Hi Bin,

On Wed, Sep 20, 2017 at 8:35 PM, Bin Meng bmeng.cn@gmail.com wrote:

Hi Suneel,

On Thu, Sep 21, 2017 at 10:17 AM, Suneel Garapati suneelglinux@gmail.com wrote:

...

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 suneelglinux@gmail.com

Changes v4:

• do not set preamble if child is block device for mass storage

Thanks for working on this.

...

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(-)

diff --git a/cmd/usb.c b/cmd/usb.c index d95bcf5..d64561e 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -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;
            }
  

@@ -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.

diff --git a/cmd/usb.c b/cmd/usb.c index d64561e..907debe 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -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);
  
  

...

                    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.

Thanks for quick testing.

You can include my tags:

Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com

to the v5. Really sorry about that!

Will send v5.

Regards, Suneel

Regards, Bin