This patch primarily:
- Gets rid of usb_interface->{driver,private_data} since those
roles are handled now in the driver model core.
- Updates the driver model core to make the rest of that possible.
That means adding new methods so driver binding can be done in
the open, not just as a side effect of registration.
Along the way:
- Resolves two minor FIXMEs, returning a status code from
a routine that can fail (changes signature and kerneldoc)
and using the right timeout symbol.
- Fixes a couple usb.c potential oopses where container_of()
result was tested for null, not the input, and (related)
a superfluous test.
- Catches what looked like a self-deadlock case in usbfs.
- Removes some now-dubious code from mdc800, and updates
a diagnostic to be more correct.
I'm not quite sure how the get_bus/put_bus stuff is supposed
to work. This stuff doesn't return the refcounts it acquires,
like other code in "bus.c". Can you sanity check that?
This is lightly tested. Driver bind/unbind seemed to behave
as it should, even "audio.c" which is one of several drivers
which needs to bind to devices in pairs (new notion for the
driver model core). I don't happen to test a user mode driver,
that'd cover code paths that changed (so it deserves testing).
- Dave
--- ./include/linux-dist/usb.h Sun Dec 8 10:57:47 2002
+++ ./include/linux/usb.h Sun Dec 8 16:20:47 2002
@@ -116,9 +116,7 @@
unsigned num_altsetting; /* number of alternate settings */
unsigned max_altsetting; /* total memory allocated */
- struct usb_driver *driver; /* driver */
struct device dev; /* interface specific device info */
- void *private_data;
};
#define to_usb_interface(d) container_of(d, struct usb_interface, dev)
#define interface_to_usbdev(intf) \
@@ -262,12 +260,16 @@
/* for drivers using iso endpoints */
extern int usb_get_current_frame_number (struct usb_device *usb_dev);
-/* used these for multi-interface device registration */
-extern void usb_driver_claim_interface(struct usb_driver *driver,
+/* use these for multi-interface drivers */
+struct usb_driver;
+
+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);
extern void usb_driver_release_interface(struct usb_driver *driver,
struct usb_interface *iface);
+
+/* */
const struct usb_device_id *usb_match_id(struct usb_interface *interface,
const struct usb_device_id *id);
--- ./drivers/usb-dist/core/usb.c Sun Dec 8 10:57:46 2002
+++ ./drivers/usb/core/usb.c Sun Dec 8 16:09:47 2002
@@ -7,8 +7,7 @@
* (C) Copyright Gregory P. Smith 1999
* (C) Copyright Deti Fliegl 1999 (new USB architecture)
* (C) Copyright Randy Dunlap 2000
- * (C) Copyright David Brownell 2000-2001 (kernel hotplug, usb_device_id,
- more docs, etc)
+ * (C) Copyright David Brownell 2000-2002
* (C) Copyright Yggdrasil Computing, Inc. 2000
* (usb_device_id matching changes by Adam J. Richter)
* (C) Copyright Greg Kroah-Hartman 2002
@@ -95,8 +94,6 @@
error = driver->probe (intf, id);
up (&driver->serialize);
}
- if (!error)
- intf->driver = driver;
module_put(driver->owner);
@@ -111,7 +108,7 @@
intf = list_entry(dev,struct usb_interface,dev);
driver = to_usb_driver(dev->driver);
- if (!driver) {
+ if (!dev->driver) {
err("%s does not have a valid driver to work with!",
__FUNCTION__);
return -ENODEV;
@@ -129,12 +126,8 @@
* module unload */
down(&driver->serialize);
- if (intf->driver && intf->driver->disconnect)
- intf->driver->disconnect(intf);
-
- /* if driver->disconnect didn't release the interface */
- if (intf->driver)
- usb_driver_release_interface(driver, intf);
+ if (driver->disconnect)
+ driver->disconnect(intf);
up(&driver->serialize);
module_put(driver->owner);
@@ -269,10 +262,13 @@
* @iface: the interface to which it will be bound
* @priv: driver data associated with that interface
*
+ * Returns zero on success, or negative errno if the interface has
+ * already been claimed.
+ *
* This is used by usb device drivers that need to claim more than one
* interface on a device when probing (audio and acm are current examples).
* No device driver should directly modify internal usb_interface or
- * usb_device structure members.
+ * usb_device structure members, or call this outside of its probe().
*
* 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
@@ -280,20 +276,20 @@
* first be sure that no other driver has claimed the interface, by
* checking with usb_interface_claimed().
*/
-void usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface
*iface, void* priv)
+int usb_driver_claim_interface (struct usb_driver *driver,
+ struct usb_interface *iface, void* priv)
{
- if (!iface || !driver)
- return;
+ int retval;
- // FIXME change API to report an error in this case
- if (iface->driver)
- err ("%s driver booted %s off interface %p",
- driver->name, iface->driver->name, iface);
- else
- dbg("%s driver claimed interface %p", driver->name, iface);
+ if (!iface || !driver)
+ return -ENODEV;
- iface->driver = driver;
- iface->private_data = priv;
+ retval = __device_bind (&iface->dev, &driver->driver);
+ if (retval == 0)
+ dev_set_drvdata (&iface->dev, priv);
+ dbg ("%s driver %s interface %p", driver->name,
+ retval ? "didn't claim" : "claimed", iface);
+ return retval;
}
/**
@@ -312,7 +308,7 @@
if (!iface)
return 0;
- return (iface->driver != NULL);
+ return (iface->dev.driver != NULL);
} /* usb_interface_claimed() */
/**
@@ -332,11 +328,11 @@
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 || iface->driver != driver)
+ if (!iface || iface->dev.driver != &driver->driver)
return;
- iface->driver = NULL;
- iface->private_data = NULL;
+ __device_unbind (&iface->dev);
+ dev_set_drvdata (&iface->dev, NULL);
}
/**
@@ -408,7 +404,7 @@
struct usb_device *dev;
/* proc_connectinfo in devio.c may call us with id == NULL. */
- if (id == NULL)
+ if (interface == NULL || id == NULL)
return NULL;
intf = &interface->altsetting [interface->act_altsetting];
@@ -521,14 +517,11 @@
if (!dev)
return -ENODEV;
+ intf = to_usb_interface(dev);
if (dev->driver == &usb_generic_driver)
return 0;
- intf = to_usb_interface(dev);
- if (!intf)
- return -ENODEV;
-
usb_dev = interface_to_usbdev (intf);
if (!usb_dev)
return -ENODEV;
@@ -844,8 +837,7 @@
int usb_set_address(struct usb_device *dev)
{
return usb_control_msg(dev, usb_snddefctrl(dev), USB_REQ_SET_ADDRESS,
- // FIXME USB_CTRL_SET_TIMEOUT
- 0, dev->devnum, 0, NULL, 0, HZ * USB_CTRL_GET_TIMEOUT);
+ 0, dev->devnum, 0, NULL, 0, HZ * USB_CTRL_SET_TIMEOUT);
}
--- ./drivers/usb-dist/core/devices.c Sun Dec 8 10:49:58 2002
+++ ./drivers/usb/core/devices.c Sun Dec 8 12:56:12 2002
@@ -235,10 +235,13 @@
static char *usb_dump_interface_descriptor(char *start, char *end, const struct
usb_interface *iface, int setno)
{
struct usb_interface_descriptor *desc = &iface->altsetting[setno].desc;
+ struct usb_driver *driver = 0;
if (start > end)
return start;
lock_kernel(); /* driver might be unloaded */
+ if (iface->dev.driver)
+ driver = to_usb_driver (iface->dev.driver);
start += sprintf(start, format_iface,
desc->bInterfaceNumber,
desc->bAlternateSetting,
@@ -247,7 +250,7 @@
class_decode(desc->bInterfaceClass),
desc->bInterfaceSubClass,
desc->bInterfaceProtocol,
- iface->driver ? iface->driver->name : "(none)");
+ driver ? driver->name : "(none)");
unlock_kernel();
return start;
}
--- ./drivers/usb-dist/core/devio.c Sun Dec 8 10:57:46 2002
+++ ./drivers/usb/core/devio.c Sun Dec 8 17:04:46 2002
@@ -347,6 +347,11 @@
.disconnect = driver_disconnect,
};
+/* NOTE: usbfs necessarily doesn't use probe() to bind to interfaces.
+ * it's a kind of "generic driver", that user mode tools bind to
+ * interfaces on usb devices (or unbind) as needed.
+ */
+
static int claimintf(struct dev_state *ps, unsigned int intf)
{
struct usb_device *dev = ps->dev;
@@ -360,14 +365,13 @@
if (test_bit(intf, &ps->ifclaimed))
return 0;
iface = &dev->actconfig->interface[intf];
- err = -EBUSY;
- lock_kernel();
- if (!usb_interface_claimed(iface)) {
- usb_driver_claim_interface(&usbdevfs_driver, iface, ps);
+
+ /* device model handles locking */
+ err = device_bind (&iface->dev, &usbdevfs_driver.driver);
+ if (err == 0) {
+ dev_set_drvdata (&iface->dev, ps);
set_bit(intf, &ps->ifclaimed);
- err = 0;
}
- unlock_kernel();
return err;
}
@@ -381,10 +385,10 @@
return -EINVAL;
err = -EINVAL;
dev = ps->dev;
+ iface = &dev->actconfig->interface[intf];
down(&dev->serialize);
if (dev && test_and_clear_bit(intf, &ps->ifclaimed)) {
- iface = &dev->actconfig->interface[intf];
- usb_driver_release_interface(&usbdevfs_driver, iface);
+ device_unbind (&iface->dev);
err = 0;
}
up(&dev->serialize);
@@ -674,6 +678,7 @@
{
struct usbdevfs_getdriver gd;
struct usb_interface *interface;
+ struct usb_driver *driver;
int ret;
if (copy_from_user(&gd, arg, sizeof(gd)))
@@ -683,9 +688,10 @@
interface = usb_ifnum_to_if(ps->dev, gd.interface);
if (!interface)
return -EINVAL;
- if (!interface->driver)
+ if (!interface->dev.driver)
return -ENODATA;
- strcpy(gd.driver, interface->driver->name);
+ driver = to_usb_driver (interface->dev.driver);
+ strcpy(gd.driver, driver->name);
if (copy_to_user(arg, &gd, sizeof(gd)))
return -EFAULT;
return 0;
@@ -718,7 +724,7 @@
continue;
err ("%s - this function is broken", __FUNCTION__);
- if (intf->driver && ps->dev) {
+ if (intf->dev.driver && ps->dev) {
usb_device_probe (&intf->dev);
}
}
@@ -739,7 +745,7 @@
interface = usb_ifnum_to_if(ps->dev, setintf.interface);
if (!interface)
return -EINVAL;
- if (interface->driver) {
+ if (interface->dev.driver) {
if ((ret = checkintf(ps, ret)))
return ret;
}
@@ -1089,13 +1095,16 @@
retval = -EINVAL;
else switch (ctrl.ioctl_code) {
- /* disconnect kernel driver from interface, leaving it unbound. */
- /* maybe unbound - you get no guarantee it stays unbound */
- case USBDEVFS_DISCONNECT:
- /* this function is misdesigned - retained for compatibility */
+ /* disconnect kernel driver from interface, so it's unbound for now */
+ case USBDEVFS_DISCONNECT:
+ if (test_bit(ctrl.ifno, &ps->ifclaimed)) {
+ // use USBDEVFS_RELEASEINTERFACE instead
+ retval = -EDEADLK;
+ break;
+ }
lock_kernel();
- driver = ifp->driver;
- if (driver) {
+ if (ifp->dev.driver) {
+ driver = to_usb_driver (ifp->dev.driver);
dbg ("disconnect '%s' from dev %d interface %d",
driver->name, ps->dev->devnum, ctrl.ifno);
usb_device_remove(&ifp->dev);
@@ -1118,8 +1127,8 @@
* driver module.
*/
lock_kernel ();
- driver = ifp->driver;
- if (driver == 0 || driver->ioctl == 0) {
+ driver = to_usb_driver (ifp->dev.driver);
+ if (ifp->dev.driver == 0 || driver->ioctl == 0) {
unlock_kernel();
retval = -ENOSYS;
} else {
--- ./drivers/usb-dist/misc/usbtest.c Sun Dec 8 10:57:46 2002
+++ ./drivers/usb/misc/usbtest.c Sun Dec 8 11:00:10 2002
@@ -978,7 +978,7 @@
dev_set_drvdata (&intf->dev, 0);
info ("unbound %s", dev->id);
- kfree (intf->private_data);
+ kfree (dev);
}
/* Basic testing only needs a device that can source or sink bulk traffic.
--- ./drivers/usb-dist/storage/scsiglue.c Sun Dec 8 10:49:56 2002
+++ ./drivers/usb/storage/scsiglue.c Sun Dec 8 13:01:50 2002
@@ -254,7 +254,7 @@
&pusb_dev_save->actconfig->interface[i];
/* if this is an unclaimed interface, skip it */
- if (!intf->driver) {
+ if (!intf->dev.driver) {
continue;
}
--- ./drivers/usb-dist/image/mdc800.c Sun Dec 8 10:57:46 2002
+++ ./drivers/usb/image/mdc800.c Sun Dec 8 13:15:07 2002
@@ -467,7 +467,6 @@
}
- usb_driver_claim_interface (&mdc800_usb_driver, intf, mdc800);
if (usb_set_interface (dev, intf_desc->desc.bInterfaceNumber, 0) < 0)
{
err ("MDC800 Configuration fails.");
@@ -549,12 +548,10 @@
usb_unlink_urb (mdc800->write_urb);
usb_unlink_urb (mdc800->download_urb);
- usb_driver_release_interface (&mdc800_usb_driver, intf);
-
mdc800->dev=0;
dev_set_drvdata(&intf->dev, NULL);
}
- info ("Mustek MDC800 disconnected from USB.");
+ info ("Mustek MDC800 driver disconnected from USB device.");
}
--- ./include/linux-dist/device.h Sun Dec 8 10:57:47 2002
+++ ./include/linux/device.h Sun Dec 8 11:57:32 2002
@@ -311,6 +311,12 @@
extern int device_add(struct device * dev);
extern void device_del(struct device * dev);
+/* locked and unlocked driver binding primitives */
+extern int __device_bind(struct device * dev, struct device_driver *driver);
+extern int device_bind(struct device * dev, struct device_driver *driver);
+extern void __device_unbind(struct device * dev);
+extern void device_unbind(struct device * dev);
+
/* driverfs interface for exporting device attributes */
struct device_attribute {
--- ./drivers/base-dist/bus.c Sun Dec 8 10:57:44 2002
+++ ./drivers/base/bus.c Sun Dec 8 15:59:56 2002
@@ -326,18 +326,67 @@
return error;
}
+/**
+ * __device_bind - bind specific driver to a device, without locking
+ * @dev: device with no current driver binding
+ * @drv: driver being bound; known to be a usable match.
+ *
+ * Some bus models (notably USB) allow logical devices to be packaged in
+ * groups, so that binding one device must atomically bind another one.
+ * This routine may be used from one of those devices' probe() routines,
+ * where the bus rwsem is already locked, to achieve that binding; no
+ * probe() routine is invoked to validate the new binding, or (re)enter
+ * initialization code.
+ *
+ * Returns zero on success, or else negative errno if binding would
+ * have overwritten an existing binding.
+ */
+int __device_bind (struct device * dev, struct device_driver * drv)
+{
+ if (dev->driver)
+ return -EINVAL;
+ dev->driver = drv;
+ attach(dev);
+ return 0;
+}
/**
- * detach - do dirty work of detaching device from its driver.
- * @dev: device.
- * @drv: its driver.
+ * device_bind - bind specific driver to a device
+ * @dev: device with no current driver binding
+ * @drv: driver being bound; known to be a usable match.
+ *
+ * When drivers are bound to device outside of a probe(), use this
+ * routine to acquire the bus rwsem before calling __device_bind().
+ * Returns zero on success, or else negative errno.
+ */
+int device_bind(struct device *dev, struct device_driver *drv)
+{
+ struct bus_type *bus = get_bus(dev->bus);
+ int retval;
+
+ if (!bus)
+ return -EINVAL;
+ down_write(&bus->subsys.rwsem);
+ retval = __device_bind(dev, drv);
+ up_write(&bus->subsys.rwsem);
+ // put_bus(bus);
+ return retval;
+}
+
+/**
+ * __device_unbind - disconnect driver from a device (unlocked)
+ * @dev: device whose driver is being unbound.
*
* Note that calls to this function are serialized by taking the
- * bus's rwsem in both bus_remove_device() and bus_remove_driver().
+ * bus's rwsem in bus_remove_device(), bus_remove_driver(),
+ * or device_unbind(). If the driver has a remove() method,
+ * that is invoked.
*/
-static void detach(struct device * dev, struct device_driver * drv)
+void __device_unbind(struct device * dev)
{
+ struct device_driver *drv = dev->driver;
+
if (drv) {
sysfs_remove_link(&drv->kobj,dev->kobj.name);
list_del_init(&dev->driver_list);
@@ -350,13 +399,32 @@
/**
+ * device_unbind - disconnect driver from a device
+ * @dev: device
+ *
+ * This acquires the bus rwsem and calls __device_unbind.
+ */
+void device_unbind(struct device * dev)
+{
+ struct bus_type *bus = get_bus(dev->bus);
+
+ if (bus) {
+ down_write(&bus->subsys.rwsem);
+ __device_unbind(dev);
+ up_write(&bus->subsys.rwsem);
+ // put_bus(bus);
+ }
+}
+
+
+/**
* device_detach - detach device from its driver.
* @dev: device.
*/
static void device_detach(struct device * dev)
{
- detach(dev,dev->driver);
+ __device_unbind (dev);
}
@@ -370,7 +438,7 @@
struct list_head * entry, * next;
list_for_each_safe(entry,next,&drv->devices) {
struct device * dev = container_of(entry,struct device,driver_list);
- detach(dev,drv);
+ __device_unbind (dev);
}
}
@@ -550,5 +618,10 @@
EXPORT_SYMBOL(get_bus);
EXPORT_SYMBOL(put_bus);
+EXPORT_SYMBOL(__device_bind);
+EXPORT_SYMBOL(device_bind);
+EXPORT_SYMBOL(__device_unbind);
+EXPORT_SYMBOL(device_unbind);
+
EXPORT_SYMBOL(bus_create_file);
EXPORT_SYMBOL(bus_remove_file);
