Backlight and LCD module patches [2]

Backlight and LCD module patches [2]

[Andrew Zabolotny]

This patch adds lcd and backlight driver classes so that the
lowlevel lcd and backlight power controls can be separated from
framebuffer drivers.

Signed-off-by: Jamey Hicks <jamey.hicks@hp.com>
Signed-off-by: Andrew Zabolotny <zap@homelink.ru>

--- linux-2.6.7-rc3/include/linux/lcd.h	1970-01-01 03:00:00.000000000 +0300
+++ linux/include/linux/lcd.h	2004-06-08 01:59:42.000000000 +0400
@@ -0,0 +1,58 @@
+/*
+ * LCD Lowlevel Control Abstraction
+ * 
+ * Copyright (C) 2003,2004 Hewlett-Packard Company
+ * 
+ */
+
+#ifndef _LINUX_LCD_H
+#define _LINUX_LCD_H
+
+#include <linux/device.h>
+
+struct lcd_device;
+
+/* This structure defines all the properties of a LCD flat panel. */
+struct lcd_properties {
+	/* Owner module */
+	struct module *owner;
+	/* Get the LCD panel power status (0: full on, 1..3: controller
+	   power on, flat panel power off, 4: full off) */
+	int (*get_power) (struct lcd_device *);
+	/* Enable or disable power to the LCD (0: on; 4: off) */
+	int (*set_power) (struct lcd_device *, int power);
+	/* The maximum value for contrast (read-only) */
+	int max_contrast;
+	/* Get the current contrast setting (0-max_contrast) */
+	int (*get_contrast) (struct lcd_device *);
+	/* Set LCD panel contrast */
+	int (*set_contrast) (struct lcd_device *, int contrast);
+};
+
+struct lcd_device {
+	/* This protects the 'props' field. If 'props' is NULL, the driver that
+	   registered this device has been unloaded, and if class_get_devdata()
+	   points to something in the body of that driver, it is also invalid. */
+	struct semaphore sem;
+	/* If this is NULL, the backing module is unloaded */
+	struct lcd_properties *props;
+	/* The class device structure */
+	struct class_device class_dev;
+};
+
+extern int lcd_device_register(const char *name, void *devdata,
+			       struct lcd_properties *lp,
+			       struct lcd_device **alloc_ld);
+extern void lcd_device_unregister(struct lcd_device *ld);
+
+extern struct lcd_device *lcd_device_find(const char *name);
+extern struct lcd_device *lcd_device_get(struct lcd_device *ld);
+extern void lcd_device_put(struct lcd_device *ld);
+
+#define to_lcd_device(obj) container_of(obj, struct lcd_device, class_dev)
+
+/* The registered clients of this notifier chain will be called every time
+   a new lcd class_device is registered */
+extern struct notifier_block *lcd_device_chain;
+
+#endif
--- linux-2.6.7-rc3/include/linux/backlight.h	1970-01-01 03:00:00.000000000 +0300
+++ linux/include/linux/backlight.h	2004-06-08 01:46:09.000000000 +0400
@@ -0,0 +1,58 @@
+/*
+ * Backlight Lowlevel Control Abstraction
+ * 
+ * Copyright (C) 2003,2004 Hewlett-Packard Company
+ * 
+ */
+
+#ifndef _LINUX_BACKLIGHT_H
+#define _LINUX_BACKLIGHT_H
+
+#include <linux/device.h>
+
+struct backlight_device;
+
+/* This structure defines all the properties of a backlight
+   (usually attached to a LCD). */
+struct backlight_properties {
+	/* Owner module */
+	struct module *owner;
+	/* Get the power status (0: on, 1..3: power saving modes; 4: off) */
+	int (*get_power) (struct backlight_device *);
+	/* Enable or disable power to the LCD (0: on; 4: off) */
+	int (*set_power) (struct backlight_device *, int power);
+	/* Maximal value for brightness (read-only) */
+	int max_brightness;
+	/* Get current backlight brightness */
+	int (*get_brightness) (struct backlight_device *);
+	/* Set backlight brightness (0..max_brightness) */
+	int (*set_brightness) (struct backlight_device *, int brightness);
+};
+
+struct backlight_device {
+	/* This protects the 'props' field. If 'props' is NULL, the driver that
+	   registered this device has been unloaded, and if class_get_devdata()
+	   points to something in the body of that driver, it is also invalid. */
+	struct semaphore sem;
+	/* If this is NULL, the backing module is unloaded */
+	struct backlight_properties *props;
+	/* The class device structure */
+	struct class_device class_dev;
+};
+
+extern int backlight_device_register(const char *name, void *devdata,
+				     struct backlight_properties *bp,
+				     struct backlight_device **alloc_bd);
+extern void backlight_device_unregister(struct backlight_device *bd);
+
+extern struct backlight_device *backlight_device_find(const char *name);
+extern struct backlight_device *backlight_device_get(struct backlight_device *bd);
+extern void backlight_device_put(struct backlight_device *bd);
+
+#define to_backlight_device(obj) container_of(obj, struct backlight_device, class_dev)
+
+/* The registered clients of this notifier chain will be called every time
+   a new backlight class_device is registered */
+extern struct notifier_block *backlight_device_chain;
+
+#endif
--- linux-2.6.7-rc3/drivers/video/lcd.c	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/video/lcd.c	2004-06-08 02:15:11.000000000 +0400
@@ -0,0 +1,306 @@
+/*
+ * LCD Lowlevel Control Abstraction
+ *
+ * Copyright (C) 2003,2004 Hewlett-Packard Company
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/lcd.h>
+#include <linux/notifier.h>
+#include <linux/ctype.h>
+#include <asm/bug.h>
+
+struct notifier_block *lcd_device_chain;
+EXPORT_SYMBOL (lcd_device_chain);
+
+static ssize_t lcd_show_power(struct class_device *cdev, char *buf)
+{
+	int rc;
+	struct lcd_device *ld = to_lcd_device(cdev);
+
+	down (&ld->sem);
+	if (likely (ld->props && ld->props->get_power))
+		rc = sprintf (buf, "%d\n", ld->props->get_power (ld));
+	else
+		rc = -ENXIO;
+	up (&ld->sem);
+
+	return rc;
+}
+
+static ssize_t lcd_store_power(struct class_device *cdev, const char *buf, size_t count)
+{
+	int rc, power;
+	char *endp;
+	struct lcd_device *ld = to_lcd_device(cdev);
+
+	power = simple_strtoul(buf, &endp, 0);
+	if (*endp && !isspace (*endp))
+		return -EINVAL;
+
+	down (&ld->sem);
+	if (likely (ld->props && ld->props->set_power)) {
+		pr_debug("lcd: set power to %d\n", power);
+		ld->props->set_power(ld, power);
+		rc = count;
+	} else
+		rc = -ENXIO;
+	up (&ld->sem);
+
+	return rc;
+}
+
+static ssize_t lcd_show_contrast(struct class_device *cdev, char *buf)
+{
+	int rc;
+	struct lcd_device *ld = to_lcd_device(cdev);
+
+	down (&ld->sem);
+	if (likely (ld->props && ld->props->get_contrast))
+		rc = sprintf (buf, "%d\n", ld->props->get_contrast(ld));
+	else
+		rc = -ENXIO;
+	up (&ld->sem);
+
+	return rc;
+}
+
+static ssize_t lcd_store_contrast(struct class_device *cdev, const char *buf, size_t count)
+{
+	int rc, contrast;
+	char *endp;
+	struct lcd_device *ld = to_lcd_device(cdev);
+
+	contrast = simple_strtoul(buf, &endp, 0);
+	if (*endp && !isspace (*endp))
+		return -EINVAL;
+
+	down (&ld->sem);
+	if (likely (ld->props && ld->props->set_contrast)) {
+		pr_debug("lcd: set contrast to %d\n", contrast);
+		ld->props->set_contrast(ld, contrast);
+		rc = count;
+	} else
+		rc = -ENXIO;
+	up (&ld->sem);
+
+	return rc;
+}
+
+static ssize_t lcd_show_max_contrast(struct class_device *cdev, char *buf)
+{
+	int rc;
+	struct lcd_device *ld = to_lcd_device(cdev);
+
+	down (&ld->sem);
+	if (likely (ld->props))
+		rc = sprintf (buf, "%d\n", ld->props->max_contrast);
+	else
+		rc = -ENXIO;
+	up (&ld->sem);
+
+	return rc;
+}
+
+static void lcd_class_release(struct class_device *dev)
+{
+	struct lcd_device *ld = to_lcd_device (dev);
+	kfree (ld);
+}
+
+struct class lcd_class = {
+	.name = "lcd",
+	.release = lcd_class_release,
+};
+
+#define DECLARE_ATTR(_name,_mode,_show,_store)			\
+	{							 	\
+	.attr	= { .name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE },	\
+	.show	= _show,					\
+	.store	= _store,					\
+	}
+
+static struct class_device_attribute lcd_class_device_attributes[] = {
+	DECLARE_ATTR (power, 0644, lcd_show_power, lcd_store_power),
+	DECLARE_ATTR (contrast, 0644, lcd_show_contrast, lcd_store_contrast),
+	DECLARE_ATTR (max_contrast, 0444, lcd_show_max_contrast, NULL),
+};
+
+/**
+ * lcd_device_register - register a new object of lcd_device class.
+ * @name: the name of the new object (must be the same as the name of the
+ *   respective framebuffer device).
+ * @devdata: an optional pointer to be stored in the class_device. The
+ *   methods may retrieve it by using class_get_devdata (ld->class_dev).
+ * @lp: the lcd properties structure.
+ * @alloc_ld: (returned) the newly allocated device. Must be used when
+ *   deregistering the device. Not relevant if the procedure does not
+ *   return 0 (success).
+ *
+ * Creates a new lcd class_device and copies data from the received ld
+ * structure into the new object. Returns after registering the new object.
+ */
+int lcd_device_register (const char *name, void *devdata,
+			 struct lcd_properties *lp,
+			 struct lcd_device **alloc_ld)
+{
+	int i, rc;
+	struct lcd_device *new_ld;
+
+	pr_debug("lcd_device_register: name=%s\n", name);
+
+	new_ld = kmalloc (sizeof (struct lcd_device), GFP_KERNEL);
+	if (unlikely (!new_ld))
+		return -ENOMEM;
+
+	init_MUTEX (&new_ld->sem);
+	new_ld->props = lp;
+	memset (&new_ld->class_dev, 0, sizeof (new_ld->class_dev));
+	new_ld->class_dev.class = &lcd_class;
+	strlcpy (new_ld->class_dev.class_id, name, KOBJ_NAME_LEN);
+	class_set_devdata (&new_ld->class_dev, devdata);
+
+	rc = class_device_register (&new_ld->class_dev);
+	if (unlikely (rc)) {
+		kfree (new_ld);
+		return rc;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(lcd_class_device_attributes); i++) {
+		rc = class_device_create_file(&new_ld->class_dev,
+					      &lcd_class_device_attributes[i]);
+		if (unlikely (rc)) {
+			while (--i >= 0)
+				class_device_remove_file(&new_ld->class_dev,
+							 &lcd_class_device_attributes[i]);
+			class_device_unregister(&new_ld->class_dev);
+			/* No need to kfree(new_ld) since release() method was called */
+			return rc;
+		}
+	}
+
+	*alloc_ld = new_ld;
+	notifier_call_chain (&lcd_device_chain, 0, new_ld);
+
+	return 0;
+}
+EXPORT_SYMBOL(lcd_device_register);
+
+/**
+ * lcd_device_unregister - unregisters a object of lcd_device class.
+ * @ld: the lcd device object to be unregistered and freed.
+ *
+ * Unregisters a previously registered via lcd_device_register object.
+ */
+void lcd_device_unregister(struct lcd_device *ld)
+{
+	int i;
+
+	if (!ld)
+		return;
+
+	pr_debug("lcd_device_unregister: name=%s\n", ld->class_dev.class_id);
+
+	for (i = 0; i < ARRAY_SIZE (lcd_class_device_attributes); i++)
+		class_device_remove_file (&ld->class_dev,
+					  &lcd_class_device_attributes[i]);
+
+	down (&ld->sem);
+	ld->props = NULL;
+	up (&ld->sem);
+
+	class_device_unregister(&ld->class_dev);
+}
+EXPORT_SYMBOL(lcd_device_unregister);
+
+/**
+ * lcd_device_find - find a LCD device by name.
+ * @name: the name of the LCD object to find.
+ *
+ * The reference counter of the returned object as well as on the module
+ * that implements the backlight functions are incremented.
+ */
+struct lcd_device *lcd_device_find(const char *name)
+{
+	struct lcd_device *ld;
+	struct class_device *class_dev =
+		class_device_find (&lcd_class, name);
+
+	if (unlikely (!class_dev))
+		return NULL;
+
+	ld = lcd_device_get (to_lcd_device (class_dev));
+	class_device_put (class_dev);
+	return ld;
+}
+EXPORT_SYMBOL(lcd_device_find);
+
+/**
+ * lcd_device_get - increment reference counter of a LCD device
+ *   and on the module that implements the LCD device methods.
+ * @ld: the LCD device object.
+ *
+ * Increments the reference counter on both the LCD device and on the
+ * module that implements this LCD device.
+ */
+struct lcd_device *lcd_device_get(struct lcd_device *ld)
+{
+	struct class_device *class_dev = class_device_get (&ld->class_dev);
+
+	if (likely (class_dev)) {
+		down (&ld->sem);
+		if (likely (ld->props && try_module_get (ld->props->owner))) {
+			up (&ld->sem);
+			return ld;
+		}
+		up (&ld->sem);
+
+		class_device_put(class_dev);
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(lcd_device_get);
+
+/**
+ * lcd_device_put - decrement reference counter of a LCD device.
+ * @ld: the LCD device object.
+ *
+ * Decrements the reference counter on both the LCD device and on the
+ * module that implements this LCD device.
+ */
+void lcd_device_put(struct lcd_device *ld)
+{
+	if (ld) {
+		down (&ld->sem);
+		if (ld->props)
+			module_put(ld->props->owner);
+		up (&ld->sem);
+		class_device_put (&ld->class_dev);
+	}
+}
+EXPORT_SYMBOL(lcd_device_put);
+
+static void __exit lcd_class_exit(void)
+{
+	class_unregister(&lcd_class);
+}
+
+static int __init lcd_class_init(void)
+{
+	return class_register(&lcd_class);
+}
+
+/*
+ * if this is compiled into the kernel, we need to ensure that the
+ * class is registered before users of the class try to register lcd's
+ */
+postcore_initcall(lcd_class_init);
+module_exit(lcd_class_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jamey Hicks <jamey.hicks@hp.com>, Andrew Zabolotny <zap@homelink.ru>");
+MODULE_DESCRIPTION("LCD Lowlevel Control Abstraction");
--- linux-2.6.7-rc3/drivers/video/backlight.c	1970-01-01 03:00:00.000000000 +0300
+++ linux/drivers/video/backlight.c	2004-06-08 02:15:45.000000000 +0400
@@ -0,0 +1,307 @@
+/*
+ * Backlight Lowlevel Control Abstraction
+ *
+ * Copyright (C) 2003,2004 Hewlett-Packard Company
+ *
+ */
+
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/backlight.h>
+#include <linux/notifier.h>
+#include <linux/ctype.h>
+#include <asm/bug.h>
+
+struct notifier_block *backlight_device_chain;
+EXPORT_SYMBOL (backlight_device_chain);
+
+static ssize_t backlight_show_power (struct class_device *cdev, char *buf)
+{
+	int rc;
+	struct backlight_device *bd = to_backlight_device (cdev);
+
+	down (&bd->sem);
+	if (likely (bd->props && bd->props->get_power))
+		rc = sprintf (buf, "%d\n", bd->props->get_power (bd));
+	else
+		rc = -ENXIO;
+	up (&bd->sem);
+
+	return rc;
+}
+
+static ssize_t backlight_store_power(struct class_device *cdev, const char *buf, size_t count)
+{
+	int rc, power;
+	char *endp;
+	struct backlight_device *bd = to_backlight_device (cdev);
+
+	power = simple_strtoul (buf, &endp, 0);
+	if (*endp && !isspace (*endp))
+		return -EINVAL;
+
+	down (&bd->sem);
+	if (likely (bd->props && bd->props->set_power)) {
+		pr_debug("backlight: set power to %d\n", power);
+		bd->props->set_power(bd, power);
+		rc = count;
+	} else
+		rc = -ENXIO;
+	up (&bd->sem);
+
+	return rc;
+}
+
+static ssize_t backlight_show_brightness(struct class_device *cdev, char *buf)
+{
+	int rc;
+	struct backlight_device *bd = to_backlight_device (cdev);
+
+	down (&bd->sem);
+	if (likely (bd->props && bd->props->get_brightness))
+		rc = sprintf (buf, "%d\n", bd->props->get_brightness (bd));
+	else
+		rc = -ENXIO;
+	up (&bd->sem);
+
+	return rc;
+}
+
+static ssize_t backlight_store_brightness(struct class_device *cdev, const char *buf, size_t count)
+{
+	int rc, brightness;
+	char *endp;
+	struct backlight_device *bd = to_backlight_device (cdev);
+
+	brightness = simple_strtoul (buf, &endp, 0);
+	if (*endp && !isspace (*endp))
+		return -EINVAL;
+
+	down (&bd->sem);
+	if (likely (bd->props && bd->props->set_brightness)) {
+		pr_debug("backlight: set brightness to %d\n", brightness);
+		bd->props->set_brightness(bd, brightness);
+		rc = count;
+	} else
+		rc = -ENXIO;
+	up (&bd->sem);
+
+	return rc;
+}
+
+static ssize_t backlight_show_max_brightness(struct class_device *cdev, char *buf)
+{
+	int rc;
+	struct backlight_device *bd = to_backlight_device (cdev);
+
+	down (&bd->sem);
+	if (likely (bd->props))
+		rc = sprintf(buf, "%d\n", bd->props->max_brightness);
+	else
+		rc = -ENXIO;
+	up (&bd->sem);
+
+	return rc;
+}
+
+static void backlight_class_release(struct class_device *dev)
+{
+	struct backlight_device *bd = to_backlight_device (dev);
+	kfree (bd);
+}
+
+struct class backlight_class = {
+	.name = "backlight",
+	.release = backlight_class_release,
+};
+
+#define DECLARE_ATTR(_name,_mode,_show,_store)			\
+	{							 	\
+	.attr	= { .name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE },	\
+	.show	= _show,					\
+	.store	= _store,					\
+	}
+
+static struct class_device_attribute bl_class_device_attributes[] = {
+	DECLARE_ATTR (power, 0644, backlight_show_power, backlight_store_power),
+	DECLARE_ATTR (brightness, 0644, backlight_show_brightness, backlight_store_brightness),
+	DECLARE_ATTR (max_brightness, 0444, backlight_show_max_brightness, NULL),
+};
+
+/**
+ * backlight_device_register - create and register a new object of
+ *   backlight_device class.
+ * @name: the name of the new object (must be the same as the name of the
+ *   respective framebuffer device).
+ * @devdata: an optional pointer to be stored in the class_device. The
+ *   methods may retrieve it by using class_get_devdata (&bd->class_dev).
+ * @bp: the backlight properties structure.
+ * @alloc_bd: (returned) the newly allocated device. Must be used when
+ *   deregistering the device. Not relevant if the procedure does not
+ *   return 0 (success).
+ *
+ * Creates and registers new backlight class_device. Returns either 0 or
+ * the negative errno.
+ */
+int backlight_device_register(const char *name, void *devdata,
+			      struct backlight_properties *bp,
+			      struct backlight_device **alloc_bd)
+{
+	int i, rc;
+	struct backlight_device *new_bd;
+
+	pr_debug("backlight_device_alloc: name=%s\n", name);
+
+	new_bd = kmalloc (sizeof (struct backlight_device), GFP_KERNEL);
+	if (unlikely (!new_bd))
+		return -ENOMEM;
+
+	init_MUTEX (&new_bd->sem);
+	new_bd->props = bp;
+	memset (&new_bd->class_dev, 0, sizeof (new_bd->class_dev));
+	new_bd->class_dev.class = &backlight_class;
+	strlcpy (new_bd->class_dev.class_id, name, KOBJ_NAME_LEN);
+	class_set_devdata (&new_bd->class_dev, devdata);
+
+	rc = class_device_register (&new_bd->class_dev);
+	if (unlikely (rc)) {
+		kfree (new_bd);
+		return rc;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(bl_class_device_attributes); i++) {
+		rc = class_device_create_file(&new_bd->class_dev,
+					      &bl_class_device_attributes[i]);
+		if (unlikely (rc)) {
+			while (--i >= 0)
+				class_device_remove_file(&new_bd->class_dev,
+							 &bl_class_device_attributes[i]);
+			class_device_unregister(&new_bd->class_dev);
+			/* No need to kfree(new_bd) since release() method was called */
+			return rc;
+		}
+	}
+
+	*alloc_bd = new_bd;
+	notifier_call_chain (&backlight_device_chain, 0, new_bd);
+
+	return 0;
+}
+EXPORT_SYMBOL(backlight_device_register);
+
+/**
+ * backlight_device_unregister - unregisters a backlight device object.
+ * @bd: the backlight device object to be unregistered and freed.
+ *
+ * Unregisters a previously registered via backlight_device_register object.
+ */
+void backlight_device_unregister(struct backlight_device *bd)
+{
+	int i;
+
+	if (!bd)
+		return;
+
+	pr_debug("backlight_device_unregister: name=%s\n", bd->class_dev.class_id);
+
+	for (i = 0; i < ARRAY_SIZE(bl_class_device_attributes); i++)
+		class_device_remove_file (&bd->class_dev,
+					  &bl_class_device_attributes[i]);
+
+	down (&bd->sem);
+	bd->props = NULL;
+	up (&bd->sem);
+
+	class_device_unregister(&bd->class_dev);
+}
+EXPORT_SYMBOL(backlight_device_unregister);
+
+/**
+ * backlight_device_find - find a backlight device by name.
+ * @name: the name of the backlight object to find.
+ *
+ * The reference counter of the returned object as well as on the module
+ * that implements the backlight functions are incremented.
+ */
+struct backlight_device *backlight_device_find(const char *name)
+{
+	struct backlight_device *bd;
+	struct class_device *class_dev =
+		class_device_find (&backlight_class, name);
+
+	if (unlikely (!class_dev))
+		return NULL;
+
+	bd = backlight_device_get (to_backlight_device (class_dev));
+	class_device_put (class_dev);
+	return bd;
+}
+EXPORT_SYMBOL(backlight_device_find);
+
+/**
+ * backlight_device_get - increment reference counter of a backlight device
+ *   and on the module that implements the backlight device methods.
+ * @bd: the backlight device object.
+ *
+ * Increments the reference counter on both the backlight device and on the
+ * module that implements this backlight device.
+ */
+struct backlight_device *backlight_device_get(struct backlight_device *bd)
+{
+	struct class_device *class_dev = class_device_get (&bd->class_dev);
+
+	if (likely (class_dev)) {
+		down (&bd->sem);
+		if (likely (bd->props && try_module_get (bd->props->owner))) {
+			up (&bd->sem);
+			return bd;
+		}
+		up (&bd->sem);
+
+		class_device_put(class_dev);
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(backlight_device_get);
+
+/**
+ * backlight_device_put - decrement reference counter of a backlight device.
+ * @ld: the backlight device object.
+ *
+ * Decrements the reference counter on both the backlight device and on the
+ * module that implements this backlight device.
+ */
+void backlight_device_put(struct backlight_device *bd)
+{
+	if (bd) {
+		down (&bd->sem);
+		if (bd->props)
+			module_put(bd->props->owner);
+		up (&bd->sem);
+		class_device_put (&bd->class_dev);
+	}
+}
+EXPORT_SYMBOL(backlight_device_put);
+
+static void __exit backlight_class_exit(void)
+{
+	class_unregister(&backlight_class);
+}
+
+static int __init backlight_class_init(void)
+{
+	return class_register(&backlight_class);
+}
+
+/*
+ * if this is compiled into the kernel, we need to ensure that the
+ * class is registered before users of the class try to register lcd's
+ */
+postcore_initcall(backlight_class_init);
+module_exit(backlight_class_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jamey Hicks <jamey.hicks@hp.com>, Andrew Zabolotny <zap@homelink.ru>");
+MODULE_DESCRIPTION("Backlight Lowlevel Control Abstraction");
--- linux-2.6.7-rc3/drivers/video/Kconfig	2004-06-11 02:39:30.000000000 +0400
+++ linux/drivers/video/Kconfig	2004-05-29 01:26:15.000000000 +0400
@@ -38,6 +38,38 @@
 	  (e.g. an accelerated X server) and that are not frame buffer
 	  device-aware may cause unexpected results. If unsure, say N.
 
+config LCD_CLASS_DEVICE
+	tristate "Lowlevel LCD controls"
+	depends on FB
+	help
+	  This framework adds support for low-level control of LCD.
+	  Some framebuffer devices connect to platform-specific LCD modules
+	  in order to have a platform-specific way to control the flat panel
+	  (contrast and applying power to the LCD (not to the backlight!)).
+
+	  To have support for your specific LCD panel you will have to
+	  select the proper drivers which depend on this option.
+
+config LCD_DEVICE
+	bool
+	depends on LCD_CLASS_DEVICE
+	default y
+
+config BACKLIGHT_CLASS_DEVICE
+	tristate "Lowlevel Backlight controls"
+	depends on FB
+	help
+	  This framework adds support for low-level control of the LCD
+          backlight. This includes support for brightness and power.
+
+	  To have support for your specific LCD panel you will have to
+	  select the proper drivers which depend on this option.
+
+config BACKLIGHT_DEVICE
+	bool
+	depends on BACKLIGHT_CLASS_DEVICE
+	default y
+
 config FB_CIRRUS
 	tristate "Cirrus Logic support"
 	depends on FB && (AMIGA || PCI) && BROKEN
--- linux-2.6.7-rc3/drivers/video/Makefile	2004-06-11 02:39:30.000000000 +0400
+++ linux/drivers/video/Makefile	2004-05-29 01:23:42.000000000 +0400
@@ -13,6 +13,9 @@
 obj-$(CONFIG_PPC)                 += macmodes.o
 endif
 
+obj-$(CONFIG_LCD_CLASS_DEVICE)    += lcd.o
+obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
+
 obj-$(CONFIG_FB_ACORN)            += acornfb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_AMIGA)            += amifb.o c2p.o
 obj-$(CONFIG_FB_PM2)              += pm2fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o

Re: Backlight and LCD module patches [2]

[Pavel Machek]

Hi!

> This patch adds lcd and backlight driver classes so that the
> lowlevel lcd and backlight power controls can be separated from
> framebuffer drivers.

Nice... Do you have any example driver that uses this?

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: Backlight and LCD module patches [2]

[John Lenz]

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]

On 06/17/04 13:35:17, Andrew Zabolotny wrote:
> 
> This patch adds lcd and backlight driver classes so that the
> lowlevel lcd and backlight power controls can be separated from
> framebuffer drivers.
>

What about something like this patch?  It still needs a little work.

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.

So for example, one class would be named fb and one class be named lcd.   
When we match two devices, they would symlink to each other like so.
/sys/class/lcd/foo/fb is a symlink to /sys/class/fb/bar and /sys/class/ 
fb/bar/lcd is a symlink to /sys/class/lcd/foo.

The provided match function would have to take care of actually linking  
the two devices together (that is, giving the fb_info device a pointer  
to the actual lcd_properties class or storing the pointer to the  
fb_info structure in the lcd_device structure or whatever).

If we had this class_match structure, we could elminate the matching  
code I added to this patch.

Lastly, wouldn't it be better to create a drivers/video/lcd directory  
so that all the lcd drivers can go in there and not clutter up the main  
drivers/video directory?

John

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lcd_device.patch --]
[-- Type: text/x-patch; charset=unknown-8bit, Size: 10013 bytes --]


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

--- /dev/null
+++ linux/include/linux/lcd.h
@@ -0,0 +1,52 @@
+/*
+ *  linux/include/lcd.h
+ *
+ *  Copyright (C) 2004 John Lenz <jelenz@wisc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * LCD Lowlevel Control Abstraction
+ *
+ * Based on a patch by Andrew Zabolotny <zap@homelink.ru> and Jamey Hicks <jamey.hicks@hp.com>
+ */
+
+#include <linux/device.h>
+
+struct lcd_class_device;
+
+enum lcd_power_status {
+	lcd_power_full_on = 0,
+	lcd_power_full_off,
+	lcd_power_controller_on,
+	lcd_power_flatpanel_off,
+};
+
+struct lcd_properties {
+	/* owner module */
+	struct module *owner;
+
+	/* device of this lcd */
+	struct device *lcd_dev;
+
+	/* get and set the current power state */
+	enum lcd_power_status	(*get_power)	(struct lcd_properties *props);
+	void			(*set_power)	(struct lcd_properties *props, enum lcd_power_status power);
+	
+	/* The maximum value for contrast (read-only) */
+	int max_contrast;
+
+	/* get and set the current contrast level. */
+	int	(*get_contrast)	(struct lcd_properties *props);
+	void	(*set_contrast)	(struct lcd_properties *props, int contrast);
+
+	/* match with a given framebuffer device */
+	int (*match) (struct lcd_properties *props, struct fb_info *fb_info);
+
+	/* private */
+	struct lcd_class_device *lcd_class_dev;
+};
+
+int lcd_device_register(struct lcd_properties *props, const char *name);
+void lcd_device_unregister(struct lcd_properties *props);
--- /dev/null
+++ linux/drivers/video/lcd.c
@@ -0,0 +1,312 @@
+/*
+ * linux/drivers/video/lcd.c
+ *
+ *   Copyright (C) 2004 John Lenz <jelenz@wisc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Based on a patch by Andrew Zabolotny <zap@homelink.ru> and Jamey Hicks <jamey.hicks@hp.com>
+ */
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/device.h>
+#include <linux/fb.h>
+#include <linux/lcd.h>
+
+struct lcd_class_device {
+	/* This protects the props field.*/
+	spinlock_t lock;
+	/* If props is NULL, the driver that registered this device has been unloaded */
+	struct lcd_properties *props;
+	struct class_device class_dev;
+
+	struct list_head list;
+};
+#define to_lcd_device(d) container_of(d, struct lcd_class_device, class_dev)
+
+struct lcd_fb_list_node {
+	struct list_head list;
+	struct fb_info *fb_info;
+};
+static spinlock_t lcd_list_lock = SPIN_LOCK_UNLOCKED;
+static LIST_HEAD(lcd_fb_list);
+static LIST_HEAD(lcd_device_list);
+
+struct lcd_power_name {
+	const char *		name;
+	enum lcd_power_status	power;
+};
+static const struct lcd_power_name lcd_power_names[] = {
+	{ "full on",		lcd_power_full_on },
+	{ "full off",		lcd_power_full_off },
+	{ "controller on",	lcd_power_controller_on },
+	{ "flatpanel off",	lcd_power_flatpanel_off },
+};
+
+static void lcd_class_release(struct class_device *dev)
+{
+	struct lcd_class_device *d = to_lcd_device(dev);
+	kfree(d);
+}
+
+static struct class lcd_class = {
+	.name		= "lcd",
+	.release	= lcd_class_release,
+};
+
+static ssize_t lcd_show_power(struct class_device *dev, char *buf)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	ssize_t ret = 0;
+
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		if (lcd_dev->props->get_power) {
+			sprintf(buf, "%s\n", lcd_power_names[(int)lcd_dev->props->get_power(lcd_dev->props)].name);
+			ret = strlen(buf) + 1;
+		}
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static ssize_t lcd_store_power(struct class_device *dev, const char *buf, size_t size)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	int ret = -EINVAL;
+	int i;
+	
+	for (i = 0; i < ARRAY_SIZE(lcd_power_names); i++) {
+		if (strncmp(buf, lcd_power_names[i].name, strlen(lcd_power_names[i].name)) == 0) {
+			ret = size;
+			spin_lock(&lcd_dev->lock);
+			if (likely(lcd_dev->props)) {
+				if (lcd_dev->props->set_power) {
+					lcd_dev->props->set_power(lcd_dev->props, lcd_power_names[i].power);
+				}
+			}
+			spin_unlock(&lcd_dev->lock);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(power, 0644, lcd_show_power, lcd_store_power);
+
+static ssize_t lcd_show_maxcontrast(struct class_device *dev, char *buf)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	ssize_t ret = 0;
+	
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		sprintf(buf, "%i\n", lcd_dev->props->max_contrast);
+		ret = strlen(buf) + 1;
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(max_contrast, 0444, lcd_show_maxcontrast, NULL);
+
+static ssize_t lcd_show_contrast(struct class_device *dev, char *buf)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	ssize_t ret = 0;
+
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		if (lcd_dev->props->get_contrast) {
+			sprintf(buf, "%i\n", lcd_dev->props->get_contrast(lcd_dev->props));
+			ret = strlen(buf) + 1;
+		}
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static ssize_t lcd_store_contrast(struct class_device *dev, const char *buf, size_t size)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	int ret = -EINVAL;
+	char *after;
+
+	unsigned long contrast = simple_strtoul(buf, &after, 0);
+	if (after - buf > 0) {
+		ret = after - buf;
+		spin_lock(&lcd_dev->lock);
+		if (likely(lcd_dev->props)) {
+			if (lcd_dev->props->set_contrast) {
+				lcd_dev->props->set_contrast(lcd_dev->props, contrast);
+			}
+		}
+		spin_unlock(&lcd_dev->lock);
+	}
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(contrast, 0644, lcd_show_contrast, lcd_store_contrast);
+
+/**
+ * lcd_device_register - register a new object of lcd_device class.
+ * @prop: the lcd properties structure for this device.
+ */
+int lcd_device_register(struct lcd_properties *props, const char *name)
+{
+	int rc;
+	struct lcd_class_device *new_lcd;
+	struct lcd_fb_list_node *node;
+
+	new_lcd = kmalloc (sizeof (struct lcd_class_device), GFP_KERNEL);
+	if (unlikely (!new_lcd))
+		return -ENOMEM;
+
+	spin_lock_init(&new_lcd->lock);
+	new_lcd->props = props;
+	props->lcd_class_dev = new_lcd;
+
+	memset (&new_lcd->class_dev, 0, sizeof (new_lcd->class_dev));
+	new_lcd->class_dev.class = &lcd_class;
+	new_lcd->class_dev.dev = props->lcd_dev;
+	strlcpy (new_lcd->class_dev.class_id, name, KOBJ_NAME_LEN);
+
+	rc = class_device_register (&new_lcd->class_dev);
+	if (unlikely (rc)) {
+		kfree (new_lcd);
+		return rc;
+	}
+
+	/* register the attributes */
+	class_device_create_file(&new_lcd->class_dev, &class_device_attr_power);
+	class_device_create_file(&new_lcd->class_dev, &class_device_attr_max_contrast);
+	class_device_create_file(&new_lcd->class_dev, &class_device_attr_contrast);
+
+	/* add to device list and attempt to match with fb devices */
+	spin_lock(&lcd_list_lock);
+	list_add_tail(&lcd_device_list, &new_lcd->list);
+
+	list_for_each_entry(node, &lcd_fb_list, list) {
+		if (props->match(props, node->fb_info)) {
+			/* node->fb->lcd_props = props 
+			 * get_device(props->lcd_deb) */
+		}
+	}
+	spin_unlock(&lcd_list_lock);
+
+	printk(KERN_INFO "Registered lcd device: name=%s\n", new_lcd->class_dev.class_id);
+
+	return 0;
+}
+EXPORT_SYMBOL(lcd_device_register);
+
+/**
+ * lcd_device_unregister - unregisters a object of lcd_properties class.
+ * @props: the property to unreigister
+ *
+ * Unregisters a previously registered via lcd_device_register object.
+ */
+void lcd_device_unregister(struct lcd_properties *props)
+{
+	struct lcd_class_device *lcd_dev;
+	if (!props)
+		return;
+
+	lcd_dev = props->lcd_class_dev;
+
+	pr_debug("lcd_device_unregister: name=%s\n", lcd_dev->class_dev.class_id);
+
+	class_device_remove_file (&lcd_dev->class_dev, &class_device_attr_contrast);
+	class_device_remove_file (&lcd_dev->class_dev, &class_device_attr_max_contrast);
+	class_device_remove_file (&lcd_dev->class_dev, &class_device_attr_power);
+
+	spin_lock(&lcd_dev->lock);
+	lcd_dev->props = NULL;
+	props->lcd_class_dev = NULL;
+	spin_unlock(&lcd_dev->lock);
+
+	/* remove from device list */
+	spin_lock(&lcd_list_lock);
+	list_del(&lcd_dev->list);
+	spin_unlock(&lcd_list_lock);
+
+	class_device_unregister(&lcd_dev->class_dev);
+}
+EXPORT_SYMBOL(lcd_device_unregister);
+
+int lcd_fb_info_register(struct fb_info *fb)
+{
+	struct lcd_fb_list_node *node;
+	struct lcd_class_device *lcd_dev;
+
+	node = kmalloc(sizeof(struct lcd_fb_list_node), GFP_KERNEL);
+	if (!node) {
+		return -ENOMEM;
+	}
+
+	node->fb_info = fb;
+	
+	/* add to fb list, and attempt to match with known lcd devices */
+	spin_lock(&lcd_list_lock);
+	list_add_tail(&lcd_fb_list, &node->list);
+	
+	list_for_each_entry(lcd_dev, &lcd_device_list, list) {
+		spin_lock(&lcd_dev->lock);
+		if (likely(lcd_dev->props)) {
+			if (lcd_dev->props->match(lcd_dev->props, fb)) {
+				/* fb->lcd_props = lcd_dev->props;
+				 * get_device(lcd_dev->props->lcd_dev); */
+			}
+		}
+		spin_unlock(&lcd_dev->lock);
+	}
+	spin_unlock(&lcd_list_lock);
+
+	return 0;
+}
+
+void lcd_fb_info_unregister(struct fb_info *fb)
+{
+	struct lcd_fb_list_node *node;
+	struct lcd_fb_list_node *tmp;
+
+	/* remove from fb list */
+	spin_lock(&lcd_list_lock);
+	list_for_each_entry_safe(node, tmp, &lcd_fb_list, list) {
+		if (node->fb_info == fb) {
+			list_del(&node->list);
+			kfree(node);
+		}
+	}
+
+	spin_unlock(&lcd_list_lock);
+}
+
+static int __init lcd_init(void)
+{
+	class_register(&lcd_class);
+	return 0;
+}
+subsys_initcall(lcd_init);
+
+static void __exit lcd_exit(void)
+{
+	class_unregister(&lcd_class);
+}
+module_exit(lcd_exit);
+
+MODULE_AUTHOR("John Lenz <jelenz@wisc.edu>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LCD class interface");
+

Re: Backlight and LCD module patches [2]

[Andrew Zabolotny]

On Sun, 25 Jul 2004 16:59:17 -0500
John Lenz <jelenz@students.wisc.edu> 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.
This is impossible to do in a device-independent way. Only the drivers know
how they can recognize each other. And if you're meaning the b/l driver will
register the match function with the core, that's very similar to the solution
I've described in my earlier messages.

> Lastly, wouldn't it be better to create a drivers/video/lcd directory  
> so that all the lcd drivers can go in there and not clutter up the main  
> drivers/video directory?
I agree but my word means nothing.

Speaking of your patch, I don't like the lcd_power_names array. The reason for
the 0-4 power status was to match that of VESA power states (0..4,
intermediate values mean intermediate power states, whatever they mean for
concrete devices). Besides, it makes end-user usage more complex (instead of
just specifying a number in the 0-4 range you now have to *know* you have to
specify "full off" and alike). Also it doesn't handle backlight, only LCD.

--
Greetings,
   Andrew

Re: Backlight and LCD module patches [2]

[John Lenz]

On 07/28/04 13:11:41, Andrew Zabolotny wrote:
> On Sun, 25 Jul 2004 16:59:17 -0500
> John Lenz <jelenz@students.wisc.edu> 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.
>  This is impossible to do in a device-independent way. Only the   
> drivers  know  how they can recognize each other. And if you're  
> meaning the b/l  driver will  register the match function with the  
> core, that's very similar to the  solution  I've described in my  
> earlier messages.

That's what the match callback function is for.  The class match stuff  
does not try and match anything together itself.  The provided match  
callback function would have to do all the device specific matching  
that needs to take place.  All the class_match stuff would do is make  
sure the match function is called with every possible combination of fb  
device and lcd device.

That is, we could remove the lcd_fb_list and lcd_device_list from this  
patch because the class code would handle making sure the match  
function is called.  The class_match->match function in the lcd_device  
driver would then call the lcd_properties->match function.

Secondly, we also wouldn't need to call lcd_fb_info_register from the  
fb code either, since the fb code would register a class_device and  
then the class_match code would attempt to match that fb device with a  
lcd device by calling the lcd provided match function.

(This is a little problematic since the fb code uses simple_class, so  
there would need to be some changes there for this to take place... see  
below)

And we wouldn't need to reimplement those lists and rewrite all that  
matching stuff in every driver (LED, Backlight, LCD)

>  Speaking of your patch, I don't like the lcd_power_names array. The   
> reason for  the 0-4 power status was to match that of VESA power  
> states (0..4,  intermediate values mean intermediate power states,  
> whatever they  mean  for  concrete devices). Besides, it makes end- 
> user usage more complex  (instead of  just specifying a number in the  
> 0-4 range you now have to *know* you  have to  specify "full off" and  
> alike). Also it doesn't handle backlight, only  LCD.

Ok.  I just tried to make it a little more verbose.  That is easy to  
change back.

I was more proposing the type of matching between fb and lcd devices  
that happens in this patch.  It is completly independent of the base  
code, each individual lcd device will implement a function to determine  
if it is the lcd device for the given fb device.

Actually, now that I think about it a bit more, I think the  
lcd_properties->match function should take a device * as a paramater  
instead of a fb_info *.  Insead of passing the fb_info pointer to the  
match function, we really should be passing the actual device  
structure.  For example, in the pxafb driver, it would register the  
platform_device that it creates with either the class code (if  
class_match is used) or with the lcdbase code.  This way the lcd driver  
could examine the device * and look at for example which resources it  
used, which memory region it was using, etc. and make its decision.

That is, I see two options
1) We use class_match.  Then we add an (optional) paramater to  
register_framebuffer in fbmem.c which would be a device *.  This device  
* would just be passed along to the class_simple_device_add function  
and nothing else.  In this way the class_match->match function would be  
able to extrat that device * and pass that along to the lcd_properties-
>match function.

2) class_match doesn't get added.  Instead we just call  
lcd_fb_device_register(struct device *) directly from the pxafb code  
with the platform_device as a paramater.

I would vote for number 1.

Re: Backlight and LCD module patches [2]

[Andrew Zabolotny]

On Thu, 29 Jul 2004 18:25:47 -0500
John Lenz <jelenz@students.wisc.edu> wrote:

> Actually, now that I think about it a bit more, I think the  
> lcd_properties->match function should take a device * as a paramater  
> instead of a fb_info *.  Insead of passing the fb_info pointer to the  
> match function, we really should be passing the actual device  
> structure.  For example, in the pxafb driver, it would register the  
> platform_device that it creates with either the class code (if  
> class_match is used) or with the lcdbase code.  This way the lcd driver  
> could examine the device * and look at for example which resources it  
> used, which memory region it was using, etc. and make its decision.
If you look here: http://lkml.org/lkml/2004/6/26/84 you can see that this is
exactly what I was proposing minus your proposal for a more generic
class device match function. I was imagining that it would happen this way:
the framebuffer device during initialization calls lcd_find_device() and
passes his own 'struct device' to it; then lcd_find_device calls the match
function of every previously registered LCD device with this parameter. The
first one that says 'match' is returned. Same about backlight.

I don't see many reasons for a generic class match function. Last but not
least the lcd_find_device() function is very small, so it will be a negligible
gain but a lot of hassle (as you said, framebuffer drivers will have to be
rewritten to not use the simple_class device class).

--
Greetings,
   Andrew

Re: Backlight and LCD module patches [2]

[John Lenz]

[-- Attachment #1: Type: text/plain, Size: 3029 bytes --]

On 07/29/04 19:06:45, Andrew Zabolotny wrote:
> On Thu, 29 Jul 2004 18:25:47 -0500
> John Lenz <jelenz@students.wisc.edu> wrote:
> 
> > Actually, now that I think about it a bit more, I think the  
> > lcd_properties->match function should take a device * as a paramater  
> > instead of a fb_info *.  Insead of passing the fb_info pointer to the  
> > match function, we really should be passing the actual device  
> > structure.  For example, in the pxafb driver, it would register the  
> > platform_device that it creates with either the class code (if  
> > class_match is used) or with the lcdbase code.  This way the lcd driver  
> > could examine the device * and look at for example which resources it  
> > used, which memory region it was using, etc. and make its decision.

> If you look here: http://lkml.org/lkml/2004/6/26/84 you can see that this is
> exactly what I was proposing minus your proposal for a more generic
> class device match function. I was imagining that it would happen this way:
> the framebuffer device during initialization calls lcd_find_device() and
> passes his own 'struct device' to it; then lcd_find_device calls the match
> function of every previously registered LCD device with this parameter. The
> first one that says 'match' is returned. Same about backlight.

The only problem is that what happens if the fb device is registered before
the lcd device?  So that means you still need to keep around a list of fb
devices that have been registered so that when a new lcd device is registered
it can check if it matches an old fb device.

> 
> I don't see many reasons for a generic class match function. Last but not
> least the lcd_find_device() function is very small, so it will be a negligible
> gain but a lot of hassle (as you said, framebuffer drivers will have to be
> rewritten to not use the simple_class device class).
> 

The only advantage is we let the core class code take care of managing the 2 lists
of devices for us (which it is doing anyway).  Then the driver doesn't need to
keep the fb list around, doesn't need to keep the lcd list around, doesn't need
to write all that locking code to make sure the lists aren't updated all at the
same time, etc.  The class code already has these lists, already provides locking
for them, etc.

The fb stuff doesn't have to be rewritten to use normal classes, the class_match
could still work with simple classes.  The only change would be we would need some way
to pass the device pointer to the class_simple_device_add() function in fbmem.c,
a pretty trivial change.  Notice in fbmem it calls class_simple_device_add with a
paramater of NULL.  We would just like the individual driver to optionally pass a
device * there instead of NULL.

Perhaps a patch would better explain the situation.  I haven't even tried to compile
this patch, but it should give you an idea of what I am thinking here.  The only
thing I didn't add to this was the actual implementation of class_match_register,
but I can write that as well.

John

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lcd_device.patch --]
[-- Type: text/x-patch; charset=unknown-8bit, Size: 11459 bytes --]


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

--- /dev/null
+++ linux/include/linux/lcd.h
@@ -0,0 +1,45 @@
+/*
+ *  linux/include/lcd.h
+ *
+ *  Copyright (C) 2004 John Lenz <jelenz@wisc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * LCD Lowlevel Control Abstraction
+ *
+ * Based on a patch by Andrew Zabolotny <zap@homelink.ru> and Jamey Hicks <jamey.hicks@hp.com>
+ */
+
+#include <linux/device.h>
+
+struct lcd_class_device;
+
+struct lcd_properties {
+	/* owner module */
+	struct module *owner;
+
+	/* device of this lcd */
+	struct device *lcd_dev;
+
+	/* get and set the current power state */
+	int	(*get_power)	(struct lcd_properties *props);
+	void	(*set_power)	(struct lcd_properties *props, int power);
+	
+	/* The maximum value for contrast (read-only) */
+	int max_contrast;
+
+	/* get and set the current contrast level. */
+	int	(*get_contrast)	(struct lcd_properties *props);
+	void	(*set_contrast)	(struct lcd_properties *props, int contrast);
+
+	/* match with a given framebuffer device */
+	int (*match) (struct lcd_properties *props, struct device *fb_dev);
+
+	/* private */
+	struct lcd_class_device *lcd_class_dev;
+};
+
+int lcd_device_register(struct lcd_properties *props, const char *name);
+void lcd_device_unregister(struct lcd_properties *props);
--- /dev/null
+++ linux/drivers/video/lcd/lcdbase.c
@@ -0,0 +1,251 @@
+/*
+ * linux/drivers/video/lcd/lcdbase.c
+ *
+ *   Copyright (C) 2004 John Lenz <jelenz@wisc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Based on a patch by Andrew Zabolotny <zap@homelink.ru> and Jamey Hicks <jamey.hicks@hp.com>
+ */
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/device.h>
+#include <linux/fb.h>
+#include <linux/lcd.h>
+
+struct lcd_class_device {
+	/* This protects the props field.*/
+	spinlock_t lock;
+	/* If props is NULL, the driver that registered this device has been unloaded */
+	struct lcd_properties *props;
+	struct class_device class_dev;
+};
+#define to_lcd_device(d) container_of(d, struct lcd_class_device, class_dev)
+
+static void lcd_class_release(struct class_device *dev)
+{
+	struct lcd_class_device *d = to_lcd_device(dev);
+	kfree(d);
+}
+
+static struct class lcd_class = {
+	.name		= "lcd",
+	.release	= lcd_class_release,
+};
+
+static ssize_t lcd_show_power(struct class_device *dev, char *buf)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	ssize_t ret = 0;
+
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		if (lcd_dev->props->get_power) {
+			sprintf(buf, "%i\n", lcd_dev->props->get_power(lcd_dev->props));
+			ret = strlen(buf) + 1;
+		}
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static ssize_t lcd_store_power(struct class_device *dev, const char *buf, size_t size)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	int ret = -EINVAL;
+	char *after;
+
+	unsigned long power = simple_strtoul(buf, &after, 0);
+	if (after - buf > 0) {
+		ret = after - buf;
+		spin_lock(&lcd_dev->lock);
+		if (likely(lcd_dev->props)) {
+			if (lcd_dev->props->set_power) {
+				lcd_dev->props->set_power(lcd_dev->props, power);
+			}
+		}
+		spin_unlock(&lcd_dev->lock);
+	}
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(power, 0644, lcd_show_power, lcd_store_power);
+
+static ssize_t lcd_show_maxcontrast(struct class_device *dev, char *buf)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	ssize_t ret = 0;
+	
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		sprintf(buf, "%i\n", lcd_dev->props->max_contrast);
+		ret = strlen(buf) + 1;
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(max_contrast, 0444, lcd_show_maxcontrast, NULL);
+
+static ssize_t lcd_show_contrast(struct class_device *dev, char *buf)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	ssize_t ret = 0;
+
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		if (lcd_dev->props->get_contrast) {
+			sprintf(buf, "%i\n", lcd_dev->props->get_contrast(lcd_dev->props));
+			ret = strlen(buf) + 1;
+		}
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static ssize_t lcd_store_contrast(struct class_device *dev, const char *buf, size_t size)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev);
+	int ret = -EINVAL;
+	char *after;
+
+	unsigned long contrast = simple_strtoul(buf, &after, 0);
+	if (after - buf > 0) {
+		ret = after - buf;
+		spin_lock(&lcd_dev->lock);
+		if (likely(lcd_dev->props)) {
+			if (lcd_dev->props->set_contrast) {
+				lcd_dev->props->set_contrast(lcd_dev->props, contrast);
+			}
+		}
+		spin_unlock(&lcd_dev->lock);
+	}
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(contrast, 0644, lcd_show_contrast, lcd_store_contrast);
+
+/**
+ * lcd_device_register - register a new object of lcd_device class.
+ * @prop: the lcd properties structure for this device.
+ */
+int lcd_device_register(struct lcd_properties *props, const char *name)
+{
+	int rc;
+	struct lcd_class_device *new_lcd;
+	struct lcd_fb_list_node *node;
+
+	new_lcd = kmalloc (sizeof (struct lcd_class_device), GFP_KERNEL);
+	if (unlikely (!new_lcd))
+		return -ENOMEM;
+
+	spin_lock_init(&new_lcd->lock);
+	new_lcd->props = props;
+	props->lcd_class_dev = new_lcd;
+
+	memset (&new_lcd->class_dev, 0, sizeof (new_lcd->class_dev));
+	new_lcd->class_dev.class = &lcd_class;
+	new_lcd->class_dev.dev = props->lcd_dev;
+	strlcpy (new_lcd->class_dev.class_id, name, KOBJ_NAME_LEN);
+
+	rc = class_device_register (&new_lcd->class_dev);
+	if (unlikely (rc)) {
+		kfree (new_lcd);
+		return rc;
+	}
+
+	/* register the attributes */
+	class_device_create_file(&new_lcd->class_dev, &class_device_attr_power);
+	class_device_create_file(&new_lcd->class_dev, &class_device_attr_max_contrast);
+	class_device_create_file(&new_lcd->class_dev, &class_device_attr_contrast);
+
+	printk(KERN_INFO "Registered lcd device: name=%s\n", new_lcd->class_dev.class_id);
+
+	return 0;
+}
+EXPORT_SYMBOL(lcd_device_register);
+
+/**
+ * lcd_device_unregister - unregisters a object of lcd_properties class.
+ * @props: the property to unreigister
+ *
+ * Unregisters a previously registered via lcd_device_register object.
+ */
+void lcd_device_unregister(struct lcd_properties *props)
+{
+	struct lcd_class_device *lcd_dev;
+	if (!props)
+		return;
+
+	lcd_dev = props->lcd_class_dev;
+
+	pr_debug("lcd_device_unregister: name=%s\n", lcd_dev->class_dev.class_id);
+
+	class_device_remove_file (&lcd_dev->class_dev, &class_device_attr_contrast);
+	class_device_remove_file (&lcd_dev->class_dev, &class_device_attr_max_contrast);
+	class_device_remove_file (&lcd_dev->class_dev, &class_device_attr_power);
+
+	spin_lock(&lcd_dev->lock);
+	lcd_dev->props = NULL;
+	props->lcd_class_dev = NULL;
+	spin_unlock(&lcd_dev->lock);
+
+	class_device_unregister(&lcd_dev->class_dev);
+}
+EXPORT_SYMBOL(lcd_device_unregister);
+
+static int lcd_class_match(struct class_device *dev1, struct class_device *dev2)
+{
+	struct lcd_class_device *lcd_dev = to_lcd_device(dev1);
+	int ret = 0;
+
+	spin_lock(&lcd_dev->lock);
+	if (likely(lcd_dev->props)) {
+		if (lcd_dev->props->match) {
+			ret = lcd_dev->props->match(lcd_dev->props, dev2->dev);
+		}
+	}
+	spin_unlock(&lcd_dev->lock);
+
+	return ret;
+}
+
+static struct class_match lcd_fb_class_match = {
+	.class1		= &lcd_class,
+
+	.match		= &lcd_class_match,
+};
+
+static int __init lcd_init(void)
+{
+	class_register(&lcd_class);
+
+	class_simple_set_match(fb_class, &lcd_fb_class_match, 2);
+
+	class_match_register(&lcd_fb_class_match);
+	return 0;
+}
+subsys_initcall(lcd_init);
+
+static void __exit lcd_exit(void)
+{
+	class_match_unregister(&lcd_fb_class_match);
+	class_unregister(&lcd_class);
+}
+module_exit(lcd_exit);
+
+MODULE_AUTHOR("John Lenz <jelenz@wisc.edu>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LCD class interface");
+
--- linux/include/linux/device.h~lcd_device
+++ linux/include/linux/device.h
@@ -240,6 +240,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 +258,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/video/fbmem.c~lcd_device
+++ linux/drivers/video/fbmem.c
@@ -1314,7 +1314,7 @@
 #endif
 };
 
-static struct class_simple *fb_class;
+struct class_simple *fb_class;
 
 /**
  *	register_framebuffer - registers a frame buffer device
@@ -1327,7 +1327,7 @@
  */
 
 int
-register_framebuffer(struct fb_info *fb_info)
+register_framebuffer(struct fb_info *fb_info, struct device *fb_device)
 {
 	int i;
 	struct class_device *c;
@@ -1340,7 +1340,7 @@
 			break;
 	fb_info->node = i;
 
-	c = class_simple_device_add(fb_class, MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
+	c = class_simple_device_add(fb_class, MKDEV(FB_MAJOR, i), fb_device, "fb%d", i);
 	if (IS_ERR(c)) {
 		/* Not fatal */
 		printk(KERN_WARNING "Unable to create class_device for framebuffer %d; errno = %ld\n", i, PTR_ERR(c));
@@ -1377,6 +1377,11 @@
 	return 0;
 }
 
+int register_framebuffer(struct fb_info *fb_info)
+{
+	register_framebuffer(fb_info, NULL);
+}
+
 
 /**
  *	unregister_framebuffer - releases a frame buffer device
--- linux/drivers/video/pxafb.c~lcd_device
+++ linux/drivers/video/pxafb.c
@@ -1318,7 +1318,7 @@
 
 	dev_set_drvdata(dev, fbi);
 
-	ret = register_framebuffer(&fbi->fb);
+	ret = register_framebuffer(&fbi->fb, dev);
 	if (ret < 0) {
 		dev_err(dev, "Failed to register framebuffer device: %d\n", ret);
 		goto failed;
--- linux/drivers/base/class_simple.c~lcd_device
+++ 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/include/linux/fb.h~lcd_device
+++ linux/include/linux/fb.h
@@ -634,6 +634,7 @@
 extern void cfb_imageblit(struct fb_info *info, const struct fb_image *image);
 
 /* drivers/video/fbmem.c */
+extern struct class_simple *fb_class;
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
 extern int fb_prepare_logo(struct fb_info *fb_info);

Re: Backlight and LCD module patches [2]

[Andrew Zabolotny]

On Thu, 29 Jul 2004 19:49:50 -0500
John Lenz <jelenz@students.wisc.edu> wrote:

> The only problem is that what happens if the fb device is registered before
> the lcd device?  So that means you still need to keep around a list of fb
> devices that have been registered so that when a new lcd device is
> registered it can check if it matches an old fb device.
Right now it happens in a different way. The patch I've published earlier uses
notifier objects (linux/notifier.h) so that if framebuffer can't find the
matched lcd device, it registers to be notified when new lcd devices appear in
the system; when it does it proceeds like usual (e.g. calls lcd_device_find).
Until it finds the LCD device framebuffer device cannot even initialize
(without powering on the LCD controller in some circumstances it's impossible
to do anything useful).

> The only advantage is we let the core class code take care of managing the 2
> lists of devices for us (which it is doing anyway).
These lists are anyway maintained by the class.c core. You just need a pointer
to the 'struct class' variable for classes you are interested in, and locking
also comes for free - everything is already implemented. Look in class.c for
list_for_each_entry macros - they all are traveling along these lists.

I personally don't like to make this device matching too generalized - if we
need it for b/l, that's fine, we'll have to implement it for b/l.
But it doesn't look too sane, so I'd rather leave it l/b specific.

--
Greetings,
   Andrew

Re: Backlight and LCD module patches [2]

[John Lenz]

On 07/30/04 15:02:05, Andrew Zabolotny wrote:
> On Thu, 29 Jul 2004 19:49:50 -0500
> John Lenz <jelenz@students.wisc.edu> wrote:
> 
> > The only problem is that what happens if the fb device is registered before
> > the lcd device?  So that means you still need to keep around a list of fb
> > devices that have been registered so that when a new lcd device is
> > registered it can check if it matches an old fb device.
> Right now it happens in a different way. The patch I've published earlier uses
> notifier objects (linux/notifier.h) so that if framebuffer can't find the
> matched lcd device, it registers to be notified when new lcd devices appear in
> the system; when it does it proceeds like usual (e.g. calls lcd_device_find).
> Until it finds the LCD device framebuffer device cannot even initialize
> (without powering on the LCD controller in some circumstances it's impossible
> to do anything useful).

I could see it going either way I guess.  The other thing the class_match could
do is create those symlinks from the two classes of devices.  What other way is
there from userspace to see which lcd device is associated with fb device?

> 
> > The only advantage is we let the core class code take care of managing the 2
> > lists of devices for us (which it is doing anyway).
> These lists are anyway maintained by the class.c core. You just need a pointer
> to the 'struct class' variable for classes you are interested in, and locking
> also comes for free - everything is already implemented. Look in class.c for
> list_for_each_entry macros - they all are traveling along these lists.

Yea I see that, but I was trying to keep all the class related code contained in
class.c  I feel kinda uncomfortable manipulating lists and locking locks in
struct class from outside code.

> 
> I personally don't like to make this device matching too generalized - if we
> need it for b/l, that's fine, we'll have to implement it for b/l.
> But it doesn't look too sane, so I'd rather leave it l/b specific.
> 

Why not?  I see it as a simple extension of the class_interface stuff... it
could even use the class_interface to implement class_match (or use
class_interface directly in the lcdbase.c code and forget the class_match stuff).

Now I could see both solutions as workable and either one or even a bunch more
could work for the lcd/fb matching.  Now the question is, which one to implement?
Or actually, the question is, which one is more likely to be accepted into the
kernel?

The way I see it, we really need a policy decision here, and neither you nor I 
are "authorized" to make that decision :)  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

Re: Backlight and LCD module patches [2]

[Andrew Zabolotny]

On Sat, 31 Jul 2004 17:17:36 -0500
John Lenz <jelenz@wisc.edu> wrote:

> I could see it going either way I guess.  The other thing the class_match
> could do is create those symlinks from the two classes of devices.  What
> other way is there from userspace to see which lcd device is associated with
> fb device?
Ok, you convinced me that a thin layer would give something. However, I
still think it would be better to use the lists that are already present in
class core, and not create new lists that duplicate the main one. First, the
main list is the 'ultimate' source of information, second list could
occasionally get out of sync.

> Yea I see that, but I was trying to keep all the class related code
> contained in class.c  I feel kinda uncomfortable manipulating lists and
> locking locks in struct class from outside code.
Well, *if* class_match will be implemented, it will become a part of class
core. So there shouldn't be a problem with manipulating that list.

> The way I see it, we really need a policy decision here, and neither you nor
> I are "authorized" to make that decision :)  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.
You did a perfect summary of the problem. Greg, what's your answer?

--
Greetings,
   Andrew

Re: Backlight and LCD module patches [2]

[Greg KH]

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?

thanks,

greg k-h

Re: Backlight and LCD module patches [2]

[John Lenz]

[-- Attachment #1: Type: text/plain, Size: 2466 bytes --]

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


[-- Attachment #2: class_match.patch --]
[-- Type: text/x-patch, Size: 9296 bytes --]


#
# 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);

Re: Backlight and LCD module patches [2]

[Greg KH]

On Sat, Aug 21, 2004 at 02:51:33AM +0000, John Lenz wrote:
> 
> 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...)

You also have some duplicated code in one function, which implies that
you didn't test this patch (it's in the updated patch you sent me too) :)

> The bigger question of how should we be linking these together in the first 
> place?

I thought you only wanted the ability to actually find the different
class devices.  Then the code would take it from there.  Not this
complex driver core linking logic that you implemented.

> Instead of using this class_match stuff, we could use class_interface.

Exactly.  Why don't you all use that instead?

thanks,

greg k-h

Re: Backlight and LCD module patches [2]

[John Lenz]

On 09/05/04 10:00:32, Greg KH wrote:
> On Sat, Aug 21, 2004 at 02:51:33AM +0000, John Lenz wrote:
> >
> > 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...)
> 
> You also have some duplicated code in one function, which implies that
> you didn't test this patch (it's in the updated patch you sent me too) :)

I didn't test it.  It was only to show what I was thinking of with  
class_match.

> 
> > The bigger question of how should we be linking these together in the
> first
> > place?
> 
> I thought you only wanted the ability to actually find the different
> class devices.  Then the code would take it from there.  Not this
> complex driver core linking logic that you implemented.
> 
> > Instead of using this class_match stuff, we could use class_interface.
> 
> Exactly.  Why don't you all use that instead?

The only benifit from class_match is the "object oriented approach".  I  
assume that the struct list_head children in struct class can be used from  
driver code?  It is the correct policy to use that directly than to call a  
function in class.c?  As well, the struct kobject in class_device (which we  
would use to create a symbolic link)?  And of course our driver code would  
have to acquire class->subsys.rwsem...

If we do it in lcdbase.c we can even solve the locking problem... that is,  
we can always acquire class->subsys.rwsem in the lcd class before the  
class->subsys.rwsem in fb_class.

John

Re: Backlight and LCD module patches [2]

[John Lenz]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

On 08/13/04 18:27:40, Greg KH wrote:
> 
> Care to provide a proposed implementation of this functionality?

Here is an updated patch.  The only difference between the previous one  
and this one was a broken continue; statement.  I tried to continue the  
outer loop from inside an inner loop.  Instead I just use a goto.

[-- Attachment #2: class_match.patch --]
[-- Type: text/x-patch, Size: 9328 bytes --]


#
# 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,134 @@
 
 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) {
+						goto already_matched;
+					}
+				}
+
+				/* 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;
+				}
+already_matched:;
+			}
+			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 +474,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 +508,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 +539,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 +637,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 +731,5 @@
 
 EXPORT_SYMBOL(class_interface_register);
 EXPORT_SYMBOL(class_interface_unregister);
+EXPORT_SYMBOL(class_match_register);
+EXPORT_SYMBOL(class_match_unregister);