Add dynamic id support to all USB drivers
Subsystems (2)
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
PATCH 2/2 USB: add dynamic id functionality to USB core
In reply to Greg KHEcho 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 dereturn -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 dreturn 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 dconst 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_iddevice 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 *dpr_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)
PATCH 1/2 USB: reorg some functions out of the main usb.c file
In reply to Greg KHThis 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 bustatic 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(structmark_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_interfacereturn 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>
Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core
In reply to Greg KHA 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
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
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
Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core
In reply to Greg KHHi,
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
Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core
In reply to Alan SternQuoted 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
Re: [linux-usb-devel] [PATCH 02/02] USB: add dynamic id functionality to USB core
In reply to Ingo OeserQuoted 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