]> git.hungrycats.org Git - linux/commitdiff
[PATCH] PATCH: 2.5.8 ehci, submit errors
authorDavid Brownell <david-b@pacbell.net>
Mon, 22 Apr 2002 06:32:58 +0000 (23:32 -0700)
committerGreg Kroah-Hartman <greg@kroah.com>
Mon, 22 Apr 2002 06:32:58 +0000 (23:32 -0700)
It fixes problems with interrupt transfers, which I think that
nobody else has run into (or I'd surely have heard of it :).
Looks like not many folk are using USB 2.0 hubs yet.

    - wasn't checking enough of the periodic schedule to
      detect bandwidth overcommit (would BUG out)
    - frames to uframes is rightshift 3, not 8 :)
    - properly cleans up (no oops!) after certain rare errors
      in the interrupt submit path (just my luck to hit one)
    - use that cleanup to bypass some old implementation
      shortcuts in the control and bulk submit paths
    - there are also some other minor updates/cleanups

drivers/usb/host/ehci-hcd.c
drivers/usb/host/ehci-q.c
drivers/usb/host/ehci-sched.c

index 212b61a84ea05850aa066c9bd7074b67fbd56bba..29fb395cf2c3aca1e6bdd34ff6eb3e7c3bada1b5 100644 (file)
@@ -70,6 +70,8 @@
  *
  * HISTORY:
  *
+ * 2002-04-19  Control/bulk/interrupt submit no longer uses giveback() on
+ *     errors in submit path.  Bugfixes to interrupt scheduling/processing.
  * 2002-03-05  Initial high-speed ISO support; reduce ITD memory; shift
  *     more checking to generic hcd framework (db).  Make it work with
  *     Philips EHCI; reduce PCI traffic; shorten IRQ path (Rory Bolt).
@@ -81,7 +83,7 @@
  * 2001-June   Works with usb-storage and NEC EHCI on 2.4
  */
 
-#define DRIVER_VERSION "$Revision: 0.27 $"
+#define DRIVER_VERSION "$Revision: 0.31 $"
 #define DRIVER_AUTHOR "David Brownell"
 #define DRIVER_DESC "USB 2.0 'Enhanced' Host Controller (EHCI) Driver"
 
@@ -512,8 +514,7 @@ static int ehci_urb_enqueue (
        case PIPE_BULK:
                if (!qh_urb_transaction (ehci, urb, &qtd_list, mem_flags))
                        return -ENOMEM;
-               submit_async (ehci, urb, &qtd_list, mem_flags);
-               break;
+               return submit_async (ehci, urb, &qtd_list, mem_flags);
 
        case PIPE_INTERRUPT:
                if (!qh_urb_transaction (ehci, urb, &qtd_list, mem_flags))
@@ -531,8 +532,9 @@ static int ehci_urb_enqueue (
                return -ENOSYS;
 #endif /* have_split_iso */
 
+       default:        /* can't happen */
+               return -ENOSYS;
        }
-       return 0;
 }
 
 /* remove from hardware lists
@@ -587,7 +589,7 @@ dbg ("wait for dequeue: state %d, reclaim %p, hcd state %d",
                // itd or sitd ...
 
                // wait till next completion, do it then.
-               // completion irqs can wait up to 128 msec,
+               // completion irqs can wait up to 1024 msec,
                urb->transfer_flags |= EHCI_STATE_UNLINK;
                return 0;
        }
@@ -614,10 +616,7 @@ static void ehci_free_config (struct usb_hcd *hcd, struct usb_device *udev)
                if (dev->ep [i]) {
                        struct ehci_qh          *qh;
 
-                       // FIXME:  this might be an itd/sitd too ...
-                       // or an interrupt urb (not on async list)
-                       // can use "union ehci_shadow"
-
+                       /* dev->ep never has ITDs or SITDs */
                        qh = (struct ehci_qh *) dev->ep [i];
                        vdbg ("free_config, ep 0x%02x qh %p", i, qh);
                        if (!list_empty (&qh->qtd_list)) {
index 8ee0a2321a42b8b0b3dc7bed8e62ddc494ec0316..c1ab43b5426f61309d1859b9153a9d82215b6822 100644 (file)
@@ -280,6 +280,8 @@ qh_completions (
 
                /* if these qtds were queued to the HC, some may be active.
                 * else we're cleaning up after a failed URB submission.
+                *
+                * FIXME can simplify: cleanup case is gone now.
                 */
                if (likely (qh != 0)) {
                        int             qh_halted;
@@ -405,6 +407,47 @@ qh_completions (
 
 /*-------------------------------------------------------------------------*/
 
+/*
+ * reverse of qh_urb_transaction:  free a list of TDs.
+ * used for cleanup after errors, before HC sees an URB's TDs.
+ */
+static void qtd_list_free (
+       struct ehci_hcd         *ehci,
+       struct urb              *urb,
+       struct list_head        *qtd_list
+) {
+       struct list_head        *entry, *temp;
+       int                     unmapped = 0;
+
+       list_for_each_safe (entry, temp, qtd_list) {
+               struct ehci_qtd *qtd;
+
+               qtd = list_entry (entry, struct ehci_qtd, qtd_list);
+               list_del (&qtd->qtd_list);
+               if (unmapped != 2) {
+                       int     direction;
+
+                       /* for ctrl unmap twice: SETUP and DATA;
+                        * else (bulk, intr) just once: DATA
+                        */
+                       if (!unmapped++ && usb_pipecontrol (urb->pipe))
+                               direction = PCI_DMA_TODEVICE;
+                       else {
+                               direction = usb_pipein (urb->pipe)
+                                       ? PCI_DMA_FROMDEVICE
+                                       : PCI_DMA_TODEVICE;
+                               unmapped++;
+                       }
+                       if (qtd->buf_dma)
+                               pci_unmap_single (ehci->hcd.pdev,
+                                       qtd->buf_dma,
+                                       qtd->urb->transfer_buffer_length,
+                                       direction);
+               }
+               ehci_qtd_free (ehci, qtd);
+       }
+}
+
 /*
  * create a list of filled qtds for this URB; won't link into qh.
  */
@@ -547,8 +590,7 @@ qh_urb_transaction (
        return head;
 
 cleanup:
-       urb->status = -ENOMEM;
-       qh_completions (ehci, head, 1);
+       qtd_list_free (ehci, urb, head);
        return 0;
 }
 
@@ -713,7 +755,7 @@ static void qh_link_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
 
 /*-------------------------------------------------------------------------*/
 
-static void
+static int
 submit_async (
        struct ehci_hcd         *ehci,
        struct urb              *urb,
@@ -807,8 +849,7 @@ submit_async (
                if (likely (qh != 0)) {
                        // dbg_qh ("new qh", ehci, qh);
                        dev->ep [epnum] = qh;
-               } else
-                       urb->status = -ENOMEM;
+               }
        }
 
        /* Control/bulk operations through TTs don't need scheduling,
@@ -820,8 +861,11 @@ submit_async (
                        qh_link_async (ehci, qh_get (qh));
        }
        spin_unlock_irqrestore (&ehci->lock, flags);
-       if (unlikely (!qh))
-               qh_completions (ehci, qtd_list, 1);
+       if (unlikely (qh == 0)) {
+               qtd_list_free (ehci, urb, qtd_list);
+               return -ENOMEM;
+       }
+       return 0;
 }
 
 /*-------------------------------------------------------------------------*/
index 396fa42a14c497754f1239b9aaf419bdd8113e1b..90c300572d43b95eae08313ecf28e7c6a2d8925c 100644 (file)
@@ -220,6 +220,8 @@ static void intr_deschedule (
 ) {
        unsigned long   flags;
 
+       period >>= 3;           // FIXME microframe periods not handled yet
+
        spin_lock_irqsave (&ehci->lock, flags);
 
        do {
@@ -256,6 +258,38 @@ static void intr_deschedule (
                atomic_read (&qh->refcount), ehci->periodic_urbs);
 }
 
+static int check_period (
+       struct ehci_hcd *ehci, 
+       unsigned        frame,
+       int             uframe,
+       unsigned        period,
+       unsigned        usecs
+) {
+       /*
+        * 80% periodic == 100 usec/uframe available
+        * convert "usecs we need" to "max already claimed" 
+        */
+       usecs = 100 - usecs;
+
+       do {
+               int     claimed;
+
+// FIXME delete when intr_submit handles non-empty queues
+// this gives us a one intr/frame limit (vs N/uframe)
+               if (ehci->pshadow [frame].ptr)
+                       return 0;
+
+               claimed = periodic_usecs (ehci, frame, uframe);
+               if (claimed > usecs)
+                       return 0;
+
+// FIXME update to handle sub-frame periods
+       } while ((frame += period) < ehci->periodic_size);
+
+       // success!
+       return 1;
+}
+
 static int intr_submit (
        struct ehci_hcd         *ehci,
        struct urb              *urb,
@@ -263,7 +297,6 @@ static int intr_submit (
        int                     mem_flags
 ) {
        unsigned                epnum, period;
-       unsigned                temp;
        unsigned short          usecs;
        unsigned long           flags;
        struct ehci_qh          *qh;
@@ -272,11 +305,8 @@ static int intr_submit (
 
        /* get endpoint and transfer data */
        epnum = usb_pipeendpoint (urb->pipe);
-       if (usb_pipein (urb->pipe)) {
-               temp = urb->dev->epmaxpacketin [epnum];
+       if (usb_pipein (urb->pipe))
                epnum |= 0x10;
-       } else
-               temp = urb->dev->epmaxpacketout [epnum];
        if (urb->dev->speed != USB_SPEED_HIGH) {
                dbg ("no intr/tt scheduling yet"); 
                status = -ENOSYS;
@@ -287,6 +317,13 @@ static int intr_submit (
         * NOTE: current completion/restart logic doesn't handle more than
         * one qtd in a periodic qh ... 16-20 KB/urb is pretty big for this.
         * such big requests need many periods to transfer.
+        *
+        * FIXME want to change hcd core submit model to expect queuing
+        * for all transfer types ... not just ISO and (with flag) BULK.
+        * that means: getting rid of this check; handling the "interrupt
+        * urb already queued" case below like bulk queuing is handled (no
+        * errors possible!); and completly getting rid of that annoying
+        * qh restart logic.  simpler/smaller overall, and more flexible.
         */
        if (unlikely (qtd_list->next != qtd_list->prev)) {
                dbg ("only one intr qtd per urb allowed"); 
@@ -297,11 +334,13 @@ static int intr_submit (
        usecs = HS_USECS (urb->transfer_buffer_length);
 
        /* FIXME handle HS periods of less than 1 frame. */
-       if (urb->interval < 8)
-               period = 1;
-       else
-               period = urb->interval >> 8;
-               
+       period = urb->interval >> 3;
+       if (period < 1) {
+               dbg ("intr period %d uframes, NYET!", urb->interval);
+               status = -EINVAL;
+               goto done;
+       }
+
        spin_lock_irqsave (&ehci->lock, flags);
 
        /* get the qh (must be empty and idle) */
@@ -326,21 +365,22 @@ static int intr_submit (
                        list_splice (qtd_list, &qh->qtd_list);
                        qh_update (qh, list_entry (qtd_list->next,
                                                struct ehci_qtd, qtd_list));
+                       qtd_list = &qh->qtd_list;
                }
        } else {
                /* can't sleep here, we have ehci->lock... */
                qh = ehci_qh_make (ehci, urb, qtd_list, SLAB_ATOMIC);
-               qtd_list = &qh->qtd_list;
                if (likely (qh != 0)) {
                        // dbg ("new INTR qh %p", qh);
                        dev->ep [epnum] = qh;
+                       qtd_list = &qh->qtd_list;
                } else
                        status = -ENOMEM;
        }
 
        /* Schedule this periodic QH. */
        if (likely (status == 0)) {
-               unsigned        frame = urb->interval;
+               unsigned        frame = period;
 
                qh->hw_next = EHCI_LIST_END;
                qh->usecs = usecs;
@@ -352,28 +392,20 @@ static int intr_submit (
                do {
                        int     uframe;
 
-                       /* Select some frame 0..(urb->interval - 1) with a
-                        * microframe that can hold this transaction.
+                       /* pick a set of slots such that all uframes have
+                        * enough periodic bandwidth available.
                         *
                         * FIXME for TT splits, need uframes for start and end.
                         * FSTNs can put end into next frame (uframes 0 or 1).
                         */
                        frame--;
                        for (uframe = 0; uframe < 8; uframe++) {
-                               int     claimed;
-                               claimed = periodic_usecs (ehci, frame, uframe);
-                               /* 80% periodic == 100 usec max committed */
-                               if ((claimed + usecs) <= 100) {
-                                       vdbg ("frame %d.%d: %d usecs, plus %d",
-                                               frame, uframe, claimed, usecs);
+                               if (check_period (ehci, frame, uframe,
+                                               period, usecs) != 0)
                                        break;
-                               }
                        }
                        if (uframe == 8)
                                continue;
-// FIXME delete when code below handles non-empty queues
-                       if (ehci->pshadow [frame].ptr)
-                               continue;
 
                        /* QH will run once each period, starting there  */
                        urb->start_frame = frame;
@@ -389,8 +421,9 @@ static int intr_submit (
                                qh, qh->usecs, period, frame, uframe);
                        do {
                                if (unlikely (ehci->pshadow [frame].ptr != 0)) {
-// FIXME -- just link to the end, before any qh with a shorter period,
+// FIXME -- just link toward the end, before any qh with a shorter period,
 // AND handle it already being (implicitly) linked into this frame
+// AS WELL AS updating the check_period() logic
                                        BUG ();
                                } else {
                                        ehci->pshadow [frame].qh = qh_get (qh);
@@ -401,7 +434,7 @@ static int intr_submit (
                        } while (frame < ehci->periodic_size);
 
                        /* update bandwidth utilization records (for usbfs) */
-                       usb_claim_bandwidth (urb->dev, urb, usecs, 0);
+                       usb_claim_bandwidth (urb->dev, urb, usecs/period, 0);
 
                        /* maybe enable periodic schedule processing */
                        if (!ehci->periodic_urbs++)
@@ -412,14 +445,9 @@ static int intr_submit (
        }
        spin_unlock_irqrestore (&ehci->lock, flags);
 done:
-       if (status) {
-               usb_complete_t  complete = urb->complete;
+       if (status)
+               qtd_list_free (ehci, urb, qtd_list);
 
-               urb->complete = 0;
-               urb->status = status;
-               qh_completions (ehci, qtd_list, 1);
-               urb->complete = complete;
-       }
        return status;
 }
 
@@ -438,6 +466,10 @@ intr_complete (
        if (likely ((qh->hw_token & __constant_cpu_to_le32 (QTD_STS_ACTIVE))
                        != 0))
                return flags;
+       if (unlikely (list_empty (&qh->qtd_list))) {
+               dbg ("intr qh %p no TDs?", qh);
+               return flags;
+       }
        
        qtd = list_entry (qh->qtd_list.next, struct ehci_qtd, qtd_list);
        urb = qtd->urb;