[RESEND][PATCH 0/3] Raspberry pi improvements usb core

[RESEND][PATCH 0/3] Raspberry pi improvements usb core

[Jason Wessel]

These patches are now in use by other developers, and I would like to
get them into the upstream u-boot.  This 3 patches were part of a 9
patch series posted in July prior to the merge window closure.

-- Travis CI checks https://github.com/u-boot/u-boot/pull/35 --

Pull Request #35 Rpi master

The commit 57805f2270c4 ("net: bcmgenet: Don't set ID_MODE_DIS when
not using RGMII") needed to be extended for the case of using the
rgmii-rxid. The latest version of the Rasbperry Pi4 dtb files for the
5.4 now specify the rgmii-rxid.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

Commit 9a21770 #35: Rpi master Branch master 

-- End Travis CI checks --


Jason Wessel (3):
      usb_kbd: succeed even if no interrupt is pending
      common/usb.c: Work around keyboard reporting "USB device not accepting new address"
      usb.c: Add a retry in the usb_prepare_device()

 common/usb.c     | 16 +++++++++++++---
 common/usb_kbd.c |  4 +++-
 2 files changed, 16 insertions(+), 4 deletions(-)

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

After the initial configuration some USB keyboard+mouse devices never
return any kind of event on the interrupt line.  In particular, the
device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
never returns a data packet until the first external input event.

I found this was also true with some newer model Dell keyboards.

When the device is plugged into a xhci controller there is also no
point in waiting 5 seconds for a device that is never going to present
data, so the call to the interrupt service was changed to a
nonblocking operation for the controllers that support this.

With the patch applied, the rpi3 and rpi4 work well with the more
complex keyboard devices.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 common/usb_kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..3c0056e1b9 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -519,7 +519,9 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum)
 			   1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
 #else
 	if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-			data->intinterval, false) < 0) {
+			data->intinterval, true) < 0) {
+		/* Read first packet if the device provides it, else pick it up later */
+		return 1;
 #endif
 		printf("Failed to get keyboard state from device %04x:%04x\n",
 		       dev->descriptor.idVendor, dev->descriptor.idProduct);
-- 
2.17.1

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Marek Vasut]

On 8/18/20 4:34 PM, Jason Wessel wrote:
> After the initial configuration some USB keyboard+mouse devices never
> return any kind of event on the interrupt line.  In particular, the
> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
> never returns a data packet until the first external input event.
> 
> I found this was also true with some newer model Dell keyboards.
> 
> When the device is plugged into a xhci controller there is also no
> point in waiting 5 seconds for a device that is never going to present
> data, so the call to the interrupt service was changed to a
> nonblocking operation for the controllers that support this.
> 
> With the patch applied, the rpi3 and rpi4 work well with the more
> complex keyboard devices.

Please extend the comment in the code, it is not clear from it or from
the commit message what the problem really is that this patch tries to fix.

Also, the printf() below the return 1 is never reached.

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

On 8/18/20 10:05 AM, Marek Vasut wrote:
> On 8/18/20 4:34 PM, Jason Wessel wrote:
>> After the initial configuration some USB keyboard+mouse devices never
>> return any kind of event on the interrupt line.  In particular, the
>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>> never returns a data packet until the first external input event.
>>
>> I found this was also true with some newer model Dell keyboards.
>>
>> When the device is plugged into a xhci controller there is also no
>> point in waiting 5 seconds for a device that is never going to present
>> data, so the call to the interrupt service was changed to a
>> nonblocking operation for the controllers that support this.
>>
>> With the patch applied, the rpi3 and rpi4 work well with the more
>> complex keyboard devices.
> 
> Please extend the comment in the code, it is not clear from it or from
> the commit message what the problem really is that this patch tries to fix.
> 
> Also, the printf() below the return 1 is never reached.
> 


The printf() is only reached in the case of the #ifdef above it being true. 

The compiler in theory should optimize it away, but for clarity it can be moved 
with in the #ifdef but that also requires fixing it in two places because 
there are multiple levels to the #ifdef.

Would the following be acceptable, which I can put in the next version.


commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master)
Author: Jason Wessel <jason.wessel@windriver.com>
Date:   Thu Jun 25 05:31:25 2020 -0700

    usb_kbd: succeed even if no interrupt is pending
    
    After the initial configuration some USB keyboard+mouse devices never
    return any kind of event on the interrupt line.  In particular, the
    device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
    3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
    never returns a data packet until the first external input event.
    
    I found this was also true with some newer model Dell keyboards.
    
    Without the patch u-boot prints the following on one of keyboards and
    leave it unable to accept input events.
    
    =====
    scanning bus xhci_pci for devices...
      Failed to get keyboard state from device 14dd:1009
    =====
    
    With the patch, all families of the Raspberry Pi boards can use this
    keyboard device.
    
    Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b316807844..9ff008d5dc 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -514,18 +514,28 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum)
                                      USB_KBD_BOOT_REPORT_SIZE, data->new,
                                      data->intinterval);
        if (!data->intq) {
+               printf("Failed to get keyboard state from device %04x:%04x\n",
+                      dev->descriptor.idVendor, dev->descriptor.idProduct);
+               /* Abort, we don't want to use that non-functional keyboard. */
+               return 0;
+       }
 #elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP)
        if (usb_get_report(dev, iface->desc.bInterfaceNumber,
                           1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE) < 0) {
-#else
-       if (usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
-                       data->intinterval, false) < 0) {
-#endif
                printf("Failed to get keyboard state from device %04x:%04x\n",
                       dev->descriptor.idVendor, dev->descriptor.idProduct);
                /* Abort, we don't want to use that non-functional keyboard. */
                return 0;
        }
+#else
+       /*
+        * Set poll to true for initial interrupt transfer
+        * because some keyboard devices do not return an
+        * event until the first keypress.
+        */
+       usb_int_msg(dev, data->intpipe, data->new, data->intpktsize,
+                   data->intinterval, true);
+#endif
 
        /* Success. */
        return 1;

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Marek Vasut]

On 8/18/20 6:54 PM, Jason Wessel wrote:
> 
> 
> On 8/18/20 10:05 AM, Marek Vasut wrote:
>> On 8/18/20 4:34 PM, Jason Wessel wrote:
>>> After the initial configuration some USB keyboard+mouse devices never
>>> return any kind of event on the interrupt line.  In particular, the
>>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>>> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>>> never returns a data packet until the first external input event.
>>>
>>> I found this was also true with some newer model Dell keyboards.
>>>
>>> When the device is plugged into a xhci controller there is also no
>>> point in waiting 5 seconds for a device that is never going to present
>>> data, so the call to the interrupt service was changed to a
>>> nonblocking operation for the controllers that support this.
>>>
>>> With the patch applied, the rpi3 and rpi4 work well with the more
>>> complex keyboard devices.
>>
>> Please extend the comment in the code, it is not clear from it or from
>> the commit message what the problem really is that this patch tries to fix.
>>
>> Also, the printf() below the return 1 is never reached.
>>
> 
> 
> The printf() is only reached in the case of the #ifdef above it being true. 

That's pretty awful and confusing code then. The original code wasn't
stellar either, but can this be somehow improved now ?

> The compiler in theory should optimize it away, but for clarity it can be moved 
> with in the #ifdef but that also requires fixing it in two places because 
> there are multiple levels to the #ifdef.

I think it's better to make it more readable if possible.

> Would the following be acceptable, which I can put in the next version.
> 
> 
> commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master)
> Author: Jason Wessel <jason.wessel@windriver.com>
> Date:   Thu Jun 25 05:31:25 2020 -0700
> 
>     usb_kbd: succeed even if no interrupt is pending
>     
>     After the initial configuration some USB keyboard+mouse devices never
>     return any kind of event on the interrupt line.  In particular, the
>     device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>     3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>     never returns a data packet until the first external input event.
>     
>     I found this was also true with some newer model Dell keyboards.
>     
>     Without the patch u-boot prints the following on one of keyboards and
>     leave it unable to accept input events.
>     
>     =====
>     scanning bus xhci_pci for devices...
>       Failed to get keyboard state from device 14dd:1009

So I have to wonder, if the keyboard never returns a data packet until
you press a key (that makes sense), how does Linux deal with this ?

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Jason Wessel]

On 8/18/20 12:20 PM, Marek Vasut wrote:
> On 8/18/20 6:54 PM, Jason Wessel wrote:
>>
>>
>> On 8/18/20 10:05 AM, Marek Vasut wrote:
>>> On 8/18/20 4:34 PM, Jason Wessel wrote:
>>>> After the initial configuration some USB keyboard+mouse devices never
>>>> return any kind of event on the interrupt line.  In particular, the
>>>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>>>> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>>>> never returns a data packet until the first external input event.
>>>>
>>>> I found this was also true with some newer model Dell keyboards.
>>>>
>>>> When the device is plugged into a xhci controller there is also no
>>>> point in waiting 5 seconds for a device that is never going to present
>>>> data, so the call to the interrupt service was changed to a
>>>> nonblocking operation for the controllers that support this.
>>>>
>>>> With the patch applied, the rpi3 and rpi4 work well with the more
>>>> complex keyboard devices.
>>>
>>> Please extend the comment in the code, it is not clear from it or from
>>> the commit message what the problem really is that this patch tries to fix.
>>>
>>> Also, the printf() below the return 1 is never reached.
>>>
>>
>>
>> The printf() is only reached in the case of the #ifdef above it being true. 
> 
> That's pretty awful and confusing code then. The original code wasn't
> stellar either, but can this be somehow improved now ?
> 
>> The compiler in theory should optimize it away, but for clarity it can be moved 
>> with in the #ifdef but that also requires fixing it in two places because 
>> there are multiple levels to the #ifdef.
> 
> I think it's better to make it more readable if possible.
> 
>> Would the following be acceptable, which I can put in the next version.
>>
>>
>> commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master)
>> Author: Jason Wessel <jason.wessel@windriver.com>
>> Date:   Thu Jun 25 05:31:25 2020 -0700
>>
>>     usb_kbd: succeed even if no interrupt is pending
>>     
>>     After the initial configuration some USB keyboard+mouse devices never
>>     return any kind of event on the interrupt line.  In particular, the
>>     device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>>     3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>>     never returns a data packet until the first external input event.
>>     
>>     I found this was also true with some newer model Dell keyboards.
>>     
>>     Without the patch u-boot prints the following on one of keyboards and
>>     leave it unable to accept input events.
>>     
>>     =====
>>     scanning bus xhci_pci for devices...
>>       Failed to get keyboard state from device 14dd:1009
> 
> So I have to wonder, if the keyboard never returns a data packet until
> you press a key (that makes sense), how does Linux deal with this ?
> 


As far as I can tell the usb_kbd_probe() probe function in the Linux kernel 
sets up the configuration and exits right away.  The keyboard drivers state 
was zeroed out in the probe function and the kernel later processes callbacks
from the interrupt handler.  

Does that imply the other 2 code paths should also just return 1 and we get
rid of the printf() entirely? 

I don't have any boards that wanted either of these two paths through code,
so I wasn't inclined to change it.

Jason.

[RESEND][PATCH 1/3] usb_kbd: succeed even if no interrupt is pending

[Marek Vasut]

On 8/18/20 8:47 PM, Jason Wessel wrote:
> 
> 
> On 8/18/20 12:20 PM, Marek Vasut wrote:
>> On 8/18/20 6:54 PM, Jason Wessel wrote:
>>>
>>>
>>> On 8/18/20 10:05 AM, Marek Vasut wrote:
>>>> On 8/18/20 4:34 PM, Jason Wessel wrote:
>>>>> After the initial configuration some USB keyboard+mouse devices never
>>>>> return any kind of event on the interrupt line.  In particular, the
>>>>> device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>>>>> 3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>>>>> never returns a data packet until the first external input event.
>>>>>
>>>>> I found this was also true with some newer model Dell keyboards.
>>>>>
>>>>> When the device is plugged into a xhci controller there is also no
>>>>> point in waiting 5 seconds for a device that is never going to present
>>>>> data, so the call to the interrupt service was changed to a
>>>>> nonblocking operation for the controllers that support this.
>>>>>
>>>>> With the patch applied, the rpi3 and rpi4 work well with the more
>>>>> complex keyboard devices.
>>>>
>>>> Please extend the comment in the code, it is not clear from it or from
>>>> the commit message what the problem really is that this patch tries to fix.
>>>>
>>>> Also, the printf() below the return 1 is never reached.
>>>>
>>>
>>>
>>> The printf() is only reached in the case of the #ifdef above it being true. 
>>
>> That's pretty awful and confusing code then. The original code wasn't
>> stellar either, but can this be somehow improved now ?
>>
>>> The compiler in theory should optimize it away, but for clarity it can be moved 
>>> with in the #ifdef but that also requires fixing it in two places because 
>>> there are multiple levels to the #ifdef.
>>
>> I think it's better to make it more readable if possible.
>>
>>> Would the following be acceptable, which I can put in the next version.
>>>
>>>
>>> commit 319c75ee3d1fce8a3389d1de857751504b5110cb (HEAD -> master)
>>> Author: Jason Wessel <jason.wessel@windriver.com>
>>> Date:   Thu Jun 25 05:31:25 2020 -0700
>>>
>>>     usb_kbd: succeed even if no interrupt is pending
>>>     
>>>     After the initial configuration some USB keyboard+mouse devices never
>>>     return any kind of event on the interrupt line.  In particular, the
>>>     device identified by "Cypress Cypress USB Keyboard / PS2 Mouse as
>>>     3f980000.usb/usb1/1-1/1-1.3/1-1.3:1.0/0003:04B4:0101.0001/input/input0"
>>>     never returns a data packet until the first external input event.
>>>     
>>>     I found this was also true with some newer model Dell keyboards.
>>>     
>>>     Without the patch u-boot prints the following on one of keyboards and
>>>     leave it unable to accept input events.
>>>     
>>>     =====
>>>     scanning bus xhci_pci for devices...
>>>       Failed to get keyboard state from device 14dd:1009
>>
>> So I have to wonder, if the keyboard never returns a data packet until
>> you press a key (that makes sense), how does Linux deal with this ?
>>
> 
> 
> As far as I can tell the usb_kbd_probe() probe function in the Linux kernel 
> sets up the configuration and exits right away.  The keyboard drivers state 
> was zeroed out in the probe function and the kernel later processes callbacks
> from the interrupt handler.  

Why can't we return right away and why do we poll/wait then ?

> Does that imply the other 2 code paths should also just return 1 and we get
> rid of the printf() entirely? 
> 
> I don't have any boards that wanted either of these two paths through code,
> so I wasn't inclined to change it.

I'd say, change it, it'd go in ~October and the next release is in 3
months from October anyway.

[RESEND][PATCH 2/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

When resetting the rpi3 board sometimes it will display:
     USB device not accepting new address (error=0)

After the message appears, the usb keyboard will not work.  It seems
that the configuration actually did succeed however.  Checking the
device status for a return code of zero and continuing allows the usb
keyboard and other usb devices to work function.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 common/usb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c5..0eb5d40a2d 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 	dev->devnum = addr;
 
 	err = usb_set_address(dev); /* set address */
-
-	if (err < 0) {
+	if (err < 0)
+		debug("\n       usb_set_address return < 0\n");
+	if (err < 0 && dev->status != 0) {
 		printf("\n      USB device not accepting new address " \
 			"(error=%lX)\n", dev->status);
-		return err;
+			return err;
 	}
 
 	mdelay(10);	/* Let the SET_ADDRESS settle */
-- 
2.17.1

[RESEND][PATCH 2/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Marek Vasut]

On 8/18/20 4:34 PM, Jason Wessel wrote:
> When resetting the rpi3 board sometimes it will display:
>      USB device not accepting new address (error=0)
> 
> After the message appears, the usb keyboard will not work.  It seems
> that the configuration actually did succeed however.  Checking the
> device status for a return code of zero and continuing allows the usb
> keyboard and other usb devices to work function.
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  common/usb.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/common/usb.c b/common/usb.c
> index aad13fd9c5..0eb5d40a2d 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
>  	dev->devnum = addr;
>  
>  	err = usb_set_address(dev); /* set address */
> -
> -	if (err < 0) {
> +	if (err < 0)
> +		debug("\n       usb_set_address return < 0\n");

Please print the return code here.

Is dev->status always set by usb_set_address() when err < 0 ?

> +	if (err < 0 && dev->status != 0) {
>  		printf("\n      USB device not accepting new address " \
>  			"(error=%lX)\n", dev->status);
> -		return err;
> +			return err;
>  	}
>  
>  	mdelay(10);	/* Let the SET_ADDRESS settle */
> 

[RESEND][PATCH 2/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Jason Wessel]

On 8/18/20 10:08 AM, Marek Vasut wrote:
> On 8/18/20 4:34 PM, Jason Wessel wrote:
>> When resetting the rpi3 board sometimes it will display:
>>      USB device not accepting new address (error=0)
>>
>> After the message appears, the usb keyboard will not work.  It seems
>> that the configuration actually did succeed however.  Checking the
>> device status for a return code of zero and continuing allows the usb
>> keyboard and other usb devices to work function.
>>
>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>> ---
>>  common/usb.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/usb.c b/common/usb.c
>> index aad13fd9c5..0eb5d40a2d 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
>>  	dev->devnum = addr;
>>  
>>  	err = usb_set_address(dev); /* set address */
>> -
>> -	if (err < 0) {
>> +	if (err < 0)
>> +		debug("\n       usb_set_address return < 0\n");
> 
> Please print the return code here.

Good idea.

> 
> Is dev->status always set by usb_set_address() when err < 0 ?

Yes.  The status is filled in as far as I can tell.  I had
received other values when exceeding timing thresholds with
too many printfs() to the serial port while debugging.

The usb hardware hardware devices seem to like their initialization
to be completed in a timely manner.

Jason.

> 
>> +	if (err < 0 && dev->status != 0) {
>>  		printf("\n      USB device not accepting new address " \
>>  			"(error=%lX)\n", dev->status);
>> -		return err;
>> +			return err;
>>  	}
>>  
>>  	mdelay(10);	/* Let the SET_ADDRESS settle */
>>

[RESEND][PATCH 2/3] common/usb.c: Work around keyboard reporting "USB device not accepting new address"

[Marek Vasut]

On 8/18/20 8:16 PM, Jason Wessel wrote:
> 
> 
> On 8/18/20 10:08 AM, Marek Vasut wrote:
>> On 8/18/20 4:34 PM, Jason Wessel wrote:
>>> When resetting the rpi3 board sometimes it will display:
>>>      USB device not accepting new address (error=0)
>>>
>>> After the message appears, the usb keyboard will not work.  It seems
>>> that the configuration actually did succeed however.  Checking the
>>> device status for a return code of zero and continuing allows the usb
>>> keyboard and other usb devices to work function.
>>>
>>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>>> ---
>>>  common/usb.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/common/usb.c b/common/usb.c
>>> index aad13fd9c5..0eb5d40a2d 100644
>>> --- a/common/usb.c
>>> +++ b/common/usb.c
>>> @@ -1054,11 +1054,12 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
>>>  	dev->devnum = addr;
>>>  
>>>  	err = usb_set_address(dev); /* set address */
>>> -
>>> -	if (err < 0) {
>>> +	if (err < 0)
>>> +		debug("\n       usb_set_address return < 0\n");
>>
>> Please print the return code here.
> 
> Good idea.
> 
>>
>> Is dev->status always set by usb_set_address() when err < 0 ?
> 
> Yes.  The status is filled in as far as I can tell.  I had
> received other values when exceeding timing thresholds with
> too many printfs() to the serial port while debugging.
> 
> The usb hardware hardware devices seem to like their initialization
> to be completed in a timely manner.

I'd say, init the variable before usb_set_address with 0. All USB_ST_*
are non-zero, so no matter what usb_set_address() does,
usb_control_msg() will set dev->status to != 0, so the check is always
true then.

[RESEND][PATCH 3/3] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

I have found through testing some USB 2 composite mouse/keyboard
devices do not response to the usb_set_address call immediately
following the port reset.  It can take anywhere from 2ms to 20ms.

This patch adds a retry and delay for usb_prepare_device() and allows
all the USB keyboards I tried to function properly.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 common/usb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 0eb5d40a2d..39bae86a11 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1032,6 +1032,7 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 			      struct usb_device *parent)
 {
 	int err;
+	int retry_msec = 0;
 
 	/*
 	 * Allocate usb 3.0 device context.
@@ -1054,6 +1055,14 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 	dev->devnum = addr;
 
 	err = usb_set_address(dev); /* set address */
+	/* Retry for old composite keyboard/mouse usb2 hardware */
+	while (err < 0 && retry_msec <= 40) {
+		retry_msec += 20;
+		mdelay(20);
+		err = usb_set_address(dev); /* set address */
+	}
+	if (retry_msec > 0)
+		debug("usb_set_address delay: %i\n", retry_msec);
 	if (err < 0)
 		debug("\n       usb_set_address return < 0\n");
 	if (err < 0 && dev->status != 0) {
-- 
2.17.1

[RESEND][PATCH 3/3] usb.c: Add a retry in the usb_prepare_device()

[Marek Vasut]

On 8/18/20 4:34 PM, Jason Wessel wrote:
> I have found through testing some USB 2 composite mouse/keyboard
> devices do not response to the usb_set_address call immediately
> following the port reset.  It can take anywhere from 2ms to 20ms.

Does it work if you do

=> setenv usb_pgood_delay 2000

before your test ?

[RESEND][PATCH 3/3] usb.c: Add a retry in the usb_prepare_device()

[Jason Wessel]

From the Raspberry Pi3 - With the patch removed:

U-Boot> setenv usb_pgood_delay 2000
U-Boot> usb reset
resetting USB...
Bus usb at 7e980000: USB DWC2
scanning bus usb at 7e980000 for devices... unable to get device descriptor (error=-22)
6 USB Device(s) found
       scanning usb for storage devices... 1 Storage Device(s) found


The only way I found to make it work reliably was to have it retry in the usb_prepare_device().

Jason.


On 8/18/20 10:08 AM, Marek Vasut wrote:
> On 8/18/20 4:34 PM, Jason Wessel wrote:
>> I have found through testing some USB 2 composite mouse/keyboard
>> devices do not response to the usb_set_address call immediately
>> following the port reset.  It can take anywhere from 2ms to 20ms.
> 
> Does it work if you do
> 
> => setenv usb_pgood_delay 2000
> 
> before your test ?
> 

[RESEND][PATCH 3/3] usb.c: Add a retry in the usb_prepare_device()

[Marek Vasut]

On 8/18/20 8:01 PM, Jason Wessel wrote:
>>From the Raspberry Pi3 - With the patch removed:
> 
> U-Boot> setenv usb_pgood_delay 2000
> U-Boot> usb reset
> resetting USB...
> Bus usb at 7e980000: USB DWC2
> scanning bus usb at 7e980000 for devices... unable to get device descriptor (error=-22)
> 6 USB Device(s) found
>        scanning usb for storage devices... 1 Storage Device(s) found
> 
> 
> The only way I found to make it work reliably was to have it retry in the usb_prepare_device().
> 
> Jason.
> 
> 
> On 8/18/20 10:08 AM, Marek Vasut wrote:
>> On 8/18/20 4:34 PM, Jason Wessel wrote:
>>> I have found through testing some USB 2 composite mouse/keyboard
>>> devices do not response to the usb_set_address call immediately
>>> following the port reset.  It can take anywhere from 2ms to 20ms.
>>
>> Does it work if you do
>>
>> => setenv usb_pgood_delay 2000
>>
>> before your test ?

Please stop top-posting.

Can you check whether the pgood_delay isn't EHCI-only ? It does sure
look similar to the problems which pgood_delay was supposed to help with.

[RESEND][PATCH 0/3] Raspberry pi improvements usb core

[Marek Vasut]

On 8/18/20 4:34 PM, Jason Wessel wrote:
> These patches are now in use by other developers, and I would like to
> get them into the upstream u-boot.  This 3 patches were part of a 9
> patch series posted in July prior to the merge window closure.
> 
> -- Travis CI checks https://github.com/u-boot/u-boot/pull/35 --
> 
> Pull Request #35 Rpi master
> 
> The commit 57805f2270c4 ("net: bcmgenet: Don't set ID_MODE_DIS when
> not using RGMII") needed to be extended for the case of using the
> rgmii-rxid. The latest version of the Rasbperry Pi4 dtb files for the
> 5.4 now specify the rgmii-rxid.

OK, so this is some networking patch series ?