Greg:

There are a few places where the code enumerates through all the USB
devices on all the buses, starting with each bus's root hub and working
down.  However a bus does not always have a root hub, and the code does
not check that the root_hub pointer is non-NULL.  This patch fixes the
problem, using the usb_bus_list_lock semaphore to synchronize access when
root hubs are added or removed.

In addition it seemed like a good idea to minimize the time that a 
non-fully-configured root hub is accessible through the bus's pointer.  So 
this patch delays setting the pointer and holds usb_bus_list_lock while 
configuring a root hub.

It turned out that a bunch of things needed to be changed for all this to 
work:

        Check for NULL root_hub pointer in usb_device_read() and
        usb_find_device().

        Pass the root-hub device as a separate argument to 
        hcd_register_root().

        Make usb_register_root_hub() acquire the usb_bus_list_lock and
        set the bus->root_hub pointer.

        For consistency's sake, move the place where the children[]
        pointer to a non-root-hub device gets stored as close as possible
        to where usb_new_device() is called.

        Make usb_disconnect() acquire the usb_bus_list_lock when removing
        a root hub.

        Change usb_hcd_pci_remove() and the non-PCI host drivers so that
        they call usb_disconnect() with a pointer to the bus's root_hub
        pointer, not a pointer to a temporary variable.

        Change all the host controller drivers not to store the root_hub
        pointer in the bus structure but instead to pass it as a new
        argument to hcd_register_root().

I made some attempt to update the hc_sl811 driver along with the rest, but 
it's pretty clear that driver won't work in the current framework.  Among 
other things, it never reads the root hub's device descriptor.  To what 
extent is the driver really supported?

Please apply.

Alan Stern



Signed-off-by: Alan Stern <[EMAIL PROTECTED]>

===== drivers/usb/core/devices.c 1.34 vs edited =====
--- 1.34/drivers/usb/core/devices.c     Sat May 15 11:48:04 2004
+++ edited/drivers/usb/core/devices.c   Tue Jun  8 14:54:13 2004
@@ -589,6 +589,8 @@
                bus = list_entry(buslist, struct usb_bus, bus_list);
 
                /* recurse through all children of the root hub */
+               if (!bus->root_hub)
+                       continue;
                down(&bus->root_hub->serialize);
                ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos, bus->root_hub, 
bus, 0, 0, 0);
                up(&bus->root_hub->serialize);
===== drivers/usb/core/hcd.h 1.74 vs edited =====
--- 1.74/drivers/usb/core/hcd.h Tue Jun  1 12:54:17 2004
+++ edited/drivers/usb/core/hcd.h       Tue Jun  8 14:54:13 2004
@@ -339,7 +339,8 @@
 extern int usb_register_root_hub (struct usb_device *usb_dev,
                struct device *parent_dev);
 
-static inline int hcd_register_root (struct usb_hcd *hcd)
+static inline int hcd_register_root (struct usb_device *usb_dev,
+               struct usb_hcd *hcd)
 {
        /* hcd->driver->start() reported can_wakeup, probably with
         * assistance from board's boot firmware.
@@ -349,8 +350,7 @@
                dev_dbg (hcd->self.controller, "supports USB remote wakeup\n");
        hcd->remote_wakeup = hcd->can_wakeup;
 
-       return usb_register_root_hub (
-               hcd_to_bus (hcd)->root_hub, hcd->self.controller);
+       return usb_register_root_hub (usb_dev, hcd->self.controller);
 }
 
 /*-------------------------------------------------------------------------*/
===== drivers/usb/core/hcd-pci.c 1.53 vs edited =====
--- 1.53/drivers/usb/core/hcd-pci.c     Sat May 15 11:48:04 2004
+++ edited/drivers/usb/core/hcd-pci.c   Tue Jun  8 14:54:13 2004
@@ -229,7 +229,6 @@
 void usb_hcd_pci_remove (struct pci_dev *dev)
 {
        struct usb_hcd          *hcd;
-       struct usb_device       *hub;
 
        hcd = pci_get_drvdata(dev);
        if (!hcd)
@@ -239,12 +238,11 @@
        if (in_interrupt ())
                BUG ();
 
-       hub = hcd->self.root_hub;
        if (HCD_IS_RUNNING (hcd->state))
                hcd->state = USB_STATE_QUIESCING;
 
        dev_dbg (hcd->self.controller, "roothub graceful disconnect\n");
-       usb_disconnect (&hub);
+       usb_disconnect (&hcd->self.root_hub);
 
        hcd->driver->stop (hcd);
        hcd_buffer_destroy (hcd);
===== drivers/usb/core/hcd.c 1.139 vs edited =====
--- 1.139/drivers/usb/core/hcd.c        Mon Jun  7 17:29:39 2004
+++ edited/drivers/usb/core/hcd.c       Tue Jun  8 15:11:00 2004
@@ -764,8 +764,9 @@
  *
  * The USB host controller calls this function to register the root hub
  * properly with the USB subsystem.  It sets up the device properly in
- * the device model tree, and then calls usb_new_device() to register the
- * usb device.  It also assigns the root hub's USB address (always 1).
+ * the device tree and stores the root_hub pointer in the bus structure,
+ * then calls usb_new_device() to register the usb device.  It also
+ * assigns the root hub's USB address (always 1).
  */
 int usb_register_root_hub (struct usb_device *usb_dev, struct device *parent_dev)
 {
@@ -779,6 +780,9 @@
        set_bit (devnum, usb_dev->bus->devmap.devicemap);
        usb_dev->state = USB_STATE_ADDRESS;
 
+       down (&usb_bus_list_lock);
+       usb_dev->bus->root_hub = usb_dev;
+
        usb_dev->epmaxpacketin[0] = usb_dev->epmaxpacketout[0] = 64;
        retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE);
        if (retval != sizeof usb_dev->descriptor) {
@@ -790,9 +794,12 @@
        down (&usb_dev->serialize);
        retval = usb_new_device (usb_dev);
        up (&usb_dev->serialize);
-       if (retval)
+       if (retval) {
+               usb_dev->bus->root_hub = NULL;
                dev_err (parent_dev, "can't register root hub for %s, %d\n",
                                usb_dev->dev.bus_id, retval);
+       }
+       up (&usb_bus_list_lock);
        return retval;
 }
 EXPORT_SYMBOL (usb_register_root_hub);
===== drivers/usb/core/hub.c 1.165 vs edited =====
--- 1.165/drivers/usb/core/hub.c        Mon Jun  7 17:29:39 2004
+++ edited/drivers/usb/core/hub.c       Tue Jun  8 14:57:52 2004
@@ -892,12 +892,14 @@
        }
        ops = bus->op;
 
-       *pdev = NULL;
-
        /* mark the device as inactive, so any further urb submissions for
         * this device will fail.
         */
        udev->state = USB_STATE_NOTATTACHED;
+
+       /* lock the bus list on behalf of HCDs unregistering their root hubs */
+       if (!udev->parent)
+               down(&usb_bus_list_lock);
        down(&udev->serialize);
 
        dev_info (&udev->dev, "USB disconnect, address %d\n", udev->devnum);
@@ -914,11 +916,18 @@
         */
        usb_disable_device(udev, 0);
 
-       /* Free the device number and remove the /proc/bus/usb entry */
+       /* Free the device number, remove the /proc/bus/usb entry,
+        * and delete the parent's children[] (or root_hub) pointer.
+        */
        dev_dbg (&udev->dev, "unregistering device\n");
        release_address(udev);
        usbfs_remove_device(udev);
+       *pdev = NULL;
+
        up(&udev->serialize);
+       if (!udev->parent)
+               up(&usb_bus_list_lock);
+
        device_unregister(&udev->dev);
 }
 
@@ -1402,8 +1411,6 @@
                goto fail;
        }
 
-       /* now dev is visible to other tasks */
-       hdev->children[port] = udev;
        retval = 0;
 
 fail:
@@ -1549,7 +1556,7 @@
                        goto loop;
                }
 
-               /* reset, get descriptor, add to hub's children */
+               /* reset and get descriptor */
                status = hub_port_init(hdev, udev, port);
                if (status < 0)
                        goto loop;
@@ -1602,6 +1609,7 @@
                        check_highspeed (hub, udev, port);
 
                /* Run it through the hoops (find a driver, etc) */
+               hdev->children[port] = udev;
                status = usb_new_device(udev);
                if (status)
                        goto loop;
===== drivers/usb/core/usb.c 1.275 vs edited =====
--- 1.275/drivers/usb/core/usb.c        Tue Jun  1 12:54:17 2004
+++ edited/drivers/usb/core/usb.c       Tue Jun  8 14:54:13 2004
@@ -883,6 +883,8 @@
             buslist != &usb_bus_list; 
             buslist = buslist->next) {
                bus = container_of(buslist, struct usb_bus, bus_list);
+               if (!bus->root_hub)
+                       continue;
                dev = match_device(bus->root_hub, vendor_id, product_id);
                if (dev)
                        goto exit;
===== drivers/usb/host/ehci-hcd.c 1.122 vs edited =====
--- 1.122/drivers/usb/host/ehci-hcd.c   Wed May 19 13:42:22 2004
+++ edited/drivers/usb/host/ehci-hcd.c  Tue Jun  8 14:54:13 2004
@@ -520,7 +520,7 @@
 
        /* wire up the root hub */
        bus = hcd_to_bus (hcd);
-       bus->root_hub = udev = usb_alloc_dev (NULL, bus, 0);
+       udev = usb_alloc_dev (NULL, bus, 0);
        if (!udev) {
 done2:
                ehci_mem_cleanup (ehci);
@@ -553,11 +553,10 @@
         * and device drivers may start it running.
         */
        udev->speed = USB_SPEED_HIGH;
-       if (hcd_register_root (hcd) != 0) {
+       if (hcd_register_root (udev, hcd) != 0) {
                if (hcd->state == USB_STATE_RUNNING)
                        ehci_ready (ehci);
                ehci_reset (ehci);
-               bus->root_hub = 0;
                usb_put_dev (udev); 
                retval = -ENODEV;
                goto done2;
===== drivers/usb/host/hc_sl811_rh.c 1.13 vs edited =====
--- 1.13/drivers/usb/host/hc_sl811_rh.c Tue Feb 10 00:33:10 2004
+++ edited/drivers/usb/host/hc_sl811_rh.c       Tue Jun  8 14:54:13 2004
@@ -557,18 +557,24 @@
 static int rh_connect_rh (hci_t * hci)
 {
        struct usb_device *usb_dev;
+       int retval;
 
        hci->rh.devnum = 0;
        usb_dev = usb_alloc_dev (NULL, hci->bus, 0);
        if (!usb_dev)
                return -ENOMEM;
 
-       hci->bus->root_hub = usb_dev;
        usb_dev->devnum = 1;
        usb_dev->bus->devnum_next = usb_dev->devnum + 1;
        set_bit (usb_dev->devnum, usb_dev->bus->devmap.devicemap);
 
-       if (usb_new_device (usb_dev) != 0) {
+       down (&usb_bus_list_lock);
+       hci->bus->root_hub = usb_dev;
+       retval = usb_new_device (usb_dev);
+       if (retval != 0)
+               hci->bus->root_hub = NULL;
+       up (&usb_bus_list_lock);
+       if (retval != 0) {
                usb_put_dev (usb_dev);
                return -ENODEV;
        }
===== drivers/usb/host/ohci-hcd.c 1.100 vs edited =====
--- 1.100/drivers/usb/host/ohci-hcd.c   Wed May 19 13:42:22 2004
+++ edited/drivers/usb/host/ohci-hcd.c  Tue Jun  8 14:54:13 2004
@@ -560,7 +560,7 @@
        }
  
        /* connect the virtual root hub */
-       bus->root_hub = udev = usb_alloc_dev (NULL, bus, 0);
+       udev = usb_alloc_dev (NULL, bus, 0);
        ohci->hcd.state = USB_STATE_RUNNING;
        if (!udev) {
                disable (ohci);
@@ -570,9 +570,8 @@
        }
 
        udev->speed = USB_SPEED_FULL;
-       if (hcd_register_root (&ohci->hcd) != 0) {
+       if (hcd_register_root (udev, &ohci->hcd) != 0) {
                usb_put_dev (udev);
-               bus->root_hub = NULL;
                disable (ohci);
                ohci->hc_control &= ~OHCI_CTRL_HCFS;
                writel (ohci->hc_control, &ohci->regs->control);
===== drivers/usb/host/ohci-omap.c 1.5 vs edited =====
--- 1.5/drivers/usb/host/ohci-omap.c    Fri Feb 27 13:09:17 2004
+++ edited/drivers/usb/host/ohci-omap.c Tue Jun  8 14:54:13 2004
@@ -454,7 +454,6 @@
  */
 void usb_hcd_omap_remove (struct usb_hcd *hcd, struct omap_dev *dev)
 {
-       struct usb_device       *hub;
        void *base;
 
        info ("remove: %s, state %x", hcd->self.bus_name, hcd->state);
@@ -462,11 +461,10 @@
        if (in_interrupt ())
                BUG ();
 
-       hub = hcd->self.root_hub;
        hcd->state = USB_STATE_QUIESCING;
 
        dbg ("%s: roothub graceful disconnect", hcd->self.bus_name);
-       usb_disconnect (&hub);
+       usb_disconnect (&hcd->self.root_hub);
 
        hcd->driver->stop (hcd);
        hcd_buffer_destroy (hcd);
===== drivers/usb/host/ohci-sa1111.c 1.34 vs edited =====
--- 1.34/drivers/usb/host/ohci-sa1111.c Thu Mar  4 11:57:51 2004
+++ edited/drivers/usb/host/ohci-sa1111.c       Tue Jun  8 14:54:13 2004
@@ -237,7 +237,6 @@
  */
 void usb_hcd_sa1111_remove (struct usb_hcd *hcd, struct sa1111_dev *dev)
 {
-       struct usb_device       *hub;
        void *base;
 
        info ("remove: %s, state %x", hcd->self.bus_name, hcd->state);
@@ -245,11 +244,10 @@
        if (in_interrupt ())
                BUG ();
 
-       hub = hcd->self.root_hub;
        hcd->state = USB_STATE_QUIESCING;
 
        dbg ("%s: roothub graceful disconnect", hcd->self.bus_name);
-       usb_disconnect (&hub);
+       usb_disconnect (&hcd->self.root_hub);
 
        hcd->driver->stop (hcd);
        hcd->state = USB_STATE_HALT;
===== drivers/usb/host/uhci-hcd.c 1.112 vs edited =====
--- 1.112/drivers/usb/host/uhci-hcd.c   Fri Jun  4 11:14:49 2004
+++ edited/drivers/usb/host/uhci-hcd.c  Tue Jun  8 14:54:13 2004
@@ -2185,7 +2185,7 @@
 
        uhci->rh_numports = port;
 
-       hcd->self.root_hub = udev = usb_alloc_dev(NULL, &hcd->self, 0);
+       udev = usb_alloc_dev(NULL, &hcd->self, 0);
        if (!udev) {
                dev_err(uhci_dev(uhci), "unable to allocate root hub\n");
                goto err_alloc_root_hub;
@@ -2267,7 +2267,7 @@
 
        udev->speed = USB_SPEED_FULL;
 
-       if (usb_register_root_hub(udev, uhci_dev(uhci)) != 0) {
+       if (hcd_register_root(udev, &uhci->hcd) != 0) {
                dev_err(uhci_dev(uhci), "unable to start root hub\n");
                retval = -ENOMEM;
                goto err_start_root_hub;
@@ -2295,7 +2295,6 @@
 
 err_alloc_term_td:
        usb_put_dev(udev);
-       hcd->self.root_hub = NULL;
 
 err_alloc_root_hub:
        dma_pool_destroy(uhci->qh_pool);
===== drivers/usb/gadget/dummy_hcd.c 1.4 vs edited =====
--- 1.4/drivers/usb/gadget/dummy_hcd.c  Mon May 17 13:12:20 2004
+++ edited/drivers/usb/gadget/dummy_hcd.c       Tue Jun  8 14:54:13 2004
@@ -1664,7 +1664,7 @@
        INIT_LIST_HEAD (&hcd->dev_list);
        usb_register_bus (bus);
 
-       bus->root_hub = root = usb_alloc_dev (0, bus, 0);
+       root = usb_alloc_dev (0, bus, 0);
        if (!root) {
                retval = -ENOMEM;
 clean1:
@@ -1678,8 +1678,7 @@
        root->speed = USB_SPEED_HIGH;
 
        /* ...then configured, so khubd sees us. */
-       if ((retval = hcd_register_root (&dum->hcd)) != 0) {
-               bus->root_hub = 0;
+       if ((retval = hcd_register_root (root, &dum->hcd)) != 0) {
                usb_put_dev (root);
 clean2:
                dum->hcd.state = USB_STATE_QUIESCING;



-------------------------------------------------------
This SF.Net email is sponsored by: GNOME Foundation
Hackers Unite!  GUADEC: The world's #1 Open Source Desktop Event.
GNOME Users and Developers European Conference, 28-30th June in Norway
http://2004/guadec.org
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to