[U-Boot] [PATCH v3] cmd: usb: add blk devices to ignore list in tree graph
Bin Meng
bmeng.cn at gmail.com
Wed Sep 20 07:42:28 UTC 2017
Hi Suneel,
On Wed, Sep 20, 2017 at 2:32 PM, Suneel Garapati <suneelglinux at gmail.com> wrote:
> Hi Bin,
>
> On Tue, Sep 19, 2017 at 8:32 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> 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.
>
> Ok. Let me be explicit here, to understand where I am going wrong.
>
> Each usb_show_tree_graph call on a device can be broken down like
> below into three line print sets
>
> <old-pre><devnum> <class description>
> <new-pre><device description>
> <repeat new-pre> (This is unconditional print, even before this fix)
>
> 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
> |
>
> +-3 Hub (480 Mb/s, 100mA)
> | | GenesysLogic USB2.0 Hub
> | | (You confirmed this as device separator)
>
> | +-6 Mass Storage (480 Mb/s, 98mA)
> | | | USBest Technology USB Mass Storage Device 101111c452b7c0
> | | | (so is this unconditional print as device separator)
>
> Call to block child is ignored here, so is the set of prints as
> described above and continues with next device below.
> It is not like only device description is cancelled but preamble is
> being printed.
>
> | +-7 Human Interface (1.5 Mb/s, 70mA)
> | Dell Dell USB Keyboard
> |
>
> +-4 Mass Storage (480 Mb/s, 300mA)
> | JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
> |
>
> Also, request you to let me know the correct tree graph you have in mind.
>
Please check the attached 'usb tree' logs, and I believe that explains
where the problem is.
Regards,
Bin
-------------- next part --------------
=> 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
|
+-3 Hub (480 Mb/s, 100mA)
| | GenesysLogic USB2.0 Hub
| |
| +-6 Mass Storage (480 Mb/s, 98mA)
| | USBest Technology USB Mass Storage Device 101111c452b7c0
| |
| +-7 Human Interface (1.5 Mb/s, 98mA)
| | USB Optical Mouse
| |
| +-8 Human Interface (1.5 Mb/s, 70mA)
| Dell Dell USB Keyboard
|
+-4 Mass Storage (480 Mb/s, 300mA)
JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
-------------- next part --------------
=> 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
|
+-3 Hub (480 Mb/s, 100mA)
| | GenesysLogic USB2.0 Hub
| |
| +-6 Mass Storage (480 Mb/s, 98mA)
| | | USBest Technology USB Mass Storage Device 101111c452b7c0
| | |
| +-7 Human Interface (1.5 Mb/s, 98mA)
| | USB Optical Mouse
| |
| +-8 Human Interface (1.5 Mb/s, 70mA)
| Dell Dell USB Keyboard
|
+-4 Mass Storage (480 Mb/s, 300mA)
| JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ
|
More information about the U-Boot
mailing list