- moves the config-specific sysfs updates out of usb_new_device() into usb_set_configuration(), where it belongs.
- makes usb_set_configuration() get rid of sysfs state for the previous configuration, including driver unbinding
- makes the bConfigurationValue device attribute be writable through sysfs.
- makes usb/core/message.c debug output work
In short, usb_set_configuration() now does what it's supposed to do: it changes the device config, and all usbcore state depending on it.
This was verified with a multi-config device: in bash, "echo 2 > bConfigurationValue" changed the device configuration, and it was properly reflected in sysfs. Enumeration of single-config devices worked too (a handful that were handy).
The patch doesn't update the existing callers of that routine, none of which can have been relying on any behavior stronger than a "light reset". Most of them probably need to change what they're doing ... hence the "RFT" aspect of this post.
- Dave
--- 1.6/drivers/usb/core/driverfs.c Wed Feb 26 03:17:52 2003
+++ edited/drivers/usb/core/driverfs.c Thu Jun 5 12:37:01 2003
@@ -35,10 +35,39 @@
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 */
+static ssize_t
+show_config (struct device *dev, char *buf)
+{
+ struct usb_device *udev = udev = to_usb_device (dev);
+ u8 config;
+
+ if (udev->actconfig)
+ config = udev->actconfig->desc.bConfigurationValue;
+ else
+ config = 0;
+ return sprintf (buf, "%u\n", config);
+}
+
+static ssize_t
+set_config (struct device *dev, const char *buf, size_t count)
+{
+ struct usb_device *udev = udev = to_usb_device (dev);
+ int config;
+ ssize_t retval;
+
+ if (sscanf (buf, "%u", &config) != 1 || config > 255)
+ return -EINVAL;
+ retval = usb_set_configuration (udev, config);
+ return (retval == 0) ? count : retval;
+}
+
+static DEVICE_ATTR(bConfigurationValue, S_IRUGO | S_IWUSR,
+ show_config, set_config);
+
/* String fields */
static ssize_t show_product (struct device *dev, char *buf)
{
@@ -141,11 +170,8 @@
{
struct device *dev = &udev->dev;
- /* current configuration's attributes */
- device_create_file (dev, &dev_attr_bNumInterfaces);
+ /* configuration is zero, or active */
device_create_file (dev, &dev_attr_bConfigurationValue);
- device_create_file (dev, &dev_attr_bmAttributes);
- device_create_file (dev, &dev_attr_bMaxPower);
/* device attributes */
device_create_file (dev, &dev_attr_idVendor);
@@ -166,6 +192,26 @@
device_create_file (dev, &dev_attr_product);
if (udev->descriptor.iSerialNumber)
device_create_file (dev, &dev_attr_serial);
+}
+
+void usb_create_driverfs_config_files (struct usb_device *udev)
+{
+ struct device *dev = &udev->dev;
+
+ /* current configuration's attributes */
+ device_create_file (dev, &dev_attr_bNumInterfaces);
+ device_create_file (dev, &dev_attr_bmAttributes);
+ device_create_file (dev, &dev_attr_bMaxPower);
+}
+
+void usb_remove_driverfs_config_files (struct usb_device *udev)
+{
+ struct device *dev = &udev->dev;
+
+ /* current configuration's attributes */
+ device_remove_file (dev, &dev_attr_bNumInterfaces);
+ device_remove_file (dev, &dev_attr_bmAttributes);
+ device_remove_file (dev, &dev_attr_bMaxPower);
}
/* Interface fields */
--- 1.27/drivers/usb/core/message.c Fri May 23 03:46:51 2003
+++ edited/drivers/usb/core/message.c Thu Jun 5 13:26:20 2003
@@ -2,6 +2,14 @@
* message.c - synchronous message handling
*/
+#include <linux/config.h>
+
+#ifdef CONFIG_USB_DEBUG
+ #define DEBUG
+#else
+ #undef DEBUG
+#endif
+
#include <linux/pci.h> /* for scatterlist macros */
#include <linux/usb.h>
#include <linux/module.h>
@@ -11,6 +19,7 @@
#include <asm/byteorder.h>
#include "hcd.h" /* for usbcore internals */
+#include "usb.h" /* other usbcore internals */
struct usb_api_data {
wait_queue_head_t wqh;
@@ -873,12 +882,12 @@
* 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 (), and without the sysfs lock for usb held.
*
* 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
- * configuration.
+ * support this kind of configurability; many devices only have a single
+ * configuration. Normally, after enumeration the first (and often only)
+ * device configuration is assigned, and won't need to change.
*
* USB devices may support one or more configurations, which affect
* power consumption and the functionality available. For example,
@@ -897,7 +906,9 @@
* This call is synchronous, and may not be used in an interrupt context.
*
* Returns zero on success, or else the status code returned by the
- * underlying usb_control_msg() call.
+ * underlying usb_control_msg() call. Any driver bindings associated
+ * with the previous configuration's interfaces are broken. On success,
+ * driver bindings for all new interfaces are established.
*/
int usb_set_configuration(struct usb_device *dev, int configuration)
{
@@ -917,11 +928,26 @@
}
/* if it's already configured, clear out old state first. */
- if (dev->state != USB_STATE_ADDRESS && disable) {
- for (i = 1 /* skip ep0 */; i < 15; i++) {
- disable (dev, i);
- disable (dev, USB_DIR_IN | i);
+ if (dev->state != USB_STATE_ADDRESS) {
+
+ /* remove config-specific sysfs files, and as a
+ * side effect unbind drivers from the interfaces.
+ */
+ for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *interface;
+
+ interface = &dev->actconfig->interface[i];
+ device_del (&interface->dev);
+ }
+ usb_remove_driverfs_config_files (dev);
+
+ if (disable) {
+ for (i = 1 /* skip ep0 */; i < 15; i++) {
+ disable (dev, i);
+ disable (dev, USB_DIR_IN | i);
+ }
}
+ dev->actconfig = NULL;
}
dev->toggle[0] = dev->toggle[1] = 0;
dev->halted[0] = dev->halted[1] = 0;
@@ -931,12 +957,54 @@
USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
NULL, 0, HZ * USB_CTRL_SET_TIMEOUT)) < 0)
return ret;
- if (configuration)
- dev->state = USB_STATE_CONFIGURED;
dev->actconfig = cp;
- /* reset more hc/hcd endpoint state */
- usb_set_maxpacket(dev);
+ if (configuration) {
+ dev->state = USB_STATE_CONFIGURED;
+
+ /* reset more hc/hcd endpoint state */
+ usb_set_maxpacket(dev);
+
+ /* add config-specific files to sysfs, and as a
+ * side effect allow drivers to bind to the interfaces.
+ */
+ usb_create_driverfs_config_files (dev);
+ for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *interface;
+ struct usb_interface_descriptor *desc;
+
+ interface = &dev->actconfig->interface[i];
+ interface->act_altsetting = 0;
+ desc = &interface->altsetting [0].desc;
+ interface->dev.parent = &dev->dev;
+ interface->dev.driver = NULL;
+ interface->dev.bus = &usb_bus_type;
+ interface->dev.dma_mask = dev->dev.dma_mask;
+ sprintf (&interface->dev.bus_id[0], "%d-%s:%d",
+ dev->bus->busnum, dev->devpath,
+ desc->bInterfaceNumber);
+ if (!desc->iInterface
+ || usb_string (dev, desc->iInterface,
+ interface->dev.name,
+ sizeof interface->dev.name)
+ <= 0) {
+ /* typically devices won't bother with interface
+ * descriptions; this is the normal case. an
+ * interface's driver might describe it better.
+ * (also: iInterface is per-altsetting ...)
+ */
+ sprintf (&interface->dev.name[0],
+ "usb-%s-%s interface %d",
+ dev->bus->bus_name, dev->devpath,
+ desc->bInterfaceNumber);
+ }
+ dev_dbg (&interface->dev,
+ "%s - registering interface\n",
+ __FUNCTION__);
+ device_add (&interface->dev);
+ usb_create_driverfs_intf_files (interface);
+ }
+ }
return 0;
}
--- 1.125/drivers/usb/core/usb.c Mon Jun 2 03:37:34 2003
+++ edited/drivers/usb/core/usb.c Thu Jun 5 13:01:26 2003
@@ -1158,73 +1158,55 @@
return 1;
}
+ /* read all current-speed configuration descriptors */
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 config descriptors, error %d\n",
+ err);
clear_bit(dev->devnum, dev->bus->devmap.devicemap);
dev->devnum = -1;
return 1;
}
- /* we set the default configuration here */
- 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);
- clear_bit(dev->devnum, dev->bus->devmap.devicemap);
- dev->devnum = -1;
- return 1;
- }
-
- /* USB device state == configured ... tell the world! */
+ /* add device-specific driverfs files */
+ err = device_add (&dev->dev);
+ if (err)
+ return err;
+ usb_create_driverfs_dev_files (dev);
- dev_dbg(&dev->dev, "new device strings: Mfr=%d, Product=%d, SerialNumber=%d\n",
- dev->descriptor.iManufacturer, dev->descriptor.iProduct,
dev->descriptor.iSerialNumber);
+ /* 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);
set_device_description (dev);
#ifdef DEBUG
if (dev->descriptor.iSerialNumber)
- usb_show_string(dev, "SerialNumber", dev->descriptor.iSerialNumber);
+ usb_show_string(dev, "SerialNumber",
+ dev->descriptor.iSerialNumber);
#endif
- /* put into sysfs, with device and config specific files */
- err = device_add (&dev->dev);
- if (err)
- return err;
- 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;
+ /* choose initial device configuration.
+ * FIXME for multi-config devices, driver hook needed.
+ * also, this ought to consider the hub's power budget.
+ */
+ i = dev->config[0].desc.bConfigurationValue;
- 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);
- if (!desc->iInterface
- || usb_string (dev, desc->iInterface,
- interface->dev.name,
- sizeof interface->dev.name) <= 0) {
- /* typically devices won't bother with interface
- * descriptions; this is the normal case. an
- * interface's driver might describe it better.
- * (also: iInterface is per-altsetting ...)
- */
- sprintf (&interface->dev.name[0],
- "usb-%s-%s interface %d",
- dev->bus->bus_name, 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);
+ /* now set that configuration ... binding drivers to device */
+ err = usb_set_configuration(dev, i);
+ if (err) {
+ dev_err(&dev->dev,
+ "failed setting device %d to config #%d (error=%d)\n",
+ dev->devnum, i, err);
+ /* FIXME once nothing oops with dev->actconfig null,
+ * just leave it alone for user mode recovery.
+ */
+ device_del (&dev->dev);
+ clear_bit(dev->devnum, dev->bus->devmap.devicemap);
+ dev->devnum = -1;
+ return 1;
}
/* add a /proc/bus/usb entry */
--- 1.1/drivers/usb/core/usb.h Mon Sep 30 16:09:05 2002
+++ edited/drivers/usb/core/usb.h Thu Jun 5 10:06:39 2003
@@ -3,3 +3,5 @@
extern void usb_create_driverfs_dev_files (struct usb_device *dev);
extern void usb_create_driverfs_intf_files (struct usb_interface *intf);
+extern void usb_create_driverfs_config_files (struct usb_device *dev);
+extern void usb_remove_driverfs_config_files (struct usb_device *dev);
