usbcore: make usb_generic a usb_device_driver

This patch (as714b) makes usb_generic into a usb_device_driver capable
of being probed and unbound, just like other drivers.  A fair amount of
the work that used to get done during discovery or removal of a USB
device have been moved to the probe and disconnect methods of
usb_generic: creating the sysfs attributes and selecting an initial
configuration.  However the normal behavior should continue to be the
same as before.

We will now have the possibility of creating other USB device drivers,
They will assist with exporting devices to remote systems
(USB-over-TCPIP) or to paravirtual guest operating systems.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/usb/core/driver.c

@@ -530,9 +530,10 @@ static int usb_uevent(struct device *dev, char **envp, int num_envp,
 	/* driver is often null here; dev_dbg() would oops */
 	pr_debug ("usb %s: uevent\n", dev->bus_id);
 
-	if (is_usb_device(dev))
-		return 0;
-	else {
+	if (is_usb_device(dev)) {
+		usb_dev = to_usb_device(dev);
+		alt = NULL;
+	} else {
 		intf = to_usb_interface(dev);
 		usb_dev = interface_to_usbdev(intf);
 		alt = intf->cur_altsetting;
@@ -579,15 +580,17 @@ static int usb_uevent(struct device *dev, char **envp, int num_envp,
 			   usb_dev->descriptor.bDeviceProtocol))
 		return -ENOMEM;
 
-	if (add_uevent_var(envp, num_envp, &i,
+	if (!is_usb_device(dev)) {
+
+		if (add_uevent_var(envp, num_envp, &i,
 			   buffer, buffer_size, &length,
 			   "INTERFACE=%d/%d/%d",
 			   alt->desc.bInterfaceClass,
 			   alt->desc.bInterfaceSubClass,
 			   alt->desc.bInterfaceProtocol))
-		return -ENOMEM;
+			return -ENOMEM;
 
-	if (add_uevent_var(envp, num_envp, &i,
+		if (add_uevent_var(envp, num_envp, &i,
 			   buffer, buffer_size, &length,
 			   "MODALIAS=usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic%02Xisc%02Xip%02X",
 			   le16_to_cpu(usb_dev->descriptor.idVendor),
@@ -599,7 +602,8 @@ static int usb_uevent(struct device *dev, char **envp, int num_envp,
 			   alt->desc.bInterfaceClass,
 			   alt->desc.bInterfaceSubClass,
 			   alt->desc.bInterfaceProtocol))
-		return -ENOMEM;
+			return -ENOMEM;
+	}
 
 	envp[i] = NULL;
 
@@ -747,31 +751,22 @@ EXPORT_SYMBOL_GPL_FUTURE(usb_deregister);
 
 #ifdef CONFIG_PM
 
-static int verify_suspended(struct device *dev, void *unused)
+static int usb_suspend(struct device *dev, pm_message_t message)
 {
-	if (dev->driver == NULL)
-		return 0;
-	return (dev->power.power_state.event == PM_EVENT_ON) ? -EBUSY : 0;
-}
+	struct usb_device		*udev;
+	struct usb_device_driver	*udriver;
+	struct usb_interface		*intf;
+	struct usb_driver		*driver;
+	int				status;
 
-static int usb_generic_suspend(struct device *dev, pm_message_t message)
-{
-	struct usb_interface	*intf;
-	struct usb_driver	*driver;
-	int			status;
-
-	/* USB devices enter SUSPEND state through their hubs, but can be
-	 * marked for FREEZE as soon as their children are already idled.
-	 * But those semantics are useless, so we equate the two (sigh).
-	 */
 	if (is_usb_device(dev)) {
+		if (dev->driver == NULL)
+			return 0;
+		udev = to_usb_device(dev);
+		udriver = to_usb_device_driver(dev->driver);
 		if (dev->power.power_state.event == message.event)
 			return 0;
-		/* we need to rule out bogus requests through sysfs */
-		status = device_for_each_child(dev, NULL, verify_suspended);
-		if (status)
-			return status;
- 		return usb_port_suspend(to_usb_device(dev));
+		return udriver->suspend(udev, message);
 	}
 
 	if (dev->driver == NULL)
@@ -799,12 +794,13 @@ static int usb_generic_suspend(struct device *dev, pm_message_t message)
 	return status;
 }
 
-static int usb_generic_resume(struct device *dev)
+static int usb_resume(struct device *dev)
 {
-	struct usb_interface	*intf;
-	struct usb_driver	*driver;
-	struct usb_device	*udev;
-	int			status;
+	struct usb_device		*udev;
+	struct usb_device_driver	*udriver;
+	struct usb_interface		*intf;
+	struct usb_driver		*driver;
+	int				status;
 
 	if (dev->power.power_state.event == PM_EVENT_ON)
 		return 0;
@@ -814,10 +810,13 @@ static int usb_generic_resume(struct device *dev)
 
 	/* devices resume through their hubs */
 	if (is_usb_device(dev)) {
+		if (dev->driver == NULL)
+			return 0;
 		udev = to_usb_device(dev);
+		udriver = to_usb_device_driver(dev->driver);
 		if (udev->state == USB_STATE_NOTATTACHED)
 			return 0;
-		return usb_port_resume(udev);
+		return udriver->resume(udev);
 	}
 
 	if (dev->driver == NULL) {
@@ -854,7 +853,7 @@ struct bus_type usb_bus_type = {
 	.match =	usb_device_match,
 	.uevent =	usb_uevent,
 #ifdef CONFIG_PM
-	.suspend =	usb_generic_suspend,
-	.resume =	usb_generic_resume,
+	.suspend =	usb_suspend,
+	.resume =	usb_resume,
 #endif
 };

drivers/usb/core/generic.c

@@ -21,24 +21,208 @@
 #include <linux/usb.h>
 #include "usb.h"
 
+static inline const char *plural(int n)
+{
+	return (n == 1 ? "" : "s");
+}
+
+static int choose_configuration(struct usb_device *udev)
+{
+	int i;
+	int num_configs;
+	int insufficient_power = 0;
+	struct usb_host_config *c, *best;
+
+	best = NULL;
+	c = udev->config;
+	num_configs = udev->descriptor.bNumConfigurations;
+	for (i = 0; i < num_configs; (i++, c++)) {
+		struct usb_interface_descriptor	*desc = NULL;
+
+		/* It's possible that a config has no interfaces! */
+		if (c->desc.bNumInterfaces > 0)
+			desc = &c->intf_cache[0]->altsetting->desc;
+
+		/*
+		 * HP's USB bus-powered keyboard has only one configuration
+		 * and it claims to be self-powered; other devices may have
+		 * similar errors in their descriptors.  If the next test
+		 * were allowed to execute, such configurations would always
+		 * be rejected and the devices would not work as expected.
+		 * In the meantime, we run the risk of selecting a config
+		 * that requires external power at a time when that power
+		 * isn't available.  It seems to be the lesser of two evils.
+		 *
+		 * Bugzilla #6448 reports a device that appears to crash
+		 * when it receives a GET_DEVICE_STATUS request!  We don't
+		 * have any other way to tell whether a device is self-powered,
+		 * but since we don't use that information anywhere but here,
+		 * the call has been removed.
+		 *
+		 * Maybe the GET_DEVICE_STATUS call and the test below can
+		 * be reinstated when device firmwares become more reliable.
+		 * Don't hold your breath.
+		 */
+#if 0
+		/* Rule out self-powered configs for a bus-powered device */
+		if (bus_powered && (c->desc.bmAttributes &
+					USB_CONFIG_ATT_SELFPOWER))
+			continue;
+#endif
+
+		/*
+		 * The next test may not be as effective as it should be.
+		 * Some hubs have errors in their descriptor, claiming
+		 * to be self-powered when they are really bus-powered.
+		 * We will overestimate the amount of current such hubs
+		 * make available for each port.
+		 *
+		 * This is a fairly benign sort of failure.  It won't
+		 * cause us to reject configurations that we should have
+		 * accepted.
+		 */
+
+		/* Rule out configs that draw too much bus current */
+		if (c->desc.bMaxPower * 2 > udev->bus_mA) {
+			insufficient_power++;
+			continue;
+		}
+
+		/* If the first config's first interface is COMM/2/0xff
+		 * (MSFT RNDIS), rule it out unless Linux has host-side
+		 * RNDIS support. */
+		if (i == 0 && desc
+				&& desc->bInterfaceClass == USB_CLASS_COMM
+				&& desc->bInterfaceSubClass == 2
+				&& desc->bInterfaceProtocol == 0xff) {
+#ifndef CONFIG_USB_NET_RNDIS_HOST
+			continue;
+#else
+			best = c;
+#endif
+		}
+
+		/* From the remaining configs, choose the first one whose
+		 * first interface is for a non-vendor-specific class.
+		 * Reason: Linux is more likely to have a class driver
+		 * than a vendor-specific driver. */
+		else if (udev->descriptor.bDeviceClass !=
+						USB_CLASS_VENDOR_SPEC &&
+				(!desc || desc->bInterfaceClass !=
+						USB_CLASS_VENDOR_SPEC)) {
+			best = c;
+			break;
+		}
+
+		/* If all the remaining configs are vendor-specific,
+		 * choose the first one. */
+		else if (!best)
+			best = c;
+	}
+
+	if (insufficient_power > 0)
+		dev_info(&udev->dev, "rejected %d configuration%s "
+			"due to insufficient available bus power\n",
+			insufficient_power, plural(insufficient_power));
+
+	if (best) {
+		i = best->desc.bConfigurationValue;
+		dev_info(&udev->dev,
+			"configuration #%d chosen from %d choice%s\n",
+			i, num_configs, plural(num_configs));
+	} else {
+		i = -1;
+		dev_warn(&udev->dev,
+			"no configuration chosen from %d choice%s\n",
+			num_configs, plural(num_configs));
+	}
+	return i;
+}
+
 static int generic_probe(struct usb_device *udev)
 {
+	int err, c;
+
+	/* put device-specific files into sysfs */
+	usb_create_sysfs_dev_files(udev);
+
+	/* Choose and set the configuration.  This registers the interfaces
+	 * with the driver core and lets interface drivers bind to them.
+	 */
+	c = choose_configuration(udev);
+	if (c >= 0) {
+		err = usb_set_configuration(udev, c);
+		if (err) {
+			dev_err(&udev->dev, "can't set config #%d, error %d\n",
+					c, err);
+			/* This need not be fatal.  The user can try to
+			 * set other configurations. */
+		}
+	}
+
+	/* USB device state == configured ... usable */
+	usb_notify_add_device(udev);
+
 	return 0;
 }
+
 static void generic_disconnect(struct usb_device *udev)
 {
+	usb_notify_remove_device(udev);
+
 	/* if this is only an unbind, not a physical disconnect, then
 	 * unconfigure the device */
 	if (udev->state == USB_STATE_CONFIGURED)
 		usb_set_configuration(udev, 0);
 
+	usb_remove_sysfs_dev_files(udev);
+
 	/* in case the call failed or the device was suspended */
 	if (udev->state >= USB_STATE_CONFIGURED)
 		usb_disable_device(udev, 0);
 }
 
+#ifdef	CONFIG_PM
+
+static int verify_suspended(struct device *dev, void *unused)
+{
+	if (dev->driver == NULL)
+		return 0;
+	return (dev->power.power_state.event == PM_EVENT_ON) ? -EBUSY : 0;
+}
+
+static int generic_suspend(struct usb_device *udev, pm_message_t msg)
+{
+	int	status;
+
+	/* rule out bogus requests through sysfs */
+	status = device_for_each_child(&udev->dev, NULL, verify_suspended);
+	if (status)
+		return status;
+
+	/* USB devices enter SUSPEND state through their hubs, but can be
+	 * marked for FREEZE as soon as their children are already idled.
+	 * But those semantics are useless, so we equate the two (sigh).
+	 */
+	return usb_port_suspend(udev);
+}
+
+static int generic_resume(struct usb_device *udev)
+{
+	if (udev->state == USB_STATE_NOTATTACHED)
+		return 0;
+
+	return usb_port_resume(udev);
+}
+
+#endif	/* CONFIG_PM */
+
 struct usb_device_driver usb_generic_driver = {
 	.name =	"usb",
 	.probe = generic_probe,
 	.disconnect = generic_disconnect,
+#ifdef	CONFIG_PM
+	.suspend = generic_suspend,
+	.resume = generic_resume,
+#endif
 };

drivers/usb/core/hub.c

@@ -1148,144 +1148,28 @@ void usb_disconnect(struct usb_device **pdev)
 	 * cleaning up all state associated with the current configuration
 	 * so that the hardware is now fully quiesced.
 	 */
+	dev_dbg (&udev->dev, "unregistering device\n");
 	usb_disable_device(udev, 0);
 
-	usb_notify_remove_device(udev);
+	usb_unlock_device(udev);
+
+	/* Unregister the device.  The device driver is responsible
+	 * for removing the device files from usbfs and sysfs and for
+	 * de-configuring the device.
+	 */
+	device_del(&udev->dev);
 
-	/* Free the device number, remove the /proc/bus/usb entry and
-	 * the sysfs attributes, and delete the parent's children[]
+	/* Free the device number and delete the parent's children[]
 	 * (or root_hub) pointer.
 	 */
-	dev_dbg (&udev->dev, "unregistering device\n");
 	release_address(udev);
-	usb_remove_sysfs_dev_files(udev);
 
 	/* Avoid races with recursively_mark_NOTATTACHED() */
 	spin_lock_irq(&device_state_lock);
 	*pdev = NULL;
 	spin_unlock_irq(&device_state_lock);
 
-	usb_unlock_device(udev);
-
-	device_unregister(&udev->dev);
-}
-
-static inline const char *plural(int n)
-{
-	return (n == 1 ? "" : "s");
-}
-
-static int choose_configuration(struct usb_device *udev)
-{
-	int i;
-	int num_configs;
-	int insufficient_power = 0;
-	struct usb_host_config *c, *best;
-
-	best = NULL;
-	c = udev->config;
-	num_configs = udev->descriptor.bNumConfigurations;
-	for (i = 0; i < num_configs; (i++, c++)) {
-		struct usb_interface_descriptor	*desc = NULL;
-
-		/* It's possible that a config has no interfaces! */
-		if (c->desc.bNumInterfaces > 0)
-			desc = &c->intf_cache[0]->altsetting->desc;
-
-		/*
-		 * HP's USB bus-powered keyboard has only one configuration
-		 * and it claims to be self-powered; other devices may have
-		 * similar errors in their descriptors.  If the next test
-		 * were allowed to execute, such configurations would always
-		 * be rejected and the devices would not work as expected.
-		 * In the meantime, we run the risk of selecting a config
-		 * that requires external power at a time when that power
-		 * isn't available.  It seems to be the lesser of two evils.
-		 *
-		 * Bugzilla #6448 reports a device that appears to crash
-		 * when it receives a GET_DEVICE_STATUS request!  We don't
-		 * have any other way to tell whether a device is self-powered,
-		 * but since we don't use that information anywhere but here,
-		 * the call has been removed.
-		 *
-		 * Maybe the GET_DEVICE_STATUS call and the test below can
-		 * be reinstated when device firmwares become more reliable.
-		 * Don't hold your breath.
-		 */
-#if 0
-		/* Rule out self-powered configs for a bus-powered device */
-		if (bus_powered && (c->desc.bmAttributes &
-					USB_CONFIG_ATT_SELFPOWER))
-			continue;
-#endif
-
-		/*
-		 * The next test may not be as effective as it should be.
-		 * Some hubs have errors in their descriptor, claiming
-		 * to be self-powered when they are really bus-powered.
-		 * We will overestimate the amount of current such hubs
-		 * make available for each port.
-		 *
-		 * This is a fairly benign sort of failure.  It won't
-		 * cause us to reject configurations that we should have
-		 * accepted.
-		 */
-
-		/* Rule out configs that draw too much bus current */
-		if (c->desc.bMaxPower * 2 > udev->bus_mA) {
-			insufficient_power++;
-			continue;
-		}
-
-		/* If the first config's first interface is COMM/2/0xff
-		 * (MSFT RNDIS), rule it out unless Linux has host-side
-		 * RNDIS support. */
-		if (i == 0 && desc
-				&& desc->bInterfaceClass == USB_CLASS_COMM
-				&& desc->bInterfaceSubClass == 2
-				&& desc->bInterfaceProtocol == 0xff) {
-#ifndef CONFIG_USB_NET_RNDIS_HOST
-			continue;
-#else
-			best = c;
-#endif
-		}
-
-		/* From the remaining configs, choose the first one whose
-		 * first interface is for a non-vendor-specific class.
-		 * Reason: Linux is more likely to have a class driver
-		 * than a vendor-specific driver. */
-		else if (udev->descriptor.bDeviceClass !=
-						USB_CLASS_VENDOR_SPEC &&
-				(!desc || desc->bInterfaceClass !=
-						USB_CLASS_VENDOR_SPEC)) {
-			best = c;
-			break;
-		}
-
-		/* If all the remaining configs are vendor-specific,
-		 * choose the first one. */
-		else if (!best)
-			best = c;
-	}
-
-	if (insufficient_power > 0)
-		dev_info(&udev->dev, "rejected %d configuration%s "
-			"due to insufficient available bus power\n",
-			insufficient_power, plural(insufficient_power));
-
-	if (best) {
-		i = best->desc.bConfigurationValue;
-		dev_info(&udev->dev,
-			"configuration #%d chosen from %d choice%s\n",
-			i, num_configs, plural(num_configs));
-	} else {
-		i = -1;
-		dev_warn(&udev->dev,
-			"no configuration chosen from %d choice%s\n",
-			num_configs, plural(num_configs));
-	}
-	return i;
+	put_device(&udev->dev);
 }
 
 #ifdef DEBUG
@@ -1328,7 +1212,6 @@ static inline void show_string(struct usb_device *udev, char *id, char *string)
 int usb_new_device(struct usb_device *udev)
 {
 	int err;
-	int c;
 
 	err = usb_get_configuration(udev);
 	if (err < 0) {
@@ -1418,34 +1301,15 @@ int usb_new_device(struct usb_device *udev)
 	}
 #endif
 
-	/* put device-specific files into sysfs */
+	/* Register the device.  The device driver is responsible
+	 * for adding the device files to usbfs and sysfs and for
+	 * configuring the device.
+	 */
 	err = device_add (&udev->dev);
 	if (err) {
 		dev_err(&udev->dev, "can't device_add, error %d\n", err);
 		goto fail;
 	}
-	usb_create_sysfs_dev_files (udev);
-
-	usb_lock_device(udev);
-
-	/* choose and set the configuration. that registers the interfaces
-	 * with the driver core, and lets usb device drivers bind to them.
-	 */
-	c = choose_configuration(udev);
-	if (c >= 0) {
-		err = usb_set_configuration(udev, c);
-		if (err) {
-			dev_err(&udev->dev, "can't set config #%d, error %d\n",
-					c, err);
-			/* This need not be fatal.  The user can try to
-			 * set other configurations. */
-		}
-	}
-
-	/* USB device state == configured ... usable */
-	usb_notify_add_device(udev);
-
-	usb_unlock_device(udev);
 
 	return 0;
 

drivers/usb/core/usb.c

@@ -205,7 +205,6 @@ usb_alloc_dev(struct usb_device *parent, struct usb_bus *bus, unsigned port1)
 	device_initialize(&dev->dev);
 	dev->dev.bus = &usb_bus_type;
 	dev->dev.dma_mask = bus->controller->dma_mask;
-	dev->dev.driver = &usb_generic_driver.drvwrap.driver;
 	dev->dev.release = usb_release_dev;
 	dev->state = USB_STATE_ATTACHED;