ChangeSet 1.1673.8.64, 2004/03/31 13:35:04-08:00, [EMAIL PROTECTED]
[PATCH] USB: remove usb_interface.driver field
Remove usb_interface.driver, and along with it the "half bound" state
previously associated with drivers binding with claim() instead of probe().
This changes usb_driver_claim_interface() semantics slightly: drivers must
now be prepared to accept disconnect() callbacks.
Fixes more locking bugs, and a claim() oops that snuck in with a
recent patch.
drivers/usb/core/devices.c | 4 +-
drivers/usb/core/devio.c | 88 ++++++++++++++++-----------------------------
drivers/usb/core/message.c | 25 +++++++++++-
drivers/usb/core/usb.c | 87 ++++++++++++++------------------------------
include/linux/usb.h | 18 ++++++++-
5 files changed, 102 insertions(+), 120 deletions(-)
diff -Nru a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
--- a/drivers/usb/core/devices.c Wed Apr 14 14:34:40 2004
+++ b/drivers/usb/core/devices.c Wed Apr 14 14:34:40 2004
@@ -247,7 +247,9 @@
class_decode(desc->bInterfaceClass),
desc->bInterfaceSubClass,
desc->bInterfaceProtocol,
- iface->driver ? iface->driver->name : "(none)");
+ iface->dev.driver
+ ? iface->dev.driver->name
+ : "(none)");
up_read(&usb_bus_type.subsys.rwsem);
return start;
}
diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c Wed Apr 14 14:34:40 2004
+++ b/drivers/usb/core/devio.c Wed Apr 14 14:34:40 2004
@@ -704,9 +704,9 @@
if ((ret = findintfif(ps->dev, gd.interface)) < 0)
return ret;
interface = ps->dev->actconfig->interface[ret];
- if (!interface->driver)
+ if (!interface->dev.driver)
return -ENODATA;
- strcpy(gd.driver, interface->driver->name);
+ strncpy(gd.driver, interface->dev.driver->name, sizeof(gd.driver));
if (copy_to_user(arg, &gd, sizeof(gd)))
return -EFAULT;
return 0;
@@ -725,26 +725,11 @@
static int proc_resetdevice(struct dev_state *ps)
{
- int i, ret;
-
- ret = usb_reset_device(ps->dev);
- if (ret < 0)
- return ret;
-
- for (i = 0; i < ps->dev->actconfig->desc.bNumInterfaces; i++) {
- struct usb_interface *intf = ps->dev->actconfig->interface[i];
-
- /* Don't simulate interfaces we've claimed */
- if (test_bit(i, &ps->ifclaimed))
- continue;
-
- err ("%s - this function is broken", __FUNCTION__);
- if (intf->driver && ps->dev) {
- usb_probe_interface (&intf->dev);
- }
- }
+ /* FIXME when usb_reset_device() is fixed we'll need to grab
+ * ps->dev->serialize before calling it.
+ */
+ return usb_reset_device(ps->dev);
- return 0;
}
static int proc_setintf(struct dev_state *ps, void __user *arg)
@@ -758,7 +743,7 @@
if ((ret = findintfif(ps->dev, setintf.interface)) < 0)
return ret;
interface = ps->dev->actconfig->interface[ret];
- if (interface->driver) {
+ if (interface->dev.driver) {
if ((ret = checkintf(ps, ret)))
return ret;
}
@@ -1141,58 +1126,51 @@
}
}
- if (!ps->dev)
- retval = -ENODEV;
- else if (!(ifp = usb_ifnum_to_if (ps->dev, ctrl.ifno)))
+ if (!ps->dev) {
+ if (buf)
+ kfree(buf);
+ return -ENODEV;
+ }
+
+ down(&ps->dev->serialize);
+ if (ps->dev->state != USB_STATE_CONFIGURED)
+ retval = -ENODEV;
+ else if (!(ifp = usb_ifnum_to_if (ps->dev, ctrl.ifno)))
retval = -EINVAL;
- else switch (ctrl.ioctl_code) {
+ 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 */
- lock_kernel();
- driver = ifp->driver;
- if (driver) {
- dbg ("disconnect '%s' from dev %d interface %d",
- driver->name, ps->dev->devnum, ctrl.ifno);
- usb_unbind_interface(&ifp->dev);
+ /* disconnect kernel driver from interface */
+ case USBDEVFS_DISCONNECT:
+ down_write(&usb_bus_type.subsys.rwsem);
+ if (ifp->dev.driver) {
+ driver = to_usb_driver(ifp->dev.driver);
+ dev_dbg (&ifp->dev, "disconnect by usbfs\n");
+ usb_driver_release_interface(driver, ifp);
} else
retval = -ENODATA;
- unlock_kernel();
+ up_write(&usb_bus_type.subsys.rwsem);
break;
/* let kernel drivers try to (re)bind to the interface */
case USBDEVFS_CONNECT:
- lock_kernel();
- retval = usb_probe_interface (&ifp->dev);
- unlock_kernel();
+ bus_rescan_devices(ifp->dev.bus);
break;
/* talk directly to the interface's driver */
default:
- /* BKL used here to protect against changing the binding
- * of this driver to this device, as well as unloading its
- * driver module.
- */
- lock_kernel ();
- driver = ifp->driver;
+ down_read(&usb_bus_type.subsys.rwsem);
+ if (ifp->dev.driver)
+ driver = to_usb_driver(ifp->dev.driver);
if (driver == 0 || driver->ioctl == 0) {
- unlock_kernel();
- retval = -ENOSYS;
+ retval = -ENOTTY;
} else {
- if (!try_module_get (driver->owner)) {
- unlock_kernel();
- retval = -ENOSYS;
- break;
- }
- unlock_kernel ();
retval = driver->ioctl (ifp, ctrl.ioctl_code, buf);
if (retval == -ENOIOCTLCMD)
retval = -ENOTTY;
- module_put (driver->owner);
}
+ up_read(&usb_bus_type.subsys.rwsem);
}
+ up(&ps->dev->serialize);
/* cleanup and return */
if (retval >= 0
diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c
--- a/drivers/usb/core/message.c Wed Apr 14 14:34:40 2004
+++ b/drivers/usb/core/message.c Wed Apr 14 14:34:40 2004
@@ -1176,15 +1176,34 @@
intf->dev.bus = &usb_bus_type;
intf->dev.dma_mask = dev->dev.dma_mask;
intf->dev.release = release_interface;
+ device_initialize (&intf->dev);
sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",
dev->bus->busnum, dev->devpath,
configuration,
alt->desc.bInterfaceNumber);
+ }
+
+ /* Now that all interfaces are setup, probe() calls
+ * may claim() any interface that's not yet bound.
+ * Many class drivers need that: CDC, audio, video, etc.
+ */
+ for (i = 0; i < cp->desc.bNumInterfaces; ++i) {
+ struct usb_interface *intf = cp->interface[i];
+ struct usb_interface_descriptor *desc;
+
+ desc = &intf->altsetting [0].desc;
dev_dbg (&dev->dev,
- "registering %s (config #%d, interface %d)\n",
+ "adding %s (config #%d, interface %d)\n",
intf->dev.bus_id, configuration,
- alt->desc.bInterfaceNumber);
- device_register (&intf->dev);
+ desc->bInterfaceNumber);
+ ret = device_add (&intf->dev);
+ if (ret != 0) {
+ dev_err(&dev->dev,
+ "device_add(%s) --> %d\n",
+ intf->dev.bus_id,
+ ret);
+ continue;
+ }
usb_create_driverfs_intf_files (intf);
}
}
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Wed Apr 14 14:34:40 2004
+++ b/drivers/usb/core/usb.c Wed Apr 14 14:34:40 2004
@@ -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-2004
* (C) Copyright Yggdrasil Computing, Inc. 2000
* (usb_device_id matching changes by Adam J. Richter)
* (C) Copyright Greg Kroah-Hartman 2002-2003
@@ -95,17 +94,11 @@
if (!driver->probe)
return error;
- /* driver claim() doesn't yet affect dev->driver... */
- if (intf->driver)
- return error;
-
id = usb_match_id (intf, driver->id_table);
if (id) {
dev_dbg (dev, "%s - got id\n", __FUNCTION__);
error = driver->probe (intf, id);
}
- if (!error)
- intf->driver = driver;
return error;
}
@@ -114,7 +107,7 @@
int usb_unbind_interface(struct device *dev)
{
struct usb_interface *intf = to_usb_interface(dev);
- struct usb_driver *driver = intf->driver;
+ struct usb_driver *driver = to_usb_driver(intf->dev.driver);
/* release all urbs for this interface */
usb_disable_interface(interface_to_usbdev(intf), intf);
@@ -127,7 +120,6 @@
intf->altsetting[0].desc.bInterfaceNumber,
0);
usb_set_intfdata(intf, NULL);
- intf->driver = NULL;
return 0;
}
@@ -290,7 +282,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
+ * @iface: the interface to which it will be bound; must be in the
+ * usb device's active configuration
* @priv: driver data associated with that interface
*
* This is used by usb device drivers that need to claim more than one
@@ -308,75 +301,52 @@
*/
int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface
*iface, void* priv)
{
- if (!iface || !driver)
- return -EINVAL;
+ struct device *dev = &iface->dev;
- if (iface->driver)
+ if (dev->driver)
return -EBUSY;
- /* FIXME should device_bind_driver() */
- iface->driver = driver;
+ dev->driver = &driver->driver;
usb_set_intfdata(iface, priv);
- return 0;
-}
-/**
- * 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
- * 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().
- *
- * Returns true (nonzero) iff the interface is claimed, else false (zero).
- */
-int usb_interface_claimed(struct usb_interface *iface)
-{
- if (!iface)
- return 0;
+ /* if interface was already added, bind now; else let
+ * the future device_add() bind it, bypassing probe()
+ */
+ if (!list_empty (&dev->bus_list))
+ device_bind_driver(dev);
- return (iface->driver != NULL);
-} /* usb_interface_claimed() */
+ return 0;
+}
/**
* usb_driver_release_interface - unbind a driver from an interface
* @driver: the driver to be unbound
* @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.
+ * for their disconnect() methods to be called. In typical cases this
+ * also causes the driver disconnect() method to be called.
*
* This call is synchronous, and may not be used in an interrupt context.
- * Callers must own the driver model's usb bus writelock. So driver
- * disconnect() entries don't need extra locking, but other call contexts
- * may need to explicitly claim that lock.
+ * Callers must own the usb_device serialize semaphore and the driver model's
+ * usb bus writelock. So driver disconnect() entries don't need extra locking,
+ * but other call contexts may need to explicitly claim those locks.
*/
-void usb_driver_release_interface(struct usb_driver *driver, struct usb_interface
*iface)
+void usb_driver_release_interface(struct usb_driver *driver,
+ struct usb_interface *iface)
{
+ struct device *dev = &iface->dev;
+
/* this should never happen, don't release something that's not ours */
- if (!iface || !iface->driver || iface->driver != driver)
+ if (!dev->driver || dev->driver != &driver->driver)
return;
- if (iface->dev.driver) {
- /* FIXME should be the ONLY case here */
- device_release_driver(&iface->dev);
- return;
- }
+ /* don't disconnect from disconnect(), or before dev_add() */
+ if (!list_empty (&dev->driver_list) && !list_empty (&dev->bus_list))
+ device_release_driver(dev);
- usb_set_interface(interface_to_usbdev(iface),
- iface->altsetting[0].desc.bInterfaceNumber,
- 0);
+ dev->driver = NULL;
usb_set_intfdata(iface, NULL);
- iface->driver = NULL;
}
/**
@@ -1633,7 +1603,6 @@
EXPORT_SYMBOL(usb_hub_tt_clear_buffer);
EXPORT_SYMBOL(usb_driver_claim_interface);
-EXPORT_SYMBOL(usb_interface_claimed);
EXPORT_SYMBOL(usb_driver_release_interface);
EXPORT_SYMBOL(usb_match_id);
EXPORT_SYMBOL(usb_find_interface);
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Wed Apr 14 14:34:40 2004
+++ b/include/linux/usb.h Wed Apr 14 14:34:40 2004
@@ -31,6 +31,7 @@
}
struct usb_device;
+struct usb_driver;
/*-------------------------------------------------------------------------*/
@@ -123,7 +124,6 @@
* 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;
@@ -318,7 +318,21 @@
/* used these for multi-interface device registration */
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);
+
+/**
+ * usb_interface_claimed - returns true iff an interface is claimed
+ * @iface: the interface being checked
+ *
+ * Returns true (nonzero) iff the interface is claimed, else false (zero).
+ * Callers must own the driver model's usb bus readlock. So driver
+ * probe() entries don't need extra locking, but other call contexts
+ * may need to explicitly claim that lock.
+ *
+ */
+static int inline usb_interface_claimed(struct usb_interface *iface) {
+ return (iface->dev.driver != NULL);
+}
+
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,
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id70&alloc_id638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
