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>

diff --git a/cmd/usb.c b/cmd/usb.c
index 6193728384097f997d2b5ca044d74f596887ef54..23253f22231c7e65e8bc4369a0d013f9ad0d900f 100644 (file)
--- a/cmd/usb.c
+++ b/cmd/usb.c
@@ -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);
                }
        }
 }