On Sat, 21 Feb 2004, Greg KH wrote:
> > One thing that would be good, whether this change gets made or not, is to
> > remove all assumptions from drivers about the order in which interfaces
> > are stored (use usb_ifnum_to_if()) and the order in which altsettings are
> > stored (replace intf.act_altsetting with a pointer and create
> > usb_altnum_to_alt() analogous to usb_ifnum_to_if()). There are plenty of
> > drivers that will need to be fixed up.
>
> I'd be glad to take patches to fix up any drivers that still have this
> problem right now.
Here's a start. This patch begins the conversion process by adding
usbcore support for cur_altsetting and deprecating act_altsetting.
So long as we assumed that altsetting numbers range from 0 to
num_altsetting-1 and that the number matches its index in the altsetting
array, there was no harm in using act_altsetting. But without that
assumption act_altsetting is merely an invitation to errors. Although the
kerneldoc says that act_altsetting is the _index_ of the active
altsetting, it's all too easy to confuse it with the _number_ of the
active altsetting. Using cur_altsetting instead (a pointer rather than a
number) will prevent that confusion.
Until all the drivers have been converted to use cur_altsetting, the core
will have to maintain act_altsetting in parallel with it. Eventually we
will be able to remove act_altsetting, but fixing all the drivers will
take a while.
Included in this patch:
Add cur_altsetting to struct usb_interface and deprecate
act_altsetting.
Add comments and kerneldoc explaining the changes. Also remove
the comments in front of struct usb_host_config (they seem to
have been left behind when usb_ch9.h was split out) and add
kerneldoc for that structure.
Add usb_altnum_to_altsetting() to help look up altsettings based
on their number.
Convert the usb_set_interface(), usb_set_configuration(), and
usb_reset_configuration() routines to support cur_altsetting
and act_altsetting in parallel. Convert a few others to use
cur_altsetting rather than act_altsetting.
Rename a few local variables to make their meaning a little
clearer. It would be nice to change struct usb_host_interface
to something like usb_host_altsetting, but that's a patch for
another time.
I believee this is ready to be applied, but I would welcome comments.
Alan Stern
diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c
--- a/drivers/usb/core/message.c Mon Mar 1 11:01:40 2004
+++ b/drivers/usb/core/message.c Mon Mar 1 11:01:40 2004
@@ -783,13 +783,12 @@
*/
void usb_disable_interface(struct usb_device *dev, struct usb_interface *intf)
{
- struct usb_host_interface *hintf =
- &intf->altsetting[intf->act_altsetting];
+ struct usb_host_interface *alt = intf->cur_altsetting;
int i;
- for (i = 0; i < hintf->desc.bNumEndpoints; ++i) {
+ for (i = 0; i < alt->desc.bNumEndpoints; ++i) {
usb_disable_endpoint(dev,
- hintf->endpoint[i].desc.bEndpointAddress);
+ alt->endpoint[i].desc.bEndpointAddress);
}
}
@@ -887,12 +886,11 @@
void usb_enable_interface(struct usb_device *dev,
struct usb_interface *intf)
{
- struct usb_host_interface *hintf =
- &intf->altsetting[intf->act_altsetting];
+ struct usb_host_interface *alt = intf->cur_altsetting;
int i;
- for (i = 0; i < hintf->desc.bNumEndpoints; ++i)
- usb_enable_endpoint(dev, &hintf->endpoint[i].desc);
+ for (i = 0; i < alt->desc.bNumEndpoints; ++i)
+ usb_enable_endpoint(dev, &alt->endpoint[i].desc);
}
/**
@@ -931,6 +929,7 @@
int usb_set_interface(struct usb_device *dev, int interface, int alternate)
{
struct usb_interface *iface;
+ struct usb_host_interface *alt;
int ret;
int manual = 0;
@@ -940,14 +939,15 @@
return -EINVAL;
}
- if (alternate < 0 || alternate >= iface->num_altsetting)
+ alt = usb_altnum_to_altsetting(iface, alternate);
+ if (!alt) {
+ warn("selecting invalid altsetting %d", alternate);
return -EINVAL;
+ }
ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
USB_REQ_SET_INTERFACE, USB_RECIP_INTERFACE,
- iface->altsetting[alternate]
- .desc.bAlternateSetting,
- interface, NULL, 0, HZ * 5);
+ alternate, interface, NULL, 0, HZ * 5);
/* 9.4.10 says devices don't need this and are free to STALL the
* request if the interface only has one alternate setting.
@@ -968,7 +968,8 @@
/* prevent submissions using previous endpoint settings */
usb_disable_interface(dev, iface);
- iface->act_altsetting = alternate;
+ iface->cur_altsetting = alt;
+ iface->act_altsetting = alt - iface->altsetting;
/* If the interface only has one altsetting and the device didn't
* accept the request, we attempt to carry out the equivalent action
@@ -976,13 +977,11 @@
* new altsetting.
*/
if (manual) {
- struct usb_host_interface *iface_as =
- &iface->altsetting[alternate];
int i;
- for (i = 0; i < iface_as->desc.bNumEndpoints; i++) {
+ for (i = 0; i < alt->desc.bNumEndpoints; i++) {
unsigned int epaddr =
- iface_as->endpoint[i].desc.bEndpointAddress;
+ alt->endpoint[i].desc.bEndpointAddress;
unsigned int pipe =
__create_pipe(dev, USB_ENDPOINT_NUMBER_MASK & epaddr)
| (usb_endpoint_out(epaddr) ? USB_DIR_OUT : USB_DIR_IN);
@@ -1056,8 +1055,20 @@
/* re-init hc/hcd interface/endpoint state */
for (i = 0; i < config->desc.bNumInterfaces; i++) {
struct usb_interface *intf = config->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->act_altsetting = 0;
+ intf->cur_altsetting = alt;
+ intf->act_altsetting = alt - intf->altsetting;
usb_enable_interface(dev, intf);
}
return 0;
@@ -1146,12 +1157,21 @@
*/
for (i = 0; i < cp->desc.bNumInterfaces; ++i) {
struct usb_interface *intf = cp->interface[i];
- struct usb_interface_descriptor *desc;
+ struct usb_host_interface *alt;
- intf->act_altsetting = 0;
- desc = &intf->altsetting [0].desc;
- usb_enable_interface(dev, intf);
+ 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;
+ intf->act_altsetting = alt - intf->altsetting;
+ usb_enable_interface(dev, intf);
intf->dev.parent = &dev->dev;
intf->dev.driver = NULL;
intf->dev.bus = &usb_bus_type;
@@ -1160,11 +1180,11 @@
sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",
dev->bus->busnum, dev->devpath,
configuration,
- desc->bInterfaceNumber);
+ alt->desc.bInterfaceNumber);
dev_dbg (&dev->dev,
"registering %s (config #%d, interface %d)\n",
intf->dev.bus_id, configuration,
- desc->bInterfaceNumber);
+ alt->desc.bInterfaceNumber);
device_register (&intf->dev);
usb_create_driverfs_intf_files (intf);
}
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Mon Mar 1 11:01:40 2004
+++ b/drivers/usb/core/usb.c Mon Mar 1 11:01:40 2004
@@ -189,7 +189,7 @@
}
/**
- * usb_ifnum_to_if - get the interface object with a given interface number
(usbcore-internal)
+ * usb_ifnum_to_if - get the interface object with a given interface number
* @dev: the device whose current configuration is considered
* @ifnum: the desired interface
*
@@ -220,6 +220,33 @@
}
/**
+ * usb_altnum_to_altsetting - get the altsetting structure with a given
+ * alternate setting number.
+ * @intf: the interface containing the altsetting in question
+ * @altnum: the desired alternate setting number
+ *
+ * This searches the altsetting array of the specified interface for
+ * an entry with the correct bAlternateSetting value and returns a pointer
+ * to that entry, or null.
+ *
+ * Note that altsettings need not be stored sequentially by number, so
+ * it would be incorrect to assume that the first altsetting entry in
+ * the array corresponds to altsetting zero. This routine helps device
+ * drivers avoid such mistakes.
+ */
+struct usb_host_interface *usb_altnum_to_altsetting(struct usb_interface *intf,
+ unsigned int altnum)
+{
+ int i;
+
+ for (i = 0; i < intf->num_altsetting; i++) {
+ if (intf->altsetting[i].desc.bAlternateSetting == altnum)
+ return &intf->altsetting[i];
+ }
+ return NULL;
+}
+
+/**
* usb_epnum_to_ep_desc - get the endpoint object with a given endpoint number
* @dev: the device whose current configuration+altsettings is considered
* @epnum: the desired endpoint, masked with USB_DIR_IN as appropriate.
@@ -247,7 +274,7 @@
/* only endpoints in current altsetting are active */
intf = config->interface[i];
- alt = intf->altsetting + intf->act_altsetting;
+ alt = intf->cur_altsetting;
for (k = 0; k < alt->desc.bNumEndpoints; k++)
if (epnum == alt->endpoint[k].desc.bEndpointAddress)
@@ -421,7 +448,7 @@
if (id == NULL)
return NULL;
- intf = &interface->altsetting [interface->act_altsetting];
+ intf = interface->cur_altsetting;
dev = interface_to_usbdev(interface);
/* It is important to check that id->driver_info is nonzero,
@@ -624,7 +651,7 @@
scratch += length;
if (usb_dev->descriptor.bDeviceClass == 0) {
- int alt = intf->act_altsetting;
+ struct usb_host_interface *alt = intf->cur_altsetting;
/* 2.4 only exposed interface zero. in 2.5, hotplug
* agents are called for all interfaces, and can use
@@ -633,9 +660,9 @@
envp [i++] = scratch;
length += snprintf (scratch, buffer_size - length,
"INTERFACE=%d/%d/%d",
- intf->altsetting[alt].desc.bInterfaceClass,
- intf->altsetting[alt].desc.bInterfaceSubClass,
- intf->altsetting[alt].desc.bInterfaceProtocol);
+ alt->desc.bInterfaceClass,
+ alt->desc.bInterfaceSubClass,
+ alt->desc.bInterfaceProtocol);
if ((buffer_size - length <= 0) || (i >= num_envp))
return -ENOMEM;
++length;
@@ -1582,6 +1609,7 @@
EXPORT_SYMBOL(usb_match_id);
EXPORT_SYMBOL(usb_find_interface);
EXPORT_SYMBOL(usb_ifnum_to_if);
+EXPORT_SYMBOL(usb_altnum_to_altsetting);
EXPORT_SYMBOL(usb_reset_device);
EXPORT_SYMBOL(usb_disconnect);
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Mon Mar 1 11:01:40 2004
+++ b/include/linux/usb.h Mon Mar 1 11:01:40 2004
@@ -72,14 +72,15 @@
/**
* struct usb_interface - what usb device drivers talk to
- * @altsetting: array of interface descriptors, one for each alternate
+ * @altsetting: array of interface structures, one for each alternate
* setting that may be selected. Each one includes a set of
- * endpoint configurations and will be in numberic order,
- * 0..num_altsetting.
+ * endpoint configurations. They will be in no particular order.
* @num_altsetting: number of altsettings defined.
- * @act_altsetting: index of current altsetting. this number is always
- * less than num_altsetting. after the device is configured, each
+ * @cur_altsetting: the current altsetting.
+ * @act_altsetting: index of current altsetting. This number is always
+ * less than num_altsetting. After the device is configured, each
* interface uses its default setting of zero.
+ * NOTE: act_altsetting is deprecated. Use cur_altsetting instead.
* @driver: the USB driver that is bound to this interface.
* @minor: the minor number assigned to this interface, if this
* interface is bound to a driver that uses the USB major number.
@@ -104,20 +105,27 @@
* calls such as dev_get_drvdata() on the dev member of this structure.
*
* Each interface may have alternate settings. The initial configuration
- * of a device sets the first of these, but the device driver can change
+ * of a device sets altsetting 0, but the device driver can change
* that setting using usb_set_interface(). Alternate settings are often
* used to control the the use of periodic endpoints, such as by having
* different endpoints use different amounts of reserved USB bandwidth.
* All standards-conformant USB devices that use isochronous endpoints
* will use them in non-default settings.
+ *
+ * The USB specification says that alternate setting numbers must run from
+ * 0 to one less than the total number of alternate settings. But some
+ * devices manage to mess this up, and the structures aren't necessarily
+ * stored in numerical order anyhow. Use usb_altnum_to_altsetting() to
+ * look up an alternate setting in the altsetting array based on its number.
*/
struct usb_interface {
- /* array of alternate settings for this interface.
- * these will be in numeric order, 0..num_altsettting
- */
+ /* array of alternate settings for this interface,
+ * stored in no particular order */
struct usb_host_interface *altsetting;
- unsigned act_altsetting; /* active alternate setting */
+ struct usb_host_interface *cur_altsetting; /* the currently
+ * active alternate setting */
+ unsigned act_altsetting; /* index of active alternate setting:
DEPRECATED */
unsigned num_altsetting; /* number of alternate settings */
struct usb_driver *driver; /* driver */
@@ -143,19 +151,43 @@
/* this maximum is arbitrary */
#define USB_MAXINTERFACES 32
-/* USB_DT_CONFIG: Configuration descriptor information.
+/**
+ * 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.
+ * @extra: pointer to buffer containing all extra descriptors associated
+ * with this configuration (those preceding the first interface
+ * descriptor).
+ * @extralen: length of the extra descriptors buffer.
+ *
+ * USB devices may have multiple configurations, but only one can be active
+ * at any time. Each encapsulates a different operational environment;
+ * for example, a dual-speed device would have separate configurations for
+ * full-speed and high-speed operation. The number of configurations
+ * available is stored in the device descriptor as bNumConfigurations.
*
- * USB_DT_OTHER_SPEED_CONFIG is the same descriptor, except that the
- * descriptor type is different. Highspeed-capable devices can look
- * different depending on what speed they're currently running. Only
- * devices with a USB_DT_DEVICE_QUALIFIER have an OTHER_SPEED_CONFIG.
+ * A configuration can contain multiple interfaces. Each corresponds to
+ * a different function of the USB device, and all are available whenever
+ * the configuration is active. The USB standard says that interfaces
+ * are supposed to be numbered from 0 to desc.bNumInterfaces-1, but a lot
+ * of devices get this wrong. In addition, the interface array is not
+ * guaranteed to be sorted in numerical order. Use usb_ifnum_to_if() to
+ * look up an interface entry based on its number.
+ *
+ * Device drivers should not attempt to activate configurations. The choice
+ * of which configuration to install is a policy decision based on such
+ * considerations as available power, functionality provided, and the user's
+ * desires (expressed through hotplug scripts). However, drivers can call
+ * usb_reset_configuration() to reinitialize the current configuration and
+ * all its interfaces.
*/
struct usb_host_config {
struct usb_config_descriptor desc;
- /* the interfaces associated with this configuration
- * these will be in numeric order, 0..desc.bNumInterfaces
- */
+ /* the interfaces associated with this configuration,
+ * stored in no particular order */
struct usb_interface *interface[USB_MAXINTERFACES];
unsigned char *extra; /* Extra descriptors */
@@ -297,8 +329,12 @@
const struct usb_device_id *usb_match_id(struct usb_interface *interface,
const struct usb_device_id *id);
-extern struct usb_interface *usb_find_interface(struct usb_driver *drv, int minor);
-extern struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum);
+extern struct usb_interface *usb_find_interface(struct usb_driver *drv,
+ int minor);
+extern struct usb_interface *usb_ifnum_to_if(struct usb_device *dev,
+ unsigned ifnum);
+extern struct usb_host_interface *usb_altnum_to_altsetting(
+ struct usb_interface *intf, unsigned int altnum);
/**
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
