patch "usb: dwc2: host: use hrtimer for NAK retries" added to usb-testing
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-testing 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 be merged to the usb-next branch sometime soon, after it passes testing, and the merge window is open.
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