[PATCH] USB: re-factor enumeration logic

This is an update to some patches from the December/January
timeframe, which will help sort out some of the mess for
drivers that need to use the reset logic.  It's one of the
last significant patches in my gadget-2.6 tree that haven't
yet been merged into the main kernel tree.


More refactoring of the enumeration code paths:

 * The first half of usb_new_device() becomes the second half of a new
   hub_port_init() routine (resets, sets address, gets descriptor)

 * The middle chunk of hub_port_connect_change() becomes the first half
   of that new hub_port_init() routine.

 * Khubd uses that new routine in hub_port_connect_change().

 * Now usb_new_device() cleans up better after faults, and has
   a more useful locking policy (caller owns dev->serialize).

 * Has related minor cleanups including commenting some of
   the curious request sequences coming from khubd.

Refactoring means a lot of the current usb_reset_device() logic won't
need to stay an imperfect clone of the enumeration code ... soon, it
can just call hub_port_init().

Even without touching usb_reset_device(), this eliminates a deadlock.
Previously, address0_sem was used both during probe and during reset,
so probe routines can't implement DFU firmware download (involves a
reset; DFU also uncovers other problems) or safely recover from probe
faults by resetting (usb-storage can try that).  Now that lock is no
longer held during probe(); so those deadlocks are gone.  (And some
drivers, like at76c503, can start to remove ugly workarounds.)

drivers/usb/core/hcd.c

@@ -779,10 +779,22 @@ int usb_register_root_hub (struct usb_device *usb_dev, struct device *parent_dev
 	set_bit (devnum, usb_dev->bus->devmap.devicemap);
 	usb_dev->state = USB_STATE_ADDRESS;
 
+	usb_dev->epmaxpacketin[0] = usb_dev->epmaxpacketout[0] = 64;
+	retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE);
+	if (retval != sizeof usb_dev->descriptor) {
+		dev_dbg (parent_dev, "can't read %s device descriptor %d\n",
+				usb_dev->dev.bus_id, retval);
+		return (retval < 0) ? retval : -EMSGSIZE;
+	}
+
+	(void) usb_get_dev (usb_dev);
+	down (&usb_dev->serialize);
 	retval = usb_new_device (usb_dev);
 	if (retval)
 		dev_err (parent_dev, "can't register root hub for %s, %d\n",
 				usb_dev->dev.bus_id, retval);
+	up (&usb_dev->serialize);
+	usb_put_dev (usb_dev);
 	return retval;
 }
 EXPORT_SYMBOL (usb_register_root_hub);

drivers/usb/core/usb.c

@@ -1064,92 +1064,29 @@ static inline void usb_show_string(struct usb_device *dev, char *id, int index)
 }
 
 /*
- * By the time we get here, we chose a new device address
- * and is in the default state. We need to identify the thing and
- * get the ball rolling..
+ * usb_new_device - perform initial device setup (usbcore-internal)
+ * @dev: newly addressed device (in ADDRESS state)
  *
- * Returns 0 for success, != 0 for error.
+ * This is called with devices which have been enumerated, but not yet
+ * configured.  The device descriptor is available, but not descriptors
+ * for any device configuration.  The caller owns dev->serialize, and
+ * the device is not visible through sysfs or other filesystem code.
+ *
+ * Returns 0 for success (device is configured and listed, with its
+ * interfaces, in sysfs); else a negative errno value.  On error, one
+ * reference count to the device has been dropped.
  *
  * This call is synchronous, and may not be used in an interrupt context.
  *
  * Only the hub driver should ever call this; root hub registration
  * uses it only indirectly.
  */
-#define NEW_DEVICE_RETRYS	2
-#define SET_ADDRESS_RETRYS	2
 int usb_new_device(struct usb_device *dev)
 {
-	int err = -EINVAL;
+	int err;
 	int i;
-	int j;
 	int config;
 
-	/* USB 2.0 section 5.5.3 talks about ep0 maxpacket ...
-	 * it's fixed size except for full speed devices.
-	 */
-	switch (dev->speed) {
-	case USB_SPEED_HIGH:		/* fixed at 64 */
-		i = 64;
-		break;
-	case USB_SPEED_FULL:		/* 8, 16, 32, or 64 */
-		/* to determine the ep0 maxpacket size, read the first 8
-		 * bytes from the device descriptor to get bMaxPacketSize0;
-		 * then correct our initial (small) guess.
-		 */
-		// FALLTHROUGH
-	case USB_SPEED_LOW:		/* fixed at 8 */
-		i = 8;
-		break;
-	default:
-		goto fail;
-	}
-	dev->epmaxpacketin [0] = i;
-	dev->epmaxpacketout[0] = i;
-
-	for (i = 0; i < NEW_DEVICE_RETRYS; ++i) {
-
-		for (j = 0; j < SET_ADDRESS_RETRYS; ++j) {
-			err = usb_set_address(dev);
-			if (err >= 0)
-				break;
-			wait_ms(200);
-		}
-		if (err < 0) {
-			dev_err(&dev->dev,
-				"device not accepting address %d, error %d\n",
-				dev->devnum, err);
-			goto fail;
-		}
-
-		wait_ms(10);	/* Let the SET_ADDRESS settle */
-
-		/* high and low speed devices don't need this... */
-		err = usb_get_device_descriptor(dev, 8);
-		if (err >= 8)
-			break;
-		wait_ms(100);
-	}
-
-	if (err < 8) {
-		dev_err(&dev->dev, "device descriptor read/8, error %d\n", err);
-		goto fail;
-	}
-	if (dev->speed == USB_SPEED_FULL) {
-		usb_disable_endpoint(dev, 0);
-		usb_endpoint_running(dev, 0, 1);
-		usb_endpoint_running(dev, 0, 0);
-		dev->epmaxpacketin [0] = dev->descriptor.bMaxPacketSize0;
-		dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;
-	}
-
-	/* USB device state == addressed ... still not usable */
-
-	err = usb_get_device_descriptor(dev, sizeof(dev->descriptor));
-	if (err != (signed)sizeof(dev->descriptor)) {
-		dev_err(&dev->dev, "device descriptor read/all, error %d\n", err);
-		goto fail;
-	}
-
 	err = usb_get_configuration(dev);
 	if (err < 0) {
 		dev_err(&dev->dev, "can't read configurations, error %d\n",
@@ -1170,13 +1107,10 @@ int usb_new_device(struct usb_device *dev)
 		usb_show_string(dev, "SerialNumber", dev->descriptor.iSerialNumber);
 #endif
 
-	down(&dev->serialize);
-
 	/* put device-specific files into sysfs */
 	err = device_add (&dev->dev);
 	if (err) {
 		dev_err(&dev->dev, "can't device_add, error %d\n", err);
-		up(&dev->serialize);
 		goto fail;
 	}
 	usb_create_driverfs_dev_files (dev);
@@ -1211,7 +1145,6 @@ int usb_new_device(struct usb_device *dev)
 			dev->descriptor.bNumConfigurations);
 	}
 	err = usb_set_configuration(dev, config);
-	up(&dev->serialize);
 	if (err) {
 		dev_err(&dev->dev, "can't set config #%d, error %d\n",
 			config, err);
@@ -1226,9 +1159,10 @@ int usb_new_device(struct usb_device *dev)
 
 	return 0;
 fail:
-	dev->state = USB_STATE_DEFAULT;
+	dev->state = USB_STATE_NOTATTACHED;
 	clear_bit(dev->devnum, dev->bus->devmap.devicemap);
 	dev->devnum = -1;
+	usb_put_dev(dev);
 	return err;
 }