[PATCH] usb: ohci: delay endpoint descriptor unlinking to reduce transfer latency

[ValdikSS <iam@valdikss.org.ru>]

The current OHCI implementation immediately unlinks Endpoint Descriptors (EDs)
when they become idle. Unlinking process waits for the next Start of Frame (SOF)
interrupt as a synchronization primitive before unlinking it.

This adds ~1ms latency per transfer cycle for rapid bidirectional communication
which leads to half the USB 1.1 speed for smaller packets at best.

When a transfer completes, takeback_td() immediately calls start_ed_unlink()
if the ED has no more TDs queued. This triggers:
1. ED marked for unlinking with SOF interrupt enabled
2. Wait for next 1ms frame boundary (SOF interrupt)
3. finish_unlinks() processes the ED removal

This patch adds 275+10 ms timeout to the idle EDs. If new transfers
arrive on the same ED before timeout expiry, clear the timeout and continue
using the ED. Only unlink EDs that remain idle after the timeout period
are cleared by the existing I/O watchdog, which runs every 275 ms (and
the ED timeout is 10 ms).

This eliminates SOF synchronization delays for consecutive transfers,
but preserves the one-frame unlinking delay which was added as a
race condition elimination measure before the modern git history
in commit 961c380cef (March 2004) by David Brownell.

Performance Impact:
Tested on Orange Pi Zero3 (Allwinner H618) with Canon LBP1120 printer:
- Before: 1.984ms average latency (write-read pattern)
- After:  0.981ms average latency

Link: https://lore.kernel.org/all/9013fce0-6764-49b1-9e54-68e915e12d7c@valdikss.org.ru/T/
---
 drivers/usb/host/ohci-hcd.c | 18 ++++++++++++++++++
 drivers/usb/host/ohci-q.c   | 15 ++++++++++++---
 drivers/usb/host/ohci.h     |  1 +
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9c7f30086..daaa03dd6 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -364,6 +364,7 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 	if (ohci->rh_state != OHCI_RH_RUNNING) {
 sanitize:
 		ed->state = ED_IDLE;
+		ed->idle_timeout = 0;
 		ohci_work(ohci);
 	}
 
@@ -384,6 +385,16 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 			break;
 		}
 		fallthrough;
+	case ED_OPER:		/* check for delayed unlinking */
+		if (list_empty(&ed->td_list) && ed->idle_timeout) {
+			/* ED marked for delayed unlinking, unlink it now */
+			ed->idle_timeout = 0;
+			start_ed_unlink(ohci, ed);
+			spin_unlock_irqrestore (&ohci->lock, flags);
+			schedule_timeout_uninterruptible(1);
+			goto rescan;
+		}
+		fallthrough;
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  can't recover; must leak ed.
@@ -795,6 +806,13 @@ static void io_watchdog_func(struct timer_list *t)
 			}
 		}
 
+		/* Check for idle EDs that have timed out and unlink them to prevent memory leaks */
+		if (ed->state == ED_OPER && list_empty(&ed->td_list) &&
+		    ed->idle_timeout && time_after(jiffies, ed->idle_timeout)) {
+			ed->idle_timeout = 0;
+			start_ed_unlink(ohci, ed);
+		}
+
 		/* Starting from the latest pending TD, */
 		td = ed->pending_td;
 
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index 3b445312b..1df57a454 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -433,6 +433,7 @@ static struct ed *ed_get (
 		ed->hwTailP = cpu_to_hc32 (ohci, td->td_dma);
 		ed->hwHeadP = ed->hwTailP;	/* ED_C, ED_H zeroed */
 		ed->state = ED_IDLE;
+		ed->idle_timeout = 0;
 
 		is_out = !(ep->desc.bEndpointAddress & USB_DIR_IN);
 
@@ -603,6 +604,9 @@ static void td_submit_urb (
 	int		i, this_sg_len, n;
 	struct scatterlist	*sg;
 
+	/* Clear idle timeout since we're adding new TDs */
+	urb_priv->ed->idle_timeout = 0;
+
 	/* OHCI handles the bulk/interrupt data toggles itself.  We just
 	 * use the device toggle bits for resetting, and rely on the fact
 	 * that resetting toggle is meaningless if the endpoint is active.
@@ -1162,10 +1166,15 @@ static void takeback_td(struct ohci_hcd *ohci, struct td *td)
 	if (urb_priv->td_cnt >= urb_priv->length)
 		finish_urb(ohci, urb, status);
 
-	/* clean schedule:  unlink EDs that are no longer busy */
+	/* clean schedule: delay unlinking EDs to avoid SOF synchronization overhead */
 	if (list_empty(&ed->td_list)) {
-		if (ed->state == ED_OPER)
-			start_ed_unlink(ohci, ed);
+		if (ed->state == ED_OPER) {
+			/* Mark ED as idle but don't unlink immediately to avoid
+			 * 1ms SOF synchronization delays on rapid consecutive transfers.
+			 * Watchdog will clean up after 10ms if truly idle.
+			 */
+			ed->idle_timeout = jiffies + msecs_to_jiffies(10);
+		}
 
 	/* ... reenabling halted EDs only after fault cleanup */
 	} else if ((ed->hwINFO & cpu_to_hc32(ohci, ED_SKIP | ED_DEQUEUE))
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 631dda617..858c7bebe 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -75,6 +75,7 @@ struct ed {
 #define	OKAY_TO_TAKEBACK(ohci, ed)			\
 		((int) (ohci->wdh_cnt - ed->takeback_wdh_cnt) >= 0)
 
+	unsigned long		idle_timeout;	/* when ED became idle (jiffies) */
 } __attribute__ ((aligned(16)));
 
 #define ED_MASK	((u32)~0x0f)		/* strip hw status in low addr bits */
-- 
2.51.0