Subject: Re: [PATCH 1/1] cmd: usb: display bus number
From: Heinrich Schuchardt <[email-masked]>
To: Marek Vasut <[email-masked]>, Tom Rini <[email-masked]>
Cc: [email-masked]
Date: Mon, 18 Mar 2019 21:05:01 +0100

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 <[email-masked]>
>>>>> ---
>>>>>  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?

Best regards

Heinrich