diff mbox series

usb: kbd: dwc2: Increase wait for dwc2 controller reset by 125us

Message ID 20230515145324.17697-1-filip.zaludek@oracle.com
State Deferred
Delegated to: Marek Vasut
Headers show
Series usb: kbd: dwc2: Increase wait for dwc2 controller reset by 125us | expand

Commit Message

Filip Žaludek May 15, 2023, 2:53 p.m. UTC 
Two following performance patches applied together occasionally harm usb keyboard on RPi3.

'dwc2: use the nonblock argument in submit_int_msg'
commit 9dcab2c4d2cb50ab1864c818b82a72393c160236

'console: usb: kbd: Limit poll frequency to improve performance'
commit 96991e652f541323a03c5b7e075d54a117091618

This empirically increased by sub-millisecond wait for dwc2 controller reset makes
keyboard reliable.

Signed-off-by: Filip Zaludek <filip.zaludek@oracle.com>
---
 drivers/usb/host/dwc2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marek Vasut May 15, 2023, 6:09 p.m. UTC | #1 
On 5/15/23 16:53, Filip Zaludek wrote:
> Two following performance patches applied together occasionally harm usb keyboard on RPi3.
> 
> 'dwc2: use the nonblock argument in submit_int_msg'
> commit 9dcab2c4d2cb50ab1864c818b82a72393c160236
> 
> 'console: usb: kbd: Limit poll frequency to improve performance'
> commit 96991e652f541323a03c5b7e075d54a117091618
> 
> This empirically increased by sub-millisecond wait for dwc2 controller reset makes
> keyboard reliable.
> 
> Signed-off-by: Filip Zaludek <filip.zaludek@oracle.com>
> ---
>   drivers/usb/host/dwc2.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index 23060fc369..71b66a52ed 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -764,7 +764,7 @@ static int dwc_otg_submit_rh_msg_out(struct dwc2_priv *priv,
>   					DWC2_HPRT0_PRTENCHNG |
>   					DWC2_HPRT0_PRTOVRCURRCHNG,
>   					DWC2_HPRT0_PRTRST);
> -			mdelay(50);
> +			udelay(50125);

Why not just use 'mdelay(51);' ?

If you can tell me how to reproduce this , I could try and use USB bus 
analyzer (Beagle 5000) to look at his. I have RPi3 somewhere I think. 
Then we would know what's up.

But that might take a while, since I am a bit busy these days.
Filip Žaludek May 15, 2023, 6:56 p.m. UTC | #2 
Hi Marek,


On 5/15/23 20:09, Marek Vasut wrote:
> On 5/15/23 16:53, Filip Zaludek wrote:
>> Two following performance patches applied together occasionally harm usb keyboard on RPi3.
>>
>> 'dwc2: use the nonblock argument in submit_int_msg'
>> commit 9dcab2c4d2cb50ab1864c818b82a72393c160236
>>
>> 'console: usb: kbd: Limit poll frequency to improve performance'
>> commit 96991e652f541323a03c5b7e075d54a117091618
>>
>> This empirically increased by sub-millisecond wait for dwc2 controller reset makes
>> keyboard reliable.
>>
>> Signed-off-by: Filip Zaludek <filip.zaludek@oracle.com>
>> ---
>>   drivers/usb/host/dwc2.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>> index 23060fc369..71b66a52ed 100644
>> --- a/drivers/usb/host/dwc2.c
>> +++ b/drivers/usb/host/dwc2.c
>> @@ -764,7 +764,7 @@ static int dwc_otg_submit_rh_msg_out(struct dwc2_priv *priv,
>>                       DWC2_HPRT0_PRTENCHNG |
>>                       DWC2_HPRT0_PRTOVRCURRCHNG,
>>                       DWC2_HPRT0_PRTRST);
>> -            mdelay(50);
>> +            udelay(50125);
> 
> Why not just use 'mdelay(51);' ?
> 
> If you can tell me how to reproduce this , I could try and use USB bus analyzer (Beagle 5000) to look at his. I have 
> RPi3 somewhere I think. Then we would know what's up.
> 
> But that might take a while, since I am a bit busy these days.


  Unfortunately mdelay(51) does not work, what is actually strange.
Be aware 'usb reset' might require 50 repetitions to reproduce,
but usually it is reproduced under 20 cycles.


Prerequisities:
* RPi3B or RPi3B+
* unplugged all usb devices except keyboard, (both usb 1.1 and 2.0 tested)
* RPi3 connected to console and HDMI monitor, (monitor is not requirement)
* u-boot from master compiled without debugging as it works as workaround (reproducible with current JeOS-20230110)

Refined reproducer:
* Enter 'U-Boot>' shell using usb keyboard, (always works)
* repeat 'usb reset' until usb keyboard responds, (press ENTER, or ARROW UP followed by ENTER [to see responsiveness])
* keyboard can be usually resurrected by subsequent 'usb reset' from console



Thanks,
Filip
Filip Žaludek May 17, 2023, 4:01 p.m. UTC | #3 
Hi Marek,

On 5/15/23 20:56, Filip Žaludek wrote:
> 
> 
> Hi Marek,
> 
> 
> On 5/15/23 20:09, Marek Vasut wrote:
>> On 5/15/23 16:53, Filip Zaludek wrote:
>>> Two following performance patches applied together occasionally harm usb keyboard on RPi3.
>>>
>>> 'dwc2: use the nonblock argument in submit_int_msg'
>>> commit 9dcab2c4d2cb50ab1864c818b82a72393c160236
>>>
>>> 'console: usb: kbd: Limit poll frequency to improve performance'
>>> commit 96991e652f541323a03c5b7e075d54a117091618
>>>
>>> This empirically increased by sub-millisecond wait for dwc2 controller reset makes
>>> keyboard reliable.
>>>
>>> Signed-off-by: Filip Zaludek <filip.zaludek@oracle.com>
>>> ---
>>>   drivers/usb/host/dwc2.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>>> index 23060fc369..71b66a52ed 100644
>>> --- a/drivers/usb/host/dwc2.c
>>> +++ b/drivers/usb/host/dwc2.c
>>> @@ -764,7 +764,7 @@ static int dwc_otg_submit_rh_msg_out(struct dwc2_priv *priv,
>>>                       DWC2_HPRT0_PRTENCHNG |
>>>                       DWC2_HPRT0_PRTOVRCURRCHNG,
>>>                       DWC2_HPRT0_PRTRST);
>>> -            mdelay(50);
>>> +            udelay(50125);
>>
>> Why not just use 'mdelay(51);' ?
>>
>> If you can tell me how to reproduce this , I could try and use USB bus analyzer (Beagle 5000) to look at his. I have 
>> RPi3 somewhere I think. Then we would know what's up.
>>
>> But that might take a while, since I am a bit busy these days.
> 
> 
>   Unfortunately mdelay(51) does not work, what is actually strange.
> Be aware 'usb reset' might require 50 repetitions to reproduce,
> but usually it is reproduced under 20 cycles.
> 
> 
> Prerequisities:
> * RPi3B or RPi3B+
> * unplugged all usb devices except keyboard, (both usb 1.1 and 2.0 tested)
> * RPi3 connected to console and HDMI monitor, (monitor is not requirement)
> * u-boot from master compiled without debugging as it works as workaround (reproducible with current JeOS-20230110)
> 
> Refined reproducer:
> * Enter 'U-Boot>' shell using usb keyboard, (always works)
> * repeat 'usb reset' until usb keyboard responds, (press ENTER, or ARROW UP followed by ENTER [to see responsiveness])
> * keyboard can be usually resurrected by subsequent 'usb reset' from console
> 
> 


  Marek Vasut suggested CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP=y is another way
how to deal with it, I can confirm. Thanks!

Not proposing changes to configs/rpi_3_* defaults, hopefully distributors are listening..

Regards,
Filip
diff mbox series

Patch

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 23060fc369..71b66a52ed 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -764,7 +764,7 @@  static int dwc_otg_submit_rh_msg_out(struct dwc2_priv *priv,
 					DWC2_HPRT0_PRTENCHNG |
 					DWC2_HPRT0_PRTOVRCURRCHNG,
 					DWC2_HPRT0_PRTRST);
-			mdelay(50);
+			udelay(50125);
 			clrbits_le32(&regs->hprt0, DWC2_HPRT0_PRTRST);
 			break;