LWN.net Logo

[patch 2.6.0-test6] rm interface.driver (1/3)

From:  David Brownell <david-b@pacbell.net>
To:  Greg KH <greg@kroah.com>, linux-usb-devel@lists.sourceforge.net
Subject:  [linux-usb-devel] [patch 2.6.0-test6] rm interface.driver (1/3)
Date:  Tue, 07 Oct 2003 13:10:16 -0700

This gets rid of the strange "half bound" state that secondary
interfaces have been in:  the driver model and sysfs haven't
yet reflected those bindings.

     - Remove interface.driver (interface.dev.driver suffices), so
       there's only the driver model notion of binding.

     - Call device_bind_driver() from usb_driver_claim_interface(),
       so the driver model and sysfs show the binding, if that
       interface has already been registered.

     - Call device_release_driver() from usb_driver_release_interface(),
       reversing the effect of that binding.  So usb_unbind_interface()
       doesn't use release_interface any more (to break the recursion).

     - Updates docs ... those driver model calls pass along a
       requirement to hold the bus writelock, which probe() and
       disconnect() calls already hold.

     - Remove usb_driver.serialize (bus writelock suffices) to get
       rid of self-deadlock if disconnect() uses release_interface.

     - Deprecates usb_interface_claimed(); it's like check_region().

Notable change:  claimed interfaces now get normal disconnect() methods,
which multi-interface drivers (audio, cdc, etc) may not expect.  Easy
to work around ... say by using a null driver data pointer to indicate
secondary interfaces, for which disconnect() should be a NOP.

- Dave



--- 1.90/include/linux/usb.h	Mon Sep 15 06:58:01 2003
+++ edited/include/linux/usb.h	Tue Oct  7 09:35:28 2003
@@ -118,7 +118,6 @@
 	unsigned act_altsetting;	/* active alternate setting */
 	unsigned num_altsetting;	/* number of alternate settings */
 
-	struct usb_driver *driver;	/* driver */
 	int minor;			/* minor number this interface is bound to */
 	struct device dev;		/* interface specific device info */
 	struct class_device *class_dev;
@@ -286,7 +285,11 @@
 /* for drivers using iso endpoints */
 extern int usb_get_current_frame_number (struct usb_device *usb_dev);
 
-/* used these for multi-interface device registration */
+struct usb_driver;
+
+/* used by drivers that control several related interfaces during
+ * probe()/disconnect(), or need other fancy driver binding models
+ */
 extern int usb_driver_claim_interface(struct usb_driver *driver,
 			struct usb_interface *iface, void* priv);
 extern int usb_interface_claimed(struct usb_interface *iface);
@@ -451,8 +454,6 @@
 	const struct usb_device_id *id_table;
 
 	struct device_driver driver;
-
-	struct semaphore serialize;
 };
 #define	to_usb_driver(d) container_of(d, struct usb_driver, driver)
 
--- 1.37/drivers/usb/core/message.c	Tue Sep 23 11:18:09 2003
+++ edited/drivers/usb/core/message.c	Mon Oct  6 11:39:36 2003
@@ -810,6 +810,7 @@
 			dev_dbg (&dev->dev, "unregistering interface %s\n",
 				interface->dev.bus_id);
 			device_del(&interface->dev);
+			interface->dev.bus = 0;
 		}
 		dev->actconfig = 0;
 		if (dev->state == USB_STATE_CONFIGURED)
@@ -1118,8 +1119,10 @@
 			desc = &intf->altsetting [0].desc;
 			usb_enable_interface(dev, intf);
 
+			/* multi-interface drivers (cdc etc) might
+			 * already have claimed this interface (i>0).
+			 */
 			intf->dev.parent = &dev->dev;
-			intf->dev.driver = NULL;
 			intf->dev.bus = &usb_bus_type;
 			intf->dev.dma_mask = dev->dev.dma_mask;
 			sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",
--- 1.143/drivers/usb/core/usb.c	Thu Sep 25 03:59:51 2003
+++ edited/drivers/usb/core/usb.c	Tue Oct  7 11:01:17 2003
@@ -80,7 +80,7 @@
 
 static int usb_generic_driver_data;
 
-/* needs to be called with BKL held */
+/* called from driver core, with usb bus writelock */
 int usb_probe_interface(struct device *dev)
 {
 	struct usb_interface * intf = to_usb_interface(dev);
@@ -88,41 +88,36 @@
 	const struct usb_device_id *id;
 	int error = -ENODEV;
 
-	dev_dbg(dev, "%s\n", __FUNCTION__);
-
 	if (!driver->probe)
 		return error;
 
 	id = usb_match_id (intf, driver->id_table);
 	if (id) {
-		dev_dbg (dev, "%s - got id\n", __FUNCTION__);
-		down (&driver->serialize);
+		dev_dbg (dev, "%s - id match\n", __FUNCTION__);
 		error = driver->probe (intf, id);
-		up (&driver->serialize);
-	}
-	if (!error)
-		intf->driver = driver;
+	} else
+		dev_dbg(dev, "%s - no match\n", __FUNCTION__);
 
 	return error;
 }
 
+/* called from driver core, with usb bus writelock */
 int usb_unbind_interface(struct device *dev)
 {
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct usb_driver *driver = to_usb_driver(dev->driver);
 
-	down(&driver->serialize);
-
 	/* release all urbs for this interface */
 	usb_disable_interface(interface_to_usbdev(intf), intf);
 
-	if (intf->driver && intf->driver->disconnect)
-		intf->driver->disconnect(intf);
+	if (dev->driver && driver->disconnect)
+		driver->disconnect(intf);
 
-	/* force a release and re-initialize the interface */
-	usb_driver_release_interface(driver, intf);
-
-	up(&driver->serialize);
+	/* reset other interface state */
+	usb_set_interface(interface_to_usbdev(intf),
+			intf->altsetting[0].desc.bInterfaceNumber,
+			0);
+	usb_set_intfdata(intf, NULL);
 
 	return 0;
 }
@@ -152,8 +147,6 @@
 	new_driver->driver.probe = usb_probe_interface;
 	new_driver->driver.remove = usb_unbind_interface;
 
-	init_MUTEX(&new_driver->serialize);
-
 	retval = driver_register(&new_driver->driver);
 
 	if (!retval) {
@@ -252,8 +245,8 @@
 
 /**
  * usb_driver_claim_interface - bind a driver to an interface
- * @driver: the driver to be bound
- * @iface: the interface to which it will be bound
+ * @driver: the driver to be bound (must be registered)
+ * @iface: the interface to which it will be bound (from active config)
  * @priv: driver data associated with that interface
  *
  * This is used by usb device drivers that need to claim more than one
@@ -264,26 +257,25 @@
  * Few drivers should need to use this routine, since the most natural
  * way to bind to an interface is to return the private data from
  * the driver's probe() method.
+ *
+ * Drivers that can control multiple interfaces -- class drivers like
+ * CDC, audio, and video, and some vendor-specific drivers -- use this
+ * when probe()ing the main interface, to claim the other interfaces.
+ * Any other users need to hold the driver core writelock for this bus.
  */
 int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface *iface, void* priv)
 {
 	if (!iface || !driver)
 		return -EINVAL;
-
-	/* this is mainly to lock against usbfs */
-	lock_kernel();
-	if (iface->driver) {
-		unlock_kernel();
-		err ("%s driver booted %s off interface %p",
-			driver->name, iface->driver->name, iface);
+	if (iface->dev.driver)
 		return -EBUSY;
-	} else {
-	    dbg("%s driver claimed interface %p", driver->name, iface);
-	}
 
-	iface->driver = driver;
+	iface->dev.driver = &driver->driver;
 	usb_set_intfdata(iface, priv);
-	unlock_kernel();
+
+	/* interfaces may not be registered yet in sysfs */
+	if (iface->dev.bus)
+		device_bind_driver(&iface->dev);
 	return 0;
 }
 
@@ -291,20 +283,17 @@
  * usb_interface_claimed - returns true iff an interface is claimed
  * @iface: the interface being checked
  *
- * This should be used by drivers to check other interfaces to see if
+ * This could be used by drivers to check other interfaces to see if
  * they are available or not.  If another driver has claimed the interface,
- * they may not claim it.  Otherwise it's OK to claim it using
- * usb_driver_claim_interface().
+ * they may not claim it.  However, success here is not a guarantee that
+ * usb_driver_claim_interface() won't report a failure later.
  *
  * Returns true (nonzero) iff the interface is claimed, else false (zero).
  */
-int usb_interface_claimed(struct usb_interface *iface)
+int __deprecated usb_interface_claimed(struct usb_interface *iface)
 {
-	if (!iface)
-		return 0;
-
-	return (iface->driver != NULL);
-} /* usb_interface_claimed() */
+	return (iface != NULL) && (iface->dev.driver != NULL);
+}
 
 /**
  * usb_driver_release_interface - unbind a driver from an interface
@@ -312,29 +301,25 @@
  * @iface: the interface from which it will be unbound
  *
  * In addition to unbinding the driver, this re-initializes the interface
- * by selecting altsetting 0, the default alternate setting.
- * 
- * This can be used by drivers to release an interface without waiting
- * for their disconnect() methods to be called.
- *
- * When the USB subsystem disconnect()s a driver from some interface,
- * it automatically invokes this method for that interface.  That
- * means that even drivers that used usb_driver_claim_interface()
- * usually won't need to call this.
+ * by selecting altsetting 0 (the default setting).  Unbinding includes
+ * calling the driver disconnect() method for that interface, and removing
+ * sysfs records of the binding.
  *
  * This call is synchronous, and may not be used in an interrupt context.
+ * The caller must own the driver core usb bus writelock.
  */
 void usb_driver_release_interface(struct usb_driver *driver, struct usb_interface *iface)
 {
 	/* this should never happen, don't release something that's not ours */
-	if (iface->driver && iface->driver != driver)
+	if (iface->dev.driver && iface->dev.driver != &driver->driver)
 		return;
 
-	usb_set_interface(interface_to_usbdev(iface),
-			iface->altsetting[0].desc.bInterfaceNumber,
-			0);
-	usb_set_intfdata(iface, NULL);
-	iface->driver = NULL;
+dev_dbg(&iface->dev, "release 1\n");
+
+	/* trigger usb_unbind_interface(), and do sysfs magic */
+	device_release_driver(&iface->dev);
+
+dev_dbg(&iface->dev, "release 2\n");
 }
 
 /**


Copyright © 2003, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds