[U-Boot] [PATCH v6] cmd: usb: ignore blk, emulation devices in usb tree/info display

[Bin Meng]

Hi Marek,

On Thu, Oct 19, 2017 at 4:43 PM, Marek Vasut <marex at denx.de> wrote:
> On 10/19/2017 10:37 AM, Bin Meng wrote:
>> Hi Marek,
>
> Hi,
>
>> On Thu, Oct 19, 2017 at 3:33 PM, Marek Vasut <marex at denx.de> wrote:
>>> On 10/19/2017 05:24 AM, Suneel Garapati wrote:
>>>> On Wed, Oct 18, 2017 at 6:39 PM, Marek Vasut <marex at denx.de> wrote:
>>>>> On 10/19/2017 03:22 AM, Suneel Garapati wrote:
>>>>>> usb tree/info commands iterate over all usb uclass devices
>>>>>> recursively. blk uclass child devices are created for mass storage,
>>>>>> treating these as usb uclass devices and referencing usb config
>>>>>> interface descriptors cause crash. To fix, ignore blk and usb_emul
>>>>>> uclass devices(sandbox)
>>>>>                  ^^^^^^^ what's this about ? USB_EMUL devices can be
>>>>> enables elsewhere too, right ?
>>>> Only disabled during the tree/info dump.
>>>
>>> I don't understand this answer. Can USB_EMUL devices be enabled on any
>>> other machine than sandbox or not ? I presume it can ...
>>>
>>
>> No, it cannot.
>
> Why ? Because of the Kconfig thing ? That can easily change and then
> this breaks ...

Yes, it's currently on on Sandbox. But whether it's on Sandbox or not
does not matter. These devices are should be filtered out as they are
not supposed to be on the USB topology.

>
>>>>> Anyway, shouldn't you rather filter for positive matches (usb uclass
>>>>> devices etc) , rather than filtering out a few negative matches (blk
>>>>> etc) which might break in the future ?
>>>> usb_for_each_root_dev does that but we dont have uclass_find_first_child_device
>>>> to call on UCLASS_USB like uclass_find_first_device.
>>>> So, device_find_first_child and check on uclass id is performed.
>>>
>>> I mean, rather than doing
>>> (device_get_uclass_id(child) != UCLASS_USB_xxx &&
>>> device_get_uclass_id(child) != UCLASS_USB_yyy)
>>>    dump
>>>
>>> do
>>>
>>> (device_get_uclass_id(child) == UCLASS_USB_nnn)
>>>    dump
>>>
>>> for nnn being only the relevant USB classes for which we actually want
>>> to dump.
>>>
>>> Does that work ?
>>>
>>
>> No, I don't think you can enumerate all USB devices here. It can be
>> any uclass device.
>
> And only the blk one breaks things ?

Yes, blk devices are not "struct usb_device" declared, as they are not
USB devices. However their parent is.

>
>>>>>> in usb_show_info and usb_tree_graph.
>>>>>> also avoid addition of preamble for blk uclass child devices,
>>>>>> otherwise tree dump gets messed up.
>>>>>
>>>>> Also, sentences start with capital letter. This should be in a separate
>>>>> patch if it's a separate change ...
>>>> Ignoring preamble and device should go together, hence cannot be
>>>> separate change.
>>>>

Regards,
Bin