cmd: usb: add blk devices to ignore list in tree graph

add blk child devices to ignore list while displaying usb tree graph, otherwise usb tree and info commands may cause crash treating blk as usb device.

Signed-off-by Suneel Garapati <[email protected]>

---

Changes v4:
- do not set preamble if child is block device for mass storage

Changes v3:
- remove 'check on parent uclass' in description
Changes v2:
- remove check on parent uclass
Changes v1:
- add separate check on blk uclass
- modify description
- add separate check on parent uclass as usb

 cmd/usb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

cmd/usb.c +16 -2

index d95bcf5..d64561e 100644
@@ -349,6 +349,16 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
        printf(" %s", pre);
 #ifdef CONFIG_DM_USB
        has_child = device_has_active_children(dev->dev);
+       if (device_get_uclass_id(dev->dev) == UCLASS_MASS_STORAGE) {
+               struct udevice *child;
+
+               for (device_find_first_child(dev->dev, &child);
+                    child;
+                    device_find_next_child(&child)) {
+                       if (device_get_uclass_id(child) == UCLASS_BLK)
+                               has_child = 0;
+               }
+       }
 #else
        /* check if the device has connected children */
        int i;

@@ -414,8 +424,12 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
 
                udev = dev_get_parent_priv(child);
 
-               /* Ignore emulators, we only want real devices */
-               if (device_get_uclass_id(child) != UCLASS_USB_EMUL) {
+               /*
+                * Ignore emulators and block child devices, we only want
+                * real devices
+                */
+               if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
+                   (device_get_uclass_id(child) != UCLASS_BLK)) {
                        usb_show_tree_graph(udev, pre);
                        pre[index] = 0;
                }

Hi Suneel,

Quoted 9 lines · Suneel Garapati

add blk child devices to ignore list while displaying usb tree graph, otherwise usb tree and info commands may cause crash treating blk as usb device.

Signed-off-by Suneel Garapati <[email protected]>

---

Changes v4:
- do not set preamble if child is block device for mass storage

Thanks for working on this.

Quoted 49 lines

Changes v3:
- remove 'check on parent uclass' in description
Changes v2:
- remove check on parent uclass
Changes v1:
- add separate check on blk uclass
- modify description
- add separate check on parent uclass as usb

 cmd/usb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

cmd/usb.c +16 -2

index d95bcf5..d64561e 100644
@@ -349,6 +349,16 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
        printf(" %s", pre);
 #ifdef CONFIG_DM_USB
        has_child = device_has_active_children(dev->dev);
+       if (device_get_uclass_id(dev->dev) == UCLASS_MASS_STORAGE) {
+               struct udevice *child;
+
+               for (device_find_first_child(dev->dev, &child);
+                    child;
+                    device_find_next_child(&child)) {
+                       if (device_get_uclass_id(child) == UCLASS_BLK)
+                               has_child = 0;
+               }
+       }
 #else
        /* check if the device has connected children */
        int i;

@@ -414,8 +424,12 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
 
                udev = dev_get_parent_priv(child);
 
-               /* Ignore emulators, we only want real devices */
-               if (device_get_uclass_id(child) != UCLASS_USB_EMUL) {
+               /*
+                * Ignore emulators and block child devices, we only want
+                * real devices
+                */
+               if ((device_get_uclass_id(child) != UCLASS_USB_EMUL) &&
+                   (device_get_uclass_id(child) != UCLASS_BLK)) {
                        usb_show_tree_graph(udev, pre);
                        pre[index] = 0;
                }

cmd/usb.c static void usb_show_info(struct usb_device *udev) · line 623

Quoted 6 lines

cmd/usb.c +2 -1

@@ -605,7 +619,8 @@ static void usb_show_info(struct usb_device *udev)
        for (device_find_first_child(udev->dev, &child);
             child;
             device_find_next_child(&child)) {
-               if (device_active(child)) {
+               if (device_active(child) &&
+                   (device_get_uclass_id(child) != UCLASS_BLK)) {

I hate this, but sorry I just managed to test this patch on Sandbox. This should also check UCLASS_USB_EMUL otherwise sandbox 'usb info' still crashes.

cmd/usb.c +1 -0

index d64561e..907debe 100644
@@ -620,6 +620,7 @@ 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_USB_EMUL) &&
                    (device_get_uclass_id(child) != UCLASS_BLK)) {
                        udev = dev_get_parent_priv(child);
                        usb_show_info(udev);

cmd/usb.c static void usb_show_info(struct usb_device *udev)

Quoted 4 lines

cmd/usb.c +0 -1

                        udev = dev_get_parent_priv(child);
                        usb_show_info(udev);
                }
--

With the above changes, I tested on both Intel MinnowMax and Sandbox, it works fine.

You can include my tags:

Reviewed-by Bin Meng <[email protected]>

Tested-by Bin Meng <[email protected]>

to the v5. Really sorry about that!

Regards,
Bin