4.20-stable review patch. If anyone has any objections, please let me know.
------------------
From: Terin Stock terin@terinstock.com
commit 6ed30a7d8ec29d3aba46e47aa8b4a44f077dda4e upstream.
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") Cc: stable@vger.kernel.org Reviewed-by: Douglas Anderson dianders@chromium.org Acked-by: Minas Harutyunyan hminas@synopsys.com Signed-off-by: Terin Stock terin@terinstock.com Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/usb/dwc2/hcd.h | 2 +- drivers/usb/dwc2/hcd_queue.c | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-)
--- a/drivers/usb/dwc2/hcd.h +++ b/drivers/usb/dwc2/hcd.h @@ -366,7 +366,7 @@ struct dwc2_qh { u32 desc_list_sz; u32 *n_bytes; struct timer_list unreserve_timer; - struct timer_list wait_timer; + struct hrtimer wait_timer; struct dwc2_tt *dwc_tt; int ttport; unsigned tt_buffer_dirty:1; --- a/drivers/usb/dwc2/hcd_queue.c +++ b/drivers/usb/dwc2/hcd_queue.c @@ -59,7 +59,7 @@ #define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
/* If we get a NAK, wait this long before retrying */ -#define DWC2_RETRY_WAIT_DELAY (msecs_to_jiffies(1)) +#define DWC2_RETRY_WAIT_DELAY 1*1E6L
/** * dwc2_periodic_channel_available() - Checks that a channel is available for a @@ -1464,10 +1464,12 @@ static void dwc2_deschedule_periodic(str * qh back to the "inactive" list, then queues transactions. * * @t: Pointer to wait_timer in a qh. + * + * Return: HRTIMER_NORESTART to not automatically restart this timer. */ -static void dwc2_wait_timer_fn(struct timer_list *t) +static enum hrtimer_restart dwc2_wait_timer_fn(struct hrtimer *t) { - struct dwc2_qh *qh = from_timer(qh, t, wait_timer); + struct dwc2_qh *qh = container_of(t, struct dwc2_qh, wait_timer); struct dwc2_hsotg *hsotg = qh->hsotg; unsigned long flags;
@@ -1491,6 +1493,7 @@ static void dwc2_wait_timer_fn(struct ti }
spin_unlock_irqrestore(&hsotg->lock, flags); + return HRTIMER_NORESTART; }
/** @@ -1521,7 +1524,8 @@ static void dwc2_qh_init(struct dwc2_hso /* Initialize QH */ qh->hsotg = hsotg; timer_setup(&qh->unreserve_timer, dwc2_unreserve_timer_fn, 0); - timer_setup(&qh->wait_timer, dwc2_wait_timer_fn, 0); + hrtimer_init(&qh->wait_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + qh->wait_timer.function = &dwc2_wait_timer_fn; qh->ep_type = ep_type; qh->ep_is_in = ep_is_in;
@@ -1690,7 +1694,7 @@ void dwc2_hcd_qh_free(struct dwc2_hsotg * won't do anything anyway, but we want it to finish before we free * memory. */ - del_timer_sync(&qh->wait_timer); + hrtimer_cancel(&qh->wait_timer);
dwc2_host_put_tt_info(hsotg, qh->dwc_tt);
@@ -1716,6 +1720,7 @@ int dwc2_hcd_qh_add(struct dwc2_hsotg *h { int status; u32 intr_mask; + ktime_t delay;
if (dbg_qh(qh)) dev_vdbg(hsotg->dev, "%s()\n", __func__); @@ -1734,8 +1739,8 @@ int dwc2_hcd_qh_add(struct dwc2_hsotg *h list_add_tail(&qh->qh_list_entry, &hsotg->non_periodic_sched_waiting); qh->wait_timer_cancel = false; - mod_timer(&qh->wait_timer, - jiffies + DWC2_RETRY_WAIT_DELAY + 1); + delay = ktime_set(0, DWC2_RETRY_WAIT_DELAY); + hrtimer_start(&qh->wait_timer, delay, HRTIMER_MODE_REL); } else { list_add_tail(&qh->qh_list_entry, &hsotg->non_periodic_sched_inactive);