lkml.org 
[lkml]   [2009]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] Samsung backlight driver
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:*");

\
 
 \ /
  Last update: 2009-08-15 09:19    [from the cache]
©2003-2011 Jasper Spaans