Greg:

This patch (as703) improves the error handling when a Set-Configuration 
request fails.  The old interfaces are all unregistered before the request 
is sent, and if the request fails then we don't know what config the 
device is using.  So it makes no sense to leave actconfig pointing to the 
old configuration with its invalid interfaces.

Alan Stern



Signed-off-by: Alan Stern <[EMAIL PROTECTED]>

---

The actual changes made in the patch are much smaller than they appear, 
because a large section of code was unindented by one level.


Index: usb-2.6/drivers/usb/core/message.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/message.c
+++ usb-2.6/drivers/usb/core/message.c
@@ -1411,15 +1411,7 @@ free_interfaces:
                                return ret;
                        }
                }
-       }
-
-       /* if it's already configured, clear out old state first.
-        * getting rid of old interfaces means unbinding their drivers.
-        */
-       if (dev->state != USB_STATE_ADDRESS)
-               usb_disable_device (dev, 1);    // Skip ep0
 
-       if (cp) {
                i = dev->bus_mA - cp->desc.bMaxPower * 2;
                if (i < 0)
                        dev_warn(&dev->dev, "new config #%d exceeds power "
@@ -1427,84 +1419,91 @@ free_interfaces:
                                        configuration, -i);
        }
 
+       /* if it's already configured, clear out old state first.
+        * getting rid of old interfaces means unbinding their drivers.
+        */
+       if (dev->state != USB_STATE_ADDRESS)
+               usb_disable_device (dev, 1);    // Skip ep0
+
        if ((ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
                        USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
-                       NULL, 0, USB_CTRL_SET_TIMEOUT)) < 0)
-               goto free_interfaces;
+                       NULL, 0, USB_CTRL_SET_TIMEOUT)) < 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)
+       if (!cp) {
                usb_set_device_state(dev, USB_STATE_ADDRESS);
-       else {
-               usb_set_device_state(dev, USB_STATE_CONFIGURED);
+               goto free_interfaces;
+       }
+       usb_set_device_state(dev, USB_STATE_CONFIGURED);
 
-               /* Initialize the new interface structures and the
-                * hc/hcd/usbcore interface/endpoint state.
+       /* Initialize the new interface structures and the
+        * hc/hcd/usbcore interface/endpoint state.
+        */
+       for (i = 0; i < nintf; ++i) {
+               struct usb_interface_cache *intfc;
+               struct usb_interface *intf;
+               struct usb_host_interface *alt;
+
+               cp->interface[i] = intf = new_interfaces[i];
+               intfc = cp->intf_cache[i];
+               intf->altsetting = intfc->altsetting;
+               intf->num_altsetting = intfc->num_altsetting;
+               kref_get(&intfc->ref);
+
+               alt = usb_altnum_to_altsetting(intf, 0);
+
+               /* No altsetting 0?  We'll assume the first altsetting.
+                * We could use a GetInterface call, but if a device is
+                * so non-compliant that it doesn't have altsetting 0
+                * then I wouldn't trust its reply anyway.
                 */
-               for (i = 0; i < nintf; ++i) {
-                       struct usb_interface_cache *intfc;
-                       struct usb_interface *intf;
-                       struct usb_host_interface *alt;
-
-                       cp->interface[i] = intf = new_interfaces[i];
-                       intfc = cp->intf_cache[i];
-                       intf->altsetting = intfc->altsetting;
-                       intf->num_altsetting = intfc->num_altsetting;
-                       kref_get(&intfc->ref);
-
-                       alt = usb_altnum_to_altsetting(intf, 0);
-
-                       /* No altsetting 0?  We'll assume the first altsetting.
-                        * We could use a GetInterface call, but if a device is
-                        * so non-compliant that it doesn't have altsetting 0
-                        * then I wouldn't trust its reply anyway.
-                        */
-                       if (!alt)
-                               alt = &intf->altsetting[0];
-
-                       intf->cur_altsetting = alt;
-                       usb_enable_interface(dev, intf);
-                       intf->dev.parent = &dev->dev;
-                       intf->dev.driver = NULL;
-                       intf->dev.bus = &usb_bus_type;
-                       intf->dev.dma_mask = dev->dev.dma_mask;
-                       intf->dev.release = release_interface;
-                       device_initialize (&intf->dev);
-                       mark_quiesced(intf);
-                       sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",
-                                dev->bus->busnum, dev->devpath,
-                                configuration,
-                                alt->desc.bInterfaceNumber);
-               }
-               kfree(new_interfaces);
+               if (!alt)
+                       alt = &intf->altsetting[0];
 
-               if (cp->string == NULL)
-                       cp->string = usb_cache_string(dev,
-                                       cp->desc.iConfiguration);
-
-               /* Now that all the interfaces are set up, register them
-                * to trigger binding of drivers to interfaces.  probe()
-                * routines may install different altsettings and may
-                * claim() any interfaces not yet bound.  Many class drivers
-                * need that: CDC, audio, video, etc.
-                */
-               for (i = 0; i < nintf; ++i) {
-                       struct usb_interface *intf = cp->interface[i];
+               intf->cur_altsetting = alt;
+               usb_enable_interface(dev, intf);
+               intf->dev.parent = &dev->dev;
+               intf->dev.driver = NULL;
+               intf->dev.bus = &usb_bus_type;
+               intf->dev.dma_mask = dev->dev.dma_mask;
+               intf->dev.release = release_interface;
+               device_initialize (&intf->dev);
+               mark_quiesced(intf);
+               sprintf (&intf->dev.bus_id[0], "%d-%s:%d.%d",
+                        dev->bus->busnum, dev->devpath,
+                        configuration, alt->desc.bInterfaceNumber);
+       }
+       kfree(new_interfaces);
 
-                       dev_dbg (&dev->dev,
-                               "adding %s (config #%d, interface %d)\n",
-                               intf->dev.bus_id, configuration,
-                               intf->cur_altsetting->desc.bInterfaceNumber);
-                       ret = device_add (&intf->dev);
-                       if (ret != 0) {
-                               dev_err(&dev->dev,
-                                       "device_add(%s) --> %d\n",
-                                       intf->dev.bus_id,
-                                       ret);
-                               continue;
-                       }
-                       usb_create_sysfs_intf_files (intf);
+       if (cp->string == NULL)
+               cp->string = usb_cache_string(dev, cp->desc.iConfiguration);
+
+       /* Now that all the interfaces are set up, register them
+        * to trigger binding of drivers to interfaces.  probe()
+        * routines may install different altsettings and may
+        * claim() any interfaces not yet bound.  Many class drivers
+        * need that: CDC, audio, video, etc.
+        */
+       for (i = 0; i < nintf; ++i) {
+               struct usb_interface *intf = cp->interface[i];
+
+               dev_dbg (&dev->dev,
+                       "adding %s (config #%d, interface %d)\n",
+                       intf->dev.bus_id, configuration,
+                       intf->cur_altsetting->desc.bInterfaceNumber);
+               ret = device_add (&intf->dev);
+               if (ret != 0) {
+                       dev_err(&dev->dev, "device_add(%s) --> %d\n",
+                               intf->dev.bus_id, ret);
+                       continue;
                }
+               usb_create_sysfs_intf_files (intf);
        }
 
        return 0;



_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to