UHCI: Fix problem caused by lack of terminating QH

This patch (as871) fixes a problem introduced by an earlier change.
It turns out that some systems really do need to have a terminating
skeleton QH present whenever FSBR is on.  I don't know any way to tell
which systems do need it and which don't; the easiest answer is to
have it there always.

This fixes the NumLock-hang bug reported by Jiri Slaby.

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

diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index 8d24d3dc0a6186173980988e010184b763e6c646..1497371583b983a212ca15db917a628d9ca5d8e6 100644 (file)
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -145,7 +145,8 @@ static int uhci_show_urbp(struct urb_priv *urbp, char *buf, int len, int space)
        return out - buf;
 }
 
-static int uhci_show_qh(struct uhci_qh *qh, char *buf, int len, int space)
+static int uhci_show_qh(struct uhci_hcd *uhci,
+               struct uhci_qh *qh, char *buf, int len, int space)
 {
        char *out = buf;
        int i, nurbs;
@@ -190,6 +191,9 @@ static int uhci_show_qh(struct uhci_qh *qh, char *buf, int len, int space)
 
        if (list_empty(&qh->queue)) {
                out += sprintf(out, "%*s  queue is empty\n", space, "");
+               if (qh == uhci->skel_async_qh)
+                       out += uhci_show_td(uhci->term_td, out,
+                                       len - (out - buf), 0);
        } else {
                struct urb_priv *urbp = list_entry(qh->queue.next,
                                struct urb_priv, node);
@@ -343,6 +347,7 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
        struct list_head *tmp, *head;
        int nframes, nerrs;
        __le32 link;
+       __le32 fsbr_link;
 
        static const char * const qh_names[] = {
                "unlink", "iso", "int128", "int64", "int32", "int16",
@@ -424,21 +429,22 @@ check_link:
 
        out += sprintf(out, "Skeleton QHs\n");
 
+       fsbr_link = 0;
        for (i = 0; i < UHCI_NUM_SKELQH; ++i) {
                int cnt = 0;
-               __le32 fsbr_link = 0;
 
                qh = uhci->skelqh[i];
                out += sprintf(out, "- skel_%s_qh\n", qh_names[i]); \
-               out += uhci_show_qh(qh, out, len - (out - buf), 4);
+               out += uhci_show_qh(uhci, qh, out, len - (out - buf), 4);
 
                /* Last QH is the Terminating QH, it's different */
                if (i == SKEL_TERM) {
                        if (qh_element(qh) != LINK_TO_TD(uhci->term_td))
                                out += sprintf(out, "    skel_term_qh element is not set to term_td!\n");
-                       if (link == LINK_TO_QH(uhci->skel_term_qh))
-                               goto check_qh_link;
-                       continue;
+                       link = fsbr_link;
+                       if (!link)
+                               link = LINK_TO_QH(uhci->skel_term_qh);
+                       goto check_qh_link;
                }
 
                head = &qh->node;
@@ -448,7 +454,7 @@ check_link:
                        qh = list_entry(tmp, struct uhci_qh, node);
                        tmp = tmp->next;
                        if (++cnt <= 10)
-                               out += uhci_show_qh(qh, out,
+                               out += uhci_show_qh(uhci, qh, out,
                                                len - (out - buf), 4);
                        if (!fsbr_link && qh->skel >= SKEL_FSBR)
                                fsbr_link = LINK_TO_QH(qh);
@@ -463,8 +469,6 @@ check_link:
                        link = LINK_TO_QH(uhci->skel_async_qh);
                else if (!uhci->fsbr_is_on)
                        ;
-               else if (fsbr_link)
-                       link = fsbr_link;
                else
                        link = LINK_TO_QH(uhci->skel_term_qh);
 check_qh_link:
@@ -573,8 +577,8 @@ static const struct file_operations uhci_debug_operations = {
 static inline void lprintk(char *buf)
 {}
 
-static inline int uhci_show_qh(struct uhci_qh *qh, char *buf,
-               int len, int space)
+static inline int uhci_show_qh(struct uhci_hcd *uhci,
+               struct uhci_qh *qh, char *buf, int len, int space)
 {
        return 0;
 }

diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 44da4334f1d63aa066b848dadaf0a9929f0bc98d..d22da26ff1675d6a4f9f8e9a4b40520ff720d443 100644 (file)
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -632,7 +632,8 @@ static int uhci_start(struct usb_hcd *hcd)
         */
        for (i = SKEL_ISO + 1; i < SKEL_ASYNC; ++i)
                uhci->skelqh[i]->link = LINK_TO_QH(uhci->skel_async_qh);
-       uhci->skel_async_qh->link = uhci->skel_term_qh->link = UHCI_PTR_TERM;
+       uhci->skel_async_qh->link = UHCI_PTR_TERM;
+       uhci->skel_term_qh->link = LINK_TO_QH(uhci->skel_term_qh);
 
        /* This dummy TD is to work around a bug in Intel PIIX controllers */
        uhci_fill_td(uhci->term_td, 0, uhci_explen(0) |

diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index f4ebdb3e488f59ad23d17ae7fc2a301b2aae0e9c..19a0cc02b9a2e2eacdea75a9e1d39315b46915e9 100644 (file)
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -45,43 +45,27 @@ static inline void uhci_clear_next_interrupt(struct uhci_hcd *uhci)
  */
 static void uhci_fsbr_on(struct uhci_hcd *uhci)
 {
-       struct uhci_qh *fsbr_qh, *lqh, *tqh;
+       struct uhci_qh *lqh;
 
+       /* The terminating skeleton QH always points back to the first
+        * FSBR QH.  Make the last async QH point to the terminating
+        * skeleton QH. */
        uhci->fsbr_is_on = 1;
        lqh = list_entry(uhci->skel_async_qh->node.prev,
                        struct uhci_qh, node);
-
-       /* Find the first FSBR QH.  Linear search through the list is
-        * acceptable because normally FSBR gets turned on as soon as
-        * one QH needs it. */
-       fsbr_qh = NULL;
-       list_for_each_entry_reverse(tqh, &uhci->skel_async_qh->node, node) {
-               if (tqh->skel < SKEL_FSBR)
-                       break;
-               fsbr_qh = tqh;
-       }
-
-       /* No FSBR QH means we must insert the terminating skeleton QH */
-       if (!fsbr_qh) {
-               uhci->skel_term_qh->link = LINK_TO_QH(uhci->skel_term_qh);
-               wmb();
-               lqh->link = uhci->skel_term_qh->link;
-
-       /* Otherwise loop the last QH to the first FSBR QH */
-       } else
-               lqh->link = LINK_TO_QH(fsbr_qh);
+       lqh->link = LINK_TO_QH(uhci->skel_term_qh);
 }
 
 static void uhci_fsbr_off(struct uhci_hcd *uhci)
 {
        struct uhci_qh *lqh;
 
+       /* Remove the link from the last async QH to the terminating
+        * skeleton QH. */
        uhci->fsbr_is_on = 0;
        lqh = list_entry(uhci->skel_async_qh->node.prev,
                        struct uhci_qh, node);
-
-       /* End the async list normally and unlink the terminating QH */
-       lqh->link = uhci->skel_term_qh->link = UHCI_PTR_TERM;
+       lqh->link = UHCI_PTR_TERM;
 }
 
 static void uhci_add_fsbr(struct uhci_hcd *uhci, struct urb *urb)
@@ -464,9 +448,8 @@ static void link_interrupt(struct uhci_hcd *uhci, struct uhci_qh *qh)
  */
 static void link_async(struct uhci_hcd *uhci, struct uhci_qh *qh)
 {
-       struct uhci_qh *pqh, *lqh;
+       struct uhci_qh *pqh;
        __le32 link_to_new_qh;
-       __le32 *extra_link = &link_to_new_qh;
 
        /* Find the predecessor QH for our new one and insert it in the list.
         * The list of QHs is expected to be short, so linear search won't
@@ -476,31 +459,17 @@ static void link_async(struct uhci_hcd *uhci, struct uhci_qh *qh)
                        break;
        }
        list_add(&qh->node, &pqh->node);
-       qh->link = pqh->link;
-
-       link_to_new_qh = LINK_TO_QH(qh);
-
-       /* If this is now the first FSBR QH, take special action */
-       if (uhci->fsbr_is_on && pqh->skel < SKEL_FSBR &&
-                       qh->skel >= SKEL_FSBR) {
-               lqh = list_entry(uhci->skel_async_qh->node.prev,
-                               struct uhci_qh, node);
-
-               /* If the new QH is also the last one, we must unlink
-                * the terminating skeleton QH and make the new QH point
-                * back to itself. */
-               if (qh == lqh) {
-                       qh->link = link_to_new_qh;
-                       extra_link = &uhci->skel_term_qh->link;
-
-               /* Otherwise the last QH must point to the new QH */
-               } else
-                       extra_link = &lqh->link;
-       }
 
        /* Link it into the schedule */
+       qh->link = pqh->link;
        wmb();
-       *extra_link = pqh->link = link_to_new_qh;
+       link_to_new_qh = LINK_TO_QH(qh);
+       pqh->link = link_to_new_qh;
+
+       /* If this is now the first FSBR QH, link the terminating skeleton
+        * QH to it. */
+       if (pqh->skel < SKEL_FSBR && qh->skel >= SKEL_FSBR)
+               uhci->skel_term_qh->link = link_to_new_qh;
 }
 
 /*
@@ -561,31 +530,16 @@ static void unlink_interrupt(struct uhci_hcd *uhci, struct uhci_qh *qh)
  */
 static void unlink_async(struct uhci_hcd *uhci, struct uhci_qh *qh)
 {
-       struct uhci_qh *pqh, *lqh;
+       struct uhci_qh *pqh;
        __le32 link_to_next_qh = qh->link;
 
        pqh = list_entry(qh->node.prev, struct uhci_qh, node);
-
-       /* If this is the first FSBQ QH, take special action */
-       if (uhci->fsbr_is_on && pqh->skel < SKEL_FSBR &&
-                       qh->skel >= SKEL_FSBR) {
-               lqh = list_entry(uhci->skel_async_qh->node.prev,
-                               struct uhci_qh, node);
-
-               /* If this QH is also the last one, we must link in
-                * the terminating skeleton QH. */
-               if (qh == lqh) {
-                       link_to_next_qh = LINK_TO_QH(uhci->skel_term_qh);
-                       uhci->skel_term_qh->link = link_to_next_qh;
-                       wmb();
-                       qh->link = link_to_next_qh;
-
-               /* Otherwise the last QH must point to the new first FSBR QH */
-               } else
-                       lqh->link = link_to_next_qh;
-       }
-
        pqh->link = link_to_next_qh;
+
+       /* If this was the old first FSBR QH, link the terminating skeleton
+        * QH to the next (new first FSBR) QH. */
+       if (pqh->skel < SKEL_FSBR && qh->skel >= SKEL_FSBR)
+               uhci->skel_term_qh->link = link_to_next_qh;
        mb();
 }
 
@@ -1217,7 +1171,7 @@ static int uhci_result_common(struct uhci_hcd *uhci, struct urb *urb)
 
                                if (debug > 1 && errbuf) {
                                        /* Print the chain for debugging */
-                                       uhci_show_qh(urbp->qh, errbuf,
+                                       uhci_show_qh(uhci, urbp->qh, errbuf,
                                                        ERRBUF_LEN, 0);
                                        lprintk(errbuf);
                                }