[U-Boot] [PATCH v4] cmd: usb: add blk devices to ignore list in tree graph

[Bin Meng]

Hi Suneel,

On Thu, Sep 21, 2017 at 10:17 AM, Suneel Garapati
<suneelglinux at 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 at 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.

You can include my tags:

Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
Tested-by: Bin Meng <bmeng.cn at gmail.com>

to the v5. Really sorry about that!

Regards,
Bin