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? >> 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.
