cmd: usb: Prevent reset in usb tree/info command

Commands causing reset in some configs:

When bootflow scan is run, this will cause a UCLASS_BOOTDEV device to
be added as sibling of those UCLASS_BLK devices found in the search
chain defined in environment variable "boot_targets", until boot
succeeds from some device. This can happen automatically as part of
the default boot process on some boards (example: Rock Pi 4) depending
on the board configuration (DISTRO_DEFAULTS, BOOTSTD, BOOTCOMMAND,
etc.) because they have bootcmd=bootflow scan.

If boot doesn't succeed from any device, and usb is in boot_targets,
and an usb storage device is plugged to some usb port at boot time,
its UCLASS_MASS_STORAGE device will have a UCLASS_BOOTDEV device as
child, besides a UCLASS_BLK child.

If once the boot fails the user enters at the U-Boot shell prompt:

usb info

or

usb tree

The code in cmd/usb.c will eventually recurse into the UCLASS_BOOTDEV
device and pass a null usb_device pointer to usb_show_tree_graph() or
usb_show_info() (because it has no parent_priv_).

This causes a reset. The expected behaviour would be to ignore the
UCLASS_BOOTDEV device, continue listing the usb information and return
to the prompt.

   Minimal test:

Another way to trigger this reset as a minimal test or on boards with
a different bootcmd would be:

- make sure "usb" is in environment variable boot_targets (might need
  setenv boot_targets usb; and/or saveenv and reset), then, with a usb
  storage device plugged to a usb port, run:

=> usb reset ; bootflow scan ; usb info

   Solution:

Fix it (twice) by checking for null parent_priv_ and adding
UCLASS_BOOTDEV to the list of ignored class ids before the recursive
call.

This prevents the current particular problem with UCLASS_BOOTDEV, even
in case it ever gets some parent_priv_ struct which is not an
usb_device, despite being the child of a usb_device->dev. And it also
prevents possible future problems if other children are added to usb
devices that don't have parent_priv_ because they are not part of the
usb tree, just abstractions of functionality (like UCLASS_BLK and
UCLASS_BOOTDEV are now).

Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Marek Vasut <marex@denx.de>
Tested-by: Marek Vasut <marex@denx.de>
This commit is contained in:
Xavier Drudis Ferran 2023-06-21 09:13:07 +02:00 committed by Marek Vasut
parent 50842b217f
commit 7a875a8e5c

View File

@ -421,7 +421,9 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
* Ignore emulators and block child devices, we only want
* real devices
*/
if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
if (udev &&
(device_get_uclass_id(child) != UCLASS_BOOTDEV) &&
(device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
(device_get_uclass_id(child) != UCLASS_BLK)) {
usb_show_tree_graph(udev, pre);
pre[index] = 0;
@ -604,10 +606,12 @@ static void usb_show_info(struct usb_device *udev)
child;
device_find_next_child(&child)) {
if (device_active(child) &&
(device_get_uclass_id(child) != UCLASS_BOOTDEV) &&
(device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
(device_get_uclass_id(child) != UCLASS_BLK)) {
udev = dev_get_parent_priv(child);
usb_show_info(udev);
if (udev)
usb_show_info(udev);
}
}
}