usbatm subdriver registration w/o .driver_info

Roman Kagan rkagan at mail.ru
Thu Jan 27 08:17:47 EST 2005


  Hi Duncan,

On Thu, Jan 27, 2005 at 02:41:59PM +0300, Roman Kagan wrote:
> [...] Note however that
> list mutation is serialized by the module loading subsystem, and the
> presence of the right driver on the list at probe time is guaranteed by
> the USB subsystem.  So the only thing we need to make sure is that list
> traversal doesn't fail.  I thought that choosing the appropriate list
> primitives would do the job without locks.

Garbage again :(  I rely on the list entries to be valid as I call
usb_match_id on them, which can't be ensured by any list primitives.

An updated version with rwlocks below.  Might want to send a patch to
usb-serial too ;)

This has, however, another problem which I missed initially: this stuff
will work only if there's only one match in the list.  If we ever happen
to have several subdrivers for one modem, it'll fail.  Dunno if we can
tolerate this.  What do you think?

Cheers,
  Roman.


Index: usbatm.h
===================================================================
RCS file: /home/cvs/usbatm/usbatm.h,v
retrieving revision 1.3
diff -u -r1.3 usbatm.h
--- usbatm.h	25 Jan 2005 08:18:09 -0000	1.3
+++ usbatm.h	27 Jan 2005 13:08:35 -0000
@@ -91,6 +91,7 @@
 
 	/* private */
 	struct usb_driver usb;
+	struct list_head driver_list;
 };
 
 int usbatm_register (struct usbatm_driver *driver);
Index: usbatm2.c
===================================================================
RCS file: /home/cvs/usbatm/usbatm2.c,v
retrieving revision 1.11
diff -u -r1.11 usbatm2.c
--- usbatm2.c	25 Jan 2005 08:18:09 -0000	1.11
+++ usbatm2.c	27 Jan 2005 13:08:35 -0000
@@ -1046,10 +1046,13 @@
 	return 0;
 }
 
+static rwlock_t usbatm_driver_list_lock = RW_LOCK_UNLOCKED;
+static LIST_HEAD(usbatm_driver_list);
+
 static int usbatm_usb_probe (struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_device *dev = interface_to_usbdev(intf);
-	struct usbatm_driver *driver = (struct usbatm_driver *) id->driver_info;
+	struct usbatm_driver *driver;
 	struct usbatm_data *instance;
 	char *buf;
 	int error = -ENOMEM;
@@ -1059,6 +1062,13 @@
 
 	dev_dbg(&intf->dev, "trying device with vendor=0x%x, product=0x%x, ifnum %d\n", dev->descriptor.idVendor, dev->descriptor.idProduct, ifnum);
 
+	/* USB core guarantees the right driver is on the list */
+	read_lock(usbatm_driver_list_lock);
+	list_for_each_entry(driver, &usbatm_driver_list, driver_list)
+		if (usb_match_id(intf, driver->id_table))
+			break;
+	read_unlock(usbatm_driver_list_lock);
+
 	if (!driver) {
 		dev_dbg(&intf->dev, "blacklisted by %s\n", usbatm_driver_name);
 		return -ENODEV;
@@ -1306,12 +1316,20 @@
         usb_driver->disconnect	= usbatm_disconnect;
         usb_driver->id_table	= driver->id_table;
 
+	write_lock(usbatm_driver_list_lock);
+	list_add(&driver->driver_list, &usbatm_driver_list);
+	write_unlock(usbatm_driver_list_lock);
+
 	return usb_register(usb_driver);
 }
 EXPORT_SYMBOL_GPL(usbatm_register);
 
 void usbatm_deregister (struct usbatm_driver *driver)
 {
+	write_lock(usbatm_driver_list_lock);
+	list_del(&driver->driver_list);
+	write_unlock(usbatm_driver_list_lock);
+
 	usb_deregister(&driver->usb);
 }
 EXPORT_SYMBOL_GPL(usbatm_deregister);
@@ -1337,6 +1355,11 @@
 }
 module_init(udsl_usb_init);
 
+static void __exit udsl_usb_exit(void)
+{
+}
+module_exit(udsl_usb_exit);
+
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");



More information about the Usbatm mailing list