usb: dwc2: host: use hrtimer for NAK retries

From: Terin Stock <hidden>
Date: 2018-08-28 18:04:58

Thanks Alan and Doug for your feedback. They've both been extremely helpful
in understanding what you're looking for in messages. A few responses below.
I think your commit will be more compelling with additional data.  As
Allen says it looks like you're not actually changing the delay.  You
could include things like:

* On systems with 100 HZ in the ideal case we'd end up delaying for 10
ms - 20 ms when we used jiffies.  Now we'll get much closer to 1 ms.

* Timer functions are not very high priority, so if we were running at
a high load then we'd sometimes see much longer delays.  (NOTE: if you
say this then please back it up with data--I think I've heard
anecdotally that the normal timer functions can be quite delayed but I
haven't done the research to back it up).  Presumably you could use
ktime to measure delays before and after your patch and you could
include this in the commit message.
Yes, this change decreases the delay, from a variable 10-20ms to 1ms.
I'll be sure to state this more clearly in the next version. I read Alan's
feedback as generally asking for the question. I'll run some tests with
ktime between now and then.
It would also be good to document what device you were plugging in
that you were having problems with and what system you were running
on.  That would help someone else if they ever wanted to modify the
same area of code and re-test.  They'd have a better chance of not
re-breaking you.
This is a rather generic USB floppy drive. Is there a preference of
vid and pid and/or product and manufacturer strings?
Overall nit: please CC LKML on patches.  If nothing else that makes
them easier to find in lore.kernel.org/patchwork
To be clear, is this CC on the email envelope?
quoted
diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
index 301ced1618f8..2d0cfd7f2cfe 100644
--- 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
@@ -1465,9 +1465,9 @@ static void dwc2_deschedule_periodic(struct dwc2_hsotg *hsotg,
  *
  * @t: Pointer to wait_timer in a qh.
  */
-static void dwc2_wait_timer_fn(struct timer_list *t)
+static enum hrtimer_restart dwc2_wait_timer_fn(struct hrtimer *t)
nit: please update function docstring to include a "Return:" clause now.
Sure, I'll fix this in the next version.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help