USB: EHCI: split needs_rescan into two flags

This patch (as1662) does some more QH-related cleanup in ehci-hcd.
The qh->needs_rescan flag is currently used for two different
purposes; the patch replaces it with two separate flags for greater
clarity: qh->dequeue_during_giveback indicates that a completion
handler dequeued an URB (implying that a rescan is needed), and
qh->exception indicates that the QH is in an exceptional state
requiring an unlink (either it encountered an I/O error or an unlink
was requested).

The new flags get set where the dequeue, exception, or unlink request
occurred, rather than where the unlink is started.  This is so that in
the future, if we need to, we will be able to tell apart unlinks that
truly were required from those that were carried out merely because
the QH wasn't being used.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 0c3314c41363cc18b523f77cc5761faef6eba251..93f4cd75845e5f1b9fcc91fbbf8039a6e06f5da1 100644 (file)
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -903,11 +903,14 @@ static int ehci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
                qh = (struct ehci_qh *) urb->hcpriv;
                if (!qh)
                        break;
+               qh->exception = 1;
                switch (qh->qh_state) {
                case QH_STATE_LINKED:
-               case QH_STATE_COMPLETING:
                        start_unlink_async(ehci, qh);
                        break;
+               case QH_STATE_COMPLETING:
+                       qh->dequeue_during_giveback = 1;
+                       break;
                case QH_STATE_UNLINK:
                case QH_STATE_UNLINK_WAIT:
                        /* already started */
@@ -923,11 +926,14 @@ static int ehci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
                qh = (struct ehci_qh *) urb->hcpriv;
                if (!qh)
                        break;
+               qh->exception = 1;
                switch (qh->qh_state) {
                case QH_STATE_LINKED:
-               case QH_STATE_COMPLETING:
                        start_unlink_intr(ehci, qh);
                        break;
+               case QH_STATE_COMPLETING:
+                       qh->dequeue_during_giveback = 1;
+                       break;
                case QH_STATE_IDLE:
                        qh_completions (ehci, qh);
                        break;
@@ -984,6 +990,7 @@ rescan:
                goto done;
        }
 
+       qh->exception = 1;
        if (ehci->rh_state < EHCI_RH_RUNNING)
                qh->qh_state = QH_STATE_IDLE;
        switch (qh->qh_state) {
@@ -1052,13 +1059,12 @@ 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 ||
-                               qh->qh_state == QH_STATE_COMPLETING) {
-
+               } else {
                        /* The toggle value in the QH can't be updated
                         * while the QH is active.  Unlink it now;
                         * re-linking will call qh_refresh().
                         */
+                       qh->exception = 1;
                        if (eptype == USB_ENDPOINT_XFER_BULK)
                                start_unlink_async(ehci, qh);
                        else

diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index c95f60d43b1a9d3603978ae0d1c21c69b0598dcb..fca741dbf9dff4e361f51a7018382dca7c2bf786 100644 (file)
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -322,7 +322,7 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
  rescan:
        last = NULL;
        last_status = -EINPROGRESS;
-       qh->needs_rescan = 0;
+       qh->dequeue_during_giveback = 0;
 
        /* remove de-activated QTDs from front of queue.
         * after faults (including short reads), cleanup this urb
@@ -518,18 +518,12 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
        }
 
        /* Do we need to rescan for URBs dequeued during a giveback? */
-       if (unlikely(qh->needs_rescan)) {
+       if (unlikely(qh->dequeue_during_giveback)) {
                /* 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;
+               /* Otherwise the caller must unlink the QH. */
        }
 
        /* restore original state; caller must unlink or relink */
@@ -538,29 +532,23 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
        /* be sure the hardware's done with the qh before refreshing
         * it after fault cleanup, or recovering from silicon wrongly
         * overlaying the dummy qtd (which reduces DMA chatter).
+        *
+        * We won't refresh a QH that's linked (after the HC
+        * stopped the queue).  That avoids a race:
+        *  - HC reads first part of QH;
+        *  - CPU updates that first part and the token;
+        *  - HC reads rest of that QH, including token
+        * Result:  HC gets an inconsistent image, and then
+        * DMAs to/from the wrong memory (corrupting it).
+        *
+        * That should be rare for interrupt transfers,
+        * except maybe high bandwidth ...
         */
-       if (stopped != 0 || hw->hw_qtd_next == EHCI_LIST_END(ehci)) {
-               if (state == QH_STATE_LINKED) {
-                       /*
-                        * We won't refresh a QH that's linked (after the HC
-                        * stopped the queue).  That avoids a race:
-                        *  - HC reads first part of QH;
-                        *  - CPU updates that first part and the token;
-                        *  - HC reads rest of that QH, including token
-                        * Result:  HC gets an inconsistent image, and then
-                        * DMAs to/from the wrong memory (corrupting it).
-                        *
-                        * That should be rare for interrupt transfers,
-                        * except maybe high bandwidth ...
-                        *
-                        * Therefore tell the caller to start an unlink.
-                        */
-                       qh->needs_rescan = 1;
-               }
-               /* otherwise, unlink already started */
-       }
+       if (stopped != 0 || hw->hw_qtd_next == EHCI_LIST_END(ehci))
+               qh->exception = 1;
 
-       return qh->needs_rescan;
+       /* Let the caller know if the QH needs to be unlinked. */
+       return qh->exception;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1002,8 +990,9 @@ static void qh_link_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
        head->qh_next.qh = qh;
        head->hw->hw_next = dma;
 
-       qh->xacterrs = 0;
        qh->qh_state = QH_STATE_LINKED;
+       qh->xacterrs = 0;
+       qh->exception = 0;
        /* qtd completions reported later by interrupt */
 
        enable_async(ehci);
@@ -1317,16 +1306,9 @@ static void unlink_empty_async_suspended(struct ehci_hcd *ehci)
 
 static void start_unlink_async(struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
-       /*
-        * 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;
+       /* If the QH isn't linked then there's nothing we can do. */
+       if (qh->qh_state != QH_STATE_LINKED)
                return;
-       }
 
        single_unlink_async(ehci, qh);
        start_iaa_cycle(ehci, false);

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 5c82bbab9a48976cf1ba27dfadb0f774f44e2a1f..e7a2dbd27b1ee484a8dce46fe8aa105e0f632095 100644 (file)
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -539,6 +539,7 @@ static void qh_link_periodic(struct ehci_hcd *ehci, struct ehci_qh *qh)
        }
        qh->qh_state = QH_STATE_LINKED;
        qh->xacterrs = 0;
+       qh->exception = 0;
 
        /* update per-qh bandwidth for usbfs */
        ehci_to_hcd(ehci)->self.bandwidth_allocated += qh->period
@@ -602,15 +603,9 @@ static void qh_unlink_periodic(struct ehci_hcd *ehci, struct ehci_qh *qh)
 
 static void start_unlink_intr(struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
-       /* 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;
+       /* If the QH isn't linked then there's nothing we can do. */
+       if (qh->qh_state != QH_STATE_LINKED)
                return;
-       }
 
        qh_unlink_periodic (ehci, qh);
 

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 36c3a82105953ba2433d12845e7e02a293fa6d35..6815209511aa94e355026bad18f06ac24392d2bf 100644 (file)
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -384,7 +384,6 @@ struct ehci_qh {
 
        unsigned                unlink_cycle;
 
-       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 */
@@ -407,6 +406,9 @@ struct ehci_qh {
        struct usb_device       *dev;           /* access to TT */
        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 */
 };
 
 /*-------------------------------------------------------------------------*/