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

[Marek Vasut]

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 ...

>>>> 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 ?

>>>>> 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.
>>>
> 
> [snip]
> 
> Regards,
> Bin
> 


-- 
Best regards,
Marek Vasut