[PATCH] add usb_reset_configuration()

Unfortunately, usb_set_configuration() is widely mis-used as a
lightweight device reset.  That's trouble because setting a
configuration must sometimes involve things that don't relate
at all to a light reset, and can't be done in contexts like
driver probe() calls.

This patch updates most usb_set_configuration() users to use a call
that provides more appropriate functionality:

  - Adds a new usb_reset_configuration() call, which never needs
    to change very much usbcore state.

  - Uses it to replace most usb_set_configuration() calls, in
    many serial drivers, hisax, dvb, irda, and so on.

  - Modifies usb_reset_device() so it issues the control request
    directly.  It's both more of a reset (hides a USB reset) and
    less of one (altsettings are unchanged).

  - Makes usbfs return the error code instead of discarding it.

Once this goes in, then usb_set_configuration() can be made to
work properly (including from sysfs).

drivers/isdn/hisax/st5481_usb.c

@@ -252,8 +252,8 @@ int __devinit st5481_setup_usb(struct st5481_adapter *adapter)
 	
 	DBG(1,"");
 	
-	if ((status = usb_set_configuration (dev,dev->config[0].desc.bConfigurationValue)) < 0) {
-		WARN("set_configuration failed,status=%d",status);
+	if ((status = usb_reset_configuration (dev)) < 0) {
+		WARN("reset_configuration failed,status=%d",status);
 		return status;
 	}
 

drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c

@@ -1017,7 +1017,7 @@ static int ttusb_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 static int ttusb_setup_interfaces(struct ttusb *ttusb)
 {
-	usb_set_configuration(ttusb->dev, 1);
+	usb_reset_configuration(ttusb->dev);
 	usb_set_interface(ttusb->dev, 1, 1);
 
 	ttusb->bulk_out_pipe = usb_sndbulkpipe(ttusb->dev, 1);

drivers/net/irda/irda-usb.c

@@ -1482,9 +1482,9 @@ static int irda_usb_probe(struct usb_interface *intf,
 		goto err_out_2;
 	}
 
-	/* Is this really necessary? */
-	if (usb_set_configuration (dev, dev->config[0].desc.bConfigurationValue) < 0) {
-		err("set_configuration failed");
+	/* Is this really necessary? (no, except maybe for broken devices) */
+	if (usb_reset_configuration (dev) < 0) {
+		err("reset_configuration failed");
 		ret = -EIO;
 		goto err_out_3;
 	}

drivers/usb/class/cdc-acm.c

@@ -593,7 +593,14 @@ static int acm_probe (struct usb_interface *intf,
 				epwrite = &ifdata->endpoint[0].desc;
 			}
 
-			usb_set_configuration(dev, cfacm->desc.bConfigurationValue);
+			/* FIXME don't scan every config. it's either correct
+			 * when we probe(), or some other task must fix this.
+			 */
+			if (dev->actconfig != cfacm) {
+				err("need inactive config #%d",
+					cfacm->desc.bConfigurationValue);
+				return -ENODEV;
+			}
 
 			for (minor = 0; minor < ACM_TTY_MINORS && acm_table[minor]; minor++);
 			if (acm_table[minor]) {

drivers/usb/core/devio.c

@@ -762,9 +762,7 @@ static int proc_setconfig(struct dev_state *ps, void __user *arg)
 
 	if (get_user(u, (unsigned int __user *)arg))
 		return -EFAULT;
-	if (usb_set_configuration(ps->dev, u) < 0)
-		return -EINVAL;
-	return 0;
+	return usb_set_configuration(ps->dev, u);
 }
 
 static int proc_submiturb(struct dev_state *ps, void __user *arg)

drivers/usb/core/hub.c

@@ -1335,7 +1335,10 @@ int usb_physical_reset_device(struct usb_device *dev)
 
 	kfree(descriptor);
 
-	ret = usb_set_configuration(dev, dev->actconfig->desc.bConfigurationValue);
+	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+			USB_REQ_SET_CONFIGURATION, 0,
+			dev->actconfig->desc.bConfigurationValue, 0,
+			NULL, 0, HZ * USB_CTRL_SET_TIMEOUT);
 	if (ret < 0) {
 		err("failed to set dev %s active configuration (error=%d)",
 			dev->devpath, ret);

drivers/usb/core/message.c

@@ -963,6 +963,55 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
 	return 0;
 }
 
+/**
+ * usb_reset_configuration - lightweight device reset
+ * @dev: the device whose configuration is being reset
+ *
+ * This issues a standard SET_CONFIGURATION request to the device using
+ * the current configuration.  The effect is to reset most USB-related
+ * state in the device, including interface altsettings (reset to zero),
+ * endpoint halts (cleared), and data toggle (only for bulk and interrupt
+ * endpoints).  Other usbcore state is unchanged, including bindings of
+ * usb device drivers to interfaces.
+ *
+ * Because this affects multiple interfaces, avoid using this with composite
+ * (multi-interface) devices.  Instead, the driver for each interface may
+ * use usb_set_interface() on the interfaces it claims.  Resetting the whole
+ * configuration would affect other drivers' interfaces.
+ *
+ * Returns zero on success, else a negative error code.
+ */
+int usb_reset_configuration(struct usb_device *dev)
+{
+	int			i, retval;
+	struct usb_host_config	*config;
+
+	for (i = 1; i < 16; ++i) {
+		usb_disable_endpoint(dev, i);
+		usb_disable_endpoint(dev, i + USB_DIR_IN);
+	}
+
+	config = dev->actconfig;
+	retval = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+			USB_REQ_SET_CONFIGURATION, 0,
+			config->desc.bConfigurationValue, 0,
+			NULL, 0, HZ * USB_CTRL_SET_TIMEOUT);
+	if (retval < 0)
+		return retval;
+
+	dev->toggle[0] = dev->toggle[1] = 0;
+	dev->halted[0] = dev->halted[1] = 0;
+
+	/* re-init hc/hcd interface/endpoint state */
+	for (i = 0; i < config->desc.bNumInterfaces; i++) {
+		struct usb_interface *intf = config->interface[i];
+
+		intf->act_altsetting = 0;
+		usb_enable_interface(dev, intf);
+	}
+	return 0;
+}
+
 /**
  * usb_set_configuration - Makes a particular device setting be current
  * @dev: the device whose configuration is being updated
@@ -1136,7 +1185,10 @@ EXPORT_SYMBOL(usb_get_device_descriptor);
 EXPORT_SYMBOL(usb_get_status);
 EXPORT_SYMBOL(usb_get_string);
 EXPORT_SYMBOL(usb_string);
+
+// synchronous calls that also maintain usbcore state
 EXPORT_SYMBOL(usb_clear_halt);
+EXPORT_SYMBOL(usb_reset_configuration);
 EXPORT_SYMBOL(usb_set_configuration);
 EXPORT_SYMBOL(usb_set_interface);
 

drivers/usb/media/dabusb.c

@@ -747,8 +747,8 @@ static int dabusb_probe (struct usb_interface *intf,
 	s->usbdev = usbdev;
 	s->devnum = intf->minor;
 
-	if (usb_set_configuration (usbdev, usbdev->config[0].desc.bConfigurationValue) < 0) {
-		err("set_configuration failed");
+	if (usb_reset_configuration (usbdev) < 0) {
+		err("reset_configuration failed");
 		goto reject;
 	}
 	if (usbdev->descriptor.idProduct == 0x2131) {

drivers/usb/media/stv680.c

@@ -225,8 +225,9 @@ static int stv_sndctrl (int set, struct usb_stv *stv680, unsigned short req, uns
 static int stv_set_config (struct usb_stv *dev, int configuration, int interface, int alternate)
 {
 
-	if (usb_set_configuration (dev->udev, configuration) < 0) {
-		PDEBUG (1, "STV(e): FAILED to set configuration %i", configuration);
+	if (configuration != dev->udev->actconfig->desc.bConfigurationValue
+			|| usb_reset_configuration (dev->udev) < 0) {
+		PDEBUG (1, "STV(e): FAILED to reset configuration %i", configuration);
 		return -1;
 	}
 	if (usb_set_interface (dev->udev, interface, alternate) < 0) {

drivers/usb/misc/tiglusb.c

@@ -57,7 +57,7 @@ static int timeout = TIMAXTIME;	/* timeout in tenth of seconds     */
 static inline int
 clear_device (struct usb_device *dev)
 {
-	if (usb_set_configuration (dev, dev->config[0].desc.bConfigurationValue) < 0) {
+	if (usb_reset_configuration (dev) < 0) {
 		err ("clear_device failed");
 		return -1;
 	}
@@ -343,8 +343,10 @@ tiglusb_probe (struct usb_interface *intf,
 	    && (dev->descriptor.idVendor != 0x451))
 		return -ENODEV;
 
-	if (usb_set_configuration (dev, dev->config[0].desc.bConfigurationValue) < 0) {
-		err ("tiglusb_probe: set_configuration failed");
+	// NOTE:  it's already in this config, this shouldn't be needed.
+	// is this working around some hardware bug?
+	if (usb_reset_configuration (dev) < 0) {
+		err ("tiglusb_probe: reset_configuration failed");
 		return -ENODEV;
 	}
 

drivers/usb/serial/empeg.c

@@ -464,8 +464,13 @@ static int  empeg_startup (struct usb_serial *serial)
 
 	dbg("%s", __FUNCTION__);
 
-	dbg("%s - Set config to 1", __FUNCTION__);
-	r = usb_set_configuration (serial->dev, 1);
+	if (serial->dev->actconfig->desc.bConfigurationValue != 1) {
+		err("active config #%d != 1 ??",
+			serial->dev->actconfig->desc.bConfigurationValue);
+		return -ENODEV;
+	}
+	dbg("%s - reset config", __FUNCTION__);
+	r = usb_reset_configuration (serial->dev);
 
 	/* continue on with initialization */
 	return r;

drivers/usb/serial/ipaq.c

@@ -555,8 +555,12 @@ static void ipaq_destroy_lists(struct usb_serial_port *port)
 static int ipaq_startup(struct usb_serial *serial)
 {
 	dbg("%s", __FUNCTION__);
-	usb_set_configuration(serial->dev, 1);
-	return 0;
+	if (serial->dev->actconfig->desc.bConfigurationValue != 1) {
+		err("active config #%d != 1 ??",
+			serial->dev->actconfig->desc.bConfigurationValue);
+		return -ENODEV;
+	}
+	return usb_reset_configuration (serial->dev);
 }
 
 static void ipaq_shutdown(struct usb_serial *serial)

drivers/usb/serial/visor.c

@@ -763,8 +763,14 @@ static int visor_probe (struct usb_serial *serial, const struct usb_device_id *i
 
 	dbg("%s", __FUNCTION__);
 
-	dbg("%s - Set config to 1", __FUNCTION__);
-	usb_set_configuration (serial->dev, 1);
+	if (serial->dev->actconfig->desc.bConfigurationValue != 1) {
+		err("active config #%d != 1 ??",
+			serial->dev->actconfig->desc.bConfigurationValue);
+		return -ENODEV;
+	}
+	dbg("%s - reset config", __FUNCTION__);
+	retval = usb_reset_configuration (serial->dev);
+
 
 	if (id->driver_info) {
 		startup = (void *)id->driver_info;

drivers/usb/storage/usb.c

@@ -921,9 +921,14 @@ static int storage_probe(struct usb_interface *intf,
 	if (us->protocol == US_PR_EUSB_SDDR09 ||
 			us->protocol == US_PR_DPCM_USB) {
 		/* set the configuration -- STALL is an acceptable response here */
-		result = usb_set_configuration(us->pusb_dev, 1);
+		if (us->pusb_dev->actconfig->desc.bConfigurationValue != 1) {
+			US_DEBUGP("active config #%d != 1 ??\n", us->pusb_dev
+				->actconfig->desc.bConfigurationValue);
+			goto BadDevice;
+		}
+		result = usb_reset_configuration(us->pusb_dev);
 
-		US_DEBUGP("Result from usb_set_configuration is %d\n", result);
+		US_DEBUGP("Result of usb_reset_configuration is %d\n", result);
 		if (result == -EPIPE) {
 			US_DEBUGP("-- stall on control interface\n");
 		} else if (result != 0) {

include/linux/usb.h

@@ -869,6 +869,7 @@ extern int usb_string(struct usb_device *dev, int index,
 
 /* wrappers that also update important state inside usbcore */
 extern int usb_clear_halt(struct usb_device *dev, int pipe);
+extern int usb_reset_configuration(struct usb_device *dev);
 extern int usb_set_configuration(struct usb_device *dev, int configuration);
 extern int usb_set_interface(struct usb_device *dev, int ifnum, int alternate);
 

sound/usb/usbaudio.c

@@ -2560,8 +2560,8 @@ static int snd_usb_extigy_boot_quirk(struct usb_device *dev, struct usb_interfac
 		err = usb_get_device_descriptor(dev);
 		config = dev->actconfig;
 		if (err < 0) snd_printdd("error usb_get_device_descriptor: %d\n", err);
-		err = usb_set_configuration(dev, get_cfg_desc(config)->bConfigurationValue);
-		if (err < 0) snd_printdd("error usb_set_configuration: %d\n", err);
+		err = usb_reset_configuration(dev);
+		if (err < 0) snd_printdd("error usb_reset_configuration: %d\n", err);
 		snd_printdd("extigy_boot: new boot length = %d\n", get_cfg_desc(config)->wTotalLength);
 		return -ENODEV; /* quit this anyway */
 	}
@@ -2761,8 +2761,8 @@ static void *snd_usb_audio_probe(struct usb_device *dev,
 		 * now look for an empty slot and create a new card instance
 		 */
 		/* first, set the current configuration for this device */
-		if (usb_set_configuration(dev, get_cfg_desc(config)->bConfigurationValue) < 0) {
-			snd_printk(KERN_ERR "cannot set configuration (value 0x%x)\n", get_cfg_desc(config)->bConfigurationValue);
+		if (usb_reset_configuration(dev) < 0) {
+			snd_printk(KERN_ERR "cannot reset configuration (value 0x%x)\n", get_cfg_desc(config)->bConfigurationValue);
 			goto __error;
 		}
 		for (i = 0; i < SNDRV_CARDS; i++)