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

[Marek Vasut]

On 10/19/2017 11:52 AM, Bin Meng wrote:
> Hi Marek,
> 
> On Thu, Oct 19, 2017 at 5:47 PM, Marek Vasut <marex at denx.de> wrote:
>> On 10/19/2017 10:56 AM, Bin Meng wrote:
>>> 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.
>>
>> I agree with that, I was just commenting on this "(sandbox)" in the
>> description, which IMO is not precise.
>>
>>>>>>>> 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 which case, _that_ should be in the commit message.
>>
>> Anyway, is there any chance something else will come in, ie. net device
>> for USB ethernet devices ?
>>
> 
> No problem with any USB devices that end up in the leaf node in the DM tree.

So what triggers this issue are partitions and co. then ?

> Regards,
> Bin
> 


-- 
Best regards,
Marek Vasut