<html><head><meta name="color-scheme" content="light dark"></head><body><pre style="word-wrap: break-word; white-space: pre-wrap;"># This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.454   -&gt; 1.455  
#	drivers/usb/host/ohci-hcd.c	1.11    -&gt; 1.12   
#	drivers/usb/host/ohci-q.c	1.7     -&gt; 1.8    
#	drivers/usb/host/ohci.h	1.4     -&gt; 1.5    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/05	david-b@pacbell.net	1.455
# [PATCH] ohci-hcd, fix urb unlink races
# 
# This patch:
# 
# - Fixes a longstanding urb unlink race, by switching to a single queue
#   for EDs being unlinked.  The previous two-queue scheme was sensitive to
#   IRQ latencies:  one extra millisecond would make it use the wrong queue.
#   This updated scheme should handle latencies of up to 32K microseconds
#   (Cthulu forfend :) and slightly shrinks object code size.
# 
# - Related (mostly) cleanup.  Some functions and one ED field renamed, ED
#   layout is a smidgeon more compact (even given more data), driver version
#   string doesn't reflect CVS, debug message only comes out in verbose mode.
# --------------------------------------------
#
diff -Nru a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
--- a/drivers/usb/host/ohci-hcd.c	Wed Jun  5 12:28:53 2002
+++ b/drivers/usb/host/ohci-hcd.c	Wed Jun  5 12:28:53 2002
@@ -12,6 +12,9 @@
  * 
  * History:
  * 
+ * 2002/06/01 remember frame when HC won't see EDs any more; use that info
+ *	to fix urb unlink races caused by interrupt latency assumptions;
+ *	minor ED field and function naming updates
  * 2002/01/18 package as a patch for 2.5.3; this should match the
  *	2.4.17 kernel modulo some bugs being fixed.
  *
@@ -106,7 +109,7 @@
  *	- lots more testing!!
  */
 
-#define DRIVER_VERSION "$Revision: 1.9 $"
+#define DRIVER_VERSION "2002-Jun-01"
 #define DRIVER_AUTHOR "Roman Weissgaerber &lt;weissg@vienna.at&gt;, David Brownell"
 #define DRIVER_DESC "USB 1.1 'Open' Host Controller (OHCI) Driver"
 
@@ -287,7 +290,7 @@
 		}
 			
 		urb_priv-&gt;state = URB_DEL; 
-		ed_unlink (urb-&gt;dev, urb_priv-&gt;ed);
+		start_urb_unlink (ohci, urb_priv-&gt;ed);
 		spin_unlock_irqrestore (&amp;ohci-&gt;lock, flags);
 	} else {
 		/*
@@ -508,16 +511,15 @@
   
 	/* could track INTR_SO to reduce available PCI/... bandwidth */
 
-	// FIXME:  this assumes SOF (1/ms) interrupts don't get lost...
-	if (ints &amp; OHCI_INTR_SF) { 
-		unsigned int frame = le16_to_cpu (ohci-&gt;hcca-&gt;frame_no) &amp; 1;
+	/* handle any pending URB/ED unlinks, leaving INTR_SF enabled
+	 * when there's still unlinking to be done (next frame).
+	 */
+	spin_lock (&amp;ohci-&gt;lock);
+	if (ohci-&gt;ed_rm_list)
+		finish_unlinks (ohci, le16_to_cpu (ohci-&gt;hcca-&gt;frame_no));
+	if ((ints &amp; OHCI_INTR_SF) != 0 &amp;&amp; !ohci-&gt;ed_rm_list)
 		writel (OHCI_INTR_SF, &amp;regs-&gt;intrdisable);	
-		if (ohci-&gt;ed_rm_list [!frame] != NULL) {
-			dl_del_list (ohci, !frame);
-		}
-		if (ohci-&gt;ed_rm_list [frame] != NULL)
-			writel (OHCI_INTR_SF, &amp;regs-&gt;intrenable);	
-	}
+	spin_unlock (&amp;ohci-&gt;lock);
 
 	writel (ints, &amp;regs-&gt;intrstatus);
 	writel (OHCI_INTR_MIE, &amp;regs-&gt;intrenable);	
@@ -719,8 +721,7 @@
 	for (i = 0; i &lt; NUM_INTS; i++) ohci-&gt;hcca-&gt;int_table [i] = 0;
 	
 	/* no EDs to remove */
-	ohci-&gt;ed_rm_list [0] = NULL;
-	ohci-&gt;ed_rm_list [1] = NULL;
+	ohci-&gt;ed_rm_list = NULL;
 
 	/* empty control and bulk lists */	 
 	ohci-&gt;ed_isotail     = NULL;
@@ -802,7 +803,7 @@
 		ohci-&gt;disabled = 0;
 		ohci-&gt;sleeping = 0;
 		ohci-&gt;hc_control = OHCI_CONTROL_INIT | OHCI_USB_OPER;
-		if (!ohci-&gt;ed_rm_list [0] &amp;&amp; !ohci-&gt;ed_rm_list [1]) {
+		if (!ohci-&gt;ed_rm_list) {
 			if (ohci-&gt;ed_controltail)
 				ohci-&gt;hc_control |= OHCI_CTRL_CLE;
 			if (ohci-&gt;ed_bulktail)
diff -Nru a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
--- a/drivers/usb/host/ohci-q.c	Wed Jun  5 12:28:53 2002
+++ b/drivers/usb/host/ohci-q.c	Wed Jun  5 12:28:53 2002
@@ -208,8 +208,7 @@
 		}
 		ed-&gt;ed_prev = ohci-&gt;ed_controltail;
 		if (!ohci-&gt;ed_controltail
-				&amp;&amp; !ohci-&gt;ed_rm_list [0]
-				&amp;&amp; !ohci-&gt;ed_rm_list [1]
+				&amp;&amp; !ohci-&gt;ed_rm_list
 				&amp;&amp; !ohci-&gt;sleeping
 				) {
 			ohci-&gt;hc_control |= OHCI_CTRL_CLE;
@@ -227,8 +226,7 @@
 		}
 		ed-&gt;ed_prev = ohci-&gt;ed_bulktail;
 		if (!ohci-&gt;ed_bulktail
-				&amp;&amp; !ohci-&gt;ed_rm_list [0]
-				&amp;&amp; !ohci-&gt;ed_rm_list [1]
+				&amp;&amp; !ohci-&gt;ed_rm_list
 				&amp;&amp; !ohci-&gt;sleeping
 				) {
 			ohci-&gt;hc_control |= OHCI_CTRL_BLE;
@@ -240,16 +238,16 @@
 	case PIPE_INTERRUPT:
 		load = ed-&gt;int_load;
 		interval = ep_2_n_interval (ed-&gt;int_period);
-		ed-&gt;int_interval = interval;
+		ed-&gt;interval = interval;
 		int_branch = ep_int_balance (ohci, interval, load);
 		ed-&gt;int_branch = int_branch;
 
 		for (i = 0; i &lt; ep_rev (6, interval); i += inter) {
 			inter = 1;
 			for (ed_p = &amp; (ohci-&gt;hcca-&gt;int_table [ep_rev (5, i) + int_branch]); 
-				 (*ed_p != 0) &amp;&amp; ((dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;int_interval &gt;= interval); 
+				 (*ed_p != 0) &amp;&amp; ((dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;interval &gt;= interval); 
 				ed_p = &amp; ((dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;hwNextED)) 
-					inter = ep_rev (6, (dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;int_interval);
+					inter = ep_rev (6, (dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;interval);
 			ed-&gt;hwNextED = *ed_p; 
 			*ed_p = cpu_to_le32 (ed-&gt;dma);
 		}
@@ -260,7 +258,7 @@
 
 	case PIPE_ISOCHRONOUS:
 		ed-&gt;hwNextED = 0;
-		ed-&gt;int_interval = 1;
+		ed-&gt;interval = 1;
 		if (ohci-&gt;ed_isotail != NULL) {
 			ohci-&gt;ed_isotail-&gt;hwNextED = cpu_to_le32 (ed-&gt;dma);
 			ed-&gt;ed_prev = ohci-&gt;ed_isotail;
@@ -270,7 +268,7 @@
 				for (ed_p = &amp; (ohci-&gt;hcca-&gt;int_table [ep_rev (5, i)]); 
 					*ed_p != 0; 
 					ed_p = &amp; ((dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;hwNextED)) 
-						inter = ep_rev (6, (dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;int_interval);
+						inter = ep_rev (6, (dma_to_ed (ohci, le32_to_cpup (ed_p)))-&gt;interval);
 				*ed_p = cpu_to_le32 (ed-&gt;dma);	
 			}	
 			ed-&gt;ed_prev = NULL;
@@ -311,7 +309,7 @@
  * the link from the ed still points to another operational ed or 0
  * so the HC can eventually finish the processing of the unlinked ed
  */
-static int ep_unlink (struct ohci_hcd *ohci, struct ed *ed) 
+static int start_ed_unlink (struct ohci_hcd *ohci, struct ed *ed) 
 {
 	int	i;
 
@@ -357,8 +355,8 @@
 		break;
 
 	case PIPE_INTERRUPT:
-		periodic_unlink (ohci, ed, ed-&gt;int_branch, ed-&gt;int_interval);
-		for (i = ed-&gt;int_branch; i &lt; NUM_INTS; i += ed-&gt;int_interval)
+		periodic_unlink (ohci, ed, ed-&gt;int_branch, ed-&gt;interval);
+		for (i = ed-&gt;int_branch; i &lt; NUM_INTS; i += ed-&gt;interval)
 			ohci-&gt;ohci_int_load [i] -= ed-&gt;int_load;
 #ifdef OHCI_VERBOSE_DEBUG
 		ohci_dump_periodic (ohci, "UNLINK_INT");
@@ -384,6 +382,10 @@
 
 	/* FIXME ED's "unlink" state is indeterminate;
 	 * the HC might still be caching it (till SOF).
+	 * - use ed_rm_list and finish_unlinks(), adding some state that
+	 *   prevents clobbering hw linkage before the appropriate SOF
+	 * - a speedup:  when only one urb is queued on the ed, save 1msec
+	 *   by making start_urb_unlink() use this routine to deschedule.
 	 */
 	ed-&gt;state = ED_UNLINK;
 	return 0;
@@ -478,11 +480,8 @@
  * put the ep on the rm_list and stop the bulk or ctrl list 
  * real work is done at the next start frame (SF) hardware interrupt
  */
-static void ed_unlink (struct usb_device *usb_dev, struct ed *ed)
+static void start_urb_unlink (struct ohci_hcd *ohci, struct ed *ed)
 {    
-	unsigned int		frame;
-   	struct ohci_hcd		*ohci = hcd_to_ohci (usb_dev-&gt;bus-&gt;hcpriv);
-
 	/* already pending? */
 	if (ed-&gt;state &amp; ED_URB_DEL)
 		return;
@@ -503,9 +502,15 @@
 			break;
 	}
 
-	frame = le16_to_cpu (ohci-&gt;hcca-&gt;frame_no) &amp; 0x1;
-	ed-&gt;ed_rm_list = ohci-&gt;ed_rm_list [frame];
-	ohci-&gt;ed_rm_list [frame] = ed;
+	/* SF interrupt might get delayed; record the frame counter value that
+	 * indicates when the HC isn't looking at it, so concurrent unlinks
+	 * behave.  frame_no wraps every 2^16 msec, and changes right before
+	 * SF is triggered.
+	 */
+	ed-&gt;tick = le16_to_cpu (ohci-&gt;hcca-&gt;frame_no) + 1;
+
+	ed-&gt;ed_rm_list = ohci-&gt;ed_rm_list;
+	ohci-&gt;ed_rm_list = ed;
 
 	/* enable SOF interrupt */
 	if (!ohci-&gt;sleeping) {
@@ -816,10 +821,12 @@
 			if (td_list-&gt;ed-&gt;hwHeadP &amp; ED_H) {
 				if (urb_priv &amp;&amp; ((td_list-&gt;index + 1)
 						&lt; urb_priv-&gt;length)) {
+#ifdef OHCI_VERBOSE_DEBUG
 					dbg ("urb %p TD %d of %d, patch ED",
 						td_list-&gt;urb,
 						1 + td_list-&gt;index,
 						urb_priv-&gt;length);
+#endif
 					td_list-&gt;ed-&gt;hwHeadP = 
 			    (urb_priv-&gt;td [urb_priv-&gt;length - 1]-&gt;hwNextTD
 				    &amp; __constant_cpu_to_le32 (TD_MASK))
@@ -841,27 +848,37 @@
 
 /*-------------------------------------------------------------------------*/
 
-/* there are some pending requests to unlink 
- * - some URBs/TDs if urb_priv-&gt;state == URB_DEL
- */
-static void dl_del_list (struct ohci_hcd *ohci, unsigned int frame)
+/* wrap-aware logic stolen from &lt;linux/jiffies.h&gt; */
+#define tick_before(t1,t2) ((((s16)(t1))-((s16)(t2))) &lt; 0)
+
+/* there are some urbs/eds to unlink; called in_irq(), with HCD locked */
+static void finish_unlinks (struct ohci_hcd *ohci, u16 tick)
 {
-	unsigned long	flags;
-	struct ed	*ed;
-	__u32		edINFO;
-	__u32		tdINFO;
-	struct td	*td = NULL, *td_next = NULL,
-			*tdHeadP = NULL, *tdTailP;
-	__u32		*td_p;
+	struct ed	*ed, **last;
 	int		ctrl = 0, bulk = 0;
 
-	spin_lock_irqsave (&amp;ohci-&gt;lock, flags);
-
-	for (ed = ohci-&gt;ed_rm_list [frame]; ed != NULL; ed = ed-&gt;ed_rm_list) {
+	for (last = &amp;ohci-&gt;ed_rm_list, ed = *last; ed != NULL; ed = *last) {
+		struct td	*td, *td_next, *tdHeadP, *tdTailP;
+		u32		*td_p;
+		int		unlinked;
+
+		/* only take off EDs that the HC isn't using, accounting for
+		 * frame counter wraps.  completion callbacks might prepend
+		 * EDs to the list, they'll be checked next irq.
+		 */
+		if (tick_before (tick, ed-&gt;tick)) {
+			last = &amp;ed-&gt;ed_rm_list;
+			continue;
+		}
+		*last = ed-&gt;ed_rm_list;
+		ed-&gt;ed_rm_list = 0;
+		unlinked = 0;
 
+		/* unlink urbs from first one requested to queue end;
+		 * leave earlier urbs alone
+		 */
 		tdTailP = dma_to_td (ohci, le32_to_cpup (&amp;ed-&gt;hwTailP));
 		tdHeadP = dma_to_td (ohci, le32_to_cpup (&amp;ed-&gt;hwHeadP));
-		edINFO = le32_to_cpup (&amp;ed-&gt;hwINFO);
 		td_p = &amp;ed-&gt;hwHeadP;
 
 		for (td = tdHeadP; td != tdTailP; td = td_next) {
@@ -870,8 +887,11 @@
 
 			td_next = dma_to_td (ohci,
 				le32_to_cpup (&amp;td-&gt;hwNextTD));
-			if ((urb_priv-&gt;state == URB_DEL)) {
-				tdINFO = le32_to_cpup (&amp;td-&gt;hwINFO);
+			if (unlinked || (urb_priv-&gt;state == URB_DEL)) {
+				u32 tdINFO = le32_to_cpup (&amp;td-&gt;hwINFO);
+
+				unlinked = 1;
+
 				/* HC may have partly processed this TD */
 				if (TD_CC_GET (tdINFO) &lt; 0xE)
 					td_done (urb, td);
@@ -880,22 +900,32 @@
 
 				/* URB is done; clean up */
 				if (++ (urb_priv-&gt;td_cnt) == urb_priv-&gt;length) {
-     					spin_unlock_irqrestore (&amp;ohci-&gt;lock,
-						flags);
+					if (urb-&gt;status == -EINPROGRESS)
+						urb-&gt;status = -ECONNRESET;
+     					spin_unlock (&amp;ohci-&gt;lock);
 					finish_urb (ohci, urb);
-					spin_lock_irqsave (&amp;ohci-&gt;lock, flags);
+					spin_lock (&amp;ohci-&gt;lock);
 				}
 			} else {
 				td_p = &amp;td-&gt;hwNextTD;
 			}
 		}
 
+		/* FIXME actually want four cases here:
+		 * (a) finishing URB unlink
+		 *     [a1] no URBs queued, so start ED unlink
+		 *     [a2] some (earlier) URBs still linked, re-enable
+		 * (b) finishing ED unlink
+		 *     [b1] no URBs queued, ED is truly idle now
+		 *     [b2] URBs now queued, link ED back into schedule
+		 * right now we only have (a)
+		 */
 		ed-&gt;state &amp;= ~ED_URB_DEL;
 		tdHeadP = dma_to_td (ohci, le32_to_cpup (&amp;ed-&gt;hwHeadP));
 
 		if (tdHeadP == tdTailP) {
 			if (ed-&gt;state == ED_OPER)
-				ep_unlink (ohci, ed);
+				start_ed_unlink (ohci, ed);
 			td_free (ohci, tdTailP);
 			ed-&gt;hwINFO = ED_SKIP;
 			ed-&gt;state = ED_NEW;
@@ -918,7 +948,7 @@
 			writel (0, &amp;ohci-&gt;regs-&gt;ed_controlcurrent);
 		if (bulk)	/* reset bulk list */
 			writel (0, &amp;ohci-&gt;regs-&gt;ed_bulkcurrent);
-		if (!ohci-&gt;ed_rm_list [!frame]) {
+		if (!ohci-&gt;ed_rm_list) {
 			if (ohci-&gt;ed_controltail)
 				ohci-&gt;hc_control |= OHCI_CTRL_CLE;
 			if (ohci-&gt;ed_bulktail)
@@ -926,9 +956,6 @@
 			writel (ohci-&gt;hc_control, &amp;ohci-&gt;regs-&gt;control);   
 		}
 	}
-
-   	ohci-&gt;ed_rm_list [frame] = NULL;
-   	spin_unlock_irqrestore (&amp;ohci-&gt;lock, flags);
 }
 
 
@@ -939,7 +966,7 @@
  * Process normal completions (error or success) and clean the schedules.
  *
  * This is the main path for handing urbs back to drivers.  The only other
- * path is dl_del_list(), which unlinks URBs by scanning EDs, instead of
+ * path is finish_unlinks(), which unlinks URBs using ed_rm_list, instead of
  * scanning the (re-reversed) donelist as this does.
  */
 static void dl_done_list (struct ohci_hcd *ohci, struct td *td)
@@ -960,7 +987,7 @@
 		/* If all this urb's TDs are done, call complete().
 		 * Interrupt transfers are the only special case:
 		 * they're reissued, until "deleted" by usb_unlink_urb
-		 * (real work done in a SOF intr, by dl_del_list).
+		 * (real work done in a SOF intr, by finish_unlinks).
 		 */
   		if (urb_priv-&gt;td_cnt == urb_priv-&gt;length) {
 			int	resubmit;
@@ -980,7 +1007,7 @@
 		if ((ed-&gt;hwHeadP &amp; __constant_cpu_to_le32 (TD_MASK))
 					== ed-&gt;hwTailP
 				&amp;&amp; (ed-&gt;state == ED_OPER)) 
-			ep_unlink (ohci, ed);
+			start_ed_unlink (ohci, ed);
     		td = td_next;
   	}  
 	spin_unlock_irqrestore (&amp;ohci-&gt;lock, flags);
diff -Nru a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
--- a/drivers/usb/host/ohci.h	Wed Jun  5 12:28:53 2002
+++ b/drivers/usb/host/ohci.h	Wed Jun  5 12:28:53 2002
@@ -27,22 +27,29 @@
 	__u32			hwNextED;	/* next ED in list */
 
 	/* rest are purely for the driver's use */
-	struct ed		*ed_prev;  
-	__u8			int_period;
-	__u8			int_branch;
-	__u8			int_load; 
-	__u8			int_interval;
-	__u8			state;			// ED_{NEW,UNLINK,OPER}
+	dma_addr_t		dma;		/* addr of ED */
+	struct ed		*ed_prev;	/* for non-interrupt EDs */
+
+	u8			type; 		/* PIPE_{BULK,...} */
+	u8			interval;	/* interrupt, isochronous */
+	union {
+		struct intr_info {		/* interrupt */
+			u8	int_period;
+			u8	int_branch;
+			u8	int_load; 
+		};
+		u16		last_iso;	/* isochronous */
+	};
+
+	u8			state;		/* ED_{NEW,UNLINK,OPER} */
 #define ED_NEW 		0x00		/* unused, no dummy td */
 #define ED_UNLINK 	0x01		/* dummy td, maybe linked to hc */
 #define ED_OPER		0x02		/* dummy td, _is_ linked to hc */
 #define ED_URB_DEL  	0x08		/* for unlinking; masked in */
 
-	__u8			type; 
-	__u16			last_iso;
+	/* HC may see EDs on rm_list until next frame (frame_no == tick) */
+	u16			tick;
 	struct ed		*ed_rm_list;
-
-	dma_addr_t		dma;			/* addr of ED */
 } __attribute__ ((aligned(16)));
 
 #define ED_MASK	((u32)~0x0f)		/* strip hw status in low addr bits */
@@ -335,7 +342,7 @@
 	struct ohci_hcca	*hcca;
 	dma_addr_t		hcca_dma;
 
-	struct ed		*ed_rm_list [2];	/* to be removed */
+	struct ed		*ed_rm_list;		/* to be removed */
 
 	struct ed		*ed_bulktail;		/* last in bulk list */
 	struct ed		*ed_controltail;	/* last in ctrl list */
</pre></body></html>