[U-Boot] [PATCH resend] usbh/ehci: Increase timeout for enumeration

[Vipin Kumar]

On 12/6/2012 1:06 PM, Igor Grinberg wrote:
> On 12/06/12 08:58, Vipin Kumar wrote:
>> On 12/6/2012 12:17 PM, Igor Grinberg wrote:
>>> On 12/06/12 08:30, Vipin Kumar wrote:
>>>> Few pen drives take longer than usual for enumeration. The u-boot unlike linux
>>>> does not depend on interrupts and works in polling and timeout mode.
>>>>
>>>> This patch increases this timeout to increase the set of usb sticks that can be
>>>> enumerated by u-boot
>>>>
>>>> Signed-off-by: Vipin Kumar<vipin.kumar at st.com>
>>>> ---
>>>>    common/usb_hub.c | 27 ++++++++++++++++++++++-----
>>>>    1 file changed, 22 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/common/usb_hub.c b/common/usb_hub.c
>>>> index e4a1201..24de9b7 100644
>>>> --- a/common/usb_hub.c
>>>> +++ b/common/usb_hub.c
>>>> @@ -393,17 +393,34 @@ static int usb_hub_configure(struct usb_device *dev)
>>>>            "" : "no ");
>>>>        usb_hub_power_on(hub);
>>>>
>>>> +    mdelay(1500);
>>>
>>> a 1.5 seconds? This looks like a huge overkill...
>>> Even for broken usb sticks...
>>>
>>
>> Yes, but we are not talking about performance in u-boot. And since we are working in a polling mode, we only have 1 chance to detect the pen-drive
>
> Of course we _do care_ about performance and 1.5 seconds is huge and not justified impact.

OK

> Where is this value come from? Any real justification? Or just: lets make it huge...

A bit of both. In routine usb_hub_power_on, we wait for max(pgood_delay, 
100) ms
for power to be stable. The pgood_delay can go max upto 512. This is
because max u8 can carry is 255 and pgood_delay is usb_hub_power_on * 2

A few pens request the maximum possible delay and are not stable even
after this. The new added delay is for power to be stable but I agree
that this value is more experimental and I do not have a real written
justification

> Also, as I understand from your commit message, this is needed only for broken pens...

Yes

> Why should all others suffer?
>
> If this is really needed, I think you can do better then this.
> For example instead of waiting 1.5 seconds no meter what each time,
> make it a busy/wait loop (like you do below) and expire after a timeout.
>

The problem is that I do not know what to wait on in that case. Can you
point me to something

Regards
Vipin