On Thu, 1 Nov 2012, Sarah Sharp wrote:
> On Wed, Oct 31, 2012 at 03:54:49PM -0400, Alan Stern wrote:
> > + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> > + USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
> > + NULL, 0, USB_CTRL_SET_TIMEOUT);
> > + if (ret < 0 && cp) {
> > + /*
> > + * All the old state is gone, so what else can we do?
> > + * The device is probably useless now anyway.
> > + */
> > + for (i = 0; i < nintf; ++i) {
> > + usb_disable_interface(dev, cp->interface[i], true);
>
> usb_disable_interface() will set the udev->ep_out and udev->ep_in array
> entries to NULL. The bandwidth functions need those arrays to figure
> out which entries to disable.
You're right. I should have realized that. :-(
> Is there a particular reason why you wanted to call
> usb_disable_interface() before calling usb_hcd_alloc_bandwidth()?
Sheer oversight. Some days I am dumber than others...
> And I do agree that this patch is much cleaner than my patch.
Okay, Matthias, here's an updated version of that patch. The only
difference is that it puts the usb_hcd_alloc_bandwidth() call _before_
the usb_disable_interface() calls.
Alan Stern
Index: usb-3.7/drivers/usb/core/message.c
===================================================================
--- usb-3.7.orig/drivers/usb/core/message.c
+++ usb-3.7/drivers/usb/core/message.c
@@ -1806,29 +1806,8 @@ free_interfaces:
goto free_interfaces;
}
- ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
- USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
- NULL, 0, USB_CTRL_SET_TIMEOUT);
- if (ret < 0) {
- /* All the old state is gone, so what else can we do?
- * The device is probably useless now anyway.
- */
- cp = NULL;
- }
-
- dev->actconfig = cp;
- if (!cp) {
- usb_set_device_state(dev, USB_STATE_ADDRESS);
- usb_hcd_alloc_bandwidth(dev, NULL, NULL, NULL);
- /* Leave LPM disabled while the device is unconfigured. */
- mutex_unlock(hcd->bandwidth_mutex);
- usb_autosuspend_device(dev);
- goto free_interfaces;
- }
- mutex_unlock(hcd->bandwidth_mutex);
- usb_set_device_state(dev, USB_STATE_CONFIGURED);
-
- /* Initialize the new interface structures and the
+ /*
+ * Initialize the new interface structures and the
* hc/hcd/usbcore interface/endpoint state.
*/
for (i = 0; i < nintf; ++i) {
@@ -1872,6 +1851,35 @@ free_interfaces:
}
kfree(new_interfaces);
+ ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+ USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
+ NULL, 0, USB_CTRL_SET_TIMEOUT);
+ if (ret < 0 && cp) {
+ /*
+ * All the old state is gone, so what else can we do?
+ * The device is probably useless now anyway.
+ */
+ usb_hcd_alloc_bandwidth(dev, NULL, NULL, NULL);
+ for (i = 0; i < nintf; ++i) {
+ usb_disable_interface(dev, cp->interface[i], true);
+ put_device(&cp->interface[i]->dev);
+ cp->interface[i] = NULL;
+ }
+ cp = NULL;
+ }
+
+ dev->actconfig = cp;
+ mutex_unlock(hcd->bandwidth_mutex);
+
+ if (!cp) {
+ usb_set_device_state(dev, USB_STATE_ADDRESS);
+
+ /* Leave LPM disabled while the device is unconfigured. */
+ usb_autosuspend_device(dev);
+ return ret;
+ }
+ usb_set_device_state(dev, USB_STATE_CONFIGURED);
+
if (cp->string == NULL &&
!(dev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS))
cp->string = usb_cache_string(dev, cp->desc.iConfiguration);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
