ChangeSet 1.1595.7.31, 2003/07/31 22:50:19-07:00, stern@rowland.harvard.edu [PATCH] USB: Use the new enable/disable routines This is the third part of what used to be as66c. The patch makes several changes in the routines that handle unbinding and selecting altsettings and configurations. Upon unbinding a driver, don't nuke all the URBs for the device -- only kill the ones on the driver's interface. Afterwards, reinitialize the interface by selecting altsetting 0 (the default). When changing an altsetting, if the interface has only one altsetting it is allowed to STALL the request. Attempt to carry out the equivalent initialization by clearing the HALT feature on each of the interface's endpoints. When changing configurations, mark each interface as being in altsetting 0. In general, use the new disable/enable routines instead of doing everything by hand. drivers/usb/core/message.c | 93 ++++++++++++++++++++++----------------------- drivers/usb/core/usb.c | 50 ++++++++---------------- 2 files changed, 65 insertions(+), 78 deletions(-) diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c --- a/drivers/usb/core/message.c Fri Aug 1 10:53:59 2003 +++ b/drivers/usb/core/message.c Fri Aug 1 10:53:59 2003 @@ -922,9 +922,8 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) { struct usb_interface *iface; - struct usb_host_interface *iface_as; - int i, ret; - void (*disable)(struct usb_device *, int) = dev->bus->op->disable; + int ret; + int manual = 0; iface = usb_ifnum_to_if(dev, interface); if (!iface) { @@ -932,22 +931,23 @@ return -EINVAL; } - /* 9.4.10 says devices don't need this, if the interface - only has one alternate setting */ - if (iface->num_altsetting == 1) { - dbg("ignoring set_interface for dev %d, iface %d, alt %d", - dev->devnum, interface, alternate); - return 0; - } - if (alternate < 0 || alternate >= iface->num_altsetting) return -EINVAL; - if ((ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), + 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)) < 0) + 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. + */ + if (ret == -EPIPE && iface->num_altsetting == 1) { + dbg("manual set_interface for dev %d, iface %d, alt %d", + dev->devnum, interface, alternate); + manual = 1; + } else if (ret < 0) return ret; /* FIXME drivers shouldn't need to replicate/bugfix the logic here @@ -957,20 +957,32 @@ */ /* prevent submissions using previous endpoint settings */ - iface_as = iface->altsetting + iface->act_altsetting; - for (i = 0; i < iface_as->desc.bNumEndpoints; i++) { - u8 ep = iface_as->endpoint [i].desc.bEndpointAddress; - int out = !(ep & USB_DIR_IN); - - /* clear out hcd state, then usbcore state */ - if (disable) - disable (dev, ep); - ep &= USB_ENDPOINT_NUMBER_MASK; - (out ? dev->epmaxpacketout : dev->epmaxpacketin ) [ep] = 0; - } + usb_disable_interface(dev, iface); + iface->act_altsetting = alternate; - /* 9.1.1.5: reset toggles for all endpoints affected by this iface-as + /* If the interface only has one altsetting and the device didn't + * accept the request, we attempt to carry out the equivalent action + * by manually clearing the HALT feature for each endpoint in the + * new altsetting. + */ + if (manual) { + struct usb_host_interface *iface_as = + &iface->altsetting[alternate]; + int i; + + for (i = 0; i < iface_as->desc.bNumEndpoints; i++) { + unsigned int epaddr = + iface_as->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); + + usb_clear_halt(dev, pipe); + } + } + + /* 9.1.1.5: reset toggles for all endpoints in the new altsetting * * Note: * Despite EP0 is always present in all interfaces/AS, the list of @@ -981,18 +993,7 @@ * 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->desc.bNumEndpoints; i++) { - u8 ep = iface_as->endpoint[i].desc.bEndpointAddress; - int out = !(ep & USB_DIR_IN); - - ep &= USB_ENDPOINT_NUMBER_MASK; - usb_settoggle (dev, ep, out, 0); - (out ? dev->epmaxpacketout : dev->epmaxpacketin) [ep] - = iface_as->endpoint [i].desc.wMaxPacketSize; - usb_endpoint_running (dev, ep, out); - } + usb_enable_interface(dev, iface); return 0; } @@ -1031,7 +1032,6 @@ { int i, ret; struct usb_host_config *cp = NULL; - void (*disable)(struct usb_device *, int) = dev->bus->op->disable; for (i=0; idescriptor.bNumConfigurations; i++) { if (dev->config[i].desc.bConfigurationValue == configuration) { @@ -1045,12 +1045,8 @@ } /* 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) + usb_disable_device (dev, 1); // Skip ep0 dev->toggle[0] = dev->toggle[1] = 0; dev->halted[0] = dev->halted[1] = 0; dev->state = USB_STATE_ADDRESS; @@ -1063,8 +1059,13 @@ dev->state = USB_STATE_CONFIGURED; dev->actconfig = cp; - /* reset more hc/hcd endpoint state */ - usb_set_maxpacket(dev); + /* reset more hc/hcd interface/endpoint state */ + for (i = 0; i < cp->desc.bNumInterfaces; ++i) { + struct usb_interface *intf = cp->interface[i]; + + intf->act_altsetting = 0; + usb_enable_interface(dev, intf); + } return 0; } diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c --- a/drivers/usb/core/usb.c Fri Aug 1 10:53:59 2003 +++ b/drivers/usb/core/usb.c Fri Aug 1 10:53:59 2003 @@ -80,23 +80,6 @@ static int usb_generic_driver_data; -/* deallocate hcd/hardware state ... and nuke all pending urbs */ -static void nuke_urbs(struct usb_device *dev) -{ - void (*disable)(struct usb_device *, int); - int i; - - if (!dev || !dev->bus || !dev->bus->op || !dev->bus->op->disable) - return; - dbg("nuking urbs assigned to %s", dev->dev.bus_id); - - disable = dev->bus->op->disable; - for (i = 0; i < 15; i++) { - disable(dev, i); - disable(dev, USB_DIR_IN | i); - } -} - /* needs to be called with BKL held */ int usb_probe_interface(struct device *dev) { @@ -125,23 +108,19 @@ int usb_unbind_interface(struct device *dev) { - struct usb_interface *intf; - struct usb_driver *driver; - - intf = list_entry(dev,struct usb_interface,dev); - driver = to_usb_driver(dev->driver); + struct usb_interface *intf = to_usb_interface(dev); + struct usb_driver *driver = to_usb_driver(dev->driver); down(&driver->serialize); - /* release all urbs for this device */ - nuke_urbs(interface_to_usbdev(intf)); + /* release all urbs for this interface */ + usb_disable_interface(interface_to_usbdev(intf), intf); if (intf->driver && intf->driver->disconnect) intf->driver->disconnect(intf); - /* if driver->disconnect didn't release the interface */ - if (intf->driver) - usb_driver_release_interface(driver, intf); + /* force a release and re-initialize the interface */ + usb_driver_release_interface(driver, intf); up(&driver->serialize); @@ -325,24 +304,31 @@ * usb_driver_release_interface - unbind a driver from an interface * @driver: the driver to be unbound * @iface: the interface from which it will be unbound + * + * In addition to unbinding the driver, this re-initializes the interface + * by selecting altsetting 0, the default alternate setting. * - * This should be used by drivers to release their claimed interfaces. - * It is normally called in their disconnect() methods, and only for - * drivers that bound to more than one interface in their probe(). + * This can be used by drivers to release an interface without waiting + * for their disconnect() methods to be called. * * When the USB subsystem disconnect()s a driver from some interface, * it automatically invokes this method for that interface. That * means that even drivers that used usb_driver_claim_interface() * usually won't need to call this. + * + * This call is synchronous, and may not be used in an interrupt context. */ void usb_driver_release_interface(struct usb_driver *driver, struct usb_interface *iface) { /* this should never happen, don't release something that's not ours */ - if (!iface || iface->driver != driver) + if (iface->driver && iface->driver != driver) return; iface->driver = NULL; usb_set_intfdata(iface, NULL); + usb_set_interface(interface_to_usbdev(iface), + iface->altsetting[0].desc.bInterfaceNumber, + 0); } /** @@ -917,7 +903,7 @@ } /* deallocate hcd/hardware state ... and nuke all pending urbs */ - nuke_urbs(dev); + usb_disable_device(dev, 0); /* disconnect() drivers from interfaces (a key side effect) */ dev_dbg (&dev->dev, "unregistering interfaces\n");