Re: Help needed for EHCI problem: removing an active bulk-in QH

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael:

Here at last is the final version of the patch, or something very close 
to it.  This should be applied on top of the other EHCI patches, but 
not the quick-and-dirty fix, which it replaces.

Please test it and let me know the results.

Alan Stern



Index: usb-4.3/drivers/usb/host/ehci.h
===================================================================
--- usb-4.3.orig/drivers/usb/host/ehci.h
+++ usb-4.3/drivers/usb/host/ehci.h
@@ -110,6 +110,7 @@ enum ehci_hrtimer_event {
 	EHCI_HRTIMER_POLL_DEAD,		/* Wait for dead controller to stop */
 	EHCI_HRTIMER_UNLINK_INTR,	/* Wait for interrupt QH unlink */
 	EHCI_HRTIMER_FREE_ITDS,		/* Wait for unused iTDs and siTDs */
+	EHCI_HRTIMER_ACTIVE_UNLINK,	/* Wait while unlinking an active QH */
 	EHCI_HRTIMER_START_UNLINK_INTR, /* Unlink empty interrupt QHs */
 	EHCI_HRTIMER_ASYNC_UNLINKS,	/* Unlink empty async QHs */
 	EHCI_HRTIMER_IAA_WATCHDOG,	/* Handle lost IAA interrupts */
@@ -156,6 +157,8 @@ struct ehci_hcd {			/* one per controlle
 	struct list_head	async_idle;
 	unsigned		async_unlink_cycle;
 	unsigned		async_count;	/* async activity count */
+	__hc32			old_current;	/* Test for QH becoming */
+	__hc32			old_token;	/*  inactive during unlink */
 
 	/* periodic schedule support */
 #define	DEFAULT_I_TDPS		1024		/* some HCs can do less */
@@ -432,13 +435,19 @@ struct ehci_qh {
 	u8			xacterrs;	/* XactErr retry counter */
 #define	QH_XACTERR_MAX		32		/* XactErr retry limit */
 
+	u8			unlink_reason;
+#define QH_UNLINK_HALTED	0x01		/* Halt flag is set */
+#define QH_UNLINK_SHORT_READ	0x02		/* Recover from a short read */
+#define QH_UNLINK_DUMMY_OVERLAY	0x04		/* QH overlayed the dummy TD */
+#define QH_UNLINK_SHUTDOWN	0x08		/* The HC isn't running */
+#define QH_UNLINK_END_OF_QUEUE	0x10		/* Reached end of the queue */
+#define QH_UNLINK_REQUESTED	0x20		/* Disable, reset, or dequeue */
+
 	u8			gap_uf;		/* uframes split/csplit gap */
 
 	unsigned		is_out:1;	/* bulk or intr OUT */
 	unsigned		clearing_tt:1;	/* Clear-TT-Buf in progress */
 	unsigned		dequeue_during_giveback:1;
-	unsigned		exception:1;	/* got a fault, or an unlink
-						   was requested */
 	unsigned		should_be_inactive:1;
 };
 
Index: usb-4.3/drivers/usb/host/ehci-hcd.c
===================================================================
--- usb-4.3.orig/drivers/usb/host/ehci-hcd.c
+++ usb-4.3/drivers/usb/host/ehci-hcd.c
@@ -306,6 +306,7 @@ static void ehci_quiesce (struct ehci_hc
 
 /*-------------------------------------------------------------------------*/
 
+static void end_iaa_cycle(struct ehci_hcd *ehci);
 static void end_unlink_async(struct ehci_hcd *ehci);
 static void unlink_empty_async(struct ehci_hcd *ehci);
 static void unlink_empty_async_suspended(struct ehci_hcd *ehci);
@@ -565,6 +566,9 @@ static int ehci_init(struct usb_hcd *hcd
 	/* Accept arbitrarily long scatter-gather lists */
 	if (!(hcd->driver->flags & HCD_LOCAL_MEM))
 		hcd->self.sg_tablesize = ~0;
+
+	/* Prepare for unlinking active QHs */
+	ehci->old_current = ~0;
 	return 0;
 }
 
@@ -758,7 +762,7 @@ static irqreturn_t ehci_irq (struct usb_
 			ehci_dbg(ehci, "IAA with IAAD still set?\n");
 		if (ehci->iaa_in_progress)
 			COUNT(ehci->stats.iaa);
-		end_unlink_async(ehci);
+		end_iaa_cycle(ehci);
 	}
 
 	/* remote wakeup [4.3.1] */
@@ -911,7 +915,7 @@ static int ehci_urb_dequeue(struct usb_h
 		 */
 	} else {
 		qh = (struct ehci_qh *) urb->hcpriv;
-		qh->exception = 1;
+		qh->unlink_reason |= QH_UNLINK_REQUESTED;
 		switch (qh->qh_state) {
 		case QH_STATE_LINKED:
 			if (usb_pipetype(urb->pipe) == PIPE_INTERRUPT)
@@ -972,7 +976,7 @@ rescan:
 		goto done;
 	}
 
-	qh->exception = 1;
+	qh->unlink_reason |= QH_UNLINK_REQUESTED;
 	switch (qh->qh_state) {
 	case QH_STATE_LINKED:
 		WARN_ON(!list_empty(&qh->qtd_list));
@@ -1042,7 +1046,7 @@ ehci_endpoint_reset(struct usb_hcd *hcd,
 			 * re-linking will call qh_refresh().
 			 */
 			usb_settoggle(qh->ps.udev, epnum, is_out, 0);
-			qh->exception = 1;
+			qh->unlink_reason |= QH_UNLINK_REQUESTED;
 			if (eptype == USB_ENDPOINT_XFER_BULK)
 				start_unlink_async(ehci, qh);
 			else
Index: usb-4.3/drivers/usb/host/ehci-q.c
===================================================================
--- usb-4.3.orig/drivers/usb/host/ehci-q.c
+++ usb-4.3/drivers/usb/host/ehci-q.c
@@ -394,6 +394,7 @@ qh_completions (struct ehci_hcd *ehci, s
 					goto retry_xacterr;
 				}
 				stopped = 1;
+				qh->unlink_reason |= QH_UNLINK_HALTED;
 
 			/* magic dummy for some short reads; qh won't advance.
 			 * that silicon quirk can kick in with this dummy too.
@@ -408,6 +409,7 @@ qh_completions (struct ehci_hcd *ehci, s
 					&& !(qtd->hw_alt_next
 						& EHCI_LIST_END(ehci))) {
 				stopped = 1;
+				qh->unlink_reason |= QH_UNLINK_SHORT_READ;
 			}
 
 		/* stop scanning when we reach qtds the hc is using */
@@ -420,8 +422,10 @@ qh_completions (struct ehci_hcd *ehci, s
 			stopped = 1;
 
 			/* cancel everything if we halt, suspend, etc */
-			if (ehci->rh_state < EHCI_RH_RUNNING)
+			if (ehci->rh_state < EHCI_RH_RUNNING) {
 				last_status = -ESHUTDOWN;
+				qh->unlink_reason |= QH_UNLINK_SHUTDOWN;
+			}
 
 			/* this qtd is active; skip it unless a previous qtd
 			 * for its urb faulted, or its urb was canceled.
@@ -538,10 +542,10 @@ qh_completions (struct ehci_hcd *ehci, s
 	 * except maybe high bandwidth ...
 	 */
 	if (stopped != 0 || hw->hw_qtd_next == EHCI_LIST_END(ehci))
-		qh->exception = 1;
+		qh->unlink_reason |= QH_UNLINK_DUMMY_OVERLAY;
 
 	/* Let the caller know if the QH needs to be unlinked. */
-	return qh->exception;
+	return qh->unlink_reason;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1003,7 +1007,7 @@ static void qh_link_async (struct ehci_h
 
 	qh->qh_state = QH_STATE_LINKED;
 	qh->xacterrs = 0;
-	qh->exception = 0;
+	qh->unlink_reason = 0;
 	/* qtd completions reported later by interrupt */
 
 	enable_async(ehci);
@@ -1301,13 +1305,8 @@ static void start_iaa_cycle(struct ehci_
 	}
 }
 
-/* the async qh for the qtds being unlinked are now gone from the HC */
-
-static void end_unlink_async(struct ehci_hcd *ehci)
+static void end_iaa_cycle(struct ehci_hcd *ehci)
 {
-	struct ehci_qh		*qh;
-	bool			early_exit;
-
 	if (ehci->has_synopsys_hc_bug)
 		ehci_writel(ehci, (u32) ehci->async->qh_dma,
 			    &ehci->regs->async_next);
@@ -1315,6 +1314,16 @@ static void end_unlink_async(struct ehci
 	/* The current IAA cycle has ended */
 	ehci->iaa_in_progress = false;
 
+	end_unlink_async(ehci);
+}
+
+/* See if the async qh for the qtds being unlinked are now gone from the HC */
+
+static void end_unlink_async(struct ehci_hcd *ehci)
+{
+	struct ehci_qh		*qh;
+	bool			early_exit;
+
 	if (list_empty(&ehci->async_unlink))
 		return;
 	qh = list_first_entry(&ehci->async_unlink, struct ehci_qh,
@@ -1335,14 +1344,60 @@ static void end_unlink_async(struct ehci
 	 * after the IAA interrupt occurs.  In self-defense, always go
 	 * through two IAA cycles for each QH.
 	 */
-	else if (qh->qh_state == QH_STATE_UNLINK_WAIT) {
+	else if (qh->qh_state == QH_STATE_UNLINK) {
+		/*
+		 * Second IAA cycle has finished.  Process only the first
+		 * waiting QH (NVIDIA (?) bug).
+		 */
+		list_move_tail(&qh->unlink_node, &ehci->async_idle);
+	}
+
+	/*
+	 * AMD/ATI (?) bug: The HC can continue to use an active QH long
+	 * after the IAA interrupt occurs.  To prevent problems, QHs that
+	 * may still be active will wait until 2 ms have passed with no
+	 * change to the hw_current and hw_token fields (this delay occurs
+	 * between the two IAA cycles).
+	 *
+	 * The EHCI spec (4.8.2) says that active QHs must not be removed
+	 * from the async schedule and recommends waiting until the QH
+	 * goes inactive.  This is ridiculous because the QH will _never_
+	 * become inactive if the endpoint NAKs indefinitely.
+	 */
+
+	/* Some reasons for unlinking guarantee the QH can't be active */
+	else if (qh->unlink_reason & (QH_UNLINK_HALTED |
+			QH_UNLINK_SHORT_READ | QH_UNLINK_DUMMY_OVERLAY))
+		goto DelayDone;
+
+	/* The QH can't be active if the queue was and still is empty... */
+	else if	((qh->unlink_reason & QH_UNLINK_END_OF_QUEUE) &&
+			list_empty(&qh->qtd_list))
+		goto DelayDone;
+
+	/* ... or if the QH has halted */
+	else if	(qh->hw->hw_token & cpu_to_hc32(ehci, QTD_STS_HALT))
+		goto DelayDone;
+
+	/* Otherwise we have to wait until the QH stops changing */
+	else {
+		__hc32		qh_current, qh_token;
+
+		qh_current = qh->hw->hw_current;
+		qh_token = qh->hw->hw_token;
+		if (qh_current != ehci->old_current ||
+				qh_token != ehci->old_token) {
+			ehci->old_current = qh_current;
+			ehci->old_token = qh_token;
+			ehci_enable_event(ehci,
+					EHCI_HRTIMER_ACTIVE_UNLINK, true);
+			return;
+		}
+ DelayDone:
 		qh->qh_state = QH_STATE_UNLINK;
 		early_exit = true;
 	}
-
-	/* Otherwise process only the first waiting QH (NVIDIA bug?) */
-	else
-		list_move_tail(&qh->unlink_node, &ehci->async_idle);
+	ehci->old_current = ~0;		/* Prepare for next QH */
 
 	/* Start a new IAA cycle if any QHs are waiting for it */
 	if (!list_empty(&ehci->async_unlink))
@@ -1395,6 +1450,7 @@ static void unlink_empty_async(struct eh
 
 	/* If nothing else is being unlinked, unlink the last empty QH */
 	if (list_empty(&ehci->async_unlink) && qh_to_unlink) {
+		qh_to_unlink->unlink_reason |= QH_UNLINK_END_OF_QUEUE;
 		start_unlink_async(ehci, qh_to_unlink);
 		--count;
 	}
Index: usb-4.3/drivers/usb/host/ehci-sched.c
===================================================================
--- usb-4.3.orig/drivers/usb/host/ehci-sched.c
+++ usb-4.3/drivers/usb/host/ehci-sched.c
@@ -595,7 +595,7 @@ static void qh_link_periodic(struct ehci
 	}
 	qh->qh_state = QH_STATE_LINKED;
 	qh->xacterrs = 0;
-	qh->exception = 0;
+	qh->unlink_reason = 0;
 
 	/* update per-qh bandwidth for debugfs */
 	ehci_to_hcd(ehci)->self.bandwidth_allocated += qh->ps.bw_period
Index: usb-4.3/drivers/usb/host/ehci-timer.c
===================================================================
--- usb-4.3.orig/drivers/usb/host/ehci-timer.c
+++ usb-4.3/drivers/usb/host/ehci-timer.c
@@ -72,6 +72,7 @@ static unsigned event_delays_ns[] = {
 	1 * NSEC_PER_MSEC,	/* EHCI_HRTIMER_POLL_DEAD */
 	1125 * NSEC_PER_USEC,	/* EHCI_HRTIMER_UNLINK_INTR */
 	2 * NSEC_PER_MSEC,	/* EHCI_HRTIMER_FREE_ITDS */
+	2 * NSEC_PER_MSEC,	/* EHCI_HRTIMER_ACTIVE_UNLINK */
 	5 * NSEC_PER_MSEC,	/* EHCI_HRTIMER_START_UNLINK_INTR */
 	6 * NSEC_PER_MSEC,	/* EHCI_HRTIMER_ASYNC_UNLINKS */
 	10 * NSEC_PER_MSEC,	/* EHCI_HRTIMER_IAA_WATCHDOG */
@@ -237,6 +238,7 @@ static void ehci_handle_start_intr_unlin
 				ehci->intr_unlink_wait_cycle))
 			break;
 		list_del_init(&qh->unlink_node);
+		qh->unlink_reason |= QH_UNLINK_END_OF_QUEUE;
 		start_unlink_intr(ehci, qh);
 	}
 
@@ -360,7 +362,7 @@ static void ehci_iaa_watchdog(struct ehc
 	}
 
 	ehci_dbg(ehci, "IAA watchdog: status %x cmd %x\n", status, cmd);
-	end_unlink_async(ehci);
+	end_iaa_cycle(ehci);
 }
 
 
@@ -394,6 +396,7 @@ static void (*event_handlers[])(struct e
 	ehci_handle_controller_death,	/* EHCI_HRTIMER_POLL_DEAD */
 	ehci_handle_intr_unlinks,	/* EHCI_HRTIMER_UNLINK_INTR */
 	end_free_itds,			/* EHCI_HRTIMER_FREE_ITDS */
+	end_unlink_async,		/* EHCI_HRTIMER_ACTIVE_UNLINK */
 	ehci_handle_start_intr_unlinks,	/* EHCI_HRTIMER_START_UNLINK_INTR */
 	unlink_empty_async,		/* EHCI_HRTIMER_ASYNC_UNLINKS */
 	ehci_iaa_watchdog,		/* EHCI_HRTIMER_IAA_WATCHDOG */

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux