[U-Boot] [PATCH 4/6] usb: usb_hub_power_on(): Use 100ms power-on delay instead of 1 sec (optionally)

[Hans de Goede]

Hi,

On 11-03-16 11:13, Stefan Roese wrote:
> Hi Hans,
>
> On 10.03.2016 20:12, Hans de Goede wrote:
>> On 10-03-16 16:50, Stefan Roese wrote:
>>> In a system with a complex USB infrastrcture (many USB hubs), the
>>> power-on delay of mininimum 1 second for each USB hub results in a quite
>>> big USB scanning time. Many USB devices can deal with much lower
>>> power-on delays. In my test system, even 10ms seems to be enough
>>> for the USB device to get detected correctly. This patch now
>>> reduces the minimum 1 second delay to a minumum of 100ms instead.
>>> This results in a big USB scan time reduction:
>>>
>>> Here the numbers for my current board:
>>>
>>> Without this patch:
>>> starting USB...
>>> USB0:   USB EHCI 1.00
>>> scanning bus 0 for devices... 9 USB Device(s) found
>>>
>>> time: 10.822 seconds
>>>
>>> With this patch:
>>> starting USB...
>>> USB0:   USB EHCI 1.00
>>> scanning bus 0 for devices... 9 USB Device(s) found
>>>
>>> time: 6.319 seconds
>>>
>>> So ~4.5 seconds of USB scanning time reduction.
>>>
>>> I'm not really sure if this patch can be accepted in general as is.
>>>
>>> In patch 0d437bca [usb: hub: fix power good delay timing] by Stephen
>>> Warren, this delay was changes according to "Connect Timing ECN.pdf"
>>> to a total of 1 second plus the hub port's power good time. Now its
>>> changes back to 100ms which is a violation of this spec. But still,
>>> for most USB devices this 100ms is more than enough. So its a valid
>>> use-case to lower this time in special (perhaps static) USB
>>> environments.
>>
>> Actually that 1 second + poweron delay is why we had the 1 sec
>> delay in usb_hub_configure(), that is where we wait for a device
>> to show up. Note that we can already save a lot of time, by
>> first powering up all ports and then doing the 1 sec wait
>> and then checking all ports without needing to delay any further.
>>
>> Or even better:
>>
>> 1) turn on all ports
>> 2) do power-on-delay (either 2ms * bPwrOn2PwrGood from descriptor, or
>>     100ms which ever one is LARGER)
>> 3) set a timeout val 1 sec from now, loop over all ports and quit
>>     the loop when either all ports are connected (we already skip the
>>     per port delay in this connected case now), or when the 1 sec
>>     expires. There is no reason for the 1 sec per port delay,
>>     one sec + bPwrOn2PwrGood delay is enough for all ports
>
> One question here: Do we really need to do this power-on-delay in
> step 2) at all?

Yes the power may bounce during this time, causing a device connect
+ disconnect + connect again, we really need to wait for the power
to be stable before looking at / talking to connected devices.

Note this is also what the kernel does, it ignores the hub after
powering its ports for this time.

> Shouldn't it be sufficient to do the port scanning
> loop with a "2ms * bPwrOn2PwrGood + 1 sec" timeout instead? As
> I've done in the patch I've attached (which works just fine on
> my platform).
>
> Note that I will also dig into the parallelizing idea as well. I'm
> just trying to implement the timeout handling correctly first.

Cool :)

About the patch, you're waiting up to 1 sec for the first port and
then scanning the other ports without waiting. For the initial
implementation this is fine, but for the parallelization of child
probing you will want to scan all ports as fast as possible, skipping
already connected and handled ports for the duration of 1 sec, so as to
also scan the children as soon as they are connected (and not delay
scanning a hub on port 2 because port 1 does not have a device connected).

Nice improvement in scan time from this simple patch btw :)

Regards,

Hans