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

[Stefan Roese <sr@denx.de>]

Hi Hans,

On 11.03.2016 11:32, Hans de Goede wrote:
> 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.

Okay. I'll change it to include this initial delay here as well.

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

Right.

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

Yep! I was also pretty impressed by this. :)

Thanks,
Stefan