ChangeSet 1.1371.759.37, 2004/04/27 16:02:32-07:00, [EMAIL PROTECTED]
[PATCH] USB: Allocate interface structures dynamically
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.
Here is a slight revision of patch as246b. This one allocates all the new
interfaces before changing any other state; otherwise it's the same.
drivers/usb/core/config.c | 75 ++++++++++++++++++---------------------------
drivers/usb/core/devices.c | 31 +++++++++++-------
drivers/usb/core/message.c | 59 +++++++++++++++++++++++++++--------
drivers/usb/core/usb.c | 2 -
include/linux/usb.h | 41 ++++++++++++++++++++++--
5 files changed, 135 insertions(+), 73 deletions(-)
diff -Nru a/drivers/usb/core/config.c b/drivers/usb/core/config.c
--- a/drivers/usb/core/config.c Fri May 14 15:31:52 2004
+++ b/drivers/usb/core/config.c Fri May 14 15:31:52 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);
diff -Nru a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
--- a/drivers/usb/core/devices.c Fri May 14 15:31:52 2004
+++ b/drivers/usb/core/devices.c Fri May 14 15:31:52 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;
diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c
--- a/drivers/usb/core/message.c Fri May 14 15:31:52 2004
+++ b/drivers/usb/core/message.c Fri May 14 15:31:52 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,19 @@
{
int i, ret;
struct usb_host_config *cp = NULL;
-
+ struct usb_interface *new_interfaces[USB_MAXINTERFACES];
+ int n;
+
/* 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,
@@ -1130,6 +1137,25 @@
if (cp && configuration == 0)
dev_warn(&dev->dev, "config 0 descriptor??\n");
+ /* Allocate memory for new interfaces before doing anything else,
+ * so that if we run out then nothing will have changed. */
+ n = 0;
+ if (cp) {
+ for (; n < cp->desc.bNumInterfaces; ++n) {
+ new_interfaces[n] = kmalloc(
+ sizeof(struct usb_interface),
+ GFP_KERNEL);
+ if (!new_interfaces[n]) {
+ dev_err(&dev->dev, "Out of memory");
+ ret = -ENOMEM;
+free_interfaces:
+ while (--n >= 0)
+ kfree(new_interfaces[n]);
+ return ret;
+ }
+ }
+ }
+
/* if it's already configured, clear out old state first.
* getting rid of old interfaces means unbinding their drivers.
*/
@@ -1139,7 +1165,7 @@
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;
+ goto free_interfaces;
dev->actconfig = cp;
if (!cp)
@@ -1152,9 +1178,17 @@
* maybe probe() calls will choose different altsettings.
*/
for (i = 0; i < cp->desc.bNumInterfaces; ++i) {
- struct usb_interface *intf = cp->interface[i];
+ struct usb_interface_cache *intfc;
+ struct usb_interface *intf;
struct usb_host_interface *alt;
+ cp->interface[i] = intf = new_interfaces[i];
+ 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.
@@ -1204,7 +1238,6 @@
}
}
-out:
return ret;
}
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Fri May 14 15:31:52 2004
+++ b/drivers/usb/core/usb.c Fri May 14 15:31:52 2004
@@ -1127,7 +1127,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;
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Fri May 14 15:31:52 2004
+++ b/include/linux/usb.h Fri May 14 15:31:52 2004
@@ -148,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).
@@ -185,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;
-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id%62&alloc_ida84&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
