]> git.hungrycats.org Git - linux/commitdiff
[PATCH] relocate error checks
authorDavid Brownell <david-b@pacbell.net>
Fri, 7 Jun 2002 03:40:00 +0000 (20:40 -0700)
committerGreg Kroah-Hartman <greg@kroah.com>
Fri, 7 Jun 2002 03:40:00 +0000 (20:40 -0700)
As was discussed a few weeks back, this moves most of the
sanity checks and input conditioning for the HCD framework's
usb_submit_urb() support directly into usb_submit_urb(), so
that all HCDs (not just those using the sharable HCD framework
support) can rely on them.

drivers/usb/core/hcd.c
drivers/usb/core/urb.c

index 9ab97833622e02295cbcc95f878d02f3cf44e60a..03423ae18f6597ca8ed667b456e4706ec3e4247d 100644 (file)
@@ -967,170 +967,21 @@ static void urb_unlink (struct urb *urb)
 }
 
 
-/* may be called in any context with a valid urb->dev usecount */
-/* caller surrenders "ownership" of urb */
-
+/* may be called in any context with a valid urb->dev usecount
+ * caller surrenders "ownership" of urb
+ * expects usb_submit_urb() to have sanity checked and conditioned all
+ * inputs in the urb
+ */
 static int hcd_submit_urb (struct urb *urb, int mem_flags)
 {
        int                     status;
-       struct usb_hcd          *hcd;
-       struct hcd_dev          *dev;
+       struct usb_hcd          *hcd = urb->dev->bus->hcpriv;
+       struct hcd_dev          *dev = urb->dev->hcpriv;
        unsigned long           flags;
-       int                     pipe, temp, max;
-
-       if (!urb || urb->hcpriv || !urb->complete)
-               return -EINVAL;
-
-       urb->status = -EINPROGRESS;
-       urb->actual_length = 0;
-       urb->bandwidth = 0;
-       INIT_LIST_HEAD (&urb->urb_list);
 
-       if (!urb->dev || !urb->dev->bus || urb->dev->devnum <= 0)
-               return -ENODEV;
-       hcd = urb->dev->bus->hcpriv;
-       dev = urb->dev->hcpriv;
        if (!hcd || !dev)
                return -ENODEV;
 
-       /* can't submit new urbs when quiescing, halted, ... */
-       if (hcd->state == USB_STATE_QUIESCING || !HCD_IS_RUNNING (hcd->state))
-               return -ESHUTDOWN;
-       pipe = urb->pipe;
-       temp = usb_pipetype (urb->pipe);
-       if (usb_endpoint_halted (urb->dev, usb_pipeendpoint (pipe),
-                       usb_pipeout (pipe)))
-               return -EPIPE;
-
-       /* FIXME there should be a sharable lock protecting us against
-        * config/altsetting changes and disconnects, kicking in here.
-        */
-
-       /* Sanity check, so HCDs can rely on clean data */
-       max = usb_maxpacket (urb->dev, pipe, usb_pipeout (pipe));
-       if (max <= 0) {
-               err ("bogus endpoint (bad maxpacket)");
-               return -EINVAL;
-       }
-
-       /* "high bandwidth" mode, 1-3 packets/uframe? */
-       if (urb->dev->speed == USB_SPEED_HIGH) {
-               int     mult;
-               switch (temp) {
-               case PIPE_ISOCHRONOUS:
-               case PIPE_INTERRUPT:
-                       mult = 1 + ((max >> 11) & 0x03);
-                       max &= 0x03ff;
-                       max *= mult;
-               }
-       }
-
-       /* periodic transfers limit size per frame/uframe */
-       switch (temp) {
-       case PIPE_ISOCHRONOUS: {
-               int     n, len;
-
-               if (urb->number_of_packets <= 0)                    
-                       return -EINVAL;
-               for (n = 0; n < urb->number_of_packets; n++) {
-                       len = urb->iso_frame_desc [n].length;
-                       if (len < 0 || len > max) 
-                               return -EINVAL;
-               }
-
-               }
-               break;
-       case PIPE_INTERRUPT:
-               if (urb->transfer_buffer_length > max)
-                       return -EINVAL;
-       }
-
-       /* the I/O buffer must usually be mapped/unmapped */
-       if (urb->transfer_buffer_length < 0)
-               return -EINVAL;
-
-#ifdef DEBUG
-       /* stuff that drivers shouldn't do, but which shouldn't
-        * cause problems in HCDs if they get it wrong.
-        */
-       {
-       unsigned int    orig_flags = urb->transfer_flags;
-       unsigned int    allowed;
-
-       /* enforce simple/standard policy */
-       allowed = USB_ASYNC_UNLINK;     // affects later unlinks
-       allowed |= USB_NO_FSBR;         // only affects UHCI
-       switch (temp) {
-       case PIPE_CONTROL:
-               allowed |= USB_DISABLE_SPD;
-               break;
-       case PIPE_BULK:
-               allowed |= USB_DISABLE_SPD | USB_QUEUE_BULK
-                               | USB_ZERO_PACKET | URB_NO_INTERRUPT;
-               break;
-       case PIPE_INTERRUPT:
-               allowed |= USB_DISABLE_SPD;
-               break;
-       case PIPE_ISOCHRONOUS:
-               allowed |= USB_ISO_ASAP;
-               break;
-       }
-       urb->transfer_flags &= allowed;
-
-       /* fail if submitter gave bogus flags */
-       if (urb->transfer_flags != orig_flags) {
-               err ("BOGUS urb flags, %x --> %x",
-                       orig_flags, urb->transfer_flags);
-               return -EINVAL;
-       }
-       }
-#endif
-       /*
-        * Force periodic transfer intervals to be legal values that are
-        * a power of two (so HCDs don't need to).
-        *
-        * FIXME want bus->{intr,iso}_sched_horizon values here.  Each HC
-        * supports different values... this uses EHCI/UHCI defaults (and
-        * EHCI can use smaller non-default values).
-        */
-       switch (temp) {
-       case PIPE_ISOCHRONOUS:
-       case PIPE_INTERRUPT:
-               /* too small? */
-               if (urb->interval <= 0)
-                       return -EINVAL;
-               /* too big? */
-               switch (urb->dev->speed) {
-               case USB_SPEED_HIGH:    /* units are microframes */
-                       // NOTE usb handles 2^15
-                       if (urb->interval > (1024 * 8))
-                               urb->interval = 1024 * 8;
-                       temp = 1024 * 8;
-                       break;
-               case USB_SPEED_FULL:    /* units are frames/msec */
-               case USB_SPEED_LOW:
-                       if (temp == PIPE_INTERRUPT) {
-                               if (urb->interval > 255)
-                                       return -EINVAL;
-                               // NOTE ohci only handles up to 32
-                               temp = 128;
-                       } else {
-                               if (urb->interval > 1024)
-                                       urb->interval = 1024;
-                               // NOTE usb and ohci handle up to 2^15
-                               temp = 1024;
-                       }
-                       break;
-               default:
-                       return -EINVAL;
-               }
-               /* power of two? */
-               while (temp > urb->interval)
-                       temp >>= 1;
-               urb->interval = temp;
-       }
-
-
        /*
         * FIXME:  make urb timeouts be generic, keeping the HCD cores
         * as simple as possible.
@@ -1140,9 +991,6 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
        // hcd_monitor_hook(MONITOR_URB_SUBMIT, urb)
        // It would catch submission paths for all urbs.
 
-       /* increment urb's reference count, we now control it. */
-       urb = usb_get_urb(urb);
-
        /*
         * Atomically queue the urb,  first to our records, then to the HCD.
         * Access to urb->status is controlled by urb->lock ... changes on
@@ -1164,18 +1012,15 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
        if (status)
                return status;
 
-       /* temporarily up refcount while queueing it in the HCD,
-        * since we report some queuing/setup errors ourselves
+       /* increment urb's reference count as part of giving it to the HCD
+        * (which now controls it).  HCD guarantees that it either returns
+        * an error or calls giveback(), but not both.
         */
        urb = usb_get_urb (urb);
        if (urb->dev == hcd->self.root_hub)
                status = rh_urb_enqueue (hcd, urb);
        else
                status = hcd->driver->urb_enqueue (hcd, urb, mem_flags);
-       /* urb->dev got nulled if hcd called giveback for us */
-       if (status && urb->dev)
-               urb_unlink (urb);
-       usb_put_urb (urb);
        return status;
 }
 
index 6cb11b083890d2962134fea57145856b5e6e1b37..0d614e893685ae19eaaebd87596d75840c7f3841 100644 (file)
@@ -182,15 +182,170 @@ struct urb * usb_get_urb(struct urb *urb)
  */
 int usb_submit_urb(struct urb *urb, int mem_flags)
 {
+       int                     pipe, temp, max;
+       struct usb_device       *dev;
+       struct usb_operations   *op;
+       int                     is_out;
 
-       if (urb && urb->dev && urb->dev->bus && urb->dev->bus->op) {
-               if (usb_maxpacket(urb->dev, urb->pipe, usb_pipeout(urb->pipe)) <= 0) {
-                       err("%s: pipe %x has invalid size (<= 0)", __FUNCTION__, urb->pipe);
+       if (!urb || urb->hcpriv || !urb->complete)
+               return -EINVAL;
+       if (!(dev = urb->dev) || !dev->bus || dev->devnum <= 0)
+               return -ENODEV;
+       if (!(op = dev->bus->op) || !op->submit_urb)
+               return -ENODEV;
+
+       urb->status = -EINPROGRESS;
+       urb->actual_length = 0;
+       urb->bandwidth = 0;
+
+       /* Lots of sanity checks, so HCDs can rely on clean data
+        * and don't need to duplicate tests
+        */
+       pipe = urb->pipe;
+       temp = usb_pipetype (pipe);
+       is_out = usb_pipeout (pipe);
+
+       /* (actually HCDs may need to duplicate this, endpoint might yet
+        * stall due to queued bulk/intr transactions that complete after
+        * we check)
+        */
+       if (usb_endpoint_halted (dev, usb_pipeendpoint (pipe), is_out))
+               return -EPIPE;
+
+       /* FIXME there should be a sharable lock protecting us against
+        * config/altsetting changes and disconnects, kicking in here.
+        * (here == before maxpacket, and eventually endpoint type,
+        * checks get made.)
+        */
+
+       max = usb_maxpacket (dev, pipe, is_out);
+       if (max <= 0) {
+               dbg ("%s: bogus endpoint %d-%s on usb-%s-%s (bad maxpacket %d)",
+                       __FUNCTION__,
+                       usb_pipeendpoint (pipe), is_out ? "OUT" : "IN",
+                       dev->bus->bus_name, dev->devpath,
+                       max);
+               return -EMSGSIZE;
+       }
+
+       /* "high bandwidth" mode, 1-3 packets/uframe? */
+       if (dev->speed == USB_SPEED_HIGH) {
+               int     mult;
+               switch (temp) {
+               case PIPE_ISOCHRONOUS:
+               case PIPE_INTERRUPT:
+                       mult = 1 + ((max >> 11) & 0x03);
+                       max &= 0x03ff;
+                       max *= mult;
+               }
+       }
+
+       /* periodic transfers limit size per frame/uframe */
+       switch (temp) {
+       case PIPE_ISOCHRONOUS: {
+               int     n, len;
+
+               if (urb->number_of_packets <= 0)                    
+                       return -EINVAL;
+               for (n = 0; n < urb->number_of_packets; n++) {
+                       len = urb->iso_frame_desc [n].length;
+                       if (len < 0 || len > max) 
+                               return -EMSGSIZE;
+               }
+
+               }
+               break;
+       case PIPE_INTERRUPT:
+               if (urb->transfer_buffer_length > max)
                        return -EMSGSIZE;
+       }
+
+       /* the I/O buffer must be mapped/unmapped, except when length=0 */
+       if (urb->transfer_buffer_length < 0)
+               return -EMSGSIZE;
+
+#ifdef DEBUG
+       /* stuff that drivers shouldn't do, but which shouldn't
+        * cause problems in HCDs if they get it wrong.
+        */
+       {
+       unsigned int    orig_flags = urb->transfer_flags;
+       unsigned int    allowed;
+
+       /* enforce simple/standard policy */
+       allowed = USB_ASYNC_UNLINK;     // affects later unlinks
+       allowed |= USB_NO_FSBR;         // only affects UHCI
+       switch (temp) {
+       case PIPE_CONTROL:
+               allowed |= USB_DISABLE_SPD;
+               break;
+       case PIPE_BULK:
+               allowed |= USB_DISABLE_SPD | USB_QUEUE_BULK
+                               | USB_ZERO_PACKET | URB_NO_INTERRUPT;
+               break;
+       case PIPE_INTERRUPT:
+               allowed |= USB_DISABLE_SPD;
+               break;
+       case PIPE_ISOCHRONOUS:
+               allowed |= USB_ISO_ASAP;
+               break;
+       }
+       urb->transfer_flags &= allowed;
+
+       /* fail if submitter gave bogus flags */
+       if (urb->transfer_flags != orig_flags) {
+               err ("BOGUS urb flags, %x --> %x",
+                       orig_flags, urb->transfer_flags);
+               return -EINVAL;
+       }
+       }
+#endif
+       /*
+        * Force periodic transfer intervals to be legal values that are
+        * a power of two (so HCDs don't need to).
+        *
+        * FIXME want bus->{intr,iso}_sched_horizon values here.  Each HC
+        * supports different values... this uses EHCI/UHCI defaults (and
+        * EHCI can use smaller non-default values).
+        */
+       switch (temp) {
+       case PIPE_ISOCHRONOUS:
+       case PIPE_INTERRUPT:
+               /* too small? */
+               if (urb->interval <= 0)
+                       return -EINVAL;
+               /* too big? */
+               switch (dev->speed) {
+               case USB_SPEED_HIGH:    /* units are microframes */
+                       // NOTE usb handles 2^15
+                       if (urb->interval > (1024 * 8))
+                               urb->interval = 1024 * 8;
+                       temp = 1024 * 8;
+                       break;
+               case USB_SPEED_FULL:    /* units are frames/msec */
+               case USB_SPEED_LOW:
+                       if (temp == PIPE_INTERRUPT) {
+                               if (urb->interval > 255)
+                                       return -EINVAL;
+                               // NOTE ohci only handles up to 32
+                               temp = 128;
+                       } else {
+                               if (urb->interval > 1024)
+                                       urb->interval = 1024;
+                               // NOTE usb and ohci handle up to 2^15
+                               temp = 1024;
+                       }
+                       break;
+               default:
+                       return -EINVAL;
                }
-               return urb->dev->bus->op->submit_urb(urb, mem_flags);
+               /* power of two? */
+               while (temp > urb->interval)
+                       temp >>= 1;
+               urb->interval = temp;
        }
-       return -ENODEV;
+
+       return op->submit_urb (urb, mem_flags);
 }
 
 /*-------------------------------------------------------------------*/