driver model: fix device interfaces.

- Remove struct intf_data. 
  It was silly, and gave us an unbalanced interface. 
  Instead, when removing a device_interface, all of a class's devices are
  iterated over, and intf->remove_device() is called for each. (Instead 
  of the device's list of interfaces it belongs to). 
  Interfaces are expected to tell if they support the device being removed.

- Remove struct device::intf_list.

- Protect classes' device list access with devclass_sem, which is taken 
  when a device or interface is added or removed. This allows access to the
  list w/o taking the class's lock, which allows subordinate objects to be
  registered (e.g. the interface itself, or objects the interfaces create
  for a device). 

- This should fix a deadlock when devices are added to interfaces, which 
  calls intf->add_device(), then interface_add_data(), which call 
  kobject_register() and hangs, since it tries to take the class's lock
  again. 

drivers/base/base.h

 #undef DEBUG
 
 extern struct semaphore device_sem;
+extern struct semaphore devclass_sem;
 
 extern int bus_add_device(struct device * dev);
 extern void bus_remove_device(struct device * dev);

drivers/base/class.c

@@ -13,6 +13,8 @@
 #define to_class_attr(_attr) container_of(_attr,struct devclass_attribute,attr)
 #define to_class(obj) container_of(obj,struct device_class,subsys.kset.kobj)
 
+DECLARE_MUTEX(devclass_sem);
+
 static ssize_t
 devclass_attr_show(struct kobject * kobj, struct attribute * attr, char * buf)
 {
@@ -163,29 +165,34 @@ int devclass_add_device(struct device * dev)
 	struct device_class * cls;
 	int error = 0;
 
+	down(&devclass_sem);
 	if (dev->driver) {
 		cls = get_devclass(dev->driver->devclass);
-		if (cls) {
-			down_write(&cls->subsys.rwsem);
-			pr_debug("device class %s: adding device %s\n",
-				 cls->name,dev->name);
-			if (cls->add_device) 
-				error = cls->add_device(dev);
-			if (!error) {
-				enum_device(cls,dev);
-				interface_add_dev(dev);
-			}
-
-			list_add_tail(&dev->class_list,&cls->devices.list);
-
-			/* notify userspace (call /sbin/hotplug) */
-			class_hotplug (dev, "add");
-
-			up_write(&cls->subsys.rwsem);
-			if (error)
-				put_devclass(cls);
+
+		if (!cls)
+			goto Done;
+
+		pr_debug("device class %s: adding device %s\n",
+			 cls->name,dev->name);
+		if (cls->add_device) 
+			error = cls->add_device(dev);
+		if (error) {
+			put_devclass(cls);
+			goto Done;
 		}
+
+		down_write(&cls->subsys.rwsem);
+		enum_device(cls,dev);
+		list_add_tail(&dev->class_list,&cls->devices.list);
+		/* notify userspace (call /sbin/hotplug) */
+		class_hotplug (dev, "add");
+
+		up_write(&cls->subsys.rwsem);
+
+		interface_add_dev(dev);
 	}
+ Done:
+	up(&devclass_sem);
 	return error;
 }
 
@@ -193,26 +200,33 @@ void devclass_remove_device(struct device * dev)
 {
 	struct device_class * cls;
 
+	down(&devclass_sem);
 	if (dev->driver) {
 		cls = dev->driver->devclass;
-		if (cls) {
-			down_write(&cls->subsys.rwsem);
-			pr_debug("device class %s: removing device %s\n",
-				 cls->name,dev->name);
-			interface_remove_dev(dev);
-			unenum_device(cls,dev);
+		if (!cls) 
+			goto Done;
 
-			list_del(&dev->class_list);
+		interface_remove_dev(dev);
 
-			/* notify userspace (call /sbin/hotplug) */
-			class_hotplug (dev, "remove");
+		down_write(&cls->subsys.rwsem);
+		pr_debug("device class %s: removing device %s\n",
+			 cls->name,dev->name);
 
-			if (cls->remove_device)
-				cls->remove_device(dev);
-			up_write(&cls->subsys.rwsem);
-			put_devclass(cls);
-		}
+		unenum_device(cls,dev);
+
+		list_del(&dev->class_list);
+
+		/* notify userspace (call /sbin/hotplug) */
+		class_hotplug (dev, "remove");
+
+		up_write(&cls->subsys.rwsem);
+
+		if (cls->remove_device)
+			cls->remove_device(dev);
+		put_devclass(cls);
 	}
+ Done:
+	up(&devclass_sem);
 }
 
 struct device_class * get_devclass(struct device_class * cls)

drivers/base/core.c

@@ -143,7 +143,6 @@ void device_initialize(struct device *dev)
 	INIT_LIST_HEAD(&dev->driver_list);
 	INIT_LIST_HEAD(&dev->bus_list);
 	INIT_LIST_HEAD(&dev->class_list);
-	INIT_LIST_HEAD(&dev->intf_list);
 }
 
 /**

drivers/base/intf.c

@@ -12,80 +12,31 @@
 
 #define to_intf(node) container_of(node,struct device_interface,kset.kobj.entry)
 
-#define to_data(e) container_of(e,struct intf_data,kobj.entry)
+#define to_dev(d) container_of(d,struct device,class_list)
 
 /**
  *	intf_dev_link - create sysfs symlink for interface.
- *	@data:	interface data descriptor.
+ *	@intf:	interface.
+ *	@dev:	device.
  *
  *	Create a symlink 'phys' in the interface's directory to 
  */
 
-static int intf_dev_link(struct intf_data * data)
+static int intf_dev_link(struct device_interface * intf, struct device * dev)
 {
-	char	name[16];
-	snprintf(name,16,"%d",data->intf_num);
-	return sysfs_create_link(&data->intf->kset.kobj,&data->dev->kobj,name);
+	return sysfs_create_link(&intf->kset.kobj,&dev->kobj,dev->bus_id);
 }
 
 /**
  *	intf_dev_unlink - remove symlink for interface.
- *	@intf:	interface data descriptor.
- *
- */
-
-static void intf_dev_unlink(struct intf_data * data)
-{
-	char	name[16];
-	snprintf(name,16,"%d",data->intf_num);
-	sysfs_remove_link(&data->intf->kset.kobj,name);
-}
-
-
-/**
- *	interface_add_data - attach data descriptor
- *	@data:	interface data descriptor.
- *
- *	This attaches the per-instance interface object to the
- *	interface (by registering its kobject) and the device
- *	itself (by inserting it into the device's list).
- *
- *	Note that there is no explicit protection done in this
- *	function. This should be called from the interface's 
- *	add_device() method, which is called under the protection
- *	of the class's rwsem.
- */
-
-int interface_add_data(struct intf_data * data)
-{
-	struct device_interface * intf = data->intf;
-
-	if (intf) {
-		data->intf_num = intf->devnum++;
-		data->kobj.kset = &intf->kset;
-		kobject_register(&data->kobj);
-
-		list_add_tail(&data->dev_entry,&data->dev->intf_list);
-		return intf_dev_link(data);
-	}
-	return -EINVAL;
-}
-
-
-/**
- *	interface_remove_data - detach data descriptor.
- *	@data:	interface data descriptor.
+ *	@intf:	interface.
+ *	@dev:	device.
  *
- *	This detaches the per-instance data descriptor by removing 
- *	it from the device's list and unregistering the kobject from
- *	the subsystem.
  */
 
-void interface_remove_data(struct intf_data * data)
+static void intf_dev_unlink(struct device_interface * intf, struct device * dev)
 {
-	intf_dev_unlink(data);
-	list_del_init(&data->dev_entry);
-	kobject_unregister(&data->kobj);
+	sysfs_remove_link(&intf->kset.kobj,dev->bus_id);
 }
 
 
@@ -103,33 +54,28 @@ static int add(struct device_interface * intf, struct device * dev)
 {
 	int error = 0;
 
-	if (intf->add_device) 
-		error = intf->add_device(dev);
+	if (intf->add_device) {
+		if (!(error = intf->add_device(dev)))
+			intf_dev_link(intf,dev);
+	}
 	pr_debug(" -> %s (%d)\n",dev->bus_id,error);
 	return error;
 }
 
 /**
  *	del - detach device from interface.
- *	@data:	interface data descriptor.
- *
- *	Another simple helper. Remove the data descriptor from 
- *	the device and the interface, then call the interface's 
- *	remove_device() method.
+ *	@intf:	interface.
+ *	@dev:	device.
  */
 
-static void del(struct intf_data * data)
+static void del(struct device_interface * intf, struct device * dev)
 {
-	struct device_interface * intf = data->intf;
-
 	pr_debug(" -> %s ",intf->name);
-	interface_remove_data(data);
 	if (intf->remove_device)
-		intf->remove_device(data);
+		intf->remove_device(dev);
+	intf_dev_unlink(intf,dev);
 }
 
-#define to_dev(entry) container_of(entry,struct device,class_list)
-
 
 /**
  *	add_intf - add class's devices to interface.
@@ -145,10 +91,8 @@ static void add_intf(struct device_interface * intf)
 	struct device_class * cls = intf->devclass;
 	struct list_head * entry;
 
-	down_write(&cls->subsys.rwsem);
 	list_for_each(entry,&cls->devices.list)
 		add(intf,to_dev(entry));
-	up_write(&cls->subsys.rwsem);
 }
 
 /**
@@ -164,6 +108,7 @@ int interface_register(struct device_interface * intf)
 {
 	struct device_class * cls = get_devclass(intf->devclass);
 
+	down(&devclass_sem);
 	if (cls) {
 		pr_debug("register interface '%s' with class '%s'\n",
 			 intf->name,cls->name);
@@ -173,6 +118,7 @@ int interface_register(struct device_interface * intf)
 		kset_register(&intf->kset);
 		add_intf(intf);
 	}
+	up(&devclass_sem);
 	return 0;
 }
 
@@ -188,14 +134,13 @@ int interface_register(struct device_interface * intf)
 
 static void del_intf(struct device_interface * intf)
 {
+	struct device_class * cls = intf->devclass;
 	struct list_head * entry;
 
-	down_write(&intf->devclass->subsys.rwsem);
-	list_for_each(entry,&intf->kset.list) {
-		struct intf_data * data = to_data(entry);
-		del(data);
+	list_for_each(entry,&cls->devices.list) {
+		struct device * dev = to_dev(entry);
+		del(intf,dev);
 	}
-	up_write(&intf->devclass->subsys.rwsem);
 }
 
 /**
@@ -210,6 +155,8 @@ static void del_intf(struct device_interface * intf)
 void interface_unregister(struct device_interface * intf)
 {
 	struct device_class * cls = intf->devclass;
+
+	down(&devclass_sem);
 	if (cls) {
 		pr_debug("unregistering interface '%s' from class '%s'\n",
 			 intf->name,cls->name);
@@ -217,6 +164,7 @@ void interface_unregister(struct device_interface * intf)
 		kset_unregister(&intf->kset);
 		put_devclass(cls);
 	}
+	up(&devclass_sem);
 }
 
 
@@ -255,20 +203,21 @@ int interface_add_dev(struct device * dev)
  *	This is another helper for the class driver core, and called
  *	when the device is being removed from the class. 
  *	
- *	We iterate over the list of interface data descriptors attached
- *	to the device, and call del() [above] for each. Again, the 
- *	class's rwsem is assumed to be held during this.
+ *	We iterate over the list of the class's devices and call del() 
+ *	[above] for each. Again, the class's rwsem is _not_ held, but
+ *	the devclass_sem is (see class.c).
  */
 
 void interface_remove_dev(struct device * dev)
 {
 	struct list_head * entry, * next;
+	struct device_class * cls = dev->driver->devclass;
 
 	pr_debug("interfaces: removing device %s\n",dev->name);
 
-	list_for_each_safe(entry,next,&dev->intf_list) {
-		struct intf_data * intf_data = to_data(entry);
-		del(intf_data);
+	list_for_each_safe(entry,next,&cls->subsys.kset.list) {
+		struct device_interface * intf = to_intf(entry);
+		del(intf,dev);
 	}
 }
 

include/linux/device.h

@@ -207,8 +207,6 @@ extern void devclass_remove_file(struct device_class *, struct devclass_attribut
  * it supports the device.
  */
 
-struct intf_data;
-
 struct device_interface {
 	char			* name;
 	struct device_class	* devclass;
@@ -217,41 +215,19 @@ struct device_interface {
 	u32			devnum;
 
 	int (*add_device)	(struct device *);
-	int (*remove_device)	(struct intf_data *);
+	int (*remove_device)	(struct device *);
 };
 
 extern int interface_register(struct device_interface *);
 extern void interface_unregister(struct device_interface *);
 
 
-/*
- * intf_data - per-device data for an interface
- * Each interface typically has a per-device data structure 
- * that it allocates. It should embed one of these structures 
- * in that structure and call interface_add_data() to add it
- * to the device's list.
- * That will also enumerate the device within the interface
- * and create a driverfs symlink for it.
- */
-struct intf_data {
-	struct device_interface	* intf;
-	struct device		* dev;
-	u32			intf_num;
-	struct list_head	dev_entry;
-	struct kobject		kobj;
-};
-
-extern int interface_add_data(struct intf_data *);
-
-
-
 struct device {
 	struct list_head node;		/* node in sibling list */
 	struct list_head bus_list;	/* node in bus's list */
 	struct list_head class_list;
 	struct list_head driver_list;
 	struct list_head children;
-	struct list_head intf_list;
 	struct device 	* parent;
 
 	struct kobject kobj;