ChangeSet 1.738.5.31, 2002/10/13 15:40:00-07:00, david-b@pacbell.net [PATCH] usbcore doc + minor fixes Cleaning out my queue of most minor patches: - Provides some kerneldoc for 'struct usb_interface' now that the API is highlighting it. - Fixes usb_set_interface() so it doesn't affect other interfaces. This provides the right place for an eventual HCD call to clean out now-invalid records of endpoint state, and also gets rid of a potential SMP issue where drivers on different interfaces calling concurrently could clobber each other. (Per-interface data doesn't need locking except against config changes.) - It's OK to pass URB_NO_INTERRUPT hints if you're queueing a bunch of interrupt transfers. The set_interface call should eventually take the interface as a parameter, it's one of the few left using the "device plus magic number" identifier. I have a partial patch for that, but it doesn't handle the (newish) ALSA usb audio driver or a few other callers. diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c --- a/drivers/usb/core/message.c Sun Oct 13 17:08:11 2002 +++ b/drivers/usb/core/message.c Sun Oct 13 17:08:11 2002 @@ -756,6 +756,7 @@ * * This is used to enable data transfers on interfaces that may not * be enabled by default. Not all devices support such configurability. + * Only the driver bound to an interface may change its setting. * * Within any given configuration, each interface may have several * alternative settings. These are often used to control levels of @@ -808,6 +809,22 @@ interface, NULL, 0, HZ * 5)) < 0) return ret; + /* FIXME drivers shouldn't need to replicate/bugfix the logic here + * when they implement async or easily-killable versions of this or + * other "should-be-internal" functions (like clear_halt). + * should hcd+usbcore postprocess control requests? + */ + + /* prevent submissions using previous endpoint settings */ + iface_as = iface->altsetting + iface->act_altsetting; + for (i = 0; i < iface_as->bNumEndpoints; i++) { + u8 ep = iface_as->endpoint [i].bEndpointAddress; + int out = !(ep & USB_DIR_IN); + + ep &= USB_ENDPOINT_NUMBER_MASK; + (out ? dev->epmaxpacketout : dev->epmaxpacketin ) [ep] = 0; + // FIXME want hcd hook here, "no such endpoint" + } iface->act_altsetting = alternate; /* 9.1.1.5: reset toggles for all endpoints affected by this iface-as @@ -819,21 +836,20 @@ * any SetInterface request and hence assume toggles need to be reset. * However, EP0 toggles are re-synced for every individual transfer * during the SETUP stage - hence EP0 toggles are "don't care" here. + * (Likewise, EP0 never "halts" on well designed devices.) */ iface_as = &iface->altsetting[alternate]; for (i = 0; i < iface_as->bNumEndpoints; i++) { u8 ep = iface_as->endpoint[i].bEndpointAddress; + int out = !(ep & USB_DIR_IN); - usb_settoggle(dev, ep&USB_ENDPOINT_NUMBER_MASK, usb_endpoint_out(ep), 0); + ep &= USB_ENDPOINT_NUMBER_MASK; + usb_settoggle (dev, ep, out, 0); + (out ? dev->epmaxpacketout : dev->epmaxpacketin) [ep] + = iface_as->endpoint [ep].wMaxPacketSize; } - /* usb_set_maxpacket() sets the maxpacket size for all EP in all - * interfaces but it shouldn't do any harm here: we have changed - * the AS for the requested interface only, hence for unaffected - * interfaces it's just re-application of still-valid values. - */ - usb_set_maxpacket(dev); return 0; } diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c --- a/drivers/usb/core/urb.c Sun Oct 13 17:08:11 2002 +++ b/drivers/usb/core/urb.c Sun Oct 13 17:08:11 2002 @@ -272,9 +272,9 @@ /* enforce simple/standard policy */ allowed = USB_ASYNC_UNLINK; // affects later unlinks allowed |= URB_NO_DMA_MAP; + allowed |= URB_NO_INTERRUPT; switch (temp) { case PIPE_BULK: - allowed |= URB_NO_INTERRUPT; if (is_out) allowed |= USB_ZERO_PACKET; /* FALLTHROUGH */ diff -Nru a/include/linux/usb.h b/include/linux/usb.h --- a/include/linux/usb.h Sun Oct 13 17:08:11 2002 +++ b/include/linux/usb.h Sun Oct 13 17:08:11 2002 @@ -222,12 +222,42 @@ int extralen; }; +/** + * struct usb_interface - what usb device drivers talk to + * @altsetting: array of interface descriptors, one for each alternate + * setting that may be selected. each one includes a set of + * endpoint configurations. + * @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 + * interface uses its default setting of zero. + * @dev: driver model's view of this device + * + * USB device drivers attach to interfaces on a physical device. Each + * interface encapsulates a single high level function, such as feeding + * an audio stream to a speaker or reporting a change in a volume control. + * Many USB devices only have one interface. The protocol used to talk to + * an interface's endpoints can be defined in a usb "class" specification, + * or by a product's vendor. The (default) control endpoint is part of + * every interface, but is never listed among the interface's descriptors. + * + * The driver that is bound to the interface can use standard driver model + * 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 + * 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. + */ struct usb_interface { struct usb_interface_descriptor *altsetting; - int act_altsetting; /* active alternate setting */ - int num_altsetting; /* number of alternate settings */ - int max_altsetting; /* total memory allocated */ + unsigned act_altsetting; /* active alternate setting */ + unsigned num_altsetting; /* number of alternate settings */ + unsigned max_altsetting; /* total memory allocated */ struct usb_driver *driver; /* driver */ struct device dev; /* interface specific device info */