Re: [PATCH v2] usb: dwc2: host: use hrtimer for NAK retries

[Doug Anderson]

Felipe,

On Tue, Nov 20, 2018 at 11:16 PM Minas Harutyunyan
<minas.harutyunyan@xxxxxxxxxxxx> wrote:
>
> On 11/14/2018 3:30 AM, Doug Anderson wrote:
> > 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
> >
> Acked-by: Minas Harutyunyan <hminas@xxxxxxxxxxxx>

It looks like Terin missed CCing you on the original patch.  I assume
you are still picking up dwc2 changes like this?  If so, should he
resend the patch targeting you (and including Minas and my tags) or do
you want to pick it up from one of the mailing lists he CCed?

Thanks!

-Doug