Skip to contentSkip to conversation

Add dynamic id support to all USB drivers

RFC PATCH In review Greg KH 4 participants 9 messages Nov 17, 2005 – Nov 18, 2005

Subsystems (2)
lore
Share & tools
lore

Greg KH

author
#0 lore raw

cover Add dynamic id support to all USB drivers

Here's two patches against the latest -git tree that add the ability to add new vendor/product device id pairs to USB drivers after they have been loaded. It works just like the PCI drivers do, with the exception that only vendor/product can be specified (no class stuff.) I did it this way to make it simpler, as that's almost all anyone ever wants to add.

Note that usb-serial drivers do not work properly with these dynamic ids yet, it's going to take some rework of the usb-serial core due to the wierd way it binds drivers to devices to get this to work properly.

Comments?

thanks,

greg k-h

author
#2 lore raw

PATCH 2/2 USB: add dynamic id functionality to USB core

In reply to Greg KH

Echo the usb vendor and product id to the "new_id" file in the driver's sysfs directory, and then that driver will be able to bind to a device with those ids if it is present.

Example:

	echo 0557 2008 > /sys/bus/usb/drivers/foo_driver/new_id

adds the hex values 0557 and 2008 to the device id table for the foo_driver.

Note, usb-serial drivers do not currently work with this capability yet. usb-storage also might have some oddities.

---
 drivers/usb/core/driver.c |  218 +++++++++++++++++++++++++++++++++++-----------
 include/linux/usb.h       |    8 +
 2 files changed, 176 insertions(+), 50 deletions(-)
drivers/usb/core/driver.c +168 -50
@@ -27,6 +27,15 @@
 #include "hcd.h"
 #include "usb.h"
 
+static int usb_match_one_id(struct usb_interface *interface,
+			    const struct usb_device_id *id);
+
+struct usb_dynid {
+	struct list_head node;
+	struct usb_device_id id;
+};
+
+
 static int generic_probe(struct device *dev)
 {
 	return 0;
@@ -58,6 +67,96 @@ struct device_driver usb_generic_driver 
  * usb device or a usb interface. */
 int usb_generic_driver_data;
 
+#ifdef CONFIG_HOTPLUG
+
+/*
+ * Adds a new dynamic USBdevice ID to this driver,
+ * and cause the driver to probe for all devices again.
+ */
+static ssize_t store_new_id(struct device_driver *driver,
+			    const char *buf, size_t count)
+{
+	struct usb_driver *usb_drv = to_usb_driver(driver);
+	struct usb_dynid *dynid;
+	u32 idVendor = 0;
+	u32 idProduct = 0;
+	int fields = 0;
+
+	fields = sscanf(buf, "%x %x", &idVendor, &idProduct);
+	if (fields < 0)
+		return -EINVAL;
+
+	dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
+	if (!dynid)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&dynid->node);
+	dynid->id.idVendor = idVendor;
+	dynid->id.idProduct = idProduct;
+	dynid->id.match_flags = USB_DEVICE_ID_MATCH_DEVICE;
+
+	spin_lock(&usb_drv->dynids.lock);
+	list_add_tail(&usb_drv->dynids.list, &dynid->node);
+	spin_unlock(&usb_drv->dynids.lock);
+
+	if (get_driver(&usb_drv->driver)) {
+		driver_attach(&usb_drv->driver);
+		put_driver(&usb_drv->driver);
+	}
+
+	return count;
+}
+static DRIVER_ATTR(new_id, S_IWUSR, NULL, store_new_id);
+
+static int usb_create_newid_file(struct usb_driver *usb_drv)
+{
+	int error = 0;
+
+	if (usb_drv->probe != NULL)
+		error = sysfs_create_file(&usb_drv->driver.kobj,
+					  &driver_attr_new_id.attr);
+	return error;
+}
+
+static void usb_free_dynids(struct usb_driver *usb_drv)
+{
+	struct usb_dynid *dynid, *n;
+
+	spin_lock(&usb_drv->dynids.lock);
+	list_for_each_entry_safe(dynid, n, &usb_drv->dynids.list, node) {
+		list_del(&dynid->node);
+		kfree(dynid);
+	}
+	spin_unlock(&usb_drv->dynids.lock);
+}
+#else
+static inline int usb_create_newid_file(struct usb_driver *usb_drv)
+{
+	return 0;
+}
+
+static inline void usb_free_dynids(struct usb_driver *usb_drv)
+{
+}
+#endif
+
+static const struct usb_device_id *usb_match_dynamic_id(struct usb_interface *intf,
+							struct usb_driver *drv)
+{
+	struct usb_dynid *dynid;
+
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (usb_match_one_id(intf, &dynid->id)) {
+			spin_unlock(&drv->dynids.lock);
+			return &dynid->id;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+	return NULL;
+}
+
+
 /* called from driver core with usb_bus_type.subsys writelock */
 static int usb_probe_interface(struct device *dev)
 {
@@ -75,6 +174,8 @@ static int usb_probe_interface(struct de
 		return -EHOSTUNREACH;
 
 	id = usb_match_id(intf, driver->id_table);
+	if (!id)
+		id = usb_match_dynamic_id(intf, driver);
 	if (id) {
 		dev_dbg(dev, "%s - got id\n", __FUNCTION__);
 
@@ -120,6 +221,64 @@ static int usb_unbind_interface(struct d
 	return 0;
 }
 
+/* returns 0 if no match, 1 if match */
+static int usb_match_one_id(struct usb_interface *interface,
+			    const struct usb_device_id *id)
+{
+	struct usb_host_interface *intf;
+	struct usb_device *dev;
+
+	/* proc_connectinfo in devio.c may call us with id == NULL. */
+	if (id == NULL)
+		return 0;
+
+	intf = interface->cur_altsetting;
+	dev = interface_to_usbdev(interface);
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
+	    id->idVendor != le16_to_cpu(dev->descriptor.idVendor))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
+	    id->idProduct != le16_to_cpu(dev->descriptor.idProduct))
+		return 0;
+
+	/* No need to test id->bcdDevice_lo != 0, since 0 is never
+	   greater than any unsigned number. */
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
+	    (id->bcdDevice_lo > le16_to_cpu(dev->descriptor.bcdDevice)))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
+	    (id->bcdDevice_hi < le16_to_cpu(dev->descriptor.bcdDevice)))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&
+	    (id->bDeviceClass != dev->descriptor.bDeviceClass))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_SUBCLASS) &&
+	    (id->bDeviceSubClass!= dev->descriptor.bDeviceSubClass))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_PROTOCOL) &&
+	    (id->bDeviceProtocol != dev->descriptor.bDeviceProtocol))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_CLASS) &&
+	    (id->bInterfaceClass != intf->desc.bInterfaceClass))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_SUBCLASS) &&
+	    (id->bInterfaceSubClass != intf->desc.bInterfaceSubClass))
+		return 0;
+
+	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_PROTOCOL) &&
+	    (id->bInterfaceProtocol != intf->desc.bInterfaceProtocol))
+		return 0;
+
+	return 1;
+}
 /**
  * usb_match_id - find first usb_device_id matching device or interface
  * @interface: the interface of interest
@@ -184,16 +343,10 @@ static int usb_unbind_interface(struct d
 const struct usb_device_id *usb_match_id(struct usb_interface *interface,
 					 const struct usb_device_id *id)
 {
-	struct usb_host_interface *intf;
-	struct usb_device *dev;
-
 	/* proc_connectinfo in devio.c may call us with id == NULL. */
 	if (id == NULL)
 		return NULL;
 
-	intf = interface->cur_altsetting;
-	dev = interface_to_usbdev(interface);
-
 	/* It is important to check that id->driver_info is nonzero,
 	   since an entry that is all zeroes except for a nonzero
 	   id->driver_info is the way to create an entry that
@@ -201,50 +354,8 @@ const struct usb_device_id *usb_match_id
 	   device and interface. */
 	for (; id->idVendor || id->bDeviceClass || id->bInterfaceClass ||
 	       id->driver_info; id++) {
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
-		    id->idVendor != le16_to_cpu(dev->descriptor.idVendor))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
-		    id->idProduct != le16_to_cpu(dev->descriptor.idProduct))
-			continue;
-
-		/* No need to test id->bcdDevice_lo != 0, since 0 is never
-		   greater than any unsigned number. */
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
-		    (id->bcdDevice_lo > le16_to_cpu(dev->descriptor.bcdDevice)))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
-		    (id->bcdDevice_hi < le16_to_cpu(dev->descriptor.bcdDevice)))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&
-		    (id->bDeviceClass != dev->descriptor.bDeviceClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_SUBCLASS) &&
-		    (id->bDeviceSubClass!= dev->descriptor.bDeviceSubClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_PROTOCOL) &&
-		    (id->bDeviceProtocol != dev->descriptor.bDeviceProtocol))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_CLASS) &&
-		    (id->bInterfaceClass != intf->desc.bInterfaceClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_SUBCLASS) &&
-		    (id->bInterfaceSubClass != intf->desc.bInterfaceSubClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_PROTOCOL) &&
-		    (id->bInterfaceProtocol != intf->desc.bInterfaceProtocol))
-			continue;
-
-		return id;
+		if (usb_match_one_id(interface, id))
+			return id;
 	}
 
 	return NULL;
@@ -268,6 +379,9 @@ int usb_device_match(struct device *dev,
 	if (id)
 		return 1;
 
+	id = usb_match_dynamic_id(intf, usb_drv);
+	if (id)
+		return 1;
 	return 0;
 }
 
@@ -296,6 +410,8 @@ int usb_register(struct usb_driver *new_
 	new_driver->driver.probe = usb_probe_interface;
 	new_driver->driver.remove = usb_unbind_interface;
 	new_driver->driver.owner = new_driver->owner;
+	spin_lock_init(&new_driver->dynids.lock);
+	INIT_LIST_HEAD(&new_driver->dynids.list);
 
 	usb_lock_all_devices();
 	retval = driver_register(&new_driver->driver);
@@ -305,6 +421,7 @@ int usb_register(struct usb_driver *new_
 		pr_info("%s: registered new driver %s\n",
 			usbcore_name, new_driver->name);
 		usbfs_update_special();
+		usb_create_newid_file(new_driver);
 	} else {
 		printk(KERN_ERR "%s: error %d registering driver %s\n",
 			usbcore_name, retval, new_driver->name);
@@ -330,6 +447,7 @@ void usb_deregister(struct usb_driver *d
 	pr_info("%s: deregistering driver %s\n", usbcore_name, driver->name);
 
 	usb_lock_all_devices();
+	usb_free_dynids(driver);
 	driver_unregister(&driver->driver);
 	usb_unlock_all_devices();
 
include/linux/usb.h +8 -0
@@ -528,6 +528,11 @@ static inline int usb_make_path (struct 
 
 /* ----------------------------------------------------------------------- */
 
+struct usb_dynids {
+	spinlock_t lock;
+	struct list_head list;
+};
+
 /**
  * struct usb_driver - identifies USB driver to usbcore
  * @owner: Pointer to the module owner of this driver; initialize
@@ -552,6 +557,8 @@ static inline int usb_make_path (struct 
  * @id_table: USB drivers use ID table to support hotplugging.
  *	Export this with MODULE_DEVICE_TABLE(usb,...).  This must be set
  *	or your driver's probe function will never get called.
+ * @dynids: used internally to hold the list of dynamically added device
+ *	ids for this driver.
  * @driver: the driver model core driver structure.
  *
  * USB drivers must provide a name, probe() and disconnect() methods,
@@ -587,6 +594,7 @@ struct usb_driver {
 
 	const struct usb_device_id *id_table;
 
+	struct usb_dynids dynids;
 	struct device_driver driver;
 };
 #define	to_usb_driver(d) container_of(d, struct usb_driver, driver)
author
#1 lore raw

PATCH 1/2 USB: reorg some functions out of the main usb.c file

In reply to Greg KH

This will make the dynamic-id stuff easier to do, as it will be self-contained.

No logic was changed at all.

---
 Documentation/DocBook/usb.tmpl |    1 
 drivers/usb/core/Makefile      |    2 
 drivers/usb/core/driver.c      |  338 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb.c         |  310 -------------------------------------
 drivers/usb/core/usb.h         |    3 
 5 files changed, 343 insertions(+), 311 deletions(-)
drivers/usb/core/usb.c +0 -310
@@ -52,161 +52,6 @@ static int nousb;	/* Disable USB when bu
 static DECLARE_RWSEM(usb_all_devices_rwsem);
 
 
-static int generic_probe (struct device *dev)
-{
-	return 0;
-}
-static int generic_remove (struct device *dev)
-{
-	struct usb_device *udev = to_usb_device(dev);
-
-	/* 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);
-
-	/* in case the call failed or the device was suspended */
-	if (udev->state >= USB_STATE_CONFIGURED)
-		usb_disable_device(udev, 0);
-	return 0;
-}
-
-static struct device_driver usb_generic_driver = {
-	.owner = THIS_MODULE,
-	.name =	"usb",
-	.bus = &usb_bus_type,
-	.probe = generic_probe,
-	.remove = generic_remove,
-};
-
-static int usb_generic_driver_data;
-
-/* called from driver core with usb_bus_type.subsys writelock */
-static int usb_probe_interface(struct device *dev)
-{
-	struct usb_interface * intf = to_usb_interface(dev);
-	struct usb_driver * driver = to_usb_driver(dev->driver);
-	const struct usb_device_id *id;
-	int error = -ENODEV;
-
-	dev_dbg(dev, "%s\n", __FUNCTION__);
-
-	if (!driver->probe)
-		return error;
-	/* FIXME we'd much prefer to just resume it ... */
-	if (interface_to_usbdev(intf)->state == USB_STATE_SUSPENDED)
-		return -EHOSTUNREACH;
-
-	id = usb_match_id (intf, driver->id_table);
-	if (id) {
-		dev_dbg (dev, "%s - got id\n", __FUNCTION__);
-
-		/* Interface "power state" doesn't correspond to any hardware
-		 * state whatsoever.  We use it to record when it's bound to
-		 * a driver that may start I/0:  it's not frozen/quiesced.
-		 */
-		mark_active(intf);
-		intf->condition = USB_INTERFACE_BINDING;
-		error = driver->probe (intf, id);
-		if (error) {
-			mark_quiesced(intf);
-			intf->condition = USB_INTERFACE_UNBOUND;
-		} else
-			intf->condition = USB_INTERFACE_BOUND;
-	}
-
-	return error;
-}
-
-/* called from driver core with usb_bus_type.subsys writelock */
-static int usb_unbind_interface(struct device *dev)
-{
-	struct usb_interface *intf = to_usb_interface(dev);
-	struct usb_driver *driver = to_usb_driver(intf->dev.driver);
-
-	intf->condition = USB_INTERFACE_UNBINDING;
-
-	/* release all urbs for this interface */
-	usb_disable_interface(interface_to_usbdev(intf), intf);
-
-	if (driver && driver->disconnect)
-		driver->disconnect(intf);
-
-	/* reset other interface state */
-	usb_set_interface(interface_to_usbdev(intf),
-			intf->altsetting[0].desc.bInterfaceNumber,
-			0);
-	usb_set_intfdata(intf, NULL);
-	intf->condition = USB_INTERFACE_UNBOUND;
-	mark_quiesced(intf);
-
-	return 0;
-}
-
-/**
- * usb_register - register a USB driver
- * @new_driver: USB operations for the driver
- *
- * Registers a USB driver with the USB core.  The list of unattached
- * interfaces will be rescanned whenever a new driver is added, allowing
- * the new driver to attach to any recognized devices.
- * Returns a negative error code on failure and 0 on success.
- * 
- * NOTE: if you want your driver to use the USB major number, you must call
- * usb_register_dev() to enable that functionality.  This function no longer
- * takes care of that.
- */
-int usb_register(struct usb_driver *new_driver)
-{
-	int retval = 0;
-
-	if (nousb)
-		return -ENODEV;
-
-	new_driver->driver.name = (char *)new_driver->name;
-	new_driver->driver.bus = &usb_bus_type;
-	new_driver->driver.probe = usb_probe_interface;
-	new_driver->driver.remove = usb_unbind_interface;
-	new_driver->driver.owner = new_driver->owner;
-
-	usb_lock_all_devices();
-	retval = driver_register(&new_driver->driver);
-	usb_unlock_all_devices();
-
-	if (!retval) {
-		pr_info("%s: registered new driver %s\n",
-			usbcore_name, new_driver->name);
-		usbfs_update_special();
-	} else {
-		printk(KERN_ERR "%s: error %d registering driver %s\n",
-			usbcore_name, retval, new_driver->name);
-	}
-
-	return retval;
-}
-
-/**
- * usb_deregister - unregister a USB driver
- * @driver: USB operations of the driver to unregister
- * Context: must be able to sleep
- *
- * Unlinks the specified driver from the internal USB driver list.
- * 
- * NOTE: If you called usb_register_dev(), you still need to call
- * usb_deregister_dev() to clean up your driver's allocated minor numbers,
- * this * call will no longer do it for you.
- */
-void usb_deregister(struct usb_driver *driver)
-{
-	pr_info("%s: deregistering driver %s\n", usbcore_name, driver->name);
-
-	usb_lock_all_devices();
-	driver_unregister (&driver->driver);
-	usb_unlock_all_devices();
-
-	usbfs_update_special();
-}
-
 /**
  * usb_ifnum_to_if - get the interface object with a given interface number
  * @dev: the device whose current configuration is considered
@@ -352,137 +197,6 @@ void usb_driver_release_interface(struct
 	mark_quiesced(iface);
 }
 
-/**
- * usb_match_id - find first usb_device_id matching device or interface
- * @interface: the interface of interest
- * @id: array of usb_device_id structures, terminated by zero entry
- *
- * usb_match_id searches an array of usb_device_id's and returns
- * the first one matching the device or interface, or null.
- * This is used when binding (or rebinding) a driver to an interface.
- * Most USB device drivers will use this indirectly, through the usb core,
- * but some layered driver frameworks use it directly.
- * These device tables are exported with MODULE_DEVICE_TABLE, through
- * modutils, to support the driver loading functionality of USB hotplugging.
- *
- * What Matches:
- *
- * The "match_flags" element in a usb_device_id controls which
- * members are used.  If the corresponding bit is set, the
- * value in the device_id must match its corresponding member
- * in the device or interface descriptor, or else the device_id
- * does not match.
- *
- * "driver_info" is normally used only by device drivers,
- * but you can create a wildcard "matches anything" usb_device_id
- * as a driver's "modules.usbmap" entry if you provide an id with
- * only a nonzero "driver_info" field.  If you do this, the USB device
- * driver's probe() routine should use additional intelligence to
- * decide whether to bind to the specified interface.
- * 
- * What Makes Good usb_device_id Tables:
- *
- * The match algorithm is very simple, so that intelligence in
- * driver selection must come from smart driver id records.
- * Unless you have good reasons to use another selection policy,
- * provide match elements only in related groups, and order match
- * specifiers from specific to general.  Use the macros provided
- * for that purpose if you can.
- *
- * The most specific match specifiers use device descriptor
- * data.  These are commonly used with product-specific matches;
- * the USB_DEVICE macro lets you provide vendor and product IDs,
- * and you can also match against ranges of product revisions.
- * These are widely used for devices with application or vendor
- * specific bDeviceClass values.
- *
- * Matches based on device class/subclass/protocol specifications
- * are slightly more general; use the USB_DEVICE_INFO macro, or
- * its siblings.  These are used with single-function devices
- * where bDeviceClass doesn't specify that each interface has
- * its own class. 
- *
- * Matches based on interface class/subclass/protocol are the
- * most general; they let drivers bind to any interface on a
- * multiple-function device.  Use the USB_INTERFACE_INFO
- * macro, or its siblings, to match class-per-interface style 
- * devices (as recorded in bDeviceClass).
- *  
- * Within those groups, remember that not all combinations are
- * meaningful.  For example, don't give a product version range
- * without vendor and product IDs; or specify a protocol without
- * its associated class and subclass.
- */   
-const struct usb_device_id *
-usb_match_id(struct usb_interface *interface, const struct usb_device_id *id)
-{
-	struct usb_host_interface *intf;
-	struct usb_device *dev;
-
-	/* proc_connectinfo in devio.c may call us with id == NULL. */
-	if (id == NULL)
-		return NULL;
-
-	intf = interface->cur_altsetting;
-	dev = interface_to_usbdev(interface);
-
-	/* It is important to check that id->driver_info is nonzero,
-	   since an entry that is all zeroes except for a nonzero
-	   id->driver_info is the way to create an entry that
-	   indicates that the driver want to examine every
-	   device and interface. */
-	for (; id->idVendor || id->bDeviceClass || id->bInterfaceClass ||
-	       id->driver_info; id++) {
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
-		    id->idVendor != le16_to_cpu(dev->descriptor.idVendor))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
-		    id->idProduct != le16_to_cpu(dev->descriptor.idProduct))
-			continue;
-
-		/* No need to test id->bcdDevice_lo != 0, since 0 is never
-		   greater than any unsigned number. */
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
-		    (id->bcdDevice_lo > le16_to_cpu(dev->descriptor.bcdDevice)))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
-		    (id->bcdDevice_hi < le16_to_cpu(dev->descriptor.bcdDevice)))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&
-		    (id->bDeviceClass != dev->descriptor.bDeviceClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_SUBCLASS) &&
-		    (id->bDeviceSubClass!= dev->descriptor.bDeviceSubClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_PROTOCOL) &&
-		    (id->bDeviceProtocol != dev->descriptor.bDeviceProtocol))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_CLASS) &&
-		    (id->bInterfaceClass != intf->desc.bInterfaceClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_SUBCLASS) &&
-		    (id->bInterfaceSubClass != intf->desc.bInterfaceSubClass))
-			continue;
-
-		if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_PROTOCOL) &&
-		    (id->bInterfaceProtocol != intf->desc.bInterfaceProtocol))
-			continue;
-
-		return id;
-	}
-
-	return NULL;
-}
-
-
 static int __find_interface(struct device * dev, void * data)
 {
 	struct usb_interface ** ret = (struct usb_interface **)data;
@@ -520,27 +234,6 @@ struct usb_interface *usb_find_interface
 	return ret ? intf : NULL;
 }
 
-static int usb_device_match (struct device *dev, struct device_driver *drv)
-{
-	struct usb_interface *intf;
-	struct usb_driver *usb_drv;
-	const struct usb_device_id *id;
-
-	/* check for generic driver, which we don't match any device with */
-	if (drv == &usb_generic_driver)
-		return 0;
-
-	intf = to_usb_interface(dev);
-	usb_drv = to_usb_driver(drv);
-	
-	id = usb_match_id (intf, usb_drv->id_table);
-	if (id)
-		return 1;
-
-	return 0;
-}
-
-
 #ifdef	CONFIG_HOTPLUG
 
 /*
@@ -1591,8 +1284,6 @@ module_exit(usb_exit);
  * driver modules to use.
  */
 
-EXPORT_SYMBOL(usb_register);
-EXPORT_SYMBOL(usb_deregister);
 EXPORT_SYMBOL(usb_disabled);
 
 EXPORT_SYMBOL_GPL(usb_get_intf);
@@ -1610,7 +1301,6 @@ EXPORT_SYMBOL(usb_unlock_device);
 
 EXPORT_SYMBOL(usb_driver_claim_interface);
 EXPORT_SYMBOL(usb_driver_release_interface);
-EXPORT_SYMBOL(usb_match_id);
 EXPORT_SYMBOL(usb_find_interface);
 EXPORT_SYMBOL(usb_ifnum_to_if);
 EXPORT_SYMBOL(usb_altnum_to_altsetting);
drivers/usb/core/Makefile +1 -1
@@ -2,7 +2,7 @@
 # Makefile for USB Core files and filesystem
 #
 
-usbcore-objs	:= usb.o hub.o hcd.o urb.o message.o \
+usbcore-objs	:= usb.o hub.o hcd.o urb.o message.o driver.o \
 			config.o file.o buffer.o sysfs.o devio.o notify.o
 
 ifeq ($(CONFIG_PCI),y)
drivers/usb/core/driver.c new file +338 -0

338 lines not on this page · raw message

drivers/usb/core/usb.h +3 -0
@@ -33,6 +33,9 @@ extern void usb_host_cleanup(void);
 extern int usb_suspend_device(struct usb_device *dev);
 extern int usb_resume_device(struct usb_device *dev);
 
+extern struct device_driver usb_generic_driver;
+extern int usb_generic_driver_data;
+extern int usb_device_match(struct device *dev, struct device_driver *drv);
 
 /* Interfaces and their "power state" are owned by usbcore */
 
Documentation/DocBook/usb.tmpl +1 -0
@@ -253,6 +253,7 @@
 !Edrivers/usb/core/urb.c
 !Edrivers/usb/core/message.c
 !Edrivers/usb/core/file.c
+!Edrivers/usb/core/driver.c
 !Edrivers/usb/core/usb.c
 !Edrivers/usb/core/hub.c
     </chapter>
#3 lore raw

Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core

In reply to Greg KH

A few minor comments...

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 87

Quoted 12 lines · Greg KH
drivers/usb/core/driver.c +12 -0
+static ssize_t store_new_id(struct device_driver *driver,
+			    const char *buf, size_t count)
+{
+	struct usb_driver *usb_drv = to_usb_driver(driver);
+	struct usb_dynid *dynid;
+	u32 idVendor = 0;
+	u32 idProduct = 0;
+	int fields = 0;
+
+	fields = sscanf(buf, "%x %x", &idVendor, &idProduct);
+	if (fields < 0)
+		return -EINVAL;

Don't you want to test (fields < 2)?

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 105

Quoted 4 lines
drivers/usb/core/driver.c +4 -0
+	if (get_driver(&usb_drv->driver)) {
+		driver_attach(&usb_drv->driver);
+		put_driver(&usb_drv->driver);
+	}

Here you don't have to refer to &usb_drv->driver; you can just refer to driver.

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 119

Quoted 9 lines
drivers/usb/core/driver.c +9 -0
+static int usb_create_newid_file(struct usb_driver *usb_drv)
+{
+	int error = 0;
+
+	if (usb_drv->probe != NULL)
+		error = sysfs_create_file(&usb_drv->driver.kobj,
+					  &driver_attr_new_id.attr);
+	return error;
+}

This deserves to be an inline function.

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 131

Quoted 11 lines
drivers/usb/core/driver.c +11 -0
+static void usb_free_dynids(struct usb_driver *usb_drv)
+{
+	struct usb_dynid *dynid, *n;
+
+	spin_lock(&usb_drv->dynids.lock);
+	list_for_each_entry_safe(dynid, n, &usb_drv->dynids.list, node) {
+		list_del(&dynid->node);
+		kfree(dynid);
+	}
+	spin_unlock(&usb_drv->dynids.lock);
+}

This could be inline as well, although being longer, its overhead is less noticeable.

Alan Stern

author
#4 lore raw

Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core

Quoted 4 lines · Alan Stern

A few minor comments...

On Wed, 16 Nov 2005, Greg KH wrote:

Quoted 14 lines

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 87

Quoted 12 lines
drivers/usb/core/driver.c +12 -0
+static ssize_t store_new_id(struct device_driver *driver,
+			    const char *buf, size_t count)
+{
+	struct usb_driver *usb_drv = to_usb_driver(driver);
+	struct usb_dynid *dynid;
+	u32 idVendor = 0;
+	u32 idProduct = 0;
+	int fields = 0;
+
+	fields = sscanf(buf, "%x %x", &idVendor, &idProduct);
+	if (fields < 0)
+		return -EINVAL;

Don't you want to test (fields < 2)?

No, it's ok to just specify the vendor, if you want a product of 0 :)

PCI does it this way too.

Quoted 7 lines

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 105

Quoted 4 lines
drivers/usb/core/driver.c +4 -0
+	if (get_driver(&usb_drv->driver)) {
+		driver_attach(&usb_drv->driver);
+		put_driver(&usb_drv->driver);
+	}

Here you don't have to refer to &usb_drv->driver; you can just refer to driver.

Good point, changed, thanks.

Quoted 12 lines

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 119

Quoted 9 lines
drivers/usb/core/driver.c +9 -0
+static int usb_create_newid_file(struct usb_driver *usb_drv)
+{
+	int error = 0;
+
+	if (usb_drv->probe != NULL)
+		error = sysfs_create_file(&usb_drv->driver.kobj,
+					  &driver_attr_new_id.attr);
+	return error;
+}

This deserves to be an inline function.

Quoted 14 lines

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 131

Quoted 11 lines
drivers/usb/core/driver.c +11 -0
+static void usb_free_dynids(struct usb_driver *usb_drv)
+{
+	struct usb_dynid *dynid, *n;
+
+	spin_lock(&usb_drv->dynids.lock);
+	list_for_each_entry_safe(dynid, n, &usb_drv->dynids.list, node) {
+		list_del(&dynid->node);
+		kfree(dynid);
+	}
+	spin_unlock(&usb_drv->dynids.lock);
+}

This could be inline as well, although being longer, its overhead is less noticeable.

It's just not worth it to inline these, it's not speed critical at all. I prefer to not inline stuff unless it help out somehow.

thanks for the review,

greg k-h

#5 lore raw

Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core

Quoted 9 lines · Greg KH
Quoted 5 lines

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 87

Quoted 3 lines
drivers/usb/core/driver.c +3 -0
+	fields = sscanf(buf, "%x %x", &idVendor, &idProduct);
+	if (fields < 0)
+		return -EINVAL;

Don't you want to test (fields < 2)?

No, it's ok to just specify the vendor, if you want a product of 0 :)

PCI does it this way too.

Well, in that case shouldn't you test (fields < 1)?

Alan Stern

#7 lore raw

Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core

In reply to Greg KH

Hi,

Quoted 13 lines · Greg KH
Quoted 12 lines · Alan Stern

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 119

Quoted 9 lines · Greg KH
drivers/usb/core/driver.c +9 -0
+static int usb_create_newid_file(struct usb_driver *usb_drv)
+{
+	int error = 0;
+
+	if (usb_drv->probe != NULL)
+		error = sysfs_create_file(&usb_drv->driver.kobj,
+					  &driver_attr_new_id.attr);
+	return error;
+}

This deserves to be an inline function.

Come on, this is just a gloryfied if :-)

static inline int usb_create_newid_file(struct usb_driver *usb_drv)

{
	if (usb_drv->probe != NULL) {
		return sysfs_create_file(&usb_drv->driver.kobj,
					  &driver_attr_new_id.attr);
	} else {
		return 0;
	}
}
Quoted 2 lines

It's just not worth it to inline these, it's not speed critical at all. I prefer to not inline stuff unless it help out somehow.

I think this could help GCC, but this should be proved, of course :-)

Regards

Ingo Oeser

author
#6 lore raw

Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core

In reply to Alan Stern
Quoted 8 lines · Alan Stern
Quoted 6 lines · Greg KH
Quoted 5 lines

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 87

Quoted 3 lines
drivers/usb/core/driver.c +3 -0
+	fields = sscanf(buf, "%x %x", &idVendor, &idProduct);
+	if (fields < 0)
+		return -EINVAL;

Don't you want to test (fields < 2)?

Quoted 5 lines
Quoted 3 lines

No, it's ok to just specify the vendor, if you want a product of 0 :)

PCI does it this way too.

Well, in that case shouldn't you test (fields < 1)?

Yeah, good point. I've also changed it to be < 2, as it doesn't really make sense to have a product number of 0.

thanks,

greg k-h

#8 lore raw

Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core

In reply to Ingo Oeser
Quoted 20 lines · Ingo Oeser

Hi,

Quoted 13 lines · Greg KH
Quoted 12 lines · Alan Stern

drivers/usb/core/driver.c struct device_driver usb_generic_driver · line 119

Quoted 9 lines · Greg KH
drivers/usb/core/driver.c +9 -0
+static int usb_create_newid_file(struct usb_driver *usb_drv)
+{
+	int error = 0;
+
+	if (usb_drv->probe != NULL)
+		error = sysfs_create_file(&usb_drv->driver.kobj,
+					  &driver_attr_new_id.attr);
+	return error;
+}

This deserves to be an inline function.

Come on, this is just a gloryfied if :-)

static inline int usb_create_newid_file(struct usb_driver *usb_drv)

Quoted 8 lines
{
	if (usb_drv->probe != NULL) {
		return sysfs_create_file(&usb_drv->driver.kobj,
					  &driver_attr_new_id.attr);
	} else {
		return 0;
	}
}

Yes it is. But it's an #ifdef if, which makes it want to be a separate function.

thanks,

greg k-h