PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Matthew Garrett]

This allows Asus backlight control to be performed via 
/sys/class/backlight. It does not currently remove the legacy /proc 
interface.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff -urp drivers/acpi.bak/asus_acpi.c drivers/acpi/asus_acpi.c
--- drivers/acpi.bak/asus_acpi.c	2006-04-03 04:22:10 +0100
+++ a/drivers/acpi/asus_acpi.c	2006-04-18 09:23:58 +0100
@@ -38,6 +38,8 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include <asm/uaccess.h>
@@ -82,6 +84,8 @@ MODULE_PARM_DESC(asus_uid, "UID for entr
 module_param(asus_gid, uint, 0);
 MODULE_PARM_DESC(asus_gid, "GID for entries in /proc/acpi/asus.\n");
 
+static struct backlight_device *asus_backlight_device;
+
 /* For each model, all features implemented, 
  * those marked with R are relative to HOTK, A for absolute */
 struct model_data {
@@ -689,7 +693,7 @@ proc_write_lcd(struct file *file, const 
 	return count;
 }
 
-static int read_brightness(void)
+static int read_brightness(struct backlight_device *bd)
 {
 	int value;
 
@@ -711,39 +715,50 @@ static int read_brightness(void)
 /*
  * Change the brightness level
  */
-static void set_brightness(int value)
+static int set_brightness(struct backlight_device *bd, int value)
 {
 	acpi_status status = 0;
 
+	value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
+	/* 0 <= value <= 15 */
+
 	/* SPLV laptop */
 	if (hotk->methods->brightness_set) {
 		if (!write_acpi_int(hotk->handle, hotk->methods->brightness_set,
 				    value, NULL))
 			printk(KERN_WARNING
 			       "Asus ACPI: Error changing brightness\n");
-		return;
+		return -EINVAL;
 	}
 
 	/* No SPLV method if we are here, act as appropriate */
-	value -= read_brightness();
+	value -= read_brightness(NULL);
 	while (value != 0) {
 		status = acpi_evaluate_object(NULL, (value > 0) ?
 					      hotk->methods->brightness_up :
 					      hotk->methods->brightness_down,
 					      NULL, NULL);
 		(value > 0) ? value-- : value++;
-		if (ACPI_FAILURE(status))
+		if (ACPI_FAILURE(status)) {
 			printk(KERN_WARNING
 			       "Asus ACPI: Error changing brightness\n");
+			return -EINVAL;
+		}		
 	}
-	return;
+	return 0;
+}
+
+static int update_brightness(struct backlight_device *bd)
+{
+	int value = bd->props->brightness;
+	return set_brightness(bd, value);
 }
 
 static int
 proc_read_brn(char *page, char **start, off_t off, int count, int *eof,
 	      void *data)
 {
-	return sprintf(page, "%d\n", read_brightness());
+	return sprintf(page, "%d\n", read_brightness(NULL));
 }
 
 static int
@@ -754,9 +769,7 @@ proc_write_brn(struct file *file, const 
 
 	count = parse_arg(buffer, count, &value);
 	if (count > 0) {
-		value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
-		/* 0 <= value <= 15 */
-		set_brightness(value);
+		set_brightness(NULL,value);
 	} else if (count < 0) {
 		printk(KERN_WARNING "Asus ACPI: Error reading user input\n");
 	}
@@ -1207,6 +1220,13 @@ static int asus_hotk_remove(struct acpi_
 	return 0;
 }
 
+static struct backlight_properties asusbl_data = {
+	.owner          = THIS_MODULE,
+	.get_brightness = read_brightness,
+	.update_status  = update_brightness,
+	.max_brightness = 15,
+};
+
 static int __init asus_acpi_init(void)
 {
 	int result;
@@ -1232,11 +1252,24 @@ static int __init asus_acpi_init(void)
 		return -ENODEV;
 	}
 
+	asus_backlight_device = backlight_device_register ("asus_bl", NULL,
+							   &asusbl_data);
+
+	if (IS_ERR (asus_backlight_device)) {
+		printk("Unable to register backlight\n");
+		acpi_bus_unregister_driver(&asus_hotk_driver);
+		remove_proc_entry(PROC_ASUS, acpi_root_dir);
+		
+		acpi_os_free(asus_info);
+		return -ENODEV;
+	}
+
 	return 0;
 }
 
 static void __exit asus_acpi_exit(void)
 {
+	backlight_device_unregister(asus_backlight_device);
 	acpi_bus_unregister_driver(&asus_hotk_driver);
 	remove_proc_entry(PROC_ASUS, acpi_root_dir);
 
diff -urp drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
--- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
+++ a/drivers/acpi/Kconfig	2006-04-18 09:20:42 +0100
@@ -167,7 +167,7 @@ config ACPI_NUMA
 
 config ACPI_ASUS
         tristate "ASUS/Medion Laptop Extras"
-	depends on X86
+	depends on X86 && BACKLIGHT_DEVICE
         ---help---
           This driver provides support for extra features of ACPI-compatible
           ASUS laptops. As some of Medion laptops are made by ASUS, it may also

-- 
Matthew Garrett | mjg59@srcf.ucam.org

PATCH [2/3]: Provide generic backlight support in IBM ACPI driver

[Matthew Garrett]

This provides support for altering IBM backlight brightness via 
/sys/class/backlight. It does not remove the legacy /proc interface.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff -urp drivers/acpi.bak/ibm_acpi.c drivers/acpi/ibm_acpi.c
--- drivers/acpi.bak/ibm_acpi.c	2006-04-03 04:22:10 +0100
+++ a/drivers/acpi/ibm_acpi.c	2006-04-18 09:19:21 +0100
@@ -78,6 +78,8 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_drivers.h>
@@ -243,6 +245,8 @@ struct ibm_struct {
 
 static struct proc_dir_entry *proc_dir = NULL;
 
+static struct backlight_device *ibm_backlight_device;
+
 #define onoff(status,bit) ((status) & (1 << (bit)) ? "on" : "off")
 #define enabled(status,bit) ((status) & (1 << (bit)) ? "enabled" : "disabled")
 #define strlencmp(a,b) (strncmp((a), (b), strlen(b)))
@@ -1318,15 +1322,25 @@ static int ecdump_write(char *buf)
 
 static int brightness_offset = 0x31;
 
+static int brightness_read_value(struct backlight_device *bd)
+{
+	u8 level;
+	if (!acpi_ec_read(brightness_offset, &level))
+		return -EIO;
+
+	level &= 0x7;
+	return level;
+}
+
 static int brightness_read(char *p)
 {
 	int len = 0;
-	u8 level;
+	int level = brightness_read_value(NULL);
 
-	if (!acpi_ec_read(brightness_offset, &level)) {
+	if (level < 0) {
 		len += sprintf(p + len, "level:\t\tunreadable\n");
 	} else {
-		len += sprintf(p + len, "level:\t\t%d\n", level & 0x7);
+		len += sprintf(p + len, "level:\t\t%d\n", level);
 		len += sprintf(p + len, "commands:\tup, down\n");
 		len += sprintf(p + len, "commands:\tlevel <level>"
 			       " (<level> is 0-7)\n");
@@ -1338,9 +1352,38 @@ static int brightness_read(char *p)
 #define BRIGHTNESS_UP	4
 #define BRIGHTNESS_DOWN	5
 
+static int brightness_write_value(struct backlight_device *bd, int value)
+{
+	int current_value = brightness_read_value(bd);
+	int inc;
+	int cmos_cmd; 
+
+	value &= 7;
+
+	cmos_cmd = value > current_value ? BRIGHTNESS_UP : BRIGHTNESS_DOWN;
+
+	inc = value > current_value ? 1 : -1;
+
+	while (value != current_value) {
+		current_value += inc;
+
+		if (!cmos_eval(cmos_cmd))
+			return -EIO;
+		if (!acpi_ec_write(brightness_offset, current_value))
+			return -EIO;
+	}
+
+	return 0;
+}
+
+static int brightness_update_value(struct backlight_device *bd)
+{
+	int value = bd->props->brightness;
+	return brightness_write_value(bd, value);
+}
+
 static int brightness_write(char *buf)
 {
-	int cmos_cmd, inc, i;
 	u8 level;
 	int new_level;
 	char *cmd;
@@ -1360,14 +1403,8 @@ static int brightness_write(char *buf)
 		} else
 			return -EINVAL;
 
-		cmos_cmd = new_level > level ? BRIGHTNESS_UP : BRIGHTNESS_DOWN;
-		inc = new_level > level ? 1 : -1;
-		for (i = level; i != new_level; i += inc) {
-			if (!cmos_eval(cmos_cmd))
-				return -EIO;
-			if (!acpi_ec_write(brightness_offset, i + inc))
-				return -EIO;
-		}
+		brightness_write_value(NULL, new_level);
+		
 	}
 
 	return 0;
@@ -1895,10 +1932,20 @@ IBM_PARAM(brightness);
 IBM_PARAM(volume);
 IBM_PARAM(fan);
 
+
+static struct backlight_properties ibmbl_data = {
+        .owner          = THIS_MODULE,
+        .get_brightness = brightness_read_value,
+        .update_status  = brightness_update_value,
+        .max_brightness = 7,
+};
+
 static void acpi_ibm_exit(void)
 {
 	int i;
 
+	backlight_device_unregister(ibm_backlight_device);
+
 	for (i = ARRAY_SIZE(ibms) - 1; i >= 0; i--)
 		ibm_exit(&ibms[i]);
 
@@ -1966,6 +2013,11 @@ static int __init acpi_ibm_init(void)
 		}
 	}
 
+	ibm_backlight_device = backlight_device_register ("ibm_bl", NULL,
+							  &ibmbl_data);
+        if (IS_ERR (ibm_backlight_device))
+		acpi_ibm_exit();
+
 	return 0;
 }
 
diff -urp drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
--- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
+++ a/drivers/acpi/Kconfig	2006-04-18 09:13:09 +0100
@@ -195,7 +195,7 @@ config ACPI_ASUS
           
 config ACPI_IBM
 	tristate "IBM ThinkPad Laptop Extras"
-	depends on X86
+	depends on X86 && BACKLIGHT_DEVICE
 	---help---
 	  This is a Linux ACPI driver for the IBM ThinkPad laptops. It adds
 	  support for Fn-Fx key combinations, Bluetooth control, video

-- 
Matthew Garrett | mjg59@srcf.ucam.org

PATCH [3/3]: Provide generic backlight support in Toshiba ACPI driver

[Matthew Garrett]

This patch provides support for altering Toshiba backlight brightness 
via /sys/class/backlight. It does not remove the legacy /proc interface.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff -urp drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
--- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
+++ a/drivers/acpi/Kconfig	2006-04-18 09:24:45 +0100
@@ -219,7 +219,7 @@ config ACPI_IBM_DOCK
 
 config ACPI_TOSHIBA
 	tristate "Toshiba Laptop Extras"
-	depends on X86
+	depends on X86 && BACKLIGHT_DEVICE
 	---help---
 	  This driver adds support for access to certain system settings
 	  on "legacy free" Toshiba laptops.  These laptops can be recognized by
diff -urp drivers/acpi.bak/toshiba_acpi.c drivers/acpi/toshiba_acpi.c
--- drivers/acpi.bak/toshiba_acpi.c	2006-04-03 04:22:10 +0100
+++ a/drivers/acpi/toshiba_acpi.c	2006-04-18 09:26:41 +0100
@@ -41,6 +41,7 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/backlight.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_drivers.h>
@@ -97,6 +98,8 @@ MODULE_LICENSE("GPL");
 #define HCI_VIDEO_OUT_CRT		0x2
 #define HCI_VIDEO_OUT_TV		0x4
 
+static struct backlight_device *tosh_backlight_device;
+
 /* utility
  */
 
@@ -289,6 +292,19 @@ static char *read_lcd(char *p)
 	return p;
 }
 
+static int tosh_get_brightness(struct backlight_device *bd)
+{
+	int brightness;
+	u32 hci_result;
+
+	hci_read1(HCI_LCD_BRIGHTNESS, &brightness, &hci_result);
+	if (hci_result == HCI_SUCCESS) {
+		brightness = brightness >> HCI_LCD_BRIGHTNESS_SHIFT;
+		return brightness;
+	} else
+		return 0;
+}
+
 static unsigned long write_lcd(const char *buffer, unsigned long count)
 {
 	int value;
@@ -307,6 +323,28 @@ static unsigned long write_lcd(const cha
 	return count;
 }
 
+static int tosh_set_brightness(struct backlight_device *bd, int brightness)
+{
+	u32 hci_result;
+
+	if (brightness >= 0 && brightness < HCI_LCD_BRIGHTNESS_LEVELS) {
+		brightness= brightness << HCI_LCD_BRIGHTNESS_SHIFT;
+		hci_write1(HCI_LCD_BRIGHTNESS, brightness, &hci_result);
+		if (hci_result != HCI_SUCCESS)
+			return -EFAULT;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tosh_update_brightness(struct backlight_device *bd)
+{
+	int value = bd->props->brightness;
+	return tosh_set_brightness(bd, value);
+}
+
 static char *read_video(char *p)
 {
 	u32 hci_result;
@@ -477,6 +515,13 @@ static ProcItem proc_items[] = {
 	{NULL}
 };
 
+static struct backlight_properties toshbl_data = {
+	.owner          = THIS_MODULE,
+	.get_brightness = tosh_get_brightness,
+	.update_status  = tosh_update_brightness,
+	.max_brightness = HCI_LCD_BRIGHTNESS_LEVELS,
+};
+
 static acpi_status __init add_device(void)
 {
 	struct proc_dir_entry *proc;
@@ -546,6 +591,12 @@ static int __init toshiba_acpi_init(void
 			remove_proc_entry(PROC_TOSHIBA, acpi_root_dir);
 	}
 
+	tosh_backlight_device = backlight_device_register ("tosh-bl", NULL,
+							   &toshbl_data);
+
+	if (IS_ERR (tosh_backlight_device))
+		printk("Failed to register Toshiba backlight device\n");
+
 	return (ACPI_SUCCESS(status)) ? 0 : -ENODEV;
 }
 
@@ -556,6 +607,8 @@ static void __exit toshiba_acpi_exit(voi
 	if (toshiba_proc_dir)
 		remove_proc_entry(PROC_TOSHIBA, acpi_root_dir);
 
+	backlight_device_unregister(tosh_backlight_device);
+
 	return;
 }

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: PATCH [2/3]: Provide generic backlight support in IBM ACPI driver

[Henrik Brix Andersen]

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

On Tue, Apr 18, 2006 at 09:30:56AM +0100, Matthew Garrett wrote:
> diff -urp drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
> --- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
> +++ a/drivers/acpi/Kconfig	2006-04-18 09:13:09 +0100
> @@ -195,7 +195,7 @@ config ACPI_ASUS
>            
>  config ACPI_IBM
>  	tristate "IBM ThinkPad Laptop Extras"
> -	depends on X86
> +	depends on X86 && BACKLIGHT_DEVICE

Wouldn't it be better to have ACPI_IBM and friends select
BACKLIGHT_DEVICE?

Regards,
Brix
-- 
Henrik Brix Andersen <brix@gentoo.org>
Gentoo Metadistribution | Mobile computing herd

[-- Attachment #2: Type: application/pgp-signature, Size: 213 bytes --]

Re: PATCH [2/3]: Provide generic backlight support in IBM ACPI driver

[Matthew Garrett]

On Tue, Apr 18, 2006 at 11:18:57AM +0200, Henrik Brix Andersen wrote:

> Wouldn't it be better to have ACPI_IBM and friends select
> BACKLIGHT_DEVICE?

Hmm. Probably, yes.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Patrick Mochel]

On Tue, Apr 18, 2006 at 09:29:52AM +0100, Matthew Garrett wrote:
> This allows Asus backlight control to be performed via 
> /sys/class/backlight. It does not currently remove the legacy /proc 
> interface.

Neat, though a few questions that apply to each of the three ACPI platform drivers..


> diff -urp drivers/acpi.bak/asus_acpi.c drivers/acpi/asus_acpi.c

> +static struct backlight_device *asus_backlight_device;
> +

Why is this dynamically allocated? If there is only one per driver, then the
register() function could take that as a parameter -- instead of passing various
variable to initialize it with -- and return an error. 

> -static int read_brightness(void)
> +static int read_brightness(struct backlight_device *bd)

It seems that you're always passing NULL to this. And, if you're not passing NULL,
aren't you always referencing the single global instance above? 

> -static void set_brightness(int value)
> +static int set_brightness(struct backlight_device *bd, int value)
>  {
>  	acpi_status status = 0;
>  
> +	value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
> +	/* 0 <= value <= 15 */

Is there something wrong with:

	if (value < 0)
		value = 0;
	else if (value > 15)
		value = 15;

? Or, could you just make the parameter an unsigned int and just keep the 2nd check? 

> -		value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
> -		/* 0 <= value <= 15 */
> -		set_brightness(value);
> +		set_brightness(NULL,value);

There should be a space between parameters. 

> +	asus_backlight_device = backlight_device_register ("asus_bl", NULL,
> +							   &asusbl_data);
> +
> +	if (IS_ERR (asus_backlight_device)) {
> +		printk("Unable to register backlight\n");

Could you print the name of the driver? 

> +		acpi_bus_unregister_driver(&asus_hotk_driver);
> +		remove_proc_entry(PROC_ASUS, acpi_root_dir);

If the backlight fails to register, does it mean that these must also fail? 


Thanks,


	Patrick

Re: PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Matthew Garrett]

On Tue, Apr 18, 2006 at 09:11:00AM -0700, Patrick Mochel wrote:

> > +static struct backlight_device *asus_backlight_device;
> > +
> 
> Why is this dynamically allocated? If there is only one per driver, then the
> register() function could take that as a parameter -- instead of passing various
> variable to initialize it with -- and return an error. 

Fair enough.

> > -static int read_brightness(void)
> > +static int read_brightness(struct backlight_device *bd)
> 
> It seems that you're always passing NULL to this. And, if you're not passing NULL,
> aren't you always referencing the single global instance above? 

Yeah, that's a holdover from an earlier version of the backlight 
interface. I'll fix that up.

> > -static void set_brightness(int value)
> > +static int set_brightness(struct backlight_device *bd, int value)
> >  {
> >  	acpi_status status = 0;
> >  
> > +	value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
> > +	/* 0 <= value <= 15 */
> 
> Is there something wrong with:
> 
> 	if (value < 0)
> 		value = 0;
> 	else if (value > 15)
> 		value = 15;

That's just cut and paste from the existing driver code - your version 
makes more sense.

> > -		value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
> > -		/* 0 <= value <= 15 */
> > -		set_brightness(value);
> > +		set_brightness(NULL,value);
> 
> There should be a space between parameters. 

Ok.

> > +	asus_backlight_device = backlight_device_register ("asus_bl", NULL,
> > +							   &asusbl_data);
> > +
> > +	if (IS_ERR (asus_backlight_device)) {
> > +		printk("Unable to register backlight\n");
> 
> Could you print the name of the driver? 

Sure.

> > +		acpi_bus_unregister_driver(&asus_hotk_driver);
> > +		remove_proc_entry(PROC_ASUS, acpi_root_dir);
> 
> If the backlight fails to register, does it mean that these must also fail? 

Hm. Good question. The rest of the driver ought to work even without 
backlight registration, but that sounds like an edge case.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Matthew Garrett]

Here are some fixed-up versions of my patches to move acpi drivers 
towards using the standard backlight interface. I've kept the dynamic 
allocation of backlight_device for now, since changing that would also 
mean changing the backlight core code and updating other drivers.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff -ur drivers/acpi.bak/asus_acpi.c drivers/acpi/asus_acpi.c
--- drivers/acpi.bak/asus_acpi.c	2006-04-03 04:22:10 +0100
+++ a/drivers/acpi/asus_acpi.c	2006-04-19 19:29:50 +0100
@@ -38,6 +38,8 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
 #include <asm/uaccess.h>
@@ -82,6 +84,8 @@
 module_param(asus_gid, uint, 0);
 MODULE_PARM_DESC(asus_gid, "GID for entries in /proc/acpi/asus.\n");
 
+static struct backlight_device *asus_backlight_device;
+
 /* For each model, all features implemented, 
  * those marked with R are relative to HOTK, A for absolute */
 struct model_data {
@@ -689,7 +693,7 @@
 	return count;
 }
 
-static int read_brightness(void)
+static int read_brightness(struct backlight_device *bd)
 {
 	int value;
 
@@ -711,39 +715,50 @@
 /*
  * Change the brightness level
  */
-static void set_brightness(int value)
+static int set_brightness(unsigned int value)
 {
 	acpi_status status = 0;
 
+	if (value > 15)
+		value = 15;
+
 	/* SPLV laptop */
 	if (hotk->methods->brightness_set) {
 		if (!write_acpi_int(hotk->handle, hotk->methods->brightness_set,
 				    value, NULL))
 			printk(KERN_WARNING
 			       "Asus ACPI: Error changing brightness\n");
-		return;
+		return -EINVAL;
 	}
 
 	/* No SPLV method if we are here, act as appropriate */
-	value -= read_brightness();
+	value -= read_brightness(NULL);
 	while (value != 0) {
 		status = acpi_evaluate_object(NULL, (value > 0) ?
 					      hotk->methods->brightness_up :
 					      hotk->methods->brightness_down,
 					      NULL, NULL);
 		(value > 0) ? value-- : value++;
-		if (ACPI_FAILURE(status))
+		if (ACPI_FAILURE(status)) {
 			printk(KERN_WARNING
 			       "Asus ACPI: Error changing brightness\n");
+			return -EINVAL;
+		}		
 	}
-	return;
+	return 0;
+}
+
+static int update_brightness(struct backlight_device *bd)
+{
+	int value = bd->props->brightness;
+	return set_brightness (value);
 }
 
 static int
 proc_read_brn(char *page, char **start, off_t off, int count, int *eof,
 	      void *data)
 {
-	return sprintf(page, "%d\n", read_brightness());
+	return sprintf(page, "%d\n", read_brightness(NULL));
 }
 
 static int
@@ -754,9 +769,7 @@
 
 	count = parse_arg(buffer, count, &value);
 	if (count > 0) {
-		value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
-		/* 0 <= value <= 15 */
-		set_brightness(value);
+		set_brightness(value);
 	} else if (count < 0) {
 		printk(KERN_WARNING "Asus ACPI: Error reading user input\n");
 	}
@@ -1207,6 +1220,13 @@
 	return 0;
 }
 
+static struct backlight_properties asusbl_data = {
+	.owner          = THIS_MODULE,
+	.get_brightness = read_brightness,
+	.update_status  = update_brightness,
+	.max_brightness = 15,
+};
+
 static int __init asus_acpi_init(void)
 {
 	int result;
@@ -1232,15 +1252,26 @@
 		return -ENODEV;
 	}
 
+	asus_backlight_device = backlight_device_register("asus_bl", NULL,
+							   &asusbl_data);
+
+	if (IS_ERR (asus_backlight_device)) {
+		printk("Unable to register ASUS backlight\n");
+		asus_backlight_device = NULL;
+	}
+
 	return 0;
 }
 
 static void __exit asus_acpi_exit(void)
 {
-	acpi_bus_unregister_driver(&asus_hotk_driver);
-	remove_proc_entry(PROC_ASUS, acpi_root_dir);
+	if (asus_backlight_device)
+		backlight_device_unregister(asus_backlight_device);
+
+	acpi_bus_unregister_driver (&asus_hotk_driver);
+	remove_proc_entry (PROC_ASUS, acpi_root_dir);
 
-	acpi_os_free(asus_info);
+	acpi_os_free (asus_info);
 
 	return;
 }
diff -ur drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
--- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
+++ a/drivers/acpi/Kconfig	2006-04-19 19:27:08 +0100
@@ -168,6 +168,7 @@
 config ACPI_ASUS
         tristate "ASUS/Medion Laptop Extras"
 	depends on X86
+	select BACKLIGHT_DEVICE
         ---help---
           This driver provides support for extra features of ACPI-compatible
           ASUS laptops. As some of Medion laptops are made by ASUS, it may also


-- 
Matthew Garrett | mjg59@srcf.ucam.org

PATCH [2/3]: Provide generic backlight support in IBM ACPI driver

[Matthew Garrett]

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff -ur drivers/acpi.bak/ibm_acpi.c drivers/acpi/ibm_acpi.c
--- drivers/acpi.bak/ibm_acpi.c	2006-04-03 04:22:10 +0100
+++ a/drivers/acpi/ibm_acpi.c	2006-04-19 19:40:48 +0100
@@ -78,6 +78,8 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_drivers.h>
@@ -243,6 +245,8 @@
 
 static struct proc_dir_entry *proc_dir = NULL;
 
+static struct backlight_device *ibm_backlight_device;
+
 #define onoff(status,bit) ((status) & (1 << (bit)) ? "on" : "off")
 #define enabled(status,bit) ((status) & (1 << (bit)) ? "enabled" : "disabled")
 #define strlencmp(a,b) (strncmp((a), (b), strlen(b)))
@@ -1318,15 +1322,25 @@
 
 static int brightness_offset = 0x31;
 
+static int brightness_read_value(struct backlight_device *bd)
+{
+	u8 level;
+	if (!acpi_ec_read(brightness_offset, &level))
+		return -EIO;
+
+	level &= 0x7;
+	return level;
+}
+
 static int brightness_read(char *p)
 {
 	int len = 0;
-	u8 level;
+	int level = brightness_read_value(NULL);
 
-	if (!acpi_ec_read(brightness_offset, &level)) {
+	if (level < 0) {
 		len += sprintf(p + len, "level:\t\tunreadable\n");
 	} else {
-		len += sprintf(p + len, "level:\t\t%d\n", level & 0x7);
+		len += sprintf(p + len, "level:\t\t%d\n", level);
 		len += sprintf(p + len, "commands:\tup, down\n");
 		len += sprintf(p + len, "commands:\tlevel <level>"
 			       " (<level> is 0-7)\n");
@@ -1338,9 +1352,38 @@
 #define BRIGHTNESS_UP	4
 #define BRIGHTNESS_DOWN	5
 
+static int brightness_write_value(int value)
+{
+	int current_value = brightness_read_value(NULL);
+	int inc;
+	int cmos_cmd; 
+
+	value &= 7;
+
+	cmos_cmd = value > current_value ? BRIGHTNESS_UP : BRIGHTNESS_DOWN;
+
+	inc = value > current_value ? 1 : -1;
+
+	while (value != current_value) {
+		current_value += inc;
+
+		if (!cmos_eval(cmos_cmd))
+			return -EIO;
+		if (!acpi_ec_write(brightness_offset, current_value))
+			return -EIO;
+	}
+
+	return 0;
+}
+
+static int brightness_update_value(struct backlight_device *bd)
+{
+	int value = bd->props->brightness;
+	return brightness_write_value(value);
+}
+
 static int brightness_write(char *buf)
 {
-	int cmos_cmd, inc, i;
 	u8 level;
 	int new_level;
 	char *cmd;
@@ -1360,14 +1403,8 @@
 		} else
 			return -EINVAL;
 
-		cmos_cmd = new_level > level ? BRIGHTNESS_UP : BRIGHTNESS_DOWN;
-		inc = new_level > level ? 1 : -1;
-		for (i = level; i != new_level; i += inc) {
-			if (!cmos_eval(cmos_cmd))
-				return -EIO;
-			if (!acpi_ec_write(brightness_offset, i + inc))
-				return -EIO;
-		}
+		brightness_write_value(new_level);
+		
 	}
 
 	return 0;
@@ -1895,10 +1932,21 @@
 IBM_PARAM(volume);
 IBM_PARAM(fan);
 
+
+static struct backlight_properties ibmbl_data = {
+        .owner          = THIS_MODULE,
+        .get_brightness = brightness_read_value,
+        .update_status  = brightness_update_value,
+        .max_brightness = 7,
+};
+
 static void acpi_ibm_exit(void)
 {
 	int i;
 
+	if (ibm_backlight_device)
+		backlight_device_unregister(ibm_backlight_device);
+
 	for (i = ARRAY_SIZE(ibms) - 1; i >= 0; i--)
 		ibm_exit(&ibms[i]);
 
@@ -1966,6 +2014,13 @@
 		}
 	}
 
+	ibm_backlight_device = backlight_device_register ("ibm_bl", NULL,
+							  &ibmbl_data);
+        if (IS_ERR (ibm_backlight_device)) {
+		printk(KERN_ERR "Unable to register IBM backlight driver\n");
+		ibm_backlight_device = NULL;
+	}
+
 	return 0;
 }
 
diff -ur drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
--- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
+++ a/drivers/acpi/Kconfig	2006-04-19 19:39:03 +0100
@@ -196,6 +196,7 @@
 config ACPI_IBM
 	tristate "IBM ThinkPad Laptop Extras"
 	depends on X86
+	select BACKLIGHT_DEVICE
 	---help---
 	  This is a Linux ACPI driver for the IBM ThinkPad laptops. It adds
 	  support for Fn-Fx key combinations, Bluetooth control, video

-- 
Matthew Garrett | mjg59@srcf.ucam.org

PATCH [3/3]: Provide generic backlight support in Toshiba ACPI driver

[Matthew Garrett]

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

diff -ur drivers/acpi.bak/Kconfig drivers/acpi/Kconfig
--- drivers/acpi.bak/Kconfig	2006-04-18 08:51:49 +0100
+++ a/drivers/acpi/Kconfig	2006-04-19 19:37:08 +0100
@@ -220,6 +220,7 @@
 config ACPI_TOSHIBA
 	tristate "Toshiba Laptop Extras"
 	depends on X86
+	select BACKLIGHT_DEVICE
 	---help---
 	  This driver adds support for access to certain system settings
 	  on "legacy free" Toshiba laptops.  These laptops can be recognized by
diff -ur drivers/acpi.bak/toshiba_acpi.c drivers/acpi/toshiba_acpi.c
--- drivers/acpi.bak/toshiba_acpi.c	2006-04-03 04:22:10 +0100
+++ a/drivers/acpi/toshiba_acpi.c	2006-04-19 19:37:48 +0100
@@ -41,6 +41,7 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/backlight.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_drivers.h>
@@ -97,6 +98,8 @@
 #define HCI_VIDEO_OUT_CRT		0x2
 #define HCI_VIDEO_OUT_TV		0x4
 
+static struct backlight_device *tosh_backlight_device;
+
 /* utility
  */
 
@@ -289,6 +292,19 @@
 	return p;
 }
 
+static int tosh_get_brightness(struct backlight_device *bd)
+{
+	int brightness;
+	u32 hci_result;
+
+	hci_read1(HCI_LCD_BRIGHTNESS, &brightness, &hci_result);
+	if (hci_result == HCI_SUCCESS) {
+		brightness = brightness >> HCI_LCD_BRIGHTNESS_SHIFT;
+		return brightness;
+	} else
+		return 0;
+}
+
 static unsigned long write_lcd(const char *buffer, unsigned long count)
 {
 	int value;
@@ -307,6 +323,28 @@
 	return count;
 }
 
+static int tosh_set_brightness(int brightness)
+{
+	u32 hci_result;
+
+	if (brightness >= 0 && brightness < HCI_LCD_BRIGHTNESS_LEVELS) {
+		brightness= brightness << HCI_LCD_BRIGHTNESS_SHIFT;
+		hci_write1(HCI_LCD_BRIGHTNESS, brightness, &hci_result);
+		if (hci_result != HCI_SUCCESS)
+			return -EFAULT;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tosh_update_brightness(struct backlight_device *bd)
+{
+	int value = bd->props->brightness;
+	return tosh_set_brightness(value);
+}
+
 static char *read_video(char *p)
 {
 	u32 hci_result;
@@ -477,6 +515,13 @@
 	{NULL}
 };
 
+static struct backlight_properties toshbl_data = {
+	.owner          = THIS_MODULE,
+	.get_brightness = tosh_get_brightness,
+	.update_status  = tosh_update_brightness,
+	.max_brightness = HCI_LCD_BRIGHTNESS_LEVELS,
+};
+
 static acpi_status __init add_device(void)
 {
 	struct proc_dir_entry *proc;
@@ -546,6 +591,14 @@
 			remove_proc_entry(PROC_TOSHIBA, acpi_root_dir);
 	}
 
+	tosh_backlight_device = backlight_device_register ("tosh-bl", NULL,
+							   &toshbl_data);
+
+	if (IS_ERR (tosh_backlight_device)) {
+		printk("Failed to register Toshiba backlight device\n");
+		tosh_backlight_device = NULL;
+	}
+
 	return (ACPI_SUCCESS(status)) ? 0 : -ENODEV;
 }
 
@@ -556,6 +609,9 @@
 	if (toshiba_proc_dir)
 		remove_proc_entry(PROC_TOSHIBA, acpi_root_dir);
 
+	if (tosh_backlight_device) 
+		backlight_device_unregister(tosh_backlight_device);
+
 	return;
 }

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Patrick Mochel]

On Wed, Apr 19, 2006 at 07:49:09PM +0100, Matthew Garrett wrote:
> Here are some fixed-up versions of my patches to move acpi drivers 
> towards using the standard backlight interface. I've kept the dynamic 
> allocation of backlight_device for now, since changing that would also 
> mean changing the backlight core code and updating other drivers.

Nice, looks good to me. 

Thanks,


	Pat

Re: PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Henrik Brix Andersen]

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

On Wed, Apr 19, 2006 at 07:49:09PM +0100, Matthew Garrett wrote:
> Here are some fixed-up versions of my patches to move acpi drivers 
> towards using the standard backlight interface. I've kept the dynamic 
> allocation of backlight_device for now, since changing that would also 
> mean changing the backlight core code and updating other drivers.

Great stuff, I very much welcome these patches. Any plans for doing
the same for the sony_acpi.c driver found in -mm?

Regards,
Brix
-- 
Henrik Brix Andersen <brix@gentoo.org>
Gentoo Metadistribution | Mobile computing herd

[-- Attachment #2: Type: application/pgp-signature, Size: 213 bytes --]

Re: PATCH [1/3]: Provide generic backlight support in Asus ACPI driver

[Matthew Garrett]

On Wed, Apr 19, 2006 at 10:13:23PM +0200, Henrik Brix Andersen wrote:

> Great stuff, I very much welcome these patches. Any plans for doing
> the same for the sony_acpi.c driver found in -mm?

I have a half-finished one lying around - I'll check it against the -mm 
code at some stage.

-- 
Matthew Garrett | mjg59@srcf.ucam.org