diff mbox series

[RFT,v1,3/5] usb: Add some delay to wait for slow USB devices to be operational

Message ID 20200322130031.10455-4-lukma@denx.de
State New
Headers show
Series usb: Improve robustness of ehci-hcd controller operation | expand

Commit Message

Lukasz Majewski March 22, 2020, 1 p.m. UTC
This change provides some extra time for some slow (or degraded) USB devices
to become fully operational.

This code is the port to newest U-Boot of the fix from - "rayvt" (from [1]).

Links:
[1] - https://forum.doozan.com/read.php?3,35295,35295#msg-35295
[2] - https://www.dropbox.com/s/nrkrd1no63viuu8/uboot-bodhi-2016.05-timeoutTD.patch?dl=0

Signed-off-by: Lukasz Majewski <lukma at denx.de>
[Unfortunately, the original patch [2] did not contain S-o-B from the original
author - "rayvt"]
---

 common/usb.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Marek Vasut March 22, 2020, 1:29 p.m. UTC | #1
On 3/22/20 2:00 PM, Lukasz Majewski wrote:
> This change provides some extra time for some slow (or degraded) USB devices
> to become fully operational.
> 
> This code is the port to newest U-Boot of the fix from - "rayvt" (from [1]).
> 
> Links:
> [1] - https://forum.doozan.com/read.php?3,35295,35295#msg-35295
> [2] - https://www.dropbox.com/s/nrkrd1no63viuu8/uboot-bodhi-2016.05-timeoutTD.patch?dl=0
> 
> Signed-off-by: Lukasz Majewski <lukma at denx.de>
> [Unfortunately, the original patch [2] did not contain S-o-B from the original
> author - "rayvt"]
> ---
> 
>  common/usb.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/common/usb.c b/common/usb.c
> index 349e838f1d..305482b5bb 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -925,14 +925,20 @@ static int get_descriptor_len(struct usb_device *dev, int len, int expect_len)
>  	__maybe_unused struct usb_device_descriptor *desc;
>  	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
>  	int err;
> +	int retry = 5;
>  
>  	desc = (struct usb_device_descriptor *)tmpbuf;
>  
> +again:
>  	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, len);
>  	if (err < expect_len) {
>  		if (err < 0) {
> -			printf("unable to get device descriptor (error=%d)\n",
> -				err);
> +			printf("unable to get device descriptor (error=%d) retry: %d\n",
> +			       err, retry);
> +			mdelay(50);

Why 50 mSec and not some other value, like 100 mSec ?
Lukasz Majewski March 23, 2020, 7:04 a.m. UTC | #2
Hi Marek,

> On 3/22/20 2:00 PM, Lukasz Majewski wrote:
> > This change provides some extra time for some slow (or degraded)
> > USB devices to become fully operational.
> > 
> > This code is the port to newest U-Boot of the fix from - "rayvt"
> > (from [1]).
> > 
> > Links:
> > [1] - https://forum.doozan.com/read.php?3,35295,35295#msg-35295
> > [2] -
> > https://www.dropbox.com/s/nrkrd1no63viuu8/uboot-bodhi-2016.05-timeoutTD.patch?dl=0
> > 
> > Signed-off-by: Lukasz Majewski <lukma at denx.de>
> > [Unfortunately, the original patch [2] did not contain S-o-B from
> > the original author - "rayvt"]
> > ---
> > 
> >  common/usb.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/common/usb.c b/common/usb.c
> > index 349e838f1d..305482b5bb 100644
> > --- a/common/usb.c
> > +++ b/common/usb.c
> > @@ -925,14 +925,20 @@ static int get_descriptor_len(struct
> > usb_device *dev, int len, int expect_len) __maybe_unused struct
> > usb_device_descriptor *desc; ALLOC_CACHE_ALIGN_BUFFER(unsigned
> > char, tmpbuf, USB_BUFSIZ); int err;
> > +	int retry = 5;
> >  
> >  	desc = (struct usb_device_descriptor *)tmpbuf;
> >  
> > +again:
> >  	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, len);
> >  	if (err < expect_len) {
> >  		if (err < 0) {
> > -			printf("unable to get device descriptor
> > (error=%d)\n",
> > -				err);
> > +			printf("unable to get device descriptor
> > (error=%d) retry: %d\n",
> > +			       err, retry);
> > +			mdelay(50);  
> 
> Why 50 mSec and not some other value, like 100 mSec ?

I think that this value (50 ms) was took from Linux in some point and
with the retry set to 5 was the ported heuristics.

If you ask why exactly there is 50 ms - I cannot say, as I've just
ported the patch.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200323/1f4f6231/attachment.sig>
Marek Vasut March 23, 2020, 11:52 a.m. UTC | #3
On 3/23/20 8:04 AM, Lukasz Majewski wrote:
> Hi Marek,
> 
>> On 3/22/20 2:00 PM, Lukasz Majewski wrote:
>>> This change provides some extra time for some slow (or degraded)
>>> USB devices to become fully operational.
>>>
>>> This code is the port to newest U-Boot of the fix from - "rayvt"
>>> (from [1]).
>>>
>>> Links:
>>> [1] - https://forum.doozan.com/read.php?3,35295,35295#msg-35295
>>> [2] -
>>> https://www.dropbox.com/s/nrkrd1no63viuu8/uboot-bodhi-2016.05-timeoutTD.patch?dl=0
>>>
>>> Signed-off-by: Lukasz Majewski <lukma at denx.de>
>>> [Unfortunately, the original patch [2] did not contain S-o-B from
>>> the original author - "rayvt"]
>>> ---
>>>
>>>  common/usb.c | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/common/usb.c b/common/usb.c
>>> index 349e838f1d..305482b5bb 100644
>>> --- a/common/usb.c
>>> +++ b/common/usb.c
>>> @@ -925,14 +925,20 @@ static int get_descriptor_len(struct
>>> usb_device *dev, int len, int expect_len) __maybe_unused struct
>>> usb_device_descriptor *desc; ALLOC_CACHE_ALIGN_BUFFER(unsigned
>>> char, tmpbuf, USB_BUFSIZ); int err;
>>> +	int retry = 5;
>>>  
>>>  	desc = (struct usb_device_descriptor *)tmpbuf;
>>>  
>>> +again:
>>>  	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, len);
>>>  	if (err < expect_len) {
>>>  		if (err < 0) {
>>> -			printf("unable to get device descriptor
>>> (error=%d)\n",
>>> -				err);
>>> +			printf("unable to get device descriptor
>>> (error=%d) retry: %d\n",
>>> +			       err, retry);
>>> +			mdelay(50);  
>>
>> Why 50 mSec and not some other value, like 100 mSec ?
> 
> I think that this value (50 ms) was took from Linux in some point and
> with the retry set to 5 was the ported heuristics.
> 
> If you ask why exactly there is 50 ms - I cannot say, as I've just
> ported the patch.

I see, then please research this. The USB stack has enough ad-hoc random
values in it already, no need to add new ones.
diff mbox series

Patch

diff --git a/common/usb.c b/common/usb.c
index 349e838f1d..305482b5bb 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -925,14 +925,20 @@  static int get_descriptor_len(struct usb_device *dev, int len, int expect_len)
 	__maybe_unused struct usb_device_descriptor *desc;
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
 	int err;
+	int retry = 5;
 
 	desc = (struct usb_device_descriptor *)tmpbuf;
 
+again:
 	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, len);
 	if (err < expect_len) {
 		if (err < 0) {
-			printf("unable to get device descriptor (error=%d)\n",
-				err);
+			printf("unable to get device descriptor (error=%d) retry: %d\n",
+			       err, retry);
+			mdelay(50);
+			if (--retry >= 0)
+				/* Some drives are just slow to wake up. */
+				goto again;
 			return err;
 		} else {
 			printf("USB device descriptor short read (expected %i, got %i)\n",