ChangeSet 1.1673.8.49, 2004/03/30 09:25:17-08:00, [EMAIL PROTECTED]
[PATCH] USB: set_configuration locking cleanups
I've posted all these before, the only notable change is
treating that one gphoto2 case as warn-and-continue rather
than return-with-failure.
usb_set_configuration() cleanup
* Remove it from the USB kernel driver API. No drivers need it now,
and the sysadmin can change bConfigurationValue using sysfs (say,
when hotplugging an otherwise problematic device).
* Simpler/cleaner locking: caller must own dev->serialize.
* Access from usbfs now uses usb_reset_configuration() sometimes,
preventing sysfs thrash, and warns about some dangerous usage
(which gphoto2 and other programs may be relying on). (This is
from Alan Stern, but I morphed an error return into a warning.)
* Prevent a couple potential "no configuration" oopses. (Alan's?)
* Remove one broken call from usbcore, in the "device morphed" path
of usb_reset_device(). This should be more polite now, hanging
that one device rather than khubd.
drivers/usb/core/devio.c | 47 +++++++++++++++++++++++++++++++++++++++++++-
drivers/usb/core/driverfs.c | 2 +
drivers/usb/core/hub.c | 34 +++++++------------------------
drivers/usb/core/message.c | 7 +-----
drivers/usb/core/usb.c | 4 +++
drivers/usb/core/usb.h | 1
include/linux/usb.h | 1
7 files changed, 63 insertions(+), 33 deletions(-)
diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c Wed Apr 14 14:36:02 2004
+++ b/drivers/usb/core/devio.c Wed Apr 14 14:36:02 2004
@@ -430,6 +430,8 @@
if (ep & ~(USB_DIR_IN|0xf))
return -EINVAL;
+ if (!dev->actconfig)
+ return -ESRCH;
for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
iface = dev->actconfig->interface[i];
for (j = 0; j < iface->num_altsetting; j++) {
@@ -450,6 +452,8 @@
if (ifn & ~0xff)
return -EINVAL;
+ if (!dev->actconfig)
+ return -ESRCH;
for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
if (dev->actconfig->interface[i]->
altsetting[0].desc.bInterfaceNumber == ifn)
@@ -766,10 +770,51 @@
static int proc_setconfig(struct dev_state *ps, void __user *arg)
{
unsigned int u;
+ int status = 0;
+ struct usb_host_config *actconfig;
if (get_user(u, (unsigned int __user *)arg))
return -EFAULT;
- return usb_set_configuration(ps->dev, u);
+
+ down(&ps->dev->serialize);
+ actconfig = ps->dev->actconfig;
+
+ /* Don't touch the device if any interfaces are claimed.
+ * It could interfere with other drivers' operations, and if
+ * an interface is claimed by usbfs it could easily deadlock.
+ */
+ if (actconfig) {
+ int i;
+
+ for (i = 0; i < actconfig->desc.bNumInterfaces; ++i) {
+ if (usb_interface_claimed(actconfig->interface[i])) {
+ dev_warn (&ps->dev->dev,
+ "usbfs: interface %d claimed "
+ "while '%s' sets config #%d\n",
+ actconfig->interface[i]
+ ->cur_altsetting
+ ->desc.bInterfaceNumber,
+ current->comm, u);
+#if 0 /* FIXME: enable in 2.6.10 or so */
+ status = -EBUSY;
+ break;
+#endif
+ }
+ }
+ }
+
+ /* SET_CONFIGURATION is often abused as a "cheap" driver reset,
+ * so avoid usb_set_configuration()'s kick to sysfs
+ */
+ if (status == 0) {
+ if (actconfig && actconfig->desc.bConfigurationValue == u)
+ status = usb_reset_configuration(ps->dev);
+ else
+ status = usb_set_configuration(ps->dev, u);
+ }
+ up(&ps->dev->serialize);
+
+ return status;
}
static int proc_submiturb(struct dev_state *ps, void __user *arg)
diff -Nru a/drivers/usb/core/driverfs.c b/drivers/usb/core/driverfs.c
--- a/drivers/usb/core/driverfs.c Wed Apr 14 14:36:02 2004
+++ b/drivers/usb/core/driverfs.c Wed Apr 14 14:36:02 2004
@@ -55,7 +55,9 @@
if (sscanf (buf, "%u", &config) != 1 || config > 255)
return -EINVAL;
+ down(&udev->serialize);
value = usb_set_configuration (udev, config);
+ up(&udev->serialize);
return (value < 0) ? value : count;
}
diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c Wed Apr 14 14:36:02 2004
+++ b/drivers/usb/core/hub.c Wed Apr 14 14:36:02 2004
@@ -1299,33 +1299,15 @@
kfree(descriptor);
usb_destroy_configuration(dev);
- ret = usb_get_device_descriptor(dev, sizeof(dev->descriptor));
- if (ret != sizeof(dev->descriptor)) {
- if (ret < 0)
- err("unable to get device %s descriptor "
- "(error=%d)", dev->devpath, ret);
- else
- err("USB device %s descriptor short read "
- "(expected %Zi, got %i)",
- dev->devpath,
- sizeof(dev->descriptor), ret);
+ /* FIXME Linux doesn't yet handle these "device morphed"
+ * paths. DFU variants need this to work ... and they
+ * include the "config descriptors changed" case this
+ * doesn't yet detect!
+ */
+ dev->state = USB_STATE_NOTATTACHED;
+ dev_err(&dev->dev, "device morphed (DFU?), nyet supported\n");
- clear_bit(dev->devnum, dev->bus->devmap.devicemap);
- dev->devnum = -1;
- return -EIO;
- }
-
- ret = usb_get_configuration(dev);
- if (ret < 0) {
- err("unable to get configuration (error=%d)", ret);
- usb_destroy_configuration(dev);
- clear_bit(dev->devnum, dev->bus->devmap.devicemap);
- dev->devnum = -1;
- return 1;
- }
-
- usb_set_configuration(dev, dev->config[0].desc.bConfigurationValue);
- return 1;
+ return -ENODEV;
}
kfree(descriptor);
diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c
--- a/drivers/usb/core/message.c Wed Apr 14 14:36:02 2004
+++ b/drivers/usb/core/message.c Wed Apr 14 14:36:02 2004
@@ -1075,11 +1075,11 @@
return 0;
}
-/**
+/*
* usb_set_configuration - Makes a particular device setting be current
* @dev: the device whose configuration is being updated
* @configuration: the configuration being chosen.
- * Context: !in_interrupt ()
+ * Context: !in_interrupt(), caller holds dev->serialize
*
* This is used to enable non-default device modes. Not all devices
* use this kind of configurability; many devices only have one
@@ -1115,7 +1115,6 @@
struct usb_host_config *cp = NULL;
/* dev->serialize guards all config changes */
- down(&dev->serialize);
for (i=0; i<dev->descriptor.bNumConfigurations; i++) {
if (dev->config[i].desc.bConfigurationValue == configuration) {
@@ -1191,7 +1190,6 @@
}
out:
- up(&dev->serialize);
return ret;
}
@@ -1300,6 +1298,5 @@
// synchronous calls that also maintain usbcore state
EXPORT_SYMBOL(usb_clear_halt);
EXPORT_SYMBOL(usb_reset_configuration);
-EXPORT_SYMBOL(usb_set_configuration);
EXPORT_SYMBOL(usb_set_interface);
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Wed Apr 14 14:36:02 2004
+++ b/drivers/usb/core/usb.c Wed Apr 14 14:36:02 2004
@@ -1173,10 +1173,13 @@
usb_show_string(dev, "SerialNumber", dev->descriptor.iSerialNumber);
#endif
+ down(&dev->serialize);
+
/* put device-specific files into sysfs */
err = device_add (&dev->dev);
if (err) {
dev_err(&dev->dev, "can't device_add, error %d\n", err);
+ up(&dev->serialize);
goto fail;
}
usb_create_driverfs_dev_files (dev);
@@ -1211,6 +1214,7 @@
dev->descriptor.bNumConfigurations);
}
err = usb_set_configuration(dev, config);
+ up(&dev->serialize);
if (err) {
dev_err(&dev->dev, "can't set config #%d, error %d\n",
config, err);
diff -Nru a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
--- a/drivers/usb/core/usb.h Wed Apr 14 14:36:02 2004
+++ b/drivers/usb/core/usb.h Wed Apr 14 14:36:02 2004
@@ -17,6 +17,7 @@
extern int usb_get_device_descriptor(struct usb_device *dev,
unsigned int size);
+extern int usb_set_configuration(struct usb_device *dev, int configuration);
/* for labeling diagnostics */
extern const char *usbcore_name;
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Wed Apr 14 14:36:02 2004
+++ b/include/linux/usb.h Wed Apr 14 14:36:02 2004
@@ -904,7 +904,6 @@
/* wrappers that also update important state inside usbcore */
extern int usb_clear_halt(struct usb_device *dev, int pipe);
extern int usb_reset_configuration(struct usb_device *dev);
-extern int usb_set_configuration(struct usb_device *dev, int configuration);
extern int usb_set_interface(struct usb_device *dev, int ifnum, int alternate);
/*
-------------------------------------------------------
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
