Greg:
This is a revised version of an earlier patch; I feel a lot better about
this one. Basically it does the same thing as before: allocate interfaces
dynamically to avoid the problems with reusing them.
The difference is that this patch adds a struct kref to the array of
usb_interface_cache's, so the array can persist if needed after the device
has been disconnected. Each interface takes a reference to it (along with
the configuration itself), so as long as the interfaces remain pinned in
memory the altsettings will also remain.
This could use a good deal of testing. Also I would like to hear
people's comments on it. However, if nothing breaks and nothing looks
bad, I think this is ready to be applied.
Alan Stern
===== include/linux/usb.h 1.181 vs edited =====
--- 1.181/include/linux/usb.h Wed Mar 31 17:13:05 2004
+++ edited/include/linux/usb.h Thu Apr 15 14:43:58 2004
@@ -18,6 +18,7 @@
#include <linux/fs.h> /* for struct file_operations */
#include <linux/completion.h> /* for struct completion */
#include <linux/sched.h> /* for current && schedule_timeout */
+#include <linux/kref.h> /* for struct kref */
static __inline__ void wait_ms(unsigned int ms)
@@ -147,11 +148,42 @@
#define USB_MAXINTERFACES 32
/**
+ * struct usb_interface_cache - long-term representation of a device interface
+ * @num_altsetting: number of altsettings defined.
+ * @ref: reference counter.
+ * @altsetting: variable-length array of interface structures, one for
+ * each alternate setting that may be selected. Each one includes a
+ * set of endpoint configurations. They will be in no particular order.
+ *
+ * These structures persist for the lifetime of a usb_device, unlike
+ * struct usb_interface (which persists only as long as its configuration
+ * is installed). The altsetting arrays can be accessed through these
+ * structures at any time, permitting comparison of configurations and
+ * providing support for the /proc/bus/usb/devices pseudo-file.
+ */
+struct usb_interface_cache {
+ unsigned num_altsetting; /* number of alternate settings */
+ struct kref ref; /* reference counter */
+
+ /* variable-length array of alternate settings for this interface,
+ * stored in no particular order */
+ struct usb_host_interface altsetting[0];
+};
+#define ref_to_usb_interface_cache(r) \
+ container_of(r, struct usb_interface_cache, ref)
+#define altsetting_to_usb_interface_cache(a) \
+ container_of(a, struct usb_interface_cache, altsetting[0])
+
+/**
* struct usb_host_config - representation of a device's configuration
* @desc: the device's configuration descriptor.
- * @interface: array of usb_interface structures, one for each interface
- * in the configuration. The number of interfaces is stored in
- * desc.bNumInterfaces.
+ * @interface: array of pointers to usb_interface structures, one for each
+ * interface in the configuration. The number of interfaces is stored
+ * in desc.bNumInterfaces. These pointers are valid only while the
+ * the configuration is active.
+ * @intf_cache: array of pointers to usb_interface_cache structures, one
+ * for each interface in the configuration. These structures exist
+ * for the entire life of the device.
* @extra: pointer to buffer containing all extra descriptors associated
* with this configuration (those preceding the first interface
* descriptor).
@@ -184,6 +216,10 @@
/* the interfaces associated with this configuration,
* stored in no particular order */
struct usb_interface *interface[USB_MAXINTERFACES];
+
+ /* Interface information available even when this is not the
+ * active configuration */
+ struct usb_interface_cache *intf_cache[USB_MAXINTERFACES];
unsigned char *extra; /* Extra descriptors */
int extralen;
===== drivers/usb/core/config.c 1.33 vs edited =====
--- 1.33/drivers/usb/core/config.c Mon Mar 29 11:17:38 2004
+++ edited/drivers/usb/core/config.c Thu Apr 15 14:45:41 2004
@@ -96,19 +96,14 @@
return buffer - buffer0 + i;
}
-static void usb_free_intf(struct usb_interface *intf)
+static void usb_release_interface_cache(struct kref *ref)
{
+ struct usb_interface_cache *intfc = ref_to_usb_interface_cache(ref);
int j;
- if (intf->altsetting) {
- for (j = 0; j < intf->num_altsetting; j++) {
- struct usb_host_interface *alt = &intf->altsetting[j];
-
- kfree(alt->endpoint);
- }
- kfree(intf->altsetting);
- }
- kfree(intf);
+ for (j = 0; j < intfc->num_altsetting; j++)
+ kfree(intfc->altsetting[j].endpoint);
+ kfree(intfc);
}
static int usb_parse_interface(struct device *ddev, int cfgno,
@@ -117,7 +112,7 @@
unsigned char *buffer0 = buffer;
struct usb_interface_descriptor *d;
int inum, asnum;
- struct usb_interface *interface;
+ struct usb_interface_cache *intfc;
struct usb_host_interface *alt;
int i, n;
int len, retval;
@@ -137,16 +132,16 @@
if (inum >= config->desc.bNumInterfaces)
goto skip_to_next_interface_descriptor;
- interface = config->interface[inum];
+ intfc = config->intf_cache[inum];
asnum = d->bAlternateSetting;
- if (asnum >= interface->num_altsetting) {
+ if (asnum >= intfc->num_altsetting) {
dev_err(ddev, "config %d interface %d has an invalid "
"alternate setting number: %d but max is %d\n",
- cfgno, inum, asnum, interface->num_altsetting - 1);
+ cfgno, inum, asnum, intfc->num_altsetting - 1);
return -EINVAL;
}
- alt = &interface->altsetting[asnum];
+ alt = &intfc->altsetting[asnum];
if (alt->desc.bLength) {
dev_err(ddev, "Duplicate descriptor for config %d "
"interface %d altsetting %d\n", cfgno, inum, asnum);
@@ -210,11 +205,12 @@
int cfgno;
int nintf, nintf_orig;
int i, j, n;
- struct usb_interface *interface;
+ struct usb_interface_cache *intfc;
unsigned char *buffer2;
int size2;
struct usb_descriptor_header *header;
int len, retval;
+ u8 nalts[USB_MAXINTERFACES];
memcpy(&config->desc, buffer, USB_DT_CONFIG_SIZE);
if (config->desc.bDescriptorType != USB_DT_CONFIG ||
@@ -237,14 +233,7 @@
cfgno, nintf, USB_MAXINTERFACES);
config->desc.bNumInterfaces = nintf = USB_MAXINTERFACES;
}
-
- for (i = 0; i < nintf; ++i) {
- interface = config->interface[i] =
- kmalloc(sizeof(struct usb_interface), GFP_KERNEL);
- if (!interface)
- return -ENOMEM;
- memset(interface, 0, sizeof(struct usb_interface));
- }
+ memset(nalts, 0, nintf);
/* Go through the descriptors, checking their length and counting the
* number of altsettings for each interface */
@@ -277,8 +266,8 @@
cfgno, i, nintf_orig - 1);
return -EINVAL;
}
- if (i < nintf)
- ++config->interface[i]->num_altsetting;
+ if (i < nintf && nalts[i] < 255)
+ ++nalts[i];
} else if (header->bDescriptorType == USB_DT_DEVICE ||
header->bDescriptorType == USB_DT_CONFIG) {
@@ -290,29 +279,29 @@
} /* for ((buffer2 = buffer, size2 = size); ...) */
- /* Allocate the altsetting arrays */
+ /* Allocate the usb_interface_caches and altsetting arrays */
for (i = 0; i < nintf; ++i) {
- interface = config->interface[i];
- if (interface->num_altsetting > USB_MAXALTSETTING) {
+ j = nalts[i];
+ if (j > USB_MAXALTSETTING) {
dev_err(ddev, "too many alternate settings for "
"config %d interface %d: %d, "
"maximum allowed: %d\n",
- cfgno, i, interface->num_altsetting,
- USB_MAXALTSETTING);
+ cfgno, i, j, USB_MAXALTSETTING);
return -EINVAL;
}
- if (interface->num_altsetting == 0) {
+ if (j == 0) {
dev_err(ddev, "config %d has no interface number "
"%d\n", cfgno, i);
return -EINVAL;
}
- len = sizeof(*interface->altsetting) *
- interface->num_altsetting;
- interface->altsetting = kmalloc(len, GFP_KERNEL);
- if (!interface->altsetting)
+ len = sizeof(*intfc) + sizeof(struct usb_host_interface) * j;
+ config->intf_cache[i] = intfc = kmalloc(len, GFP_KERNEL);
+ if (!intfc)
return -ENOMEM;
- memset(interface->altsetting, 0, len);
+ memset(intfc, 0, len);
+ intfc->num_altsetting = j;
+ kref_init(&intfc->ref, usb_release_interface_cache);
}
/* Skip over any Class Specific or Vendor Specific descriptors;
@@ -340,9 +329,9 @@
/* Check for missing altsettings */
for (i = 0; i < nintf; ++i) {
- interface = config->interface[i];
- for (j = 0; j < interface->num_altsetting; ++j) {
- if (!interface->altsetting[j].desc.bLength) {
+ intfc = config->intf_cache[i];
+ for (j = 0; j < intfc->num_altsetting; ++j) {
+ if (!intfc->altsetting[j].desc.bLength) {
dev_err(ddev, "config %d interface %d has no "
"altsetting %d\n", cfgno, i, j);
return -EINVAL;
@@ -374,10 +363,8 @@
struct usb_host_config *cf = &dev->config[c];
for (i = 0; i < cf->desc.bNumInterfaces; i++) {
- struct usb_interface *ifp = cf->interface[i];
-
- if (ifp)
- usb_free_intf(ifp);
+ if (cf->intf_cache[i])
+ kref_put(&cf->intf_cache[i]->ref);
}
}
kfree(dev->config);
===== drivers/usb/core/devices.c 1.29 vs edited =====
--- 1.29/drivers/usb/core/devices.c Tue Mar 30 11:50:40 2004
+++ edited/drivers/usb/core/devices.c Thu Apr 15 12:21:07 2004
@@ -232,13 +232,21 @@
return start;
}
-static char *usb_dump_interface_descriptor(char *start, char *end, const struct
usb_interface *iface, int setno)
+static char *usb_dump_interface_descriptor(char *start, char *end,
+ const struct usb_interface_cache *intfc,
+ const struct usb_interface *iface,
+ int setno)
{
- struct usb_interface_descriptor *desc = &iface->altsetting[setno].desc;
+ struct usb_interface_descriptor *desc = &intfc->altsetting[setno].desc;
+ char *driver_name = "";
if (start > end)
return start;
down_read(&usb_bus_type.subsys.rwsem);
+ if (iface)
+ driver_name = (iface->dev.driver
+ ? iface->dev.driver->name
+ : "(none)");
start += sprintf(start, format_iface,
desc->bInterfaceNumber,
desc->bAlternateSetting,
@@ -247,9 +255,7 @@
class_decode(desc->bInterfaceClass),
desc->bInterfaceSubClass,
desc->bInterfaceProtocol,
- iface->dev.driver
- ? iface->dev.driver->name
- : "(none)");
+ driver_name);
up_read(&usb_bus_type.subsys.rwsem);
return start;
}
@@ -258,13 +264,14 @@
int speed,
char *start,
char *end,
+ const struct usb_interface_cache *intfc,
const struct usb_interface *iface,
int setno
) {
- struct usb_host_interface *desc = &iface->altsetting[setno];
+ struct usb_host_interface *desc = &intfc->altsetting[setno];
int i;
- start = usb_dump_interface_descriptor(start, end, iface, setno);
+ start = usb_dump_interface_descriptor(start, end, intfc, iface, setno);
for (i = 0; i < desc->desc.bNumEndpoints; i++) {
if (start > end)
return start;
@@ -303,6 +310,7 @@
)
{
int i, j;
+ struct usb_interface_cache *intfc;
struct usb_interface *interface;
if (start > end)
@@ -311,14 +319,13 @@
return start + sprintf(start, "(null Cfg. desc.)\n");
start = usb_dump_config_descriptor(start, end, &config->desc, active);
for (i = 0; i < config->desc.bNumInterfaces; i++) {
+ intfc = config->intf_cache[i];
interface = config->interface[i];
- if (!interface)
- break;
- for (j = 0; j < interface->num_altsetting; j++) {
+ for (j = 0; j < intfc->num_altsetting; j++) {
if (start > end)
return start;
start = usb_dump_interface(speed,
- start, end, interface, j);
+ start, end, intfc, interface, j);
}
}
return start;
@@ -395,7 +402,7 @@
return start;
start = usb_dump_device_strings (start, end, dev);
-
+
for (i = 0; i < dev->descriptor.bNumConfigurations; i++) {
if (start > end)
return start;
===== drivers/usb/core/message.c 1.80 vs edited =====
--- 1.80/drivers/usb/core/message.c Tue Apr 6 05:52:25 2004
+++ edited/drivers/usb/core/message.c Thu Apr 15 12:28:25 2004
@@ -796,10 +796,6 @@
}
}
-static void release_interface(struct device *dev)
-{
-}
-
/*
* usb_disable_device - Disable all the endpoints for a USB device
* @dev: the device whose endpoints are being disabled
@@ -835,6 +831,7 @@
dev_dbg (&dev->dev, "unregistering interface %s\n",
interface->dev.bus_id);
device_unregister (&interface->dev);
+ dev->actconfig->interface[i] = NULL;
}
dev->actconfig = 0;
if (dev->state == USB_STATE_CONFIGURED)
@@ -1071,6 +1068,16 @@
return 0;
}
+static void release_interface(struct device *dev)
+{
+ struct usb_interface *intf = to_usb_interface(dev);
+ struct usb_interface_cache *intfc =
+ altsetting_to_usb_interface_cache(intf->altsetting);
+
+ kref_put(&intfc->ref);
+ kfree(intf);
+}
+
/*
* usb_set_configuration - Makes a particular device setting be current
* @dev: the device whose configuration is being updated
@@ -1109,19 +1116,17 @@
{
int i, ret;
struct usb_host_config *cp = NULL;
-
+
/* dev->serialize guards all config changes */
- for (i=0; i<dev->descriptor.bNumConfigurations; i++) {
+ for (i = 0; i < dev->descriptor.bNumConfigurations; i++) {
if (dev->config[i].desc.bConfigurationValue == configuration) {
cp = &dev->config[i];
break;
}
}
- if ((!cp && configuration != 0)) {
- ret = -EINVAL;
- goto out;
- }
+ if ((!cp && configuration != 0))
+ return -EINVAL;
/* The USB spec says configuration 0 means unconfigured.
* But if a device includes a configuration numbered 0,
@@ -1139,72 +1144,86 @@
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)
- goto out;
+ return ret;
dev->actconfig = cp;
- if (!cp)
+ if (!cp) {
dev->state = USB_STATE_ADDRESS;
- else {
- dev->state = USB_STATE_CONFIGURED;
+ return 0;
+ }
- /* 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; i < cp->desc.bNumInterfaces; ++i) {
- struct usb_interface *intf = cp->interface[i];
- struct usb_host_interface *alt;
-
- alt = usb_altnum_to_altsetting(intf, 0);
-
- /* No altsetting 0? We'll assume the first altsetting.
- * We could use a GetInterface call, but if a device is
- * so non-compliant that it doesn't have altsetting 0
- * then I wouldn't trust its reply anyway.
- */
- if (!alt)
- alt = &intf->altsetting[0];
-
- intf->cur_altsetting = alt;
- 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;
- 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);
+ dev->state = USB_STATE_CONFIGURED;
+
+ /* 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; i < cp->desc.bNumInterfaces; ++i) {
+ struct usb_interface_cache *intfc;
+ struct usb_interface *intf;
+ struct usb_host_interface *alt;
+
+ cp->interface[i] = intf = kmalloc(sizeof(*intf), GFP_KERNEL);
+ if (!intf) {
+ dev_err(&dev->dev, "Out of memory");
+ while (--i >= 0) {
+ put_device(&cp->interface[i]->dev);
+ cp->interface[i] = NULL;
+ }
+ dev->actconfig = NULL;
+ return -ENOMEM;
}
- /* 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.
+ memset(intf, 0, sizeof(*intf));
+ intfc = cp->intf_cache[i];
+ intf->altsetting = intfc->altsetting;
+ intf->num_altsetting = intfc->num_altsetting;
+ kref_get(&intfc->ref);
+
+ alt = usb_altnum_to_altsetting(intf, 0);
+
+ /* No altsetting 0? We'll assume the first altsetting.
+ * We could use a GetInterface call, but if a device is
+ * so non-compliant that it doesn't have altsetting 0
+ * then I wouldn't trust its reply anyway.
*/
- 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,
- "adding %s (config #%d, interface %d)\n",
- intf->dev.bus_id, configuration,
- 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);
+ if (!alt)
+ alt = &intf->altsetting[0];
+
+ intf->cur_altsetting = alt;
+ 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;
+ 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, "adding %s (config #%d, interface %d)\n",
+ intf->dev.bus_id, configuration,
+ 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);
}
-out:
return ret;
}
===== drivers/usb/core/usb.c 1.260 vs edited =====
--- 1.260/drivers/usb/core/usb.c Wed Mar 31 17:13:05 2004
+++ edited/drivers/usb/core/usb.c Thu Apr 15 12:19:20 2004
@@ -1193,7 +1193,7 @@
/* heuristic: Linux is more likely to have class
* drivers, so avoid vendor-specific interfaces.
*/
- desc = &dev->config[i].interface[0]
+ desc = &dev->config[i].intf_cache[0]
->altsetting->desc;
if (desc->bInterfaceClass == USB_CLASS_VENDOR_SPEC)
continue;
-------------------------------------------------------
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_id=1470&alloc_id=3638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
