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

[Suneel Garapati]

Hi Bin,

On Wed, Sep 20, 2017 at 8:35 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
> 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.
Thanks for quick testing.

>
> 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!
Will send v5.

Regards,
Suneel
>
> Regards,
> Bin