[PATCH] Samsung backlight driver

[PATCH] Samsung backlight driver

[Greg KH]

From: Greg Kroah-Hartman <gregkh@suse.de>

This driver implements backlight controls for Samsung laptops that
currently do not have ACPI support for this control.

It has been tested on the N130 laptop and properly works there.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/platform/x86/Kconfig             |   12 ++
 drivers/platform/x86/Makefile            |    1 
 drivers/platform/x86/samsung-backlight.c |  146 +++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)

--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -425,4 +425,16 @@ config ACPI_TOSHIBA
 
 	  If you have a legacy free Toshiba laptop (such as the Libretto L1
 	  series), say Y.
+
+config SAMSUNG_BACKLIGHT
+	tristate "Samsung Backlight driver"
+	depends on BACKLIGHT_CLASS_DEVICE
+	depends on DMI
+	---help---
+	  This driver adds support to control the backlight on a number of
+	  Samsung laptops, like the N130.
+
+	  It will only be loaded on laptops that properly need it, so it is
+	  safe to say Y here.
+
 endif # X86_PLATFORM_DEVICES
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_INTEL_MENLOW)	+= intel_menl
 obj-$(CONFIG_ACPI_WMI)		+= wmi.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
+obj-$(CONFIG_SAMSUNG_BACKLIGHT)	+= samsung-backlight.o
--- /dev/null
+++ b/drivers/platform/x86/samsung-backlight.c
@@ -0,0 +1,146 @@
+/*
+ * Samsung N130 Laptop Backlight driver
+ *
+ * Copyright (C) 2009 Greg Kroah-Hartman (gregkh@suse.de)
+ * Copyright (C) 2009 Novell Inc.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/backlight.h>
+#include <linux/fb.h>
+#include <linux/dmi.h>
+
+#define MAX_BRIGHT	0xff
+#define OFFSET		0xf4
+
+static struct pci_dev *pci_device;
+static struct backlight_device *backlight_device;
+static int offset = OFFSET;
+static u8 current_brightness;
+
+static void read_brightness(void)
+{
+	if (!pci_device)
+		return;
+	pci_read_config_byte(pci_device, offset, &current_brightness);
+}
+
+static void set_brightness(void)
+{
+	if (!pci_device)
+		return;
+	pci_write_config_byte(pci_device, offset, current_brightness);
+}
+
+static int get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static int update_status(struct backlight_device *bd)
+{
+	if (!pci_device)
+		return -ENODEV;
+	if (!backlight_device)
+		return -ENODEV;
+
+	current_brightness = bd->props.brightness;
+	set_brightness();
+	return 0;
+}
+
+static struct backlight_ops backlight_ops = {
+	.get_brightness	= get_brightness,
+	.update_status	= update_status,
+};
+
+static int find_video_card(void)
+{
+	struct pci_dev *dev = NULL;
+
+	while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) {
+		/* Found one, so let's save it off */
+		if (!pci_device)
+			pci_device = pci_dev_get(dev);
+	}
+
+	if (!pci_device)
+		return 0;
+
+	/* create a backlight device to talk to this one */
+	backlight_device = backlight_device_register("samsung",
+						     &pci_device->dev,
+						     NULL, &backlight_ops);
+	if (IS_ERR(backlight_device))
+		return PTR_ERR(backlight_device);
+	read_brightness();
+	backlight_device->props.max_brightness = MAX_BRIGHT;
+	backlight_device->props.brightness = current_brightness;
+	backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(backlight_device);
+	return 0;
+}
+
+static void remove_video_card(void)
+{
+	if (!pci_device)
+		return;
+
+	backlight_device_unregister(backlight_device);
+	backlight_device = NULL;
+
+	/* we are done with the PCI device, put it back */
+	pci_dev_put(pci_device);
+	pci_device = NULL;
+}
+
+static int dmi_check_cb(const struct dmi_system_id *id)
+{
+	printk(KERN_INFO KBUILD_MODNAME ": found laptop model '%s'\n",
+		id->ident);
+	return 0;
+}
+
+static struct dmi_system_id __initdata samsung_dmi_table[] = {
+	{
+		.ident = "N130",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "N130"),
+			DMI_MATCH(DMI_BOARD_NAME, "N130"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{ },
+};
+
+static int __init samsung_init(void)
+{
+	if (!dmi_check_system(samsung_dmi_table))
+		return -ENODEV;
+
+	return find_video_card();
+}
+
+static void __exit samsung_exit(void)
+{
+	remove_video_card();
+}
+
+module_init(samsung_init);
+module_exit(samsung_exit);
+
+MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@suse.de>");
+MODULE_DESCRIPTION("Samsung N130 Backlight driver");
+MODULE_LICENSE("GPL");
+module_param(offset, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(offset, "The offset into the PCI device for the brightness control");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnN130:*:rnN130:*");

[PATCH] samsung-backlight: Added support for Samsung NC10 laptop

[Greg KH]

Info was provided by Soeren Sonnenburg <bugreports@nn7.de>

Cc: Soeren Sonnenburg <bugreports@nn7.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

--- a/drivers/platform/x86/samsung-backlight.c
+++ b/drivers/platform/x86/samsung-backlight.c
@@ -119,6 +119,15 @@ static struct dmi_system_id __initdata samsung_dmi_table[] = {
 		},
 		.callback = dmi_check_cb,
 	},
+	{
+		.ident = "NC10",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "NC10"),
+			DMI_MATCH(DMI_BOARD_NAME, "NC10"),
+		},
+		.callback = dmi_check_cb,
+	},
 	{ },
 };
 
@@ -144,3 +153,4 @@ MODULE_LICENSE("GPL");
 module_param(offset, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(offset, "The offset into the PCI device for the brightness control");
 MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnN130:*:rnN130:*");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnNC10:*:rnNC10:*");

Re: [PATCH] Samsung backlight driver

[Dmitry Torokhov]

Hi Greg,

On Fri, Aug 14, 2009 at 12:48:15PM -0700, Greg KH wrote:
> +
> +static int update_status(struct backlight_device *bd)
> +{
> +	if (!pci_device)
> +		return -ENODEV;
> +	if (!backlight_device)

How is this possible?

> +		return -ENODEV;
> +
> +	current_brightness = bd->props.brightness;
> +	set_brightness();
> +	return 0;
> +}
> +
> +static struct backlight_ops backlight_ops = {
> +	.get_brightness	= get_brightness,
> +	.update_status	= update_status,
> +};
> +
> +static int find_video_card(void)
> +{
> +	struct pci_dev *dev = NULL;
> +
> +	while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) {
> +		/* Found one, so let's save it off */
> +		if (!pci_device)
> +			pci_device = pci_dev_get(dev);

If you need to iterate all matching devices you need to save them all or
release references to "extras" right here... Or, better yet, just break
after you find the first one.

> +	}
> +
> +	if (!pci_device)
> +		return 0;

Why not -ENODEV? What is the point of having this driver loaded if the
device is not available? Then you could drop all silly "if (!pci_device)"
checks.

-- 
Dmitry

Re: [PATCH] Samsung backlight driver

[Greg KH]

On Fri, Aug 14, 2009 at 02:54:40PM -0700, Dmitry Torokhov wrote:
> Hi Greg,
> 
> On Fri, Aug 14, 2009 at 12:48:15PM -0700, Greg KH wrote:
> > +
> > +static int update_status(struct backlight_device *bd)
> > +{
> > +	if (!pci_device)
> > +		return -ENODEV;
> > +	if (!backlight_device)
> 
> How is this possible?

It's probably not, I was just being paranoid.  You're right, I can drop
all of these checks and rework the find logic a bit.

> > +		return -ENODEV;
> > +
> > +	current_brightness = bd->props.brightness;
> > +	set_brightness();
> > +	return 0;
> > +}
> > +
> > +static struct backlight_ops backlight_ops = {
> > +	.get_brightness	= get_brightness,
> > +	.update_status	= update_status,
> > +};
> > +
> > +static int find_video_card(void)
> > +{
> > +	struct pci_dev *dev = NULL;
> > +
> > +	while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) {
> > +		/* Found one, so let's save it off */
> > +		if (!pci_device)
> > +			pci_device = pci_dev_get(dev);
> 
> If you need to iterate all matching devices you need to save them all or
> release references to "extras" right here... Or, better yet, just break
> after you find the first one.

It's a trivially small loop, this way I get the "hm, do I need to drop
the reference or not" logic correct :)

> > +	}
> > +
> > +	if (!pci_device)
> > +		return 0;
> 
> Why not -ENODEV? What is the point of having this driver loaded if the
> device is not available? Then you could drop all silly "if (!pci_device)"
> checks.

Ah, you are right, thanks, I'll go fix that.

thanks for the review, I appreciate it.

greg k-h

Re: [PATCH] Samsung backlight driver

[Greg KH]

Here's an updated version, with two more laptop models supported, and
the comments from Dmitry added in.

thanks,

greg k-h

----------------


From: Greg Kroah-Hartman <gregkh@suse.de>

This driver implements backlight controls for Samsung laptops that
currently do not have ACPI support for this control.

It has been tested on the N130 laptop and properly works there.

Info for the NC10 was provided by Soeren Sonnenburg <bugreports@nn7.de>
Info for the NP-Q45 from Jérémie Huchet <jeremie@lamah.info>

Cc: Soeren Sonnenburg <bugreports@nn7.de>
Cc: Jérémie Huchet <jeremie@lamah.info>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/platform/x86/Kconfig             |   12 ++
 drivers/platform/x86/Makefile            |    1 
 drivers/platform/x86/samsung-backlight.c |  168 +++++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)

--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -425,4 +425,16 @@ config ACPI_TOSHIBA
 
 	  If you have a legacy free Toshiba laptop (such as the Libretto L1
 	  series), say Y.
+
+config SAMSUNG_BACKLIGHT
+	tristate "Samsung Backlight driver"
+	depends on BACKLIGHT_CLASS_DEVICE
+	depends on DMI
+	---help---
+	  This driver adds support to control the backlight on a number of
+	  Samsung laptops, like the N130.
+
+	  It will only be loaded on laptops that properly need it, so it is
+	  safe to say Y here.
+
 endif # X86_PLATFORM_DEVICES
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_INTEL_MENLOW)	+= intel_menl
 obj-$(CONFIG_ACPI_WMI)		+= wmi.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
+obj-$(CONFIG_SAMSUNG_BACKLIGHT)	+= samsung-backlight.o
--- /dev/null
+++ b/drivers/platform/x86/samsung-backlight.c
@@ -0,0 +1,168 @@
+/*
+ * Samsung N130 and NC10 Laptop Backlight driver
+ *
+ * Copyright (C) 2009 Greg Kroah-Hartman (gregkh@suse.de)
+ * Copyright (C) 2009 Novell Inc.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/backlight.h>
+#include <linux/fb.h>
+#include <linux/dmi.h>
+
+#define MAX_BRIGHT	0xff
+#define OFFSET		0xf4
+
+static struct pci_dev *pci_device;
+static struct backlight_device *backlight_device;
+static int offset = OFFSET;
+static u8 current_brightness;
+
+static void read_brightness(void)
+{
+	if (!pci_device)
+		return;
+	pci_read_config_byte(pci_device, offset, &current_brightness);
+}
+
+static void set_brightness(void)
+{
+	if (!pci_device)
+		return;
+	pci_write_config_byte(pci_device, offset, current_brightness);
+}
+
+static int get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static int update_status(struct backlight_device *bd)
+{
+	if (!pci_device)
+		return -ENODEV;
+
+	current_brightness = bd->props.brightness;
+	set_brightness();
+	return 0;
+}
+
+static struct backlight_ops backlight_ops = {
+	.get_brightness	= get_brightness,
+	.update_status	= update_status,
+};
+
+static int find_video_card(void)
+{
+	struct pci_dev *dev = NULL;
+
+	while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) {
+		/*
+		 * Found one, so let's save it off and break
+		 * Note that the reference is still raised on
+		 * the PCI device here.
+		 */
+		pci_device = dev;
+		break;
+	}
+
+	if (!pci_device)
+		return -ENODEV;
+
+	/* create a backlight device to talk to this one */
+	backlight_device = backlight_device_register("samsung",
+						     &pci_device->dev,
+						     NULL, &backlight_ops);
+	if (IS_ERR(backlight_device))
+		return PTR_ERR(backlight_device);
+	read_brightness();
+	backlight_device->props.max_brightness = MAX_BRIGHT;
+	backlight_device->props.brightness = current_brightness;
+	backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(backlight_device);
+	return 0;
+}
+
+static void remove_video_card(void)
+{
+	if (!pci_device)
+		return;
+
+	backlight_device_unregister(backlight_device);
+	backlight_device = NULL;
+
+	/* we are done with the PCI device, put it back */
+	pci_dev_put(pci_device);
+	pci_device = NULL;
+}
+
+static int dmi_check_cb(const struct dmi_system_id *id)
+{
+	printk(KERN_INFO KBUILD_MODNAME ": found laptop model '%s'\n",
+		id->ident);
+	return 0;
+}
+
+static struct dmi_system_id __initdata samsung_dmi_table[] = {
+	{
+		.ident = "N130",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "N130"),
+			DMI_MATCH(DMI_BOARD_NAME, "N130"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
+		.ident = "NC10",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "NC10"),
+			DMI_MATCH(DMI_BOARD_NAME, "NC10"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
+		.ident = "NP-Q45",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "SQ45S70S"),
+			DMI_MATCH(DMI_BOARD_NAME, "SQ45S70S"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{ },
+};
+
+static int __init samsung_init(void)
+{
+	if (!dmi_check_system(samsung_dmi_table))
+		return -ENODEV;
+
+	return find_video_card();
+}
+
+static void __exit samsung_exit(void)
+{
+	remove_video_card();
+}
+
+module_init(samsung_init);
+module_exit(samsung_exit);
+
+MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@suse.de>");
+MODULE_DESCRIPTION("Samsung Backlight driver");
+MODULE_LICENSE("GPL");
+module_param(offset, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(offset, "The offset into the PCI device for the brightness control");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnN130:*:rnN130:*");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnNC10:*:rnNC10:*");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnSQ45S70S:*:rnSQ45S70S:*");

Re: [PATCH] Samsung backlight driver

[Dmitry Torokhov]

On Fri, Aug 14, 2009 at 03:48:35PM -0700, Greg KH wrote:
> Here's an updated version, with two more laptop models supported, and
> the comments from Dmitry added in.
> 

You are leaking pci device reference in error unwinding path....

Here you go, just fold it together with yours.

-- 
Dmitry

Fix PCI device reference leak in error handling path; move more code
into __init/__exit sections; simplify searching for video card and
use less globals.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/platform/x86/samsung-backlight.c |  100 +++++++++++-------------------
 1 files changed, 36 insertions(+), 64 deletions(-)


diff --git a/drivers/platform/x86/samsung-backlight.c b/drivers/platform/x86/samsung-backlight.c
index accc20e..8df5fc5 100644
--- a/drivers/platform/x86/samsung-backlight.c
+++ b/drivers/platform/x86/samsung-backlight.c
@@ -21,23 +21,24 @@
 #define MAX_BRIGHT	0xff
 #define OFFSET		0xf4
 
+static int offset = OFFSET;
+module_param(offset, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(offset, "The offset into the PCI device for the brightness control");
+
 static struct pci_dev *pci_device;
 static struct backlight_device *backlight_device;
-static int offset = OFFSET;
-static u8 current_brightness;
 
-static void read_brightness(void)
+static u8 read_brightness(void)
 {
-	if (!pci_device)
-		return;
-	pci_read_config_byte(pci_device, offset, &current_brightness);
+	u8 brightness;
+
+	pci_read_config_byte(pci_device, offset, &brightness);
+	return brightness;
 }
 
-static void set_brightness(void)
+static void set_brightness(u8 brightness)
 {
-	if (!pci_device)
-		return;
-	pci_write_config_byte(pci_device, offset, current_brightness);
+	pci_write_config_byte(pci_device, offset, brightness);
 }
 
 static int get_brightness(struct backlight_device *bd)
@@ -47,11 +48,7 @@ static int get_brightness(struct backlight_device *bd)
 
 static int update_status(struct backlight_device *bd)
 {
-	if (!pci_device)
-		return -ENODEV;
-
-	current_brightness = bd->props.brightness;
-	set_brightness();
+	set_brightness(bd->props.brightness);
 	return 0;
 }
 
@@ -60,51 +57,7 @@ static struct backlight_ops backlight_ops = {
 	.update_status	= update_status,
 };
 
-static int find_video_card(void)
-{
-	struct pci_dev *dev = NULL;
-
-	while ((dev = pci_get_device(0x8086, 0x27ae, dev)) != NULL) {
-		/*
-		 * Found one, so let's save it off and break
-		 * Note that the reference is still raised on
-		 * the PCI device here.
-		 */
-		pci_device = dev;
-		break;
-	}
-
-	if (!pci_device)
-		return -ENODEV;
-
-	/* create a backlight device to talk to this one */
-	backlight_device = backlight_device_register("samsung",
-						     &pci_device->dev,
-						     NULL, &backlight_ops);
-	if (IS_ERR(backlight_device))
-		return PTR_ERR(backlight_device);
-	read_brightness();
-	backlight_device->props.max_brightness = MAX_BRIGHT;
-	backlight_device->props.brightness = current_brightness;
-	backlight_device->props.power = FB_BLANK_UNBLANK;
-	backlight_update_status(backlight_device);
-	return 0;
-}
-
-static void remove_video_card(void)
-{
-	if (!pci_device)
-		return;
-
-	backlight_device_unregister(backlight_device);
-	backlight_device = NULL;
-
-	/* we are done with the PCI device, put it back */
-	pci_dev_put(pci_device);
-	pci_device = NULL;
-}
-
-static int dmi_check_cb(const struct dmi_system_id *id)
+static int __init dmi_check_cb(const struct dmi_system_id *id)
 {
 	printk(KERN_INFO KBUILD_MODNAME ": found laptop model '%s'\n",
 		id->ident);
@@ -147,12 +100,33 @@ static int __init samsung_init(void)
 	if (!dmi_check_system(samsung_dmi_table))
 		return -ENODEV;
 
-	return find_video_card();
+	pci_device = pci_get_device(0x8086, 0x27ae, NULL);
+	if (!pci_device)
+		return -ENODEV;
+
+	/* create a backlight device to talk to this one */
+	backlight_device = backlight_device_register("samsung",
+						     &pci_device->dev,
+						     NULL, &backlight_ops);
+	if (IS_ERR(backlight_device)) {
+		pci_dev_put(pci_device);
+		return PTR_ERR(backlight_device);
+	}
+
+	backlight_device->props.max_brightness = MAX_BRIGHT;
+	backlight_device->props.brightness = read_brightness();
+	backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(backlight_device);
+
+	return 0;
 }
 
 static void __exit samsung_exit(void)
 {
-	remove_video_card();
+	backlight_device_unregister(backlight_device);
+
+	/* we are done with the PCI device, put it back */
+	pci_dev_put(pci_device);
 }
 
 module_init(samsung_init);
@@ -161,8 +135,6 @@ module_exit(samsung_exit);
 MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@suse.de>");
 MODULE_DESCRIPTION("Samsung Backlight driver");
 MODULE_LICENSE("GPL");
-module_param(offset, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(offset, "The offset into the PCI device for the brightness control");
 MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnN130:*:rnN130:*");
 MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnNC10:*:rnNC10:*");
 MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnSQ45S70S:*:rnSQ45S70S:*");

Re: [PATCH] Samsung backlight driver

[Greg KH]

On Sat, Aug 15, 2009 at 12:15:50AM -0700, Dmitry Torokhov wrote:
> On Fri, Aug 14, 2009 at 03:48:35PM -0700, Greg KH wrote:
> > Here's an updated version, with two more laptop models supported, and
> > the comments from Dmitry added in.
> > 
> 
> You are leaking pci device reference in error unwinding path....
> 
> Here you go, just fold it together with yours.

Thank you very much for the patch, I appreciate it.

Jérémie, I've now added the pci id for your device, so the driver should
now work for it as well.  Let me know if you have any problems with it.

I've included an updated version below that also adds support for the
N120 laptop series.

thanks,

greg k-h

-----------------------

From: Greg Kroah-Hartman <gregkh@suse.de>
Subject: Samsung backlight driver

This driver implements backlight controls for Samsung laptops that
currently do not have ACPI support for this control.

It has been tested on the N130 laptop and properly works there.

Info for the NC10 was provided by Soeren Sonnenburg <bugreports@nn7.de>
Info for the NP-Q45 from Jérémie Huchet <jeremie@lamah.info>

Many thanks to Dmitry Torokhov <dmitry.torokhov@gmail.com> for cleanups
and other suggestions on how to make the driver simpler.

Cc: Soeren Sonnenburg <bugreports@nn7.de>
Cc: Jérémie Huchet <jeremie@lamah.info>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/platform/x86/Kconfig             |   12 ++
 drivers/platform/x86/Makefile            |    1 
 drivers/platform/x86/samsung-backlight.c |  157 +++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)

--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -425,4 +425,16 @@ config ACPI_TOSHIBA
 
 	  If you have a legacy free Toshiba laptop (such as the Libretto L1
 	  series), say Y.
+
+config SAMSUNG_BACKLIGHT
+	tristate "Samsung Backlight driver"
+	depends on BACKLIGHT_CLASS_DEVICE
+	depends on DMI
+	---help---
+	  This driver adds support to control the backlight on a number of
+	  Samsung laptops, like the N130.
+
+	  It will only be loaded on laptops that properly need it, so it is
+	  safe to say Y here.
+
 endif # X86_PLATFORM_DEVICES
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_INTEL_MENLOW)	+= intel_menl
 obj-$(CONFIG_ACPI_WMI)		+= wmi.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
+obj-$(CONFIG_SAMSUNG_BACKLIGHT)	+= samsung-backlight.o
--- /dev/null
+++ b/drivers/platform/x86/samsung-backlight.c
@@ -0,0 +1,157 @@
+/*
+ * Samsung N130 and NC10 Laptop Backlight driver
+ *
+ * Copyright (C) 2009 Greg Kroah-Hartman (gregkh@suse.de)
+ * Copyright (C) 2009 Novell Inc.
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/backlight.h>
+#include <linux/fb.h>
+#include <linux/dmi.h>
+
+#define MAX_BRIGHT	0xff
+#define OFFSET		0xf4
+
+static int offset = OFFSET;
+module_param(offset, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(offset, "The offset into the PCI device for the brightness control");
+
+static struct pci_dev *pci_device;
+static struct backlight_device *backlight_device;
+
+static u8 read_brightness(void)
+{
+	u8 brightness;
+
+	pci_read_config_byte(pci_device, offset, &brightness);
+	return brightness;
+}
+
+static void set_brightness(u8 brightness)
+{
+	pci_write_config_byte(pci_device, offset, brightness);
+}
+
+static int get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static int update_status(struct backlight_device *bd)
+{
+	set_brightness(bd->props.brightness);
+	return 0;
+}
+
+static struct backlight_ops backlight_ops = {
+	.get_brightness	= get_brightness,
+	.update_status	= update_status,
+};
+
+static int __init dmi_check_cb(const struct dmi_system_id *id)
+{
+	printk(KERN_INFO KBUILD_MODNAME ": found laptop model '%s'\n",
+		id->ident);
+	return 0;
+}
+
+static struct dmi_system_id __initdata samsung_dmi_table[] = {
+	{
+		.ident = "N120",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "N120"),
+			DMI_MATCH(DMI_BOARD_NAME, "N120"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
+		.ident = "N130",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "N130"),
+			DMI_MATCH(DMI_BOARD_NAME, "N130"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
+		.ident = "NC10",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "NC10"),
+			DMI_MATCH(DMI_BOARD_NAME, "NC10"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
+		.ident = "NP-Q45",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "SQ45S70S"),
+			DMI_MATCH(DMI_BOARD_NAME, "SQ45S70S"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{ },
+};
+
+static int __init samsung_init(void)
+{
+	if (!dmi_check_system(samsung_dmi_table))
+		return -ENODEV;
+
+	/*
+	 * The Samsung N120, N130, and NC10 use pci device id 0x27ae, while the
+	 * NP-Q45 uses 0x2a02.  Odds are we might need to add more to the
+	 * list over time...
+	 */
+	pci_device = pci_get_device(PCI_VENDOR_ID_INTEL, 0x27ae, NULL);
+	if (!pci_device) {
+		pci_device = pci_get_device(PCI_VENDOR_ID_INTEL, 0x2a02, NULL);
+		if (!pci_device)
+			return -ENODEV;
+	}
+
+	/* create a backlight device to talk to this one */
+	backlight_device = backlight_device_register("samsung",
+						     &pci_device->dev,
+						     NULL, &backlight_ops);
+	if (IS_ERR(backlight_device)) {
+		pci_dev_put(pci_device);
+		return PTR_ERR(backlight_device);
+	}
+
+	backlight_device->props.max_brightness = MAX_BRIGHT;
+	backlight_device->props.brightness = read_brightness();
+	backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(backlight_device);
+
+	return 0;
+}
+
+static void __exit samsung_exit(void)
+{
+	backlight_device_unregister(backlight_device);
+
+	/* we are done with the PCI device, put it back */
+	pci_dev_put(pci_device);
+}
+
+module_init(samsung_init);
+module_exit(samsung_exit);
+
+MODULE_AUTHOR("Greg Kroah-Hartman <gregkh@suse.de>");
+MODULE_DESCRIPTION("Samsung Backlight driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnN120:*:rnN120:*");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnN130:*:rnN130:*");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnNC10:*:rnNC10:*");
+MODULE_ALIAS("dmi:*:svnSAMSUNGELECTRONICSCO.,LTD.:pnSQ45S70S:*:rnSQ45S70S:*");