[PATCH v3] usbhid: tolerate intermittent errors

[Liam Mitchell]

Modifies usbhid error handling to better tolerate intermittent
errors, reducing URB resubmission delay and device resets.

---
Protocol errors like EPROTO can occur randomly, sometimes frequently and
are often not fixed by a device reset.

The current error handling will only resubmit the URB after at least 13ms
delay and may reset the USB device if another error occurs 1-1.5s later, 
regardless of error type or count.

These delays and device resets increase the chance that input events will be
missed and that users see symptoms like missed or sticky keyboard keys.

This patch reduces the threshold used to assume device is working after
discussion in V2. Truly disconnected devices should error after 3 polling
intervals.

It allows single protocol errors to be retried immediately for minimal
downtime.

It changes the retry timeout to be relative to the last input URB
submission so time spent active counts towards the delay.
This makes it more likely that subsequent intermittent errors can be
retried on the next tick.

It only tries reset after 20 errors, roughly about 2s, longer than the
previous time based threshold of 1-1.5s.

Signed-off-by: Liam Mitchell <mitchell.liam@xxxxxxxxx>
Link: https://lore.kernel.org/linux-input/CAOQ1CL6Q+4GNy=kgisLzs0UBXFT3b02PG8t-0rPuW-Wf6NhQ6g@xxxxxxxxxxxxxx/
---
Changes in v3:
- uses shorter threshold to assume device is working
- stop_retry & retry_delay -> error_count & last_in
- retry after 20 errors
- includes running time in retry timeout calc
- immediate retry case is integrated with other error logic
- shortens initial retry delay to 1ms
- changes max delay to 128ms
- Link to v2: https://lore.kernel.org/r/20260307-usbhid-eproto-v2-1-e5a8abce4652@xxxxxxxxx

Changes in v2:
- revert changes to hid_io_error
- add more specific fix in hid_irq_in
- Link to v1: https://lore.kernel.org/r/20260208-usbhid-eproto-v1-1-5872c10d90bb@xxxxxxxxx
---
 drivers/hid/usbhid/hid-core.c | 44 ++++++++++++++++++++++++++++---------------
 drivers/hid/usbhid/usbhid.h   |  4 ++--
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 758eb21430cd..233c1950632a 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -70,6 +70,12 @@ MODULE_PARM_DESC(quirks, "Add/modify USB HID quirks by specifying "
 		" quirks=vendorID:productID:quirks"
 		" where vendorID, productID, and quirks are all in"
 		" 0x-prefixed hex");
+
+/* Threshold at which we can assume a device is working correctly.
+ * A disconnected device should fail within 3 polling intervals.
+ * Most HID devices poll 8ms or faster. */
+#define HID_ASSUME_WORKING msecs_to_jiffies(100)
+
 /*
  * Input submission and I/O error handler.
  */
@@ -90,6 +96,7 @@ static int hid_start_in(struct hid_device *hid)
 	    !test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
 	    !test_bit(HID_SUSPENDED, &usbhid->iofl) &&
 	    !test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
+		usbhid->last_in = jiffies;
 		rc = usb_submit_urb(usbhid->urbin, GFP_ATOMIC);
 		if (rc != 0) {
 			clear_bit(HID_IN_RUNNING, &usbhid->iofl);
@@ -154,19 +161,13 @@ static void hid_io_error(struct hid_device *hid)
 		goto done;
 
 	/* If it has been a while since the last error, we'll assume
-	 * this a brand new error and reset the retry timeout. */
-	if (time_after(jiffies, usbhid->stop_retry + HZ/2))
-		usbhid->retry_delay = 0;
-
-	/* When an error occurs, retry at increasing intervals */
-	if (usbhid->retry_delay == 0) {
-		usbhid->retry_delay = 13;	/* Then 26, 52, 104, 104, ... */
-		usbhid->stop_retry = jiffies + msecs_to_jiffies(1000);
-	} else if (usbhid->retry_delay < 100)
-		usbhid->retry_delay *= 2;
+	 * this a brand new error and reset the error count. */
+	if (time_after(jiffies, usbhid->last_in + HID_ASSUME_WORKING))
+		usbhid->error_count = 0;
 
-	if (time_after(jiffies, usbhid->stop_retry)) {
+	usbhid->error_count++;
 
+	if (usbhid->error_count >= 20) {
 		/* Retries failed, so do a port reset unless we lack bandwidth*/
 		if (!test_bit(HID_NO_BANDWIDTH, &usbhid->iofl)
 		     && !test_and_set_bit(HID_RESET_PENDING, &usbhid->iofl)) {
@@ -176,8 +177,13 @@ static void hid_io_error(struct hid_device *hid)
 		}
 	}
 
-	mod_timer(&usbhid->io_retry,
-			jiffies + msecs_to_jiffies(usbhid->retry_delay));
+	/* Retry time is relative to the last start time and increases
+	 * with error count: 1, 2, 4, 8, 16, 32, 64, 128, 128... ms.
+	 * By including running time in the backoff, it should be possible
+	 * to retry many intermittent errors in the next tick. */
+	mod_timer(&usbhid->io_retry, usbhid->last_in +
+			msecs_to_jiffies(1U << (min(usbhid->error_count, 8U) - 1)));
+
 done:
 	spin_unlock_irqrestore(&usbhid->lock, flags);
 }
@@ -278,7 +284,7 @@ static void hid_irq_in(struct urb *urb)
 
 	switch (urb->status) {
 	case 0:			/* success */
-		usbhid->retry_delay = 0;
+		usbhid->error_count = 0;
 		if (!test_bit(HID_OPENED, &usbhid->iofl))
 			break;
 		usbhid_mark_busy(usbhid);
@@ -312,6 +318,13 @@ static void hid_irq_in(struct urb *urb)
 	case -EPROTO:		/* protocol error or unplug */
 	case -ETIME:		/* protocol error or unplug */
 	case -ETIMEDOUT:	/* Should never happen, but... */
+		/* Allow first error to retry immediately */
+		if (usbhid->error_count == 0 ||
+		    time_after(jiffies, usbhid->last_in + HID_ASSUME_WORKING)) {
+			dev_dbg(&usbhid->intf->dev, "retrying intr urb immediately\n");
+			usbhid->error_count = 1;
+			break;
+		}
 		usbhid_mark_busy(usbhid);
 		clear_bit(HID_IN_RUNNING, &usbhid->iofl);
 		hid_io_error(hid);
@@ -321,6 +334,7 @@ static void hid_irq_in(struct urb *urb)
 			 urb->status);
 	}
 
+	usbhid->last_in = jiffies;
 	status = usb_submit_urb(urb, GFP_ATOMIC);
 	if (status) {
 		clear_bit(HID_IN_RUNNING, &usbhid->iofl);
@@ -1504,7 +1518,7 @@ static void hid_restart_io(struct hid_device *hid)
 
 	if (clear_halt || reset_pending)
 		schedule_work(&usbhid->reset_work);
-	usbhid->retry_delay = 0;
+	usbhid->error_count = 0;
 	spin_unlock_irq(&usbhid->lock);
 
 	if (reset_pending || !test_bit(HID_STARTED, &usbhid->iofl))
diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
index 75fe85d3d27a..b5dbfa588dc8 100644
--- a/drivers/hid/usbhid/usbhid.h
+++ b/drivers/hid/usbhid/usbhid.h
@@ -84,8 +84,8 @@ struct usbhid_device {
 	spinlock_t lock;						/* fifo spinlock */
 	unsigned long iofl;                                             /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */
 	struct timer_list io_retry;                                     /* Retry timer */
-	unsigned long stop_retry;                                       /* Time to give up, in jiffies */
-	unsigned int retry_delay;                                       /* Delay length in ms */
+	unsigned int error_count;                                       /* Number of consecutive errors */
+	unsigned long last_in;                                          /* Time of last in URB submission */
 	struct work_struct reset_work;                                  /* Task context for resets */
 	wait_queue_head_t wait;						/* For sleeping */
 };

---
base-commit: b91e36222ccfb1b0985d1fcc4fb13b68fb99c972
change-id: 20260208-usbhid-eproto-152c7abcb185

Best regards,
-- 
Liam Mitchell <mitchell.liam@xxxxxxxxx>