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.ku...@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.
Where is this value come from? Any real justification? Or just: lets make it
huge...
Also, as I understand from your commit message, this is needed only for broken
pens...
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.
>
>>> +
>>> for (i = 0; i< dev->maxchild; i++) {
>>> ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
>>> unsigned short portstatus, portchange;
>>> + int ret;
>>> + ulong start = get_timer(0);
>>> +
>>> + do {
>>> + ret = usb_get_port_status(dev, i + 1, portsts);
>>> + if (ret< 0) {
>>> + USB_HUB_PRINTF("get_port_status failed\n");
>>> + break;
>>> + }
>>> +
>>> + portstatus = le16_to_cpu(portsts->wPortStatus);
>>> + portchange = le16_to_cpu(portsts->wPortChange);
>>> +
>>> + if ((portchange& USB_PORT_STAT_C_CONNECTION)&&
>>> + (portstatus& USB_PORT_STAT_CONNECTION))
>>> + break;
>>>
>>> - if (usb_get_port_status(dev, i + 1, portsts)< 0) {
>>> - USB_HUB_PRINTF("get_port_status failed\n");
>>> + mdelay(100);
>>> + } while (get_timer(start)< CONFIG_SYS_HZ * 10);
>>> +
>>> + if (ret< 0)
>>> continue;
>>> - }
>>>
>>> - portstatus = le16_to_cpu(portsts->wPortStatus);
>>> - portchange = le16_to_cpu(portsts->wPortChange);
>>> USB_HUB_PRINTF("Port %d Status %X Change %X\n",
>>> i + 1, portstatus, portchange);
>>>
>>
>
>
--
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
