aboutsummaryrefslogtreecommitdiff
path: root/drivers/usb
diff options
context:
space:
mode:
authorAlan Stern <stern@rowland.harvard.edu>2009-08-19 12:22:06 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2009-09-23 06:46:37 -0700
commit3a44494e233c0fdd818d485cfea8998500543589 (patch)
tree04b66268a2efc1236d3f9e477a04dc2d3ec87582 /drivers/usb
parent04c4ab17c7c39603c5017bee20d3b8ccb2f19816 (diff)
USB: EHCI: rescan the queue after an unlink
This patch (as1280) fixes an obscure bug in ehci-hcd's dequeuing logic for async URBs. If a later URB is unlinked and the completion routine unlinks an earlier URB, then the earlier URB won't be given back in a timely manner because the endpoint queue isn't rescanned as it should be. Similar bugs occur if an endpoint is reset or a halt is cleared while a completion routine is running, because the subroutines don't test for the COMPLETING state. All these problems are solved by adding a new needs_rescan flag to the ehci_qh structure. If the flag is set while scanning through an idle QH, the scan will be repeated. If the QH isn't idle then an unlink cycle will be initiated, and the proper action will be taken when it becomes idle. Also, an unnecessary test is removed from qh_link_async(): That routine is never called if the QH's state isn't IDLE. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> CC: David Brownell <david-b@pacbell.net> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'drivers/usb')
-rw-r--r--drivers/usb/host/ehci-hcd.c18
-rw-r--r--drivers/usb/host/ehci-q.c43
-rw-r--r--drivers/usb/host/ehci.h1
3 files changed, 50 insertions, 12 deletions
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 6887aac5e73..d7e85b6231b 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -863,12 +863,18 @@ static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
if (!HC_IS_RUNNING(ehci_to_hcd(ehci)->state) && ehci->reclaim)
end_unlink_async(ehci);
- /* if it's not linked then there's nothing to do */
- if (qh->qh_state != QH_STATE_LINKED)
- ;
+ /* If the QH isn't linked then there's nothing we can do
+ * unless we were called during a giveback, in which case
+ * qh_completions() has to deal with it.
+ */
+ if (qh->qh_state != QH_STATE_LINKED) {
+ if (qh->qh_state == QH_STATE_COMPLETING)
+ qh->needs_rescan = 1;
+ return;
+ }
/* defer till later if busy */
- else if (ehci->reclaim) {
+ if (ehci->reclaim) {
struct ehci_qh *last;
for (last = ehci->reclaim;
@@ -1001,6 +1007,7 @@ rescan:
qh->qh_state = QH_STATE_IDLE;
switch (qh->qh_state) {
case QH_STATE_LINKED:
+ case QH_STATE_COMPLETING:
for (tmp = ehci->async->qh_next.qh;
tmp && tmp != qh;
tmp = tmp->qh_next.qh)
@@ -1065,7 +1072,8 @@ ehci_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep)
usb_settoggle(qh->dev, epnum, is_out, 0);
if (!list_empty(&qh->qtd_list)) {
WARN_ONCE(1, "clear_halt for a busy endpoint\n");
- } else if (qh->qh_state == QH_STATE_LINKED) {
+ } else if (qh->qh_state == QH_STATE_LINKED ||
+ qh->qh_state == QH_STATE_COMPLETING) {
/* The toggle value in the QH can't be updated
* while the QH is active. Unlink it now;
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 377ed530b92..57a84795c43 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -310,13 +310,13 @@ static int qh_schedule (struct ehci_hcd *ehci, struct ehci_qh *qh);
static unsigned
qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
{
- struct ehci_qtd *last = NULL, *end = qh->dummy;
+ struct ehci_qtd *last, *end = qh->dummy;
struct list_head *entry, *tmp;
- int last_status = -EINPROGRESS;
+ int last_status;
int stopped;
unsigned count = 0;
u8 state;
- __le32 halt = HALT_BIT(ehci);
+ const __le32 halt = HALT_BIT(ehci);
struct ehci_qh_hw *hw = qh->hw;
if (unlikely (list_empty (&qh->qtd_list)))
@@ -327,11 +327,20 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
* they add urbs to this qh's queue or mark them for unlinking.
*
* NOTE: unlinking expects to be done in queue order.
+ *
+ * It's a bug for qh->qh_state to be anything other than
+ * QH_STATE_IDLE, unless our caller is scan_async() or
+ * scan_periodic().
*/
state = qh->qh_state;
qh->qh_state = QH_STATE_COMPLETING;
stopped = (state == QH_STATE_IDLE);
+ rescan:
+ last = NULL;
+ last_status = -EINPROGRESS;
+ qh->needs_rescan = 0;
+
/* remove de-activated QTDs from front of queue.
* after faults (including short reads), cleanup this urb
* then let the queue advance.
@@ -507,6 +516,21 @@ halt:
ehci_qtd_free (ehci, last);
}
+ /* Do we need to rescan for URBs dequeued during a giveback? */
+ if (unlikely(qh->needs_rescan)) {
+ /* If the QH is already unlinked, do the rescan now. */
+ if (state == QH_STATE_IDLE)
+ goto rescan;
+
+ /* Otherwise we have to wait until the QH is fully unlinked.
+ * Our caller will start an unlink if qh->needs_rescan is
+ * set. But if an unlink has already started, nothing needs
+ * to be done.
+ */
+ if (state != QH_STATE_LINKED)
+ qh->needs_rescan = 0;
+ }
+
/* restore original state; caller must unlink or relink */
qh->qh_state = state;
@@ -535,8 +559,10 @@ halt:
& hw->hw_info2) != 0) {
intr_deschedule (ehci, qh);
(void) qh_schedule (ehci, qh);
- } else
- unlink_async (ehci, qh);
+ } else {
+ /* Tell the caller to start an unlink */
+ qh->needs_rescan = 1;
+ }
break;
/* otherwise, unlink already started */
}
@@ -916,6 +942,8 @@ static void qh_link_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
if (unlikely(qh->clearing_tt))
return;
+ WARN_ON(qh->qh_state != QH_STATE_IDLE);
+
/* (re)start the async schedule? */
head = ehci->async;
timer_action_done (ehci, TIMER_ASYNC_OFF);
@@ -934,8 +962,7 @@ static void qh_link_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
}
/* clear halt and/or toggle; and maybe recover from silicon quirk */
- if (qh->qh_state == QH_STATE_IDLE)
- qh_refresh (ehci, qh);
+ qh_refresh(ehci, qh);
/* splice right after start */
qh->qh_next = head->qh_next;
@@ -1220,6 +1247,8 @@ rescan:
qh = qh_get (qh);
qh->stamp = ehci->stamp;
temp = qh_completions (ehci, qh);
+ if (qh->needs_rescan)
+ unlink_async(ehci, qh);
qh_put (qh);
if (temp != 0) {
goto rescan;
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index ec3dba6b8e4..064e76821ff 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -341,6 +341,7 @@ struct ehci_qh {
u32 refcount;
unsigned stamp;
+ u8 needs_rescan; /* Dequeue during giveback */
u8 qh_state;
#define QH_STATE_LINKED 1 /* HC sees this */
#define QH_STATE_UNLINK 2 /* HC may still see this */