ChangeSet 1.1123.18.8, 2003/08/11 16:20:45-07:00, [EMAIL PROTECTED]
[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 | 4 -
drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c | 2
drivers/net/irda/irda-usb.c | 6 +-
drivers/usb/class/cdc-acm.c | 9 +++
drivers/usb/core/devio.c | 4 -
drivers/usb/core/hub.c | 5 +-
drivers/usb/core/message.c | 52 ++++++++++++++++++++++
drivers/usb/media/dabusb.c | 4 -
drivers/usb/media/stv680.c | 5 +-
drivers/usb/misc/tiglusb.c | 8 ++-
drivers/usb/serial/empeg.c | 9 ++-
drivers/usb/serial/ipaq.c | 8 ++-
drivers/usb/serial/visor.c | 10 +++-
drivers/usb/storage/usb.c | 9 ++-
include/linux/usb.h | 1
sound/usb/usbaudio.c | 8 +--
16 files changed, 114 insertions(+), 30 deletions(-)
diff -Nru a/drivers/isdn/hisax/st5481_usb.c b/drivers/isdn/hisax/st5481_usb.c
--- a/drivers/isdn/hisax/st5481_usb.c Fri Aug 15 10:47:16 2003
+++ b/drivers/isdn/hisax/st5481_usb.c Fri Aug 15 10:47:16 2003
@@ -252,8 +252,8 @@
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;
}
diff -Nru a/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c
b/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c
--- a/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c Fri Aug 15 10:47:16 2003
+++ b/drivers/media/dvb/ttusb-budget/dvb-ttusb-budget.c Fri Aug 15 10:47:16 2003
@@ -1017,7 +1017,7 @@
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);
diff -Nru a/drivers/net/irda/irda-usb.c b/drivers/net/irda/irda-usb.c
--- a/drivers/net/irda/irda-usb.c Fri Aug 15 10:47:16 2003
+++ b/drivers/net/irda/irda-usb.c Fri Aug 15 10:47:16 2003
@@ -1482,9 +1482,9 @@
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;
}
diff -Nru a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
--- a/drivers/usb/class/cdc-acm.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/class/cdc-acm.c Fri Aug 15 10:47:16 2003
@@ -593,7 +593,14 @@
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]) {
diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/core/devio.c Fri Aug 15 10:47:16 2003
@@ -762,9 +762,7 @@
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)
diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/core/hub.c Fri Aug 15 10:47:16 2003
@@ -1335,7 +1335,10 @@
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);
diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c
--- a/drivers/usb/core/message.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/core/message.c Fri Aug 15 10:47:16 2003
@@ -964,6 +964,55 @@
}
/**
+ * 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
* @configuration: the configuration being chosen.
@@ -1136,7 +1185,10 @@
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);
diff -Nru a/drivers/usb/media/dabusb.c b/drivers/usb/media/dabusb.c
--- a/drivers/usb/media/dabusb.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/media/dabusb.c Fri Aug 15 10:47:16 2003
@@ -747,8 +747,8 @@
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) {
diff -Nru a/drivers/usb/media/stv680.c b/drivers/usb/media/stv680.c
--- a/drivers/usb/media/stv680.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/media/stv680.c Fri Aug 15 10:47:16 2003
@@ -225,8 +225,9 @@
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) {
diff -Nru a/drivers/usb/misc/tiglusb.c b/drivers/usb/misc/tiglusb.c
--- a/drivers/usb/misc/tiglusb.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/misc/tiglusb.c Fri Aug 15 10:47:16 2003
@@ -57,7 +57,7 @@
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 @@
&& (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;
}
diff -Nru a/drivers/usb/serial/empeg.c b/drivers/usb/serial/empeg.c
--- a/drivers/usb/serial/empeg.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/serial/empeg.c Fri Aug 15 10:47:16 2003
@@ -464,8 +464,13 @@
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;
diff -Nru a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
--- a/drivers/usb/serial/ipaq.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/serial/ipaq.c Fri Aug 15 10:47:16 2003
@@ -555,8 +555,12 @@
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)
diff -Nru a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
--- a/drivers/usb/serial/visor.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/serial/visor.c Fri Aug 15 10:47:16 2003
@@ -763,8 +763,14 @@
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;
diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
--- a/drivers/usb/storage/usb.c Fri Aug 15 10:47:16 2003
+++ b/drivers/usb/storage/usb.c Fri Aug 15 10:47:16 2003
@@ -921,9 +921,14 @@
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) {
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Fri Aug 15 10:47:16 2003
+++ b/include/linux/usb.h Fri Aug 15 10:47:16 2003
@@ -869,6 +869,7 @@
/* 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);
diff -Nru a/sound/usb/usbaudio.c b/sound/usb/usbaudio.c
--- a/sound/usb/usbaudio.c Fri Aug 15 10:47:16 2003
+++ b/sound/usb/usbaudio.c Fri Aug 15 10:47:16 2003
@@ -2560,8 +2560,8 @@
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 @@
* 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++)
-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
