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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
