This patch is an update of one I sent around with 2.5.70, when the kernel wasn't quite ready to do this. It resolves some bugs in the way changing device configurations were reflected in the driver model:
- Moves code around so that usb_set_configuration() updates sysfs to reflect the current configuration. Previously, that only worked right for the initial configuration chosen by khubd.
* The old interfaces are all gone. The code to handle this
moved from usb_disconnect() into usb_disable_device(), which
is called both on disconnect and set_configuration paths. * There are new interfaces. The code to handle this moved
from usb_new_device() into usb_set_configuration().Its kerneldoc is updated appropriately. The main point being that calling this in driver probe() methods is even more of a no-no, since it'll self-deadlock.
- Moves usb_new_interface() code around a bit, so that the device is visible in sysfs before usb_set_configuration() is called.
- Makes the bConfigurationValue be writable through sysfs, so device configurations can be easily changed from user mode.
I think this is ready to merge, and that it should be merged unless someone notices a significant problem.
- Dave
--- 1.34/drivers/usb/core/message.c Mon Aug 11 07:56:25 2003
+++ edited/drivers/usb/core/message.c Wed Aug 13 13:50:07 2003
@@ -788,18 +788,39 @@
* @skip_ep0: 0 to disable endpoint 0, 1 to skip it.
*
* Disables all the device's endpoints, potentially including endpoint 0.
- * Deallocates hcd/hardware state for the endpoints ... and nukes all
- * pending urbs.
+ * Deallocates hcd/hardware state for the endpoints (nuking all or most
+ * pending urbs) and usbcore state for the interfaces, so that usbcore
+ * must usb_set_configuration() before any interfaces could be used.
*/
void usb_disable_device(struct usb_device *dev, int skip_ep0)
{
int i;
- dbg("nuking URBs for device %s", dev->dev.bus_id);
+ dev_dbg(&dev->dev, "%s nuking %s URBs\n", __FUNCTION__,
+ skip_ep0 ? "non-ep0" : "all");
for (i = skip_ep0; i < 16; ++i) {
usb_disable_endpoint(dev, i);
usb_disable_endpoint(dev, i + USB_DIR_IN);
}
+ dev->toggle[0] = dev->toggle[1] = 0;
+ dev->halted[0] = dev->halted[1] = 0;
+
+ /* getting rid of interfaces will disconnect
+ * any drivers bound to them (a key side effect)
+ */
+ if (dev->actconfig) {
+ for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *interface;
+
+ /* remove this interface */
+ interface = dev->actconfig->interface[i];
+ dev_dbg (&dev->dev, "unregistering interface %s\n",
+ interface->dev.bus_id);
+ device_unregister(&interface->dev);
+ }
+ dev->actconfig = 0;
+ }
+ dev->state = USB_STATE_ADDRESS;
}
@@ -1019,28 +1040,33 @@
* Context: !in_interrupt ()
*
* This is used to enable non-default device modes. Not all devices
- * support this kind of configurability. By default, configuration
- * zero is selected after enumeration; many devices only have a single
+ * support this kind of configurability; many devices only have one
* configuration.
*
- * USB devices may support one or more configurations, which affect
+ * USB device configuration affect
* power consumption and the functionality available. For example,
* the default configuration is limited to using 100mA of bus power,
* so that when certain device functionality requires more power,
- * and the device is bus powered, that functionality will be in some
+ * and the device is bus powered, that functionality should be in some
* non-default device configuration. Other device modes may also be
* reflected as configuration options, such as whether two ISDN
* channels are presented as independent 64Kb/s interfaces or as one
- * bonded 128Kb/s interface.
+ * bonded 128Kb/s interface; or whether the device uses standard protocols
+ * (maybe CDC Ethernet) or proprietary ones (maybe RNDIS).
*
* Note that USB has an additional level of device configurability,
* associated with interfaces. That configurability is accessed using
* usb_set_interface().
*
- * This call is synchronous, and may not be used in an interrupt context.
+ * This call is synchronous. The calling context must be able to sleep,
+ * and must not hold the driver model lock for USB; usb device driver
+ * probe() methods may not use this routine.
*
* Returns zero on success, or else the status code returned by the
- * underlying usb_control_msg() call.
+ * underlying call that failed. On succesful completion, each interface
+ * in the original device configuration has been destroied, and each one
+ * in the new configuration has been probed by all relevant usb device
+ * drivers currently known to the kernel.
*/
int usb_set_configuration(struct usb_device *dev, int configuration)
{
@@ -1058,29 +1084,51 @@
return -EINVAL;
}
- /* if it's already configured, clear out old state first. */
+ /* dev->serialize guards all config changes */
+ down(&dev->serialize);
+
+ /* if it's already configured, clear out old state first.
+ * getting rid of old interfaces means unbinding their drivers.
+ */
if (dev->state != USB_STATE_ADDRESS)
usb_disable_device (dev, 1); // Skip ep0
- dev->toggle[0] = dev->toggle[1] = 0;
- dev->halted[0] = dev->halted[1] = 0;
- dev->state = USB_STATE_ADDRESS;
if ((ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
- NULL, 0, HZ * USB_CTRL_SET_TIMEOUT)) < 0)
+ NULL, 0, HZ * USB_CTRL_SET_TIMEOUT)) < 0) {
+ up(&dev->serialize);
return ret;
+ }
if (configuration)
dev->state = USB_STATE_CONFIGURED;
dev->actconfig = cp;
- /* reset more hc/hcd interface/endpoint state */
- for (i = 0; i < cp->desc.bNumInterfaces; ++i) {
+ /* re-initialize hc/hcd/usbcore interface/endpoint state.
+ * this triggers binding of drivers to interfaces; and
+ * maybe probe() calls will choose different altsettings.
+ */
+ for (i = 0; cp && i < cp->desc.bNumInterfaces; ++i) {
struct usb_interface *intf = cp->interface[i];
+ struct usb_interface_descriptor *desc;
intf->act_altsetting = 0;
+ desc = &intf->altsetting [0].desc;
usb_enable_interface(dev, intf);
+
+ 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",
+ dev->bus->busnum, dev->devpath,
+ desc->bInterfaceNumber);
+ dev_dbg (&dev->dev, "registering interface %s\n",
+ intf->dev.bus_id);
+ device_add (&intf->dev);
+ usb_create_driverfs_intf_files (intf);
}
+ up(&dev->serialize);
return 0;
}
--- 1.136/drivers/usb/core/usb.c Mon Aug 11 16:09:09 2003
+++ edited/drivers/usb/core/usb.c Wed Aug 13 13:52:50 2003
@@ -906,21 +906,11 @@
usb_disconnect(child);
}
- /* deallocate hcd/hardware state ... and nuke all pending urbs */
+ /* deallocate hcd/hardware state ... nuking all pending urbs and
+ * cleaning up all state associated with the current configuration
+ */
usb_disable_device(dev, 0);
- /* disconnect() drivers from interfaces (a key side effect) */
- dev_dbg (&dev->dev, "unregistering interfaces\n");
- if (dev->actconfig) {
- for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
- struct usb_interface *interface;
-
- /* remove this interface */
- interface = dev->actconfig->interface[i];
- device_unregister(&interface->dev);
- }
- }
-
dev_dbg (&dev->dev, "unregistering device\n");
/* Free the device number and remove the /proc/bus/usb entry */
if (dev->devnum > 0) {
@@ -1088,27 +1078,12 @@
err = usb_get_configuration(dev);
if (err < 0) {
- dev_err(&dev->dev, "unable to get device %d configuration
(error=%d)\n",
- dev->devnum, err);
+ dev_err(&dev->dev, "can't read configurations, error %d\n",
+ err);
goto fail;
}
- /* choose and set the configuration here */
- if (dev->descriptor.bNumConfigurations != 1) {
- dev_info(&dev->dev,
- "configuration #%d chosen from %d choices\n",
- dev->config[0].desc.bConfigurationValue,
- dev->descriptor.bNumConfigurations);
- }
- err = usb_set_configuration(dev, dev->config[0].desc.bConfigurationValue);
- if (err) {
- dev_err(&dev->dev, "failed to set device %d default configuration
(error=%d)\n",
- dev->devnum, err);
- goto fail;
- }
-
- /* USB device state == configured ... tell the world! */
-
+ /* Tell the world! */
dev_dbg(&dev->dev, "new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n",
dev->descriptor.iManufacturer, dev->descriptor.iProduct,
dev->descriptor.iSerialNumber);
@@ -1121,30 +1096,30 @@
usb_show_string(dev, "SerialNumber", dev->descriptor.iSerialNumber);
#endif
- /* put into sysfs, with device and config specific files */
+ /* put device-specific files into sysfs */
err = device_add (&dev->dev);
if (err)
goto fail;
usb_create_driverfs_dev_files (dev);
- /* Register all of the interfaces for this device with the driver core.
- * Remember, interfaces get bound to drivers, not devices. */
- for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
- struct usb_interface *interface = dev->actconfig->interface[i];
- struct usb_interface_descriptor *desc;
-
- desc = &interface->altsetting [interface->act_altsetting].desc;
- interface->dev.parent = &dev->dev;
- interface->dev.driver = NULL;
- interface->dev.bus = &usb_bus_type;
- interface->dev.dma_mask = parent->dma_mask;
- sprintf (&interface->dev.bus_id[0], "%d-%s:%d",
- dev->bus->busnum, dev->devpath,
- desc->bInterfaceNumber);
- dev_dbg (&dev->dev, "%s - registering interface %s\n", __FUNCTION__,
interface->dev.bus_id);
- device_add (&interface->dev);
- usb_create_driverfs_intf_files (interface);
+ /* choose and set the configuration. that registers the interfaces
+ * with the driver core, and lets usb device drivers bind to them.
+ */
+ if (dev->descriptor.bNumConfigurations != 1) {
+ dev_info(&dev->dev,
+ "configuration #%d chosen from %d choices\n",
+ dev->config[0].desc.bConfigurationValue,
+ dev->descriptor.bNumConfigurations);
+ }
+ err = usb_set_configuration(dev,
+ dev->config[0].desc.bConfigurationValue);
+ if (err) {
+ dev_err(&dev->dev, "can't set config #%d, error %d\n",
+ dev->config[0].desc.bConfigurationValue, err);
+ goto fail;
}
+
+ /* USB device state == configured ... usable */
/* add a /proc/bus/usb entry */
usbfs_add_device(dev);
--- 1.6/drivers/usb/core/driverfs.c Wed Feb 26 03:17:52 2003
+++ edited/drivers/usb/core/driverfs.c Wed Aug 13 13:37:32 2003
@@ -23,21 +23,43 @@
#include "usb.h"
/* Active configuration fields */
-#define usb_actconfig_attr(field, format_string) \
+#define usb_actconfig_show(field, format_string) \
static ssize_t \
show_##field (struct device *dev, char *buf) \
{ \
struct usb_device *udev; \
\
udev = to_usb_device (dev); \
+ if (udev->actconfig) \
return sprintf (buf, format_string, udev->actconfig->desc.field); \
+ else return 0; \
} \
+
+#define usb_actconfig_attr(field, format_string) \
+usb_actconfig_show(field,format_string) \
static DEVICE_ATTR(field, S_IRUGO, show_##field, NULL);
usb_actconfig_attr (bNumInterfaces, "%2d\n")
-usb_actconfig_attr (bConfigurationValue, "%2d\n")
usb_actconfig_attr (bmAttributes, "%2x\n")
usb_actconfig_attr (bMaxPower, "%3dmA\n")
+
+/* configuration value is always present, and r/w */
+usb_actconfig_show(bConfigurationValue,"%u\n");
+
+static ssize_t
+set_bConfigurationValue (struct device *dev, const char *buf, size_t count)
+{
+ struct usb_device *udev = udev = to_usb_device (dev);
+ int config, value;
+
+ if (sscanf (buf, "%u", &config) != 1 || config > 255)
+ return -EINVAL;
+ value = usb_set_configuration (udev, config);
+ return (value < 0) ? value : count;
+}
+
+static DEVICE_ATTR(bConfigurationValue, S_IRUGO | S_IWUSR,
+ show_bConfigurationValue, set_bConfigurationValue);
/* String fields */
static ssize_t show_product (struct device *dev, char *buf)
