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.

diff --git a/drivers/base/base.h b/drivers/base/base.h
index ab31ff74207c9a973b968d519de20ae97d69b3c3..a13680120c479d26becf81947284e9c010ccc634 100644 (file)
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -1,6 +1,7 @@
 #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);

diff --git a/drivers/base/class.c b/drivers/base/class.c
index 52c42ad287bb3b3bfee577e72a9a2cbeb76c5c1e..cf8260017f2351b6f0e6f5dd6bf9168ba62d8b35 100644 (file)
--- a/drivers/base/class.c
+++ b/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)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 40fe758731baf176f535aa6aefe8cf99e01f16b5..e37311995422cb71f6193981964b7ecc898eeb52 100644 (file)
--- a/drivers/base/core.c
+++ b/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);
 }
 
 /**

diff --git a/drivers/base/intf.c b/drivers/base/intf.c
index 809547aed60d5a62e012dbf20f1495567991cb3d..f1422dae312d38a86c59c4001c363231f80c6d5f 100644 (file)
--- a/drivers/base/intf.c
+++ b/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);
        }
 }
 

diff --git a/include/linux/device.h b/include/linux/device.h
index b6b3385f31dce12411f8261356ad318d6c38f555..f059058eb3ac9af490fe838126d993f69182af80 100644 (file)
--- a/include/linux/device.h
+++ b/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;