[U-Boot] [PATCH 1/1] cmd: usb: display bus number

[Marek Vasut]

On 3/18/19 9:05 PM, Heinrich Schuchardt wrote:
> On 1/2/19 7:33 PM, Heinrich Schuchardt wrote:
>> On 1/2/19 5:37 PM, Marek Vasut wrote:
>>> On 1/2/19 5:03 PM, Heinrich Schuchardt wrote:
>>>> On 1/2/19 3:21 PM, Marek Vasut wrote:
>>>>> On 1/2/19 12:29 PM, Heinrich Schuchardt wrote:
>>>>>> If multiple USB buses exist, the output of the commands 'usb tree' and 'usb
>>>>>> info' is confusing because it is not clear where the output for a new bus
>>>>>> starts.
>>>>>>
>>>>>> Print an additional line for each bus indicating the bus number, e.g.
>>>>>>
>>>>>> => usb tree
>>>>>> USB device tree:
>>>>>> USB bus 0
>>>>>>   1  Hub (5 Gb/s, 0mA)
>>>>>>      U-Boot XHCI Host Controller
>>>>>>
>>>>>> USB bus 1
>>>>>>   1  Hub (5 Gb/s, 0mA)
>>>>>>      U-Boot XHCI Host Controller
>>>>>>
>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>> ---
>>>>>>  cmd/usb.c | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/cmd/usb.c b/cmd/usb.c
>>>>>> index 0ccb1b5148..6da945c376 100644
>>>>>> --- a/cmd/usb.c
>>>>>> +++ b/cmd/usb.c
>>>>>> @@ -470,6 +470,7 @@ static void usb_for_each_root_dev(usb_dev_func_t func)
>>>>>>  		if (!device_active(bus))
>>>>>>  			continue;
>>>>>>  
>>>>>> +		printf("USB bus %d\n", bus->seq);
>>>>>
>>>>> Isn't this only valid if you use SEQ_ALIAS ?
>>>>
>>>> Kconfig symbol SEQ_ALIAS does not exist. I assume you mean DM_SEQ_ALIAS.
>>>>
>>>> In device_probe() seq is set unconditionally
>>>> (drivers/core/device.c:306).
>>>>
>>>> CONFIG_DM_SEQ_ALIAS=y allows to influence the value of seq via the
>>>> device tree.
>>>
>>> I know what it does. Without this option being set, the bus->seq number
>>> will be either some obscure gigantic number or undefined, right ?
>>>
>>
>> I disabled CONFIG_DM_SEQ_ALIAS and this is the output on my MACCHIATObin
>> (with the second USB bus enabled in the device tree).
>>
>> => usb start
>> starting USB...
>> USB0:   Register 2000120 NbrPorts 2
>> Starting the controller
>> USB XHCI 1.00
>> USB1:   Register 2000120 NbrPorts 2
>> Starting the controller
>> USB XHCI 1.00
>> scanning bus 0 for devices... 1 USB Device(s) found
>> scanning bus 1 for devices... 1 USB Device(s) found
>>        scanning usb for storage devices... 0 Storage Device(s) found
>> => usb tree
>> USB device tree:
>> USB bus 0
>>   1  Hub (5 Gb/s, 0mA)
>>      U-Boot XHCI Host Controller
>>
>> USB bus 1
>>   1  Hub (5 Gb/s, 0mA)
>>      U-Boot XHCI Host Controller
>>
>> Could you, please, indicate under which circumstances seq might be
>> undefined.
>>
>> Best regards
>>
>> Heinrich
>>
> 
> Hello Marek,
> 
> In device_bind_common() dev->seq is set to -1 independent of
> DM_SEQ_ALIAS. dev->req_seq is set to -1 if DM_SEQ_ALIAS is not set.
> 
> When the device is probed in device_probe() a call to function
> uclass_resolve_seq() is used to set the actual value of dev->seq.
> 
> As shown in the example above devices are number beginning with zero if
> dev->req_seq is not set via DM_SEQ_ALIAS.
> 
> Do you still see any reason why this patch should not be merged?

Would it rather make sense to print the bus name ? That might be a much
better identifier than a random number.

-- 
Best regards,
Marek Vasut