USB: EHCI: changes related to qh_refresh()
authorAlan Stern <stern@rowland.harvard.edu>
Fri, 22 Mar 2013 17:30:43 +0000 (13:30 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 25 Mar 2013 20:35:05 +0000 (13:35 -0700)
This patch (as1638) makes several changes to the ehci-hcd driver, all
related to the qh_refresh() function.  This function must be called
whenever an idle QH gets linked back into either the async or the
periodic schedule.

Change a BUG_ON() in the qh_update routine to a WARN_ON().
Since this code runs in atomic context, a BUG_ON() would
immediately freeze the whole system.

Remove two unneeded calls to qh_refresh(), one when a QH is
initialized and one when a QH becomes idle.  Adjust the
adjacent comments accordingly.

Move the qh_refresh() and qh_link_periodic() calls for new
interrupt URBs to after the new TDs have been added.

As a result of the previous two changes, qh_refresh() is never
called when the qtd_list is empty.  The corresponding check in
qh_refresh() can be removed, along with an indentation level.

These changes should not cause any alteration of behavior.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/host/ehci-q.c
drivers/usb/host/ehci-sched.c

index 23d13690428591b3c71d6c10bc9b29a4ffd0c973..b824cb6748985352917ad0f4f34727781dbd2ebf 100644 (file)
@@ -90,7 +90,7 @@ qh_update (struct ehci_hcd *ehci, struct ehci_qh *qh, struct ehci_qtd *qtd)
        struct ehci_qh_hw *hw = qh->hw;
 
        /* writes to an active overlay are unsafe */
-       BUG_ON(qh->qh_state != QH_STATE_IDLE);
+       WARN_ON(qh->qh_state != QH_STATE_IDLE);
 
        hw->hw_qtd_next = QTD_NEXT(ehci, qtd->qtd_dma);
        hw->hw_alt_next = EHCI_LIST_END(ehci);
@@ -123,26 +123,19 @@ qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
        struct ehci_qtd *qtd;
 
-       if (list_empty (&qh->qtd_list))
-               qtd = qh->dummy;
-       else {
-               qtd = list_entry (qh->qtd_list.next,
-                               struct ehci_qtd, qtd_list);
-               /*
-                * first qtd may already be partially processed.
-                * If we come here during unlink, the QH overlay region
-                * might have reference to the just unlinked qtd. The
-                * qtd is updated in qh_completions(). Update the QH
-                * overlay here.
-                */
-               if (qh->hw->hw_token & ACTIVE_BIT(ehci)) {
-                       qh->hw->hw_qtd_next = qtd->hw_next;
-                       qtd = NULL;
-               }
-       }
+       qtd = list_entry(qh->qtd_list.next, struct ehci_qtd, qtd_list);
 
-       if (qtd)
-               qh_update (ehci, qh, qtd);
+       /*
+        * first qtd may already be partially processed.
+        * If we come here during unlink, the QH overlay region
+        * might have reference to the just unlinked qtd. The
+        * qtd is updated in qh_completions(). Update the QH
+        * overlay here.
+        */
+       if (qh->hw->hw_token & ACTIVE_BIT(ehci))
+               qh->hw->hw_qtd_next = qtd->hw_next;
+       else
+               qh_update(ehci, qh, qtd);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -553,12 +546,9 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
         * overlaying the dummy qtd (which reduces DMA chatter).
         */
        if (stopped != 0 || hw->hw_qtd_next == EHCI_LIST_END(ehci)) {
-               switch (state) {
-               case QH_STATE_IDLE:
-                       qh_refresh(ehci, qh);
-                       break;
-               case QH_STATE_LINKED:
-                       /* We won't refresh a QH that's linked (after the HC
+               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;
@@ -568,13 +558,12 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
                         *
                         * That should be rare for interrupt transfers,
                         * except maybe high bandwidth ...
+                        *
+                        * Therefore tell the caller to start an unlink.
                         */
-
-                       /* Tell the caller to start an unlink */
                        qh->needs_rescan = 1;
-                       break;
-               /* otherwise, unlink already started */
                }
+               /* otherwise, unlink already started */
        }
 
        return count;
@@ -957,14 +946,13 @@ done:
 
        /* NOTE:  if (PIPE_INTERRUPT) { scheduler sets s-mask } */
 
-       /* init as live, toggle clear, advance to dummy */
+       /* init as live, toggle clear */
        qh->qh_state = QH_STATE_IDLE;
        hw = qh->hw;
        hw->hw_info1 = cpu_to_hc32(ehci, info1);
        hw->hw_info2 = cpu_to_hc32(ehci, info2);
        qh->is_out = !is_input;
        usb_settoggle (urb->dev, usb_pipeendpoint (urb->pipe), !is_input, 1);
-       qh_refresh (ehci, qh);
        return qh;
 }
 
index b476daf49f6f3c6cf226b920f5c8ff01ca5d23aa..66259dc7822e90145f7e286ca8ff5213a131829f 100644 (file)
@@ -792,7 +792,6 @@ static int qh_schedule(struct ehci_hcd *ehci, struct ehci_qh *qh)
        unsigned        frame;          /* 0..(qh->period - 1), or NO_FRAME */
        struct ehci_qh_hw       *hw = qh->hw;
 
-       qh_refresh(ehci, qh);
        hw->hw_next = EHCI_LIST_END(ehci);
        frame = qh->start;
 
@@ -844,8 +843,6 @@ static int qh_schedule(struct ehci_hcd *ehci, struct ehci_qh *qh)
        } else
                ehci_dbg (ehci, "reused qh %p schedule\n", qh);
 
-       /* stuff into the periodic schedule */
-       qh_link_periodic(ehci, qh);
 done:
        return status;
 }
@@ -891,6 +888,12 @@ static int intr_submit (
        qh = qh_append_tds(ehci, urb, qtd_list, epnum, &urb->ep->hcpriv);
        BUG_ON (qh == NULL);
 
+       /* stuff into the periodic schedule */
+       if (qh->qh_state == QH_STATE_IDLE) {
+               qh_refresh(ehci, qh);
+               qh_link_periodic(ehci, qh);
+       }
+
        /* ... update usbfs periodic stats */
        ehci_to_hcd(ehci)->self.bandwidth_int_reqs++;