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

[Bin Meng]

Hi Suneel,

On Wed, Sep 20, 2017 at 2:31 AM, Suneel Garapati <suneelglinux at gmail.com> wrote:
> Hi Bin,
>
> On Tue, Sep 19, 2017 at 12:32 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Suneel,
>>
>> On Tue, Sep 19, 2017 at 1:55 PM, 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 v3:
>>>  - remove 'check on parent uclass' in description
>>
>> thanks for making the changes.
>>
>>> 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 | 11 ++++++++---
>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/cmd/usb.c b/cmd/usb.c
>>> index d95bcf5..3889994 100644
>>> --- a/cmd/usb.c
>>> +++ b/cmd/usb.c
>>> @@ -414,8 +414,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 +609,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)) {
>>>                         udev = dev_get_parent_priv(child);
>>>                         usb_show_info(udev);
>>>                 }
>>> --
>>
>> My testing of 'usb info' looks OK, however 'usb tree' still has some
>> issues below:
>>
>> => usb tree
>> USB device tree:
>>   1  Hub (5 Gb/s, 0mA)
>>   |  U-Boot XHCI Host Controller
>>   |
>>   +-2  Hub (5 Gb/s, 0mA)
>>   | |  GenesysLogic USB3.0 Hub
>>   | |
>>   | +-5  Vendor specific (5 Gb/s, 36mA)
>>   |      Realtek USB 10/100/1000 LAN 00E04C680977
>>   |
> Leaving block devices, why the extra print here for lan port?

There is nothing wrong here. Every device has a separation line.

>
>>   +-3  Hub (480 Mb/s, 100mA)
>>   | |  GenesysLogic USB2.0 Hub
>>   | |
> And here?
>

Again, nothing wrong here.

>>   | +-6  Mass Storage (480 Mb/s, 98mA)
>>   | | |  USBest Technology USB Mass Storage Device 101111c452b7c0
>>   | | |
>>
>> As you see, we just don't print out the BLK device, but we still print
>> out the | here.
> I believe if the extra print for other devices is correct, then this
> tree is fine.

It's not correct. The tree graphic does not look correct now.

> Also, I believe this is not related to the fix this patch aims at.
> Let me know if otherwise.

No, you should not fix one thing but introduce another thing.

>
> Regards,
> Suneel
>>
>>   | +-7  Human Interface (1.5 Mb/s, 70mA)
>>   |      Dell Dell USB Keyboard
>>   |
>>   +-4  Mass Storage (480 Mb/s, 300mA)
>>     |  JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
>>     |
>>
>> And here.

Regards,
Bin