Hi Bin,
On Wed, Sep 20, 2017 at 12:42 AM, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Suneel,
>
> On Wed, Sep 20, 2017 at 2:32 PM, Suneel Garapati <suneelgli...@gmail.com>
> wrote:
>> Hi Bin,
>>
>> On Tue, Sep 19, 2017 at 8:32 PM, Bin Meng <bmeng...@gmail.com> wrote:
>>> Hi Suneel,
>>>
>>> On Wed, Sep 20, 2017 at 2:31 AM, Suneel Garapati <suneelgli...@gmail.com>
>>> wrote:
>>>> Hi Bin,
>>>>
>>>> On Tue, Sep 19, 2017 at 12:32 AM, Bin Meng <bmeng...@gmail.com> wrote:
>>>>> Hi Suneel,
>>>>>
>>>>> On Tue, Sep 19, 2017 at 1:55 PM, Suneel Garapati <suneelgli...@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 <suneelgli...@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.
Thanks for the inputs. I have sent v4 including the fix for unwanted
preamble for child blk devices.
Regards,
Suneel
>
> Regards,
> Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
