patch "usb: dwc2: host: use hrtimer for NAK retries" added to usb-next
Subsystems (3)
This is a note to let you know that I've just added the patch titled
usb: dwc2: host: use hrtimer for NAK retries
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-next branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will also be merged in the next major kernel release during the merge window.
If you have any questions about this process, please let me know.
Quoted 1 line
From
6ed30a7d8ec29d3aba46e47aa8b4a44f077dda4eMon Sep 17 00:00:00 2001
From: Terin Stock <[email protected]>
Date: Sun, 9 Sep 2018 21:24:31 -0700
Subject: usb: dwc2: host: use hrtimer for NAK retries
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: 11977After 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: 1002The floppy drive operates properly with delays up to approximately 5ms, and sends NAKs for any delays that are longer.
--- drivers/usb/dwc2/hcd.h | 2 +- drivers/usb/dwc2/hcd_queue.c | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-)
drivers/usb/dwc2/hcd.h +1 -1
@@ -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;
drivers/usb/dwc2/hcd_queue.c +12 -7
@@ -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(struct dwc2_hsotg *hsotg,* 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 timer_list *t) }spin_unlock_irqrestore (&hsotg->lock, flags); +return HRTIMER_NORESTART ; }/**
@@ -1521,7 +1524,8 @@ static void dwc2_qh_init(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,/* 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 *hsotg, struct dwc2_qh *qh)* 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 *hsotg, struct dwc2_qh *qh) {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 *hsotg, struct dwc2_qh *qh)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);
Signature · 1 line
2.20.0