Re: Backlight and LCD module patches [2]

[John Lenz]

On 08/13/04 18:27:40, Greg KH wrote:
> On Sun, Jul 25, 2004 at 04:59:17PM -0500, John Lenz wrote:
> > The problem I see is that we would like something like a bus to match  
> > together class devices.  What would be really nice is something like  
> > this.
> > 
> > struct class_match {
> >  struct class *class1;
> >  struct class *class2;
> > 
> >  int (*match)(struct class_device *dev1, struct class_device *dev2);
> > };
> > 
> > This class match structure would be very similar to a bus, in that it  
> > matches together two classes instead of matching a device to a driver.   
> > All the class code would have to do is call the match function for all  
> > possible combinations of class devices in class1 and in class2.  If  
> > match returned true, then it would create symbolic links between the  
> > two.
>
> Care to provide a proposed implementation of this functionality?

Here.  A few notes on the implementation.  I have a global lock protecting
all match operations because otherwise we get a dining philosophers problem.
(Say the same class is in two class_match structures, class1 in the first one
and class2 in the second...)

The bigger question of how should we be linking these together in the first place?
Instead of using this class_match stuff, we could use class_interface.

From my other email:

We have two completly seperate devices,
both have a struct device and both have a struct device_driver.  They can sit on
completly seperate buses and be mixed together in lots of different combinations.
One of the devices "knows" if it matches with the other device.

What is the linux preferred way to dynamicly match the two devices together?  Should
each driver pair make its own decisions and its own constructs, or should we try and
extend the device/driver/class/bus/etc model to support matching two devices together?
Use waitqueues and probe functions?  notify.h?  match by a char *name? class_match?
class_interface? create a virtual bus and use the bus matching code (not a very good
solution, but one nonetheless)?  Secondly, what is the best way to represent this
matching to userspace (sysfs)?

We already tried matching together devices by their name, and that was shot down.
And yet the mtd code matches chips and maps together by their name...  We haven't
really gotten much feedback in which ways of matching devices together is acceptable
and which isn't.

John


#
# Patch managed by http://www.holgerschurig.de/patcher.html
#

--- linux/include/linux/device.h~class_match
+++ linux/include/linux/device.h
@@ -147,6 +147,7 @@
 	struct subsystem	subsys;
 	struct list_head	children;
 	struct list_head	interfaces;
+	struct list_head	matches;
 
 	struct class_attribute		* class_attrs;
 	struct class_device_attribute	* class_dev_attrs;
@@ -187,6 +188,8 @@
 	void			* class_data;	/* class-specific data */
 
 	char	class_id[BUS_ID_SIZE];	/* unique to this class */
+
+	struct list_head	matched_classes;
 };
 
 static inline void *
@@ -240,6 +243,16 @@
 extern int class_interface_register(struct class_interface *);
 extern void class_interface_unregister(struct class_interface *);
 
+struct class_match {
+	struct class*	class1;
+	struct class*	class2;
+
+	int (*match)(struct class_device *dev1, struct class_device *dev2);
+};
+
+extern int class_match_register(struct class_match *match);
+extern void class_match_unregister(struct class_match *match);
+
 /* interface for class simple stuff */
 extern struct class_simple *class_simple_create(struct module *owner, char *name);
 extern void class_simple_destroy(struct class_simple *cs);
@@ -248,6 +261,7 @@
 extern int class_simple_set_hotplug(struct class_simple *, 
 	int (*hotplug)(struct class_device *dev, char **envp, int num_envp, char *buffer, int buffer_size));
 extern void class_simple_device_remove(dev_t dev);
+extern void class_simple_set_match(struct class_simple *cs, struct class_match *match, int location);
 
 
 struct device {
--- linux/drivers/base/class_simple.c~class_match
+++ linux/drivers/base/class_simple.c
@@ -214,3 +214,13 @@
 	}
 }
 EXPORT_SYMBOL(class_simple_device_remove);
+
+extern void class_simple_set_match(struct class_simple *cs, struct class_match *match, int location)
+{
+	if (location == 1) {
+		match->class1 = &cs->class;
+	} else if (location == 2) {
+		match->class2 = &cs->class;
+	}
+}
+EXPORT_SYMBOL(class_simple_set_match);
--- linux/drivers/base/class.c~class_match
+++ linux/drivers/base/class.c
@@ -139,6 +139,7 @@
 
 	INIT_LIST_HEAD(&cls->children);
 	INIT_LIST_HEAD(&cls->interfaces);
+	INIT_LIST_HEAD(&cls->matches);
 	error = kobject_set_name(&cls->subsys.kset.kobj, cls->name);
 	if (error)
 		return error;
@@ -306,6 +307,133 @@
 
 static decl_subsys(class_obj, &ktype_class_device, &class_hotplug_ops);
 
+static spinlock_t class_match_lock = SPIN_LOCK_UNLOCKED;
+
+struct class_match_node {
+	struct class_match *match;
+	struct class_device *dev;
+	struct list_head node;
+};
+/* Makes the calls to class_match->match functions and connects devices together */
+static void class_match_check(struct class_device *dev)
+{
+	struct class *parent = dev->class;
+	struct class_match_node *match_node, *other_match_node;
+
+	struct class *other_class;
+	struct class_device *other_device;
+	
+	int ret;
+
+	spin_lock(&class_match_lock);
+
+	down_read(&parent->subsys.rwsem);
+	list_for_each_entry(match_node, &parent->matches, node) {
+		/* call the match() function and try and match devices together */
+		if (parent == match_node->match->class1) {
+			other_class = match_node->match->class2;
+		} else if (parent == match_node->match->class2) {
+			other_class = match_node->match->class1;
+		} else {
+			other_class = NULL;
+		}
+
+		if (other_class) {
+			down_read(&other_class->subsys.rwsem);
+
+			list_for_each_entry(other_device, &other_class->children, node) {
+				/* first check if this device is already matched... */
+				list_for_each_entry(other_match_node, &other_device->matched_classes, node) {
+					if (other_match_node->match == match_node->match) {
+						continue;
+					}
+				}
+
+				/* now call the match function */
+				if (parent == match_node->match->class1) {
+					ret = match_node->match->match(dev, other_device);
+				} else {
+					ret = match_node->match->match(other_device, dev);
+				}
+				if (ret) {
+
+					/* now add this to the matched_classes lists */
+					other_match_node = (struct class_match_node *) kmalloc(sizeof(struct class_match_node), GFP_KERNEL);
+					if (other_match_node) {
+						other_match_node->dev = other_device;
+						other_match_node->match = match_node->match;
+
+						list_add_tail(&other_match_node->node, &dev->matched_classes);
+						sysfs_create_link(&dev->kobj, &other_device->kobj, other_class->name);
+					}
+
+					other_match_node = (struct class_match_node *) kmalloc(sizeof(struct class_match_node), GFP_KERNEL);
+					if (other_match_node) {
+						other_match_node->dev = dev;
+						other_match_node->match = match_node->match;
+
+						list_add_tail(&other_match_node->node, &other_device->matched_classes);
+						sysfs_create_link(&other_device->kobj, &dev->kobj, parent->name);
+					}
+					
+					break;
+				}
+			}
+			up_read(&other_class->subsys.rwsem);
+		}
+	}
+	up_read(&parent->subsys.rwsem);
+
+	spin_unlock(&class_match_lock);
+}
+
+static void class_match_unlink(struct class_device *class_dev)
+{
+	struct class * parent = class_dev->class;
+	struct class_match_node *match_node, *other_match_node, *temp_match_node;
+	struct class *other_class;
+
+	spin_lock(&class_match_lock);
+
+	down_read(&parent->subsys.rwsem);
+
+	/* remove symlinks from devices matched to this device */
+	list_for_each_entry(match_node, &class_dev->matched_classes, node) {
+		if (parent == match_node->match->class1) {
+			other_class = match_node->match->class2;
+		} else if (parent == match_node->match->class2) {
+			other_class = match_node->match->class1;
+		} else {
+			/* should never happen */
+			other_class = NULL;
+		}
+		if (other_class) {
+			list_for_each_entry_safe(other_match_node, temp_match_node, &match_node->dev->matched_classes, node) {
+				if (match_node->match == other_match_node->match) {
+					list_del(&other_match_node->node);
+					kfree(other_match_node);
+					sysfs_remove_link(&match_node->dev->kobj, parent->name);
+					break;
+				}
+			}
+		}
+	}
+
+	/* free entries in matched_classes */
+	list_for_each_entry_safe(match_node, temp_match_node, &class_dev->matched_classes, node) {
+		list_del(&match_node->node);
+		if (parent == match_node->match->class1) {
+			sysfs_remove_link(&class_dev->kobj, match_node->match->class2->name);
+		} else {
+			sysfs_remove_link(&class_dev->kobj, match_node->match->class1->name);
+		}
+		kfree(match_node);
+	}
+
+	up_read(&parent->subsys.rwsem);
+
+	spin_unlock(&class_match_lock);
+}
 
 static int class_device_add_attrs(struct class_device * cd)
 {
@@ -345,6 +473,7 @@
 	kobj_set_kset_s(class_dev, class_obj_subsys);
 	kobject_init(&class_dev->kobj);
 	INIT_LIST_HEAD(&class_dev->node);
+	INIT_LIST_HEAD(&class_dev->matched_classes);
 }
 
 int class_device_add(struct class_device *class_dev)
@@ -378,6 +507,7 @@
 			if (class_intf->add)
 				class_intf->add(class_dev);
 		up_write(&parent->subsys.rwsem);
+		class_match_check(class_dev);
 	}
 	class_device_add_attrs(class_dev);
 	class_device_dev_link(class_dev);
@@ -408,6 +538,7 @@
 			if (class_intf->remove)
 				class_intf->remove(class_dev);
 		up_write(&parent->subsys.rwsem);
+		class_match_unlink(class_dev);
 	}
 
 	class_device_dev_unlink(class_dev);
@@ -505,7 +636,64 @@
 	class_put(parent);
 }
 
+int class_match_register(struct class_match *match)
+{
+	struct class_match_node *match_node;
+	
+	if (!match)
+		return -ENODEV;
+	if (!match->class1 || !match->class2)
+		return -ENODEV;
+	if (!match->match || match->class1 == match->class2)
+		return -EINVAL;
 
+	spin_lock(&class_match_lock);
+	
+	match_node = (struct class_match_node *) kmalloc(sizeof(struct class_match_node), GFP_KERNEL);
+	if (match_node) {
+		match_node->match = match;
+		match_node->dev = NULL;
+		down_write(&match->class1->subsys.rwsem);
+		list_add_tail(&match_node->node, &match->class1->matches);
+		up_write(&match->class1->subsys.rwsem);
+	} else {
+		return -ENOMEM;
+	}
+
+	match_node = (struct class_match_node *) kmalloc(sizeof(struct class_match_node), GFP_KERNEL);
+	if (match_node) {
+		match_node->match = match;
+		match_node->dev = NULL;
+		down_write(&match->class2->subsys.rwsem);
+		list_add_tail(&match_node->node, &match->class2->matches);
+		up_write(&match->class2->subsys.rwsem);
+	} else {
+		return -ENOMEM;
+	}
+
+	spin_unlock(&class_match_lock);
+	
+	return 0;
+}
+
+void class_match_unregister(struct class_match *match)
+{
+	struct class_device *class_dev;
+
+	if (!match)
+		return;
+	if (!match->class1 || !match->class2)
+		return;
+	if (!match->match || match->class1 == match->class2)
+		return;
+	
+	/* we only need to call class_match_unlink on one of the class devices */
+
+	down_read(&match->class1->subsys.rwsem);
+	list_for_each_entry(class_dev, &match->class1->children, node)
+		class_match_unlink(class_dev);
+	up_read(&match->class1->subsys.rwsem);
+}
 
 int __init classes_init(void)
 {
@@ -542,3 +730,5 @@
 
 EXPORT_SYMBOL(class_interface_register);
 EXPORT_SYMBOL(class_interface_unregister);
+EXPORT_SYMBOL(class_match_register);
+EXPORT_SYMBOL(class_match_unregister);