[PATCH] usbcore: rm hub oops, message cleanups, unlink

These changes are unrelated except I ran into them all at once:

- Fixes an oops from a partial hub_configure() clean; let
   hub_disconnect() do the whole thing, simpler.

- Since I was there, modify that routine's err() messages
   to use dev_err().  Then eliminate a redundant diagnostic
   in hub_probe(), and merge the "bad descriptor" cases into
   one diagnostic.  Saves a few hundred rodata bytes, and
   the messages now say what hub's involved.

- Unlink fixes:  if lower level code reports a submit error,
   make sure the urb gets unlinked from the device's urb_list;
   and report "it's already being unlinked" as -EBUSY so callers
   can do something smarter than wonder "what did I do wrong".

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 075c3c2a62e36635a3d8774da8f3fb67b063c28b..74a3993826fda7e8319a8e57b14c4550e35bc53e 100644 (file)
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1022,7 +1022,10 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
                 * they could clobber root hub response data.
                 */
                urb->transfer_flags |= URB_NO_DMA_MAP;
-               return rh_urb_enqueue (hcd, urb);
+               status = rh_urb_enqueue (hcd, urb);
+               if (status)
+                       urb_unlink (urb);
+               return status;
        }
 
        /* lower level hcd code should use *_dma exclusively */
@@ -1043,7 +1046,10 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
                                            : PCI_DMA_TODEVICE);
        }
 
-       return hcd->driver->urb_enqueue (hcd, urb, mem_flags);
+       status = hcd->driver->urb_enqueue (hcd, urb, mem_flags);
+       if (status)
+               urb_unlink (urb);
+       return status;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1137,7 +1143,7 @@ static int hcd_unlink_urb (struct urb *urb)
         * FIXME use better explicit urb state
         */
        if (urb->status != -EINPROGRESS) {
-               retval = -EINVAL;
+               retval = -EBUSY;
                goto done;
        }
 

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 22245f2d483245919afe07b113e80b18afc0c286..3ccdaf5bdb6561358389fd7b05294d61ed088fbd 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -279,12 +279,13 @@ static int usb_hub_configure(struct usb_hub *hub,
        struct usb_hub_status hubstatus;
        unsigned int pipe;
        int maxp, ret;
+       char *message;
 
        hub->descriptor = kmalloc(sizeof(*hub->descriptor), GFP_KERNEL);
        if (!hub->descriptor) {
-               err("Unable to kmalloc %Zd bytes for hub descriptor",
-                       sizeof(*hub->descriptor));
-               return -1;
+               message = "can't kmalloc hub descriptor";
+               ret = -ENOMEM;
+               goto fail;
        }
 
        /* Request the entire hub descriptor.
@@ -294,13 +295,12 @@ static int usb_hub_configure(struct usb_hub *hub,
        ret = usb_get_hub_descriptor(dev, hub->descriptor,
                        sizeof(*hub->descriptor));
        if (ret < 0) {
-               err("Unable to get hub descriptor (err = %d)", ret);
-               kfree(hub->descriptor);
-               return -1;
+               message = "can't read hub descriptor";
+               goto fail;
        } else if (hub->descriptor->bNbrPorts > USB_MAXCHILDREN) {
-               err("Hub is too big! %d children", hub->descriptor->bNbrPorts);
-               kfree(hub->descriptor);
-               return -1;
+               message = "hub has too many ports!";
+               ret = -ENODEV;
+               goto fail;
        }
 
        dev->maxchild = hub->descriptor->bNbrPorts;
@@ -396,9 +396,8 @@ static int usb_hub_configure(struct usb_hub *hub,
 
        ret = usb_get_hub_status(dev, &hubstatus);
        if (ret < 0) {
-               err("Unable to get hub status (err = %d)", ret);
-               kfree(hub->descriptor);
-               return -1;
+               message = "can't get hub status";
+               goto fail;
        }
 
        le16_to_cpus(&hubstatus.wHubStatus);
@@ -419,18 +418,17 @@ static int usb_hub_configure(struct usb_hub *hub,
 
        hub->urb = usb_alloc_urb(0, GFP_KERNEL);
        if (!hub->urb) {
-               err("couldn't allocate interrupt urb");
-               kfree(hub->descriptor);
-               return -1;
+               message = "couldn't allocate interrupt urb";
+               ret = -ENOMEM;
+               goto fail;
        }
 
        usb_fill_int_urb(hub->urb, dev, pipe, hub->buffer, maxp, hub_irq,
                hub, endpoint->bInterval);
        ret = usb_submit_urb(hub->urb, GFP_KERNEL);
        if (ret) {
-               err("usb_submit_urb failed (%d)", ret);
-               kfree(hub->descriptor);
-               return -1;
+               message = "couldn't submit status urb";
+               goto fail;
        }
                
        /* Wake up khubd */
@@ -439,6 +437,12 @@ static int usb_hub_configure(struct usb_hub *hub,
        usb_hub_power_on(hub);
 
        return 0;
+
+fail:
+       dev_err (hub->intf->dev, "config failed, %s (err %d)\n",
+                       message, ret);
+       /* hub_disconnect() frees urb and descriptor */
+       return ret;
 }
 
 static void hub_disconnect(struct usb_interface *intf)
@@ -497,32 +501,27 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
        /*  specs is not defined, but it works */
        if ((desc->desc.bInterfaceSubClass != 0) &&
            (desc->desc.bInterfaceSubClass != 1)) {
-               err("invalid subclass (%d) for USB hub device #%d",
-                       desc->desc.bInterfaceSubClass, dev->devnum);
+descriptor_error:
+               dev_err (intf->dev, "bad descriptor, ignoring hub\n");
                return -EIO;
        }
 
        /* Multiple endpoints? What kind of mutant ninja-hub is this? */
        if (desc->desc.bNumEndpoints != 1) {
-               err("invalid bNumEndpoints (%d) for USB hub device #%d",
-                       desc->desc.bNumEndpoints, dev->devnum);
-               return -EIO;
+               goto descriptor_error;
        }
 
        endpoint = &desc->endpoint[0].desc;
 
        /* Output endpoint? Curiousier and curiousier.. */
        if (!(endpoint->bEndpointAddress & USB_DIR_IN)) {
-               err("Device #%d is hub class, but has output endpoint?",
-                       dev->devnum);
-               return -EIO;
+               goto descriptor_error;
        }
 
        /* If it's not an interrupt endpoint, we'd better punt! */
        if ((endpoint->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
                        != USB_ENDPOINT_XFER_INT) {
-               err("Device #%d is hub class, but endpoint is not interrupt?",
-                       dev->devnum);
+               goto descriptor_error;
                return -EIO;
        }
 
@@ -554,8 +553,6 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
                return 0;
        }
 
-       err("hub configuration failed for device at %s", dev->devpath);
-
        hub_disconnect (intf);
        return -ENODEV;
 }