Re: [PATCH v2] usb: dwc2: host: use hrtimer for NAK retries
From: Doug Anderson
Date: Tue Nov 13 2018 - 18:30:42 EST
Hi,
On Sun, Sep 9, 2018 at 9:24 PM Terin Stock <terin@xxxxxxxxxxxxxx> wrote:
>
> Modify the wait delay utilize the high resolution timer API to allow for
> more precisely scheduled callbacks.
>
> A previous commit added a 1ms retry delay after multiple consecutive
> NAKed transactions using jiffies. On systems with a low timer interrupt
> frequency, this delay may be significantly longer than specified,
> resulting in misbehavior with some USB devices.
>
> This scenario was reached on a Raspberry Pi 3B with a Macally FDD-USB
> floppy drive (identified as 0424:0fdc Standard Microsystems Corp.
> Floppy, based on the USB97CFDC USB FDC). With the relay delay, the drive
> would be unable to mount a disk, replying with NAKs until the device was
> reset.
>
> Using ktime, the delta between starting the timer (in dwc2_hcd_qh_add)
> and the callback function can be determined. With the original delay
> implementation, this value was consistently approximately 12ms. (output
> in us).
>
> <idle>-0 [000] ..s. 1600.559974: dwc2_wait_timer_fn: wait_timer delta: 11976
> <idle>-0 [000] ..s. 1600.571974: dwc2_wait_timer_fn: wait_timer delta: 11977
> <idle>-0 [000] ..s. 1600.583974: dwc2_wait_timer_fn: wait_timer delta: 11976
> <idle>-0 [000] ..s. 1600.595974: dwc2_wait_timer_fn: wait_timer delta: 11977
>
> After converting the relay delay to using a higher resolution timer, the
> delay was much closer to 1ms.
>
> <idle>-0 [000] d.h. 1956.553017: dwc2_wait_timer_fn: wait_timer delta: 1002
> <idle>-0 [000] d.h. 1956.554114: dwc2_wait_timer_fn: wait_timer delta: 1002
> <idle>-0 [000] d.h. 1957.542660: dwc2_wait_timer_fn: wait_timer delta: 1004
> <idle>-0 [000] d.h. 1957.543701: dwc2_wait_timer_fn: wait_timer delta: 1002
>
> The floppy drive operates properly with delays up to approximately 5ms,
> and sends NAKs for any delays that are longer.
>
> Fixes: 38d2b5fb75c1 ("usb: dwc2: host: Don't retry NAKed transactions right away")
> Signed-off-by: Terin Stock <terin@xxxxxxxxxxxxxx>
> ---
> drivers/usb/dwc2/hcd.h | 2 +-
> drivers/usb/dwc2/hcd_queue.c | 19 ++++++++++++-------
> 2 files changed, 13 insertions(+), 8 deletions(-)
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx