[PATCH 1/1] PXAFB: Support for backlight control

[PATCH 1/1] PXAFB: Support for backlight control

[Rodolfo Giometti]

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

Backlight control support for the PXA fram buffer.

Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>

---

Each platform should define the backlight properties in its own setup
file in "linux/arch/arm/mach-pxa/" as follow:

   static int pxafb_bl_get_brightness(struct backlight_device *bl_dev)
   {
   	return read_the_backlight_brightness();
   }

   static int pxafb_bl_update_status(struct backlight_device *bl_dev)
   {
           int perc, ret;

           if (bl_dev->props->power != FB_BLANK_UNBLANK ||
               bl_dev->props->fb_blank != FB_BLANK_UNBLANK)
                   perc = 0;
           else
                   perc = bl_dev->props->brightness;

   	write_the_backlight_brightness(perc);

           return 0;
   }

   static struct backlight_properties wwpc1100_backlight_props = {
           .get_brightness         = pxafb_bl_get_brightness,
           .update_status          = pxafb_bl_update_status,
           .max_brightness         = 100,
   };
      
and then seting up the fb info as follow:

   wwpc1100_pxafb_info.modes = &special_modes;
   wwpc1100_pxafb_info.bl_props = &wwpc1100_backlight_props;
   set_pxa_fb_info(&wwpc1100_pxafb_info);

[-- Attachment #2: patch-pxafb-backlight --]
[-- Type: text/plain, Size: 6009 bytes --]

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index c1536d7..1ee589e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1514,6 +1514,14 @@ config FB_PXA_PARAMETERS
 
 	  <file:Documentation/fb/pxafb.txt> describes the available parameters.
 
+config FB_PXA_BACKLIGHT
+	bool "Support for backlight control"
+	default y
+	depends on FB_PXA
+	select FB_BACKLIGHT
+	help
+	  Say Y here if you want to control the backlight of your display.
+
 config FB_MBX
 	tristate "2700G LCD framebuffer support"
 	depends on FB && ARCH_PXA
diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
index b4947c8..489174a 100644
--- a/drivers/video/pxafb.c
+++ b/drivers/video/pxafb.c
@@ -9,6 +9,8 @@
  *  which in turn is
  *   Based on acornfb.c Copyright (C) Russell King.
  *
+ * Backlight support by Rodolfo Giometti <giometti@linux.it>
+ *
  * This file is subject to the terms and conditions of the GNU General Public
  * License.  See the file COPYING in the main directory of this archive for
  * more details.
@@ -37,6 +39,7 @@
 #include <linux/cpufreq.h>
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
+#include <linux/backlight.h>
 
 #include <asm/hardware.h>
 #include <asm/io.h>
@@ -58,7 +61,6 @@
 #define LCCR0_INVALID_CONFIG_MASK (LCCR0_OUM|LCCR0_BM|LCCR0_QDM|LCCR0_DIS|LCCR0_EFM|LCCR0_IUM|LCCR0_SFM|LCCR0_LDM|LCCR0_ENB)
 #define LCCR3_INVALID_CONFIG_MASK (LCCR3_HSP|LCCR3_VSP|LCCR3_PCD|LCCR3_BPP)
 
-static void (*pxafb_backlight_power)(int);
 static void (*pxafb_lcd_power)(int, struct fb_var_screeninfo *);
 
 static int pxafb_activate_var(struct fb_var_screeninfo *var, struct pxafb_info *);
@@ -69,6 +71,71 @@ static void set_ctrlr_state(struct pxafb_info *fbi, u_int state);
 static char g_options[PXAFB_OPTIONS_SIZE] __initdata = "";
 #endif
 
+
+/*
+ * Backlight control
+ */
+#ifdef CONFIG_FB_BACKLIGHT
+static void pxafb_bl_suspend(struct pxafb_info *fbi)
+{
+	struct backlight_device *bl_dev = fbi->fb.bl_dev;
+
+	if (bl_dev) {
+		down(&bl_dev->sem);
+		bl_dev->props->fb_blank = FB_BLANK_POWERDOWN;
+		bl_dev->props->update_status(bl_dev);
+		up(&bl_dev->sem);
+	}
+}
+
+static void pxafb_bl_resume(struct pxafb_info *fbi)
+{
+	struct backlight_device *bl_dev = fbi->fb.bl_dev;
+
+	if (bl_dev) {
+		down(&bl_dev->sem);
+		bl_dev->props->fb_blank = FB_BLANK_UNBLANK;
+		bl_dev->props->update_status(bl_dev);
+		up(&bl_dev->sem);
+	}
+}
+
+static void pxafb_bl_init(struct fb_info *info, struct backlight_properties *bl_props)
+{
+	struct backlight_device *bl_dev;
+	char name[16];
+
+	snprintf(name, sizeof(name), "pxabl%d", info->node);
+
+	bl_dev = backlight_device_register(name, info->dev, info, bl_props);
+	if (IS_ERR(bl_dev)) {
+		info->bl_dev = NULL;
+		printk(KERN_WARNING "pxafb: backlight registration failed\n");
+		return;
+	}
+
+	mutex_lock(&info->bl_mutex);
+	info->bl_dev = bl_dev;
+	fb_bl_default_curve(info, 0, 0, 100);	/* level: 0 - 100 */
+	mutex_unlock(&info->bl_mutex);
+
+	down(&bl_dev->sem);
+	bl_dev->props->brightness = bl_props->max_brightness;
+	bl_dev->props->power = FB_BLANK_UNBLANK;
+	bl_dev->props->update_status(bl_dev);
+	up(&bl_dev->sem);
+
+	printk("pxafb: backlight initialized (%s)\n", name);
+}
+#else
+static inline void pxafb_bl_init(struct fb_info *info, struct backlight_properties *bl_props) {}
+static inline void pxafb_bl_suspend(struct pxafb_info *fbi) {}
+static inline void pxafb_bl_resume(struct pxafb_info *fbi) {}
+#endif /* CONFIG_FB_BACKLIGHT */
+
+/*
+ *
+ */
 static inline void pxafb_schedule_work(struct pxafb_info *fbi, u_int state)
 {
 	unsigned long flags;
@@ -736,14 +803,6 @@ static int pxafb_activate_var(struct fb_var_screeninfo *var, struct pxafb_info *
  * to ensure that things happen in the right way 100% of time time.
  *	-- rmk
  */
-static inline void __pxafb_backlight_power(struct pxafb_info *fbi, int on)
-{
-	pr_debug("pxafb: backlight o%s\n", on ? "n" : "ff");
-
- 	if (pxafb_backlight_power)
- 		pxafb_backlight_power(on);
-}
-
 static inline void __pxafb_lcd_power(struct pxafb_info *fbi, int on)
 {
 	pr_debug("pxafb: LCD power o%s\n", on ? "n" : "ff");
@@ -899,7 +958,7 @@ static void set_ctrlr_state(struct pxafb_info *fbi, u_int state)
 		 */
 		if (old_state != C_DISABLE) {
 			fbi->state = state;
-			__pxafb_backlight_power(fbi, 0);
+			pxafb_bl_suspend(fbi);
 			__pxafb_lcd_power(fbi, 0);
 			if (old_state != C_DISABLE_CLKCHANGE)
 				pxafb_disable_controller(fbi);
@@ -953,7 +1012,7 @@ static void set_ctrlr_state(struct pxafb_info *fbi, u_int state)
 			pxafb_setup_gpio(fbi);
 			pxafb_enable_controller(fbi);
 			__pxafb_lcd_power(fbi, 1);
-			__pxafb_backlight_power(fbi, 1);
+			pxafb_bl_resume(fbi);
 		}
 		break;
 	}
@@ -1112,11 +1171,10 @@ static struct pxafb_info * __init pxafb_init_fbinfo(struct device *dev)
 	int i, smemlen;
 
 	/* Alloc the pxafb_info and pseudo_palette in one step */
-	fbi = kmalloc(sizeof(struct pxafb_info) + sizeof(u32) * 16, GFP_KERNEL);
+	fbi = (struct pxafb_info *) framebuffer_alloc(sizeof(u32) * 16, dev);
 	if (!fbi)
 		return NULL;
 
-	memset(fbi, 0, sizeof(struct pxafb_info));
 	fbi->dev = dev;
 
 	strcpy(fbi->fb.fix.id, PXA_NAME);
@@ -1368,7 +1426,6 @@ int __init pxafb_probe(struct platform_device *dev)
 		ret = -EINVAL;
 		goto failed;
 	}
-	pxafb_backlight_power = inf->pxafb_backlight_power;
 	pxafb_lcd_power = inf->pxafb_lcd_power;
 	fbi = pxafb_init_fbinfo(&dev->dev);
 	if (!fbi) {
@@ -1407,6 +1464,9 @@ int __init pxafb_probe(struct platform_device *dev)
 		goto failed;
 	}
 
+	/* Register the backlight support */
+	pxafb_bl_init(&fbi->fb, inf->bl_props);
+
 #ifdef CONFIG_PM
 	// TODO
 #endif
diff --git a/include/asm-arm/arch-pxa/pxafb.h b/include/asm-arm/arch-pxa/pxafb.h
index 81c3928..5c89664 100644
--- a/include/asm-arm/arch-pxa/pxafb.h
+++ b/include/asm-arm/arch-pxa/pxafb.h
@@ -71,7 +71,7 @@ struct pxafb_mach_info {
 	 */
 	u_int		lccr3;
 
-	void (*pxafb_backlight_power)(int);
+	struct backlight_properties	*bl_props;
 	void (*pxafb_lcd_power)(int, struct fb_var_screeninfo *);
 
 };

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Paul Sokolovsky]

Hello Rodolfo,

Wednesday, February 21, 2007, 4:53:53 PM, you wrote:

> Backlight control support for the PXA fram buffer.


  Here're some comments: backlight support is already confusing
matter, and your patch IMHO makes it even more confusing for PXAFB.

  Before even start with details, let's first think what backlight has
to do with framebuffer? The facts that the physical object implementing
the former is usually put in close proximity to the latter, and they mostly
should be powered on synchronously, probably don't yet mean that
each and every FB driver should include good chunk of code to support
doing BL in its own special way.

  On the other hand, there's already
drivers/video/backlight/backlight.c which provides generic BL support,
implemented using notifier callback for FB core. Moreover, there's
corgi_bl.c driver which, contrary to its name is a generic driver for
embedded/PDA device backlight. It essentially subclassses pretty
abstract backlight.c, and provides good implementation for most BL
implementation. What you really need to do to use it, is to supply
single machine-dependent method, set_bl_intensity(). That method is
usually <10 lines.

  With this in mind, adhoc backlight handlers in pxafb and few other
drivers are artifacts of older times. And it's sad they are tried to
be resurrected instead of being removed.


> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>

> ---

> Each platform should define the backlight properties in its own setup
> file in "linux/arch/arm/mach-pxa/" as follow:

>    static int pxafb_bl_get_brightness(struct backlight_device *bl_dev)
>    {
>         return read_the_backlight_brightness();
>    }

>    static int pxafb_bl_update_status(struct backlight_device *bl_dev)
>    {
>            int perc, ret;

>            if (bl_dev->props->power != FB_BLANK_UNBLANK ||
>                bl_dev->props->fb_blank != FB_BLANK_UNBLANK)
>                    perc = 0;
>            else
>                    perc = bl_dev->props->brightness;

>         write_the_backlight_brightness(perc);

>            return 0;
>    }

>    static struct backlight_properties wwpc1100_backlight_props = {
>            .get_brightness         = pxafb_bl_get_brightness,
>            .update_status          = pxafb_bl_update_status,
>            .max_brightness         = 100,
>    };
>       
> and then seting up the fb info as follow:

>    wwpc1100_pxafb_info.modes = &special_modes;
>    wwpc1100_pxafb_info.bl_props = &wwpc1100_backlight_props;
>    set_pxa_fb_info(&wwpc1100_pxafb_info);



-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Paul Sokolovsky]

Hello Rodolfo,

Wednesday, February 21, 2007, 4:53:53 PM, you wrote:

> Backlight control support for the PXA fram buffer.


  Here're some comments: backlight support is already confusing
matter, and your patch IMHO makes it even more confusing for PXAFB.

  Before even start with details, let's first think what backlight has
to do with framebuffer? The facts that the physical object implementing
the former is usually put in close proximity to the latter, and they mostly
should be powered on synchronously, probably don't yet mean that
each and every FB driver should include good chunk of code to support
doing BL in its own special way.

  On the other hand, there's already
drivers/video/backlight/backlight.c which provides generic BL support,
implemented using notifier callback for FB core. Moreover, there's
corgi_bl.c driver which, contrary to its name is a generic driver for
embedded/PDA device backlight. It essentially subclassses pretty
abstract backlight.c, and provides good implementation for most BL
implementation. What you really need to do to use it, is to supply
single machine-dependent method, set_bl_intensity(). That method is
usually <10 lines.

  With this in mind, adhoc backlight handlers in pxafb and few other
drivers are artifacts of older times. And it's sad they are tried to
be resurrected instead of being removed.


> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>

> ---

> Each platform should define the backlight properties in its own setup
> file in "linux/arch/arm/mach-pxa/" as follow:

>    static int pxafb_bl_get_brightness(struct backlight_device *bl_dev)
>    {
>         return read_the_backlight_brightness();
>    }

>    static int pxafb_bl_update_status(struct backlight_device *bl_dev)
>    {
>            int perc, ret;

>            if (bl_dev->props->power != FB_BLANK_UNBLANK ||
>                bl_dev->props->fb_blank != FB_BLANK_UNBLANK)
>                    perc = 0;
>            else
>                    perc = bl_dev->props->brightness;

>         write_the_backlight_brightness(perc);

>            return 0;
>    }

>    static struct backlight_properties wwpc1100_backlight_props = {
>            .get_brightness         = pxafb_bl_get_brightness,
>            .update_status          = pxafb_bl_update_status,
>            .max_brightness         = 100,
>    };
>       
> and then seting up the fb info as follow:

>    wwpc1100_pxafb_info.modes = &special_modes;
>    wwpc1100_pxafb_info.bl_props = &wwpc1100_backlight_props;
>    set_pxa_fb_info(&wwpc1100_pxafb_info);



-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Rodolfo Giometti]

On Wed, Feb 21, 2007 at 06:00:37PM +0200, Paul Sokolovsky wrote:
> 
>   On the other hand, there's already
> drivers/video/backlight/backlight.c which provides generic BL support,
> implemented using notifier callback for FB core. Moreover, there's

My patch _uses_ that support.

> corgi_bl.c driver which, contrary to its name is a generic driver for
> embedded/PDA device backlight. It essentially subclassses pretty
> abstract backlight.c, and provides good implementation for most BL
> implementation. What you really need to do to use it, is to supply
> single machine-dependent method, set_bl_intensity(). That method is
> usually <10 lines.

I see, but in this manner you need to make a complete file for each
board, while with my patch you can just put few lines into machine's
definition file (a struct and 2 functions).

>   With this in mind, adhoc backlight handlers in pxafb and few other
> drivers are artifacts of older times. And it's sad they are tried to
> be resurrected instead of being removed.

IMHO, the actual backlight support is not so much, infact I'd like to
generalize it to support also backlighted keyboards (or input
devices). :)

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Paul Sokolovsky]

Hello Rodolfo,

Wednesday, February 21, 2007, 6:12:10 PM, you wrote:

> On Wed, Feb 21, 2007 at 06:00:37PM +0200, Paul Sokolovsky wrote:
>> 
>>   On the other hand, there's already
>> drivers/video/backlight/backlight.c which provides generic BL support,
>> implemented using notifier callback for FB core. Moreover, there's

> My patch _uses_ that support.

  Sorry if I missed it in quick review, as I told, there's a bit much
code ;-I

>> corgi_bl.c driver which, contrary to its name is a generic driver for
>> embedded/PDA device backlight. It essentially subclassses pretty
>> abstract backlight.c, and provides good implementation for most BL
>> implementation. What you really need to do to use it, is to supply
>> single machine-dependent method, set_bl_intensity(). That method is
>> usually <10 lines.

> I see, but in this manner you need to make a complete file for each
> board, while with my patch you can just put few lines into machine's
> definition file (a struct and 2 functions).

  Why? It's the same, except that it already exists, generic one (not
limited to pxafb), and requires 1 function (too bad that C doesn't
support lambda's):

==============
static void h4000_set_bl_intensity(int intensity)
{
        /* LCD brightness is driven by PWM0.
         * We'll set the pre-scaler to 8, and the period to 1024, this
         * means the backlight refresh rate will be 3686400/(8*1024) =
         * 450 Hz which is quite enough.
         */
        PWM_CTRL0 = 7;            /* pre-scaler */
        PWM_PWDUTY0 = intensity; /* duty cycle */
        PWM_PERVAL0 = H4000_MAX_INTENSITY;      /* period */

        if (intensity > 0) {
                pxa_set_cken(CKEN0_PWM0, 1);
                asic3_set_gpio_out_b(&h4000_asic3.dev,
                        GPIOB_BACKLIGHT_POWER_ON, GPIOB_BACKLIGHT_POWER_ON);
        } else {
                pxa_set_cken(CKEN0_PWM0, 0);
                asic3_set_gpio_out_b(&h4000_asic3.dev,
                        GPIOB_BACKLIGHT_POWER_ON, 0);
        }
}

static struct corgibl_machinfo h4000_bl_machinfo = {
        .default_intensity = H4000_MAX_INTENSITY / 4,
        .limit_mask = 0x0fff,
        .max_intensity = H4000_MAX_INTENSITY,
        .set_bl_intensity = h4000_set_bl_intensity,
};

struct platform_device h4000_bl = {
        .name = "corgi-bl",
        .dev = {
                .platform_data = &h4000_bl_machinfo,
        },
};
==============

>>   With this in mind, adhoc backlight handlers in pxafb and few other
>> drivers are artifacts of older times. And it's sad they are tried to
>> be resurrected instead of being removed.

> IMHO, the actual backlight support is not so much, infact I'd like to
> generalize it to support also backlighted keyboards (or input
> devices). :)

  I sent a bit of criticism for that too ;-). YMMV, but kernel
solutions are just bound to be pretty simple and generic and lack
any "niceties", which you'd likely want to do anyway eventually. For
example, what if you'll want to implement "fade out" effect for
keyboard backlight? Doing it in adhoc manner in kernel? Whereas with
the LCD classdev, you can write generic "fade out" trigger and
attach/detach it from userspace.

> Ciao,

> Rodolfo




-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Paul Sokolovsky]

Hello Rodolfo,

Wednesday, February 21, 2007, 6:12:10 PM, you wrote:

> On Wed, Feb 21, 2007 at 06:00:37PM +0200, Paul Sokolovsky wrote:
>> 
>>   On the other hand, there's already
>> drivers/video/backlight/backlight.c which provides generic BL support,
>> implemented using notifier callback for FB core. Moreover, there's

> My patch _uses_ that support.

  Sorry if I missed it in quick review, as I told, there's a bit much
code ;-I

>> corgi_bl.c driver which, contrary to its name is a generic driver for
>> embedded/PDA device backlight. It essentially subclassses pretty
>> abstract backlight.c, and provides good implementation for most BL
>> implementation. What you really need to do to use it, is to supply
>> single machine-dependent method, set_bl_intensity(). That method is
>> usually <10 lines.

> I see, but in this manner you need to make a complete file for each
> board, while with my patch you can just put few lines into machine's
> definition file (a struct and 2 functions).

  Why? It's the same, except that it already exists, generic one (not
limited to pxafb), and requires 1 function (too bad that C doesn't
support lambda's):

==============
static void h4000_set_bl_intensity(int intensity)
{
        /* LCD brightness is driven by PWM0.
         * We'll set the pre-scaler to 8, and the period to 1024, this
         * means the backlight refresh rate will be 3686400/(8*1024) =
         * 450 Hz which is quite enough.
         */
        PWM_CTRL0 = 7;            /* pre-scaler */
        PWM_PWDUTY0 = intensity; /* duty cycle */
        PWM_PERVAL0 = H4000_MAX_INTENSITY;      /* period */

        if (intensity > 0) {
                pxa_set_cken(CKEN0_PWM0, 1);
                asic3_set_gpio_out_b(&h4000_asic3.dev,
                        GPIOB_BACKLIGHT_POWER_ON, GPIOB_BACKLIGHT_POWER_ON);
        } else {
                pxa_set_cken(CKEN0_PWM0, 0);
                asic3_set_gpio_out_b(&h4000_asic3.dev,
                        GPIOB_BACKLIGHT_POWER_ON, 0);
        }
}

static struct corgibl_machinfo h4000_bl_machinfo = {
        .default_intensity = H4000_MAX_INTENSITY / 4,
        .limit_mask = 0x0fff,
        .max_intensity = H4000_MAX_INTENSITY,
        .set_bl_intensity = h4000_set_bl_intensity,
};

struct platform_device h4000_bl = {
        .name = "corgi-bl",
        .dev = {
                .platform_data = &h4000_bl_machinfo,
        },
};
==============

>>   With this in mind, adhoc backlight handlers in pxafb and few other
>> drivers are artifacts of older times. And it's sad they are tried to
>> be resurrected instead of being removed.

> IMHO, the actual backlight support is not so much, infact I'd like to
> generalize it to support also backlighted keyboards (or input
> devices). :)

  I sent a bit of criticism for that too ;-). YMMV, but kernel
solutions are just bound to be pretty simple and generic and lack
any "niceties", which you'd likely want to do anyway eventually. For
example, what if you'll want to implement "fade out" effect for
keyboard backlight? Doing it in adhoc manner in kernel? Whereas with
the LCD classdev, you can write generic "fade out" trigger and
attach/detach it from userspace.

> Ciao,

> Rodolfo




-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Richard Purdie]

On Wed, 2007-02-21 at 15:53 +0100, Rodolfo Giometti wrote:
> Backlight control support for the PXA fram buffer.
> 
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> 
> ---
> 
> Each platform should define the backlight properties in its own setup
> file in "linux/arch/arm/mach-pxa/" as follow:
> 
>    static int pxafb_bl_get_brightness(struct backlight_device *bl_dev)
>    {
>         return read_the_backlight_brightness();
>    }
> 
>    static int pxafb_bl_update_status(struct backlight_device *bl_dev)
>    {
>            int perc, ret;
> 
>            if (bl_dev->props->power != FB_BLANK_UNBLANK ||
>                bl_dev->props->fb_blank != FB_BLANK_UNBLANK)
>                    perc = 0;
>            else
>                    perc = bl_dev->props->brightness;
> 
>         write_the_backlight_brightness(perc);
> 
>            return 0;
>    }
> 
>    static struct backlight_properties wwpc1100_backlight_props = {
>            .get_brightness         = pxafb_bl_get_brightness,
>            .update_status          = pxafb_bl_update_status,
>            .max_brightness         = 100,
>    };
>       
> and then seting up the fb info as follow:
> 
>    wwpc1100_pxafb_info.modes = &special_modes;
>    wwpc1100_pxafb_info.bl_props = &wwpc1100_backlight_props;
>    set_pxa_fb_info(&wwpc1100_pxafb_info);

Reading through the patch its:

1) Not against any mainline kernel
2) Not against a recent kernel

There were a number of backlight class changes just merged into mainline
and you need to sync up any patch against them.

As mentioned by others, there is no need to tie the backlight driver
into the framebuffer any more. Have a look at
drivers/video/backlight/corgi_bl.c for an example (its used by PXA
devices).

I have said elsewhere I will take patches to make corgi_bl a more
generic driver (or maybe create a simple generic backlight driver) along
the lines of what Paul mentioned.

Regards,

Richard

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Richard Purdie]

On Wed, 2007-02-21 at 15:53 +0100, Rodolfo Giometti wrote:
> Backlight control support for the PXA fram buffer.
> 
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> 
> ---
> 
> Each platform should define the backlight properties in its own setup
> file in "linux/arch/arm/mach-pxa/" as follow:
> 
>    static int pxafb_bl_get_brightness(struct backlight_device *bl_dev)
>    {
>         return read_the_backlight_brightness();
>    }
> 
>    static int pxafb_bl_update_status(struct backlight_device *bl_dev)
>    {
>            int perc, ret;
> 
>            if (bl_dev->props->power != FB_BLANK_UNBLANK ||
>                bl_dev->props->fb_blank != FB_BLANK_UNBLANK)
>                    perc = 0;
>            else
>                    perc = bl_dev->props->brightness;
> 
>         write_the_backlight_brightness(perc);
> 
>            return 0;
>    }
> 
>    static struct backlight_properties wwpc1100_backlight_props = {
>            .get_brightness         = pxafb_bl_get_brightness,
>            .update_status          = pxafb_bl_update_status,
>            .max_brightness         = 100,
>    };
>       
> and then seting up the fb info as follow:
> 
>    wwpc1100_pxafb_info.modes = &special_modes;
>    wwpc1100_pxafb_info.bl_props = &wwpc1100_backlight_props;
>    set_pxa_fb_info(&wwpc1100_pxafb_info);

Reading through the patch its:

1) Not against any mainline kernel
2) Not against a recent kernel

There were a number of backlight class changes just merged into mainline
and you need to sync up any patch against them.

As mentioned by others, there is no need to tie the backlight driver
into the framebuffer any more. Have a look at
drivers/video/backlight/corgi_bl.c for an example (its used by PXA
devices).

I have said elsewhere I will take patches to make corgi_bl a more
generic driver (or maybe create a simple generic backlight driver) along
the lines of what Paul mentioned.

Regards,

Richard



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Rodolfo Giometti]

On Thu, Feb 22, 2007 at 12:59:06AM +0000, Richard Purdie wrote:
> 
> Reading through the patch its:
> 
> 1) Not against any mainline kernel
> 2) Not against a recent kernel

I'm sorry, but the patch applay against the latest kernel. Please, try
it.

> There were a number of backlight class changes just merged into mainline
> and you need to sync up any patch against them.

My patch uses current backlight class support in the kernel.

> As mentioned by others, there is no need to tie the backlight driver
> into the framebuffer any more. Have a look at
> drivers/video/backlight/corgi_bl.c for an example (its used by PXA
> devices).

That driver uses the backlight class support as my patch does into
pxafb.

> I have said elsewhere I will take patches to make corgi_bl a more
> generic driver (or maybe create a simple generic backlight driver) along
> the lines of what Paul mentioned.

I see.

I suppose you are the backlight support mantainer, so what do you
suggest to do to "make corgi_bl a more generic driver"?

I have to rename and modify it? Or just copy it to have backward
compatibility and the modify the new file?

I should mv backlight directory from the video one?

Thanks for your suggestions,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Rodolfo Giometti]

On Wed, Feb 21, 2007 at 06:26:08PM +0200, Paul Sokolovsky wrote:
k>   Why? It's the same, except that it already exists, generic one (not
> limited to pxafb), and requires 1 function (too bad that C doesn't
> support lambda's):

Ah, ok.

>   I sent a bit of criticism for that too ;-). YMMV, but kernel
> solutions are just bound to be pretty simple and generic and lack
> any "niceties", which you'd likely want to do anyway eventually. For
> example, what if you'll want to implement "fade out" effect for
> keyboard backlight? Doing it in adhoc manner in kernel? Whereas with
> the LCD classdev, you can write generic "fade out" trigger and
> attach/detach it from userspace.

I agree. I just wish to add a backlight support for my LCD and
minikeypad.

What do you suggest to me in order to accomplish such task?

Thanks for your suggestions,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Richard Purdie]

On Thu, 2007-02-22 at 09:28 +0100, Rodolfo Giometti wrote:
> On Thu, Feb 22, 2007 at 12:59:06AM +0000, Richard Purdie wrote:
> > 
> > Reading through the patch its:
> > 
> > 1) Not against any mainline kernel

Sorry, I'd missed a patch entering mainline,

> > 2) Not against a recent kernel
> 
> I'm sorry, but the patch applay against the latest kernel. Please, try
> it.

but this still applies as there were a number of backlight changes
merged just before 2.6.21-rc1 and your driver will not work with
2.6.21-rc1.

> > As mentioned by others, there is no need to tie the backlight driver
> > into the framebuffer any more. Have a look at
> > drivers/video/backlight/corgi_bl.c for an example (its used by PXA
> > devices).
> z
> That driver uses the backlight class support as my patch does into
> pxafb.

Yes, my point is that you shouldn't need to touch pxafb if you use the
backlight class. I know pxafb has backlight hooks but they are probably
going to get removed at some point as they should no longer be needed.

> > I have said elsewhere I will take patches to make corgi_bl a more
> > generic driver (or maybe create a simple generic backlight driver) along
> > the lines of what Paul mentioned.
> 
> I see.
> 
> I suppose you are the backlight support mantainer, so what do you
> suggest to do to "make corgi_bl a more generic driver"?

What changes do you need to it to be able to use it as a generic driver?

The main issue is that the structure definition is in one of the
sharpsl.h files at the moment so most drivers can't get to it. Fix that
and it should make a good generic driver.

Ideally I'd prefer to leave the name alone since there is broken
userspace on the Zaurus that uses that name but I can see why people
want it renamed if its to be used as a generic driver...

Richard

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Richard Purdie]

On Thu, 2007-02-22 at 09:28 +0100, Rodolfo Giometti wrote:
> On Thu, Feb 22, 2007 at 12:59:06AM +0000, Richard Purdie wrote:
> > 
> > Reading through the patch its:
> > 
> > 1) Not against any mainline kernel

Sorry, I'd missed a patch entering mainline,

> > 2) Not against a recent kernel
> 
> I'm sorry, but the patch applay against the latest kernel. Please, try
> it.

but this still applies as there were a number of backlight changes
merged just before 2.6.21-rc1 and your driver will not work with
2.6.21-rc1.

> > As mentioned by others, there is no need to tie the backlight driver
> > into the framebuffer any more. Have a look at
> > drivers/video/backlight/corgi_bl.c for an example (its used by PXA
> > devices).
> z
> That driver uses the backlight class support as my patch does into
> pxafb.

Yes, my point is that you shouldn't need to touch pxafb if you use the
backlight class. I know pxafb has backlight hooks but they are probably
going to get removed at some point as they should no longer be needed.

> > I have said elsewhere I will take patches to make corgi_bl a more
> > generic driver (or maybe create a simple generic backlight driver) along
> > the lines of what Paul mentioned.
> 
> I see.
> 
> I suppose you are the backlight support mantainer, so what do you
> suggest to do to "make corgi_bl a more generic driver"?

What changes do you need to it to be able to use it as a generic driver?

The main issue is that the structure definition is in one of the
sharpsl.h files at the moment so most drivers can't get to it. Fix that
and it should make a good generic driver.

Ideally I'd prefer to leave the name alone since there is broken
userspace on the Zaurus that uses that name but I can see why people
want it renamed if its to be used as a generic driver...

Richard


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Rodolfo Giometti]

On Thu, Feb 22, 2007 at 09:27:09AM +0000, Richard Purdie wrote:
> 
> Yes, my point is that you shouldn't need to touch pxafb if you use the
> backlight class. I know pxafb has backlight hooks but they are probably
> going to get removed at some point as they should no longer be needed.

I see.

> What changes do you need to it to be able to use it as a generic driver?
> 
> The main issue is that the structure definition is in one of the
> sharpsl.h files at the moment so most drivers can't get to it. Fix that
> and it should make a good generic driver.

I'll do as you suggest!

> Ideally I'd prefer to leave the name alone since there is broken
> userspace on the Zaurus that uses that name but I can see why people
> want it renamed if its to be used as a generic driver...

Yes. :)

I'll wait for the 2.6.21 to see what to do... thanks a lot,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Paul Sokolovsky]

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

Hello Rodolfo,

Thursday, February 22, 2007, 10:32:04 AM, you wrote:

> On Wed, Feb 21, 2007 at 06:26:08PM +0200, Paul Sokolovsky wrote:
k>>   Why? It's the same, except that it already exists, generic one (not
>> limited to pxafb), and requires 1 function (too bad that C doesn't
>> support lambda's):

> Ah, ok.

>>   I sent a bit of criticism for that too ;-). YMMV, but kernel
>> solutions are just bound to be pretty simple and generic and lack
>> any "niceties", which you'd likely want to do anyway eventually. For
>> example, what if you'll want to implement "fade out" effect for
>> keyboard backlight? Doing it in adhoc manner in kernel? Whereas with
>> the LCD classdev, you can write generic "fade out" trigger and
>> attach/detach it from userspace.

> I agree. I just wish to add a backlight support for my LCD and
> minikeypad.

> What do you suggest to me in order to accomplish such task?

  Well, I write exactly to share experience and work towards having
best practices for backlight, etc. control, reusable on wide range of
embedded/handheld devices.

  We in handhelds.org codebase have attached patch* to make corgi_bl
more suitable for general use. This patch was submitted to Richard
(so, more votes needed ;-) ). Otherwise, snippet I pasted is from real
machine implementation, HP iPaq h4000.

  As for keyboard backlight, another port we have, HTC Universal, has:
normal indicator LEDs, keyboard backlight, flashlight, ring vibra. All
of these are handled via Generic LED API:
http://handhelds.org/cgi-bin/cvsweb.cgi/linux/kernel26/arch/arm/mach-pxa/htcuniversal/htcuniversal_leds.c?rev=1.6&content-type=text/x-cvsweb-markup

  To show that it is not an anomaly, but hopefully, a trend, here's
vibra driver for the upcoming OpenMoko phone:
http://wiki.openmoko.org/wiki/Neo1973_Hardware#Vibrator

  So, if you have freedom to add keyboard backlight control to your
userspace, that would allow you to do many interesting things without
disturbing the kernel.

[*] Optimized for size, full one should patch <asm/arch/sharpsl.h> of
course.


> Thanks for your suggestions,

> Rodolfo




-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

[-- Attachment #2: driver-corgi-bl-generalize.patch --]
[-- Type: application/octet-stream, Size: 1636 bytes --]

diff -N -U3 -r -x CVS ../linux-2.6.20/drivers/video/backlight/Kconfig ../linux-2.6.20-hh/drivers/video/backlight/Kconfig
--- ../linux-2.6.20/drivers/video/backlight/Kconfig	2007-02-04 18:44:54.000000000 +0000
+++ ../linux-2.6.20-hh/drivers/video/backlight/Kconfig	2006-12-18 12:41:24.000000000 +0000
@@ -44,7 +44,7 @@
 
 config BACKLIGHT_CORGI
 	tristate "Sharp Corgi Backlight Driver (SL Series)"
-	depends on BACKLIGHT_DEVICE && PXA_SHARPSL
+	depends on BACKLIGHT_DEVICE
 	default y
 	help
 	  If you have a Sharp Zaurus SL-C7xx, SL-Cxx00 or SL-6000x say y to enable the
diff -N -U3 -r -x CVS ../linux-2.6.20/drivers/video/backlight/corgi_bl.c ../linux-2.6.20-hh/drivers/video/backlight/corgi_bl.c
--- ../linux-2.6.20/drivers/video/backlight/corgi_bl.c	2007-02-04 18:44:54.000000000 +0000
+++ ../linux-2.6.20-hh/drivers/video/backlight/corgi_bl.c	2007-02-13 20:12:14.000000000 +0000
@@ -18,7 +18,7 @@
 #include <linux/mutex.h>
 #include <linux/fb.h>
 #include <linux/backlight.h>
-#include <asm/arch/sharpsl.h>
+#include <linux/corgi_bl.h>
 #include <asm/hardware/sharpsl_pm.h>
 
 static int corgibl_intensity;
diff -N -U3 -r -x CVS ../linux-2.6.20/include/linux/corgi_bl.h ../linux-2.6.20-hh/include/linux/corgi_bl.h
--- ../linux-2.6.20/include/linux/corgi_bl.h	1970-01-01 00:00:00.000000000 +0000
+++ ../linux-2.6.20-hh/include/linux/corgi_bl.h	2006-11-04 17:36:48.000000000 +0000
@@ -0,0 +1,10 @@
+/*
+ * Generic Backlight, from sharpsl.h
+ */
+struct corgibl_machinfo {
+	int max_intensity;
+	int default_intensity;
+	int limit_mask;
+	void (*set_bl_intensity)(int intensity);
+};
+extern void corgibl_limit_intensity(int limit);

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Paul Sokolovsky]

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

Hello Rodolfo,

Thursday, February 22, 2007, 10:32:04 AM, you wrote:

> On Wed, Feb 21, 2007 at 06:26:08PM +0200, Paul Sokolovsky wrote:
k>>   Why? It's the same, except that it already exists, generic one (not
>> limited to pxafb), and requires 1 function (too bad that C doesn't
>> support lambda's):

> Ah, ok.

>>   I sent a bit of criticism for that too ;-). YMMV, but kernel
>> solutions are just bound to be pretty simple and generic and lack
>> any "niceties", which you'd likely want to do anyway eventually. For
>> example, what if you'll want to implement "fade out" effect for
>> keyboard backlight? Doing it in adhoc manner in kernel? Whereas with
>> the LCD classdev, you can write generic "fade out" trigger and
>> attach/detach it from userspace.

> I agree. I just wish to add a backlight support for my LCD and
> minikeypad.

> What do you suggest to me in order to accomplish such task?

  Well, I write exactly to share experience and work towards having
best practices for backlight, etc. control, reusable on wide range of
embedded/handheld devices.

  We in handhelds.org codebase have attached patch* to make corgi_bl
more suitable for general use. This patch was submitted to Richard
(so, more votes needed ;-) ). Otherwise, snippet I pasted is from real
machine implementation, HP iPaq h4000.

  As for keyboard backlight, another port we have, HTC Universal, has:
normal indicator LEDs, keyboard backlight, flashlight, ring vibra. All
of these are handled via Generic LED API:
http://handhelds.org/cgi-bin/cvsweb.cgi/linux/kernel26/arch/arm/mach-pxa/htcuniversal/htcuniversal_leds.c?rev=1.6&content-type=text/x-cvsweb-markup

  To show that it is not an anomaly, but hopefully, a trend, here's
vibra driver for the upcoming OpenMoko phone:
http://wiki.openmoko.org/wiki/Neo1973_Hardware#Vibrator

  So, if you have freedom to add keyboard backlight control to your
userspace, that would allow you to do many interesting things without
disturbing the kernel.

[*] Optimized for size, full one should patch <asm/arch/sharpsl.h> of
course.


> Thanks for your suggestions,

> Rodolfo




-- 
Best regards,
 Paul                            mailto:pmiscml@gmail.com

[-- Attachment #2: driver-corgi-bl-generalize.patch --]
[-- Type: application/octet-stream, Size: 1636 bytes --]

diff -N -U3 -r -x CVS ../linux-2.6.20/drivers/video/backlight/Kconfig ../linux-2.6.20-hh/drivers/video/backlight/Kconfig
--- ../linux-2.6.20/drivers/video/backlight/Kconfig	2007-02-04 18:44:54.000000000 +0000
+++ ../linux-2.6.20-hh/drivers/video/backlight/Kconfig	2006-12-18 12:41:24.000000000 +0000
@@ -44,7 +44,7 @@
 
 config BACKLIGHT_CORGI
 	tristate "Sharp Corgi Backlight Driver (SL Series)"
-	depends on BACKLIGHT_DEVICE && PXA_SHARPSL
+	depends on BACKLIGHT_DEVICE
 	default y
 	help
 	  If you have a Sharp Zaurus SL-C7xx, SL-Cxx00 or SL-6000x say y to enable the
diff -N -U3 -r -x CVS ../linux-2.6.20/drivers/video/backlight/corgi_bl.c ../linux-2.6.20-hh/drivers/video/backlight/corgi_bl.c
--- ../linux-2.6.20/drivers/video/backlight/corgi_bl.c	2007-02-04 18:44:54.000000000 +0000
+++ ../linux-2.6.20-hh/drivers/video/backlight/corgi_bl.c	2007-02-13 20:12:14.000000000 +0000
@@ -18,7 +18,7 @@
 #include <linux/mutex.h>
 #include <linux/fb.h>
 #include <linux/backlight.h>
-#include <asm/arch/sharpsl.h>
+#include <linux/corgi_bl.h>
 #include <asm/hardware/sharpsl_pm.h>
 
 static int corgibl_intensity;
diff -N -U3 -r -x CVS ../linux-2.6.20/include/linux/corgi_bl.h ../linux-2.6.20-hh/include/linux/corgi_bl.h
--- ../linux-2.6.20/include/linux/corgi_bl.h	1970-01-01 00:00:00.000000000 +0000
+++ ../linux-2.6.20-hh/include/linux/corgi_bl.h	2006-11-04 17:36:48.000000000 +0000
@@ -0,0 +1,10 @@
+/*
+ * Generic Backlight, from sharpsl.h
+ */
+struct corgibl_machinfo {
+	int max_intensity;
+	int default_intensity;
+	int limit_mask;
+	void (*set_bl_intensity)(int intensity);
+};
+extern void corgibl_limit_intensity(int limit);

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Rodolfo Giometti]

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

On Thu, Feb 22, 2007 at 12:33:35PM +0200, Paul Sokolovsky wrote:
> 
>   We in handhelds.org codebase have attached patch* to make corgi_bl
> more suitable for general use. This patch was submitted to Richard
> (so, more votes needed ;-) ). Otherwise, snippet I pasted is from real
> machine implementation, HP iPaq h4000.

You have my vote! :)

So let me suggest this patch who allows multiple driver instances (I
use it for both LCD and keypad backlighti devices).

Hope it could be merged into main line (maybe without the "corgi"
prefix ;-).

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                     		giometti@linux.it
UNIX programming                     phone:     +39 349 2432127

[-- Attachment #2: patch-multiple-backlight-devices --]
[-- Type: text/plain, Size: 6951 bytes --]

diff --git a/drivers/video/backlight/corgi_bl.c b/drivers/video/backlight/corgi_bl.c
index cc2ae61..063f0a1 100644
--- a/drivers/video/backlight/corgi_bl.c
+++ b/drivers/video/backlight/corgi_bl.c
@@ -15,24 +15,14 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
-#include <linux/mutex.h>
 #include <linux/fb.h>
 #include <linux/backlight.h>
 #include <linux/corgi_bl.h>
 #include <asm/hardware/sharpsl_pm.h>
 
-static int corgibl_intensity;
-static DEFINE_MUTEX(bl_mutex);
-static struct backlight_properties corgibl_data;
-static struct backlight_device *corgi_backlight_device;
-static struct corgibl_machinfo *bl_machinfo;
-
-static unsigned long corgibl_flags;
-#define CORGIBL_SUSPENDED     0x01
-#define CORGIBL_BATTLOW       0x02
-
 static int corgibl_send_intensity(struct backlight_device *bd)
 {
+	struct corgibl_info *info = to_backlight_info(bd->props);
 	void (*corgi_kick_batt)(void);
 	int intensity = bd->props->brightness;
 
@@ -40,16 +30,16 @@ static int corgibl_send_intensity(struct backlight_device *bd)
 		intensity = 0;
 	if (bd->props->fb_blank != FB_BLANK_UNBLANK)
 		intensity = 0;
-	if (corgibl_flags & CORGIBL_SUSPENDED)
+	if (info->flags & CORGIBL_SUSPENDED)
 		intensity = 0;
-	if (corgibl_flags & CORGIBL_BATTLOW)
-		intensity &= bl_machinfo->limit_mask;
+	if (info->flags & CORGIBL_BATTLOW)
+		intensity &= info->machinfo->limit_mask;
 
- 	mutex_lock(&bl_mutex);
-	bl_machinfo->set_bl_intensity(intensity);
-	mutex_unlock(&bl_mutex);
+ 	mutex_lock(&info->mutex);
+	info->machinfo->set_bl_intensity(intensity);
+	mutex_unlock(&info->mutex);
 
-	corgibl_intensity = intensity;
+	info->intensity = intensity;
 
  	corgi_kick_batt = symbol_get(sharpsl_battery_kick);
  	if (corgi_kick_batt) {
@@ -61,17 +51,21 @@ static int corgibl_send_intensity(struct backlight_device *bd)
 }
 
 #ifdef CONFIG_PM
-static int corgibl_suspend(struct platform_device *dev, pm_message_t state)
+static int corgibl_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	corgibl_flags |= CORGIBL_SUSPENDED;
-	corgibl_send_intensity(corgi_backlight_device);
+	struct corgibl_info *info = platform_get_drvdata(pdev);
+
+	info->flags |= CORGIBL_SUSPENDED;
+	corgibl_send_intensity(info->bd);
 	return 0;
 }
 
-static int corgibl_resume(struct platform_device *dev)
+static int corgibl_resume(struct platform_device *pdev)
 {
-	corgibl_flags &= ~CORGIBL_SUSPENDED;
-	corgibl_send_intensity(corgi_backlight_device);
+	struct corgibl_info *info = platform_get_drvdata(pdev);
+
+	info->flags &= ~CORGIBL_SUSPENDED;
+	corgibl_send_intensity(info->bd);
 	return 0;
 }
 #else
@@ -81,12 +75,14 @@ static int corgibl_resume(struct platform_device *dev)
 
 static int corgibl_get_intensity(struct backlight_device *bd)
 {
-	return corgibl_intensity;
+	struct corgibl_info *info = to_backlight_info(bd->props);
+
+	return info->intensity;
 }
 
 static int corgibl_set_intensity(struct backlight_device *bd)
 {
-	corgibl_send_intensity(corgi_backlight_device);
+	corgibl_send_intensity(bd);
 	return 0;
 }
 
@@ -94,52 +90,71 @@ static int corgibl_set_intensity(struct backlight_device *bd)
  * Called when the battery is low to limit the backlight intensity.
  * If limit==0 clear any limit, otherwise limit the intensity
  */
-void corgibl_limit_intensity(int limit)
+void corgibl_limit_intensity(struct backlight_device *bd, int limit)
 {
+	struct corgibl_info *info = to_backlight_info(bd->props);
+
 	if (limit)
-		corgibl_flags |= CORGIBL_BATTLOW;
+		info->flags |= CORGIBL_BATTLOW;
 	else
-		corgibl_flags &= ~CORGIBL_BATTLOW;
-	corgibl_send_intensity(corgi_backlight_device);
+		info->flags &= ~CORGIBL_BATTLOW;
+	corgibl_send_intensity(bd);
 }
 EXPORT_SYMBOL(corgibl_limit_intensity);
 
-
-static struct backlight_properties corgibl_data = {
-	.owner          = THIS_MODULE,
-	.get_brightness = corgibl_get_intensity,
-	.update_status  = corgibl_set_intensity,
-};
-
 static int corgibl_probe(struct platform_device *pdev)
 {
 	struct corgibl_machinfo *machinfo = pdev->dev.platform_data;
-
-	bl_machinfo = machinfo;
-	corgibl_data.max_brightness = machinfo->max_intensity;
+	struct corgibl_info *info;
+	char name[16];
+	int ret;
+
+	info = kmalloc(sizeof(struct corgibl_info), GFP_KERNEL);
+	if (IS_ERR(info))
+		return -ENOMEM;
+	memset(info, 0, sizeof(struct corgibl_info));
+	info->props.owner = THIS_MODULE;
+	info->props.get_brightness = corgibl_get_intensity;
+	info->props.update_status  = corgibl_set_intensity;
+	mutex_init(&info->mutex);
+
+	info->machinfo = machinfo;
+	info->props.max_brightness = machinfo->max_intensity;
 	if (!machinfo->limit_mask)
 		machinfo->limit_mask = -1;
 
-	corgi_backlight_device = backlight_device_register ("corgi-bl",
-		&pdev->dev, NULL, &corgibl_data);
-	if (IS_ERR (corgi_backlight_device))
-		return PTR_ERR (corgi_backlight_device);
+	snprintf(name, sizeof(name), "corgi-bl%d", pdev->id);
+	info->bd = backlight_device_register (name,
+		&pdev->dev, info, &info->props);
+	if (IS_ERR (info->bd)) {
+		ret = PTR_ERR(info->bd);
+		goto error;
+	}
 
-	corgibl_data.power = FB_BLANK_UNBLANK;
-	corgibl_data.brightness = machinfo->default_intensity;
-	corgibl_send_intensity(corgi_backlight_device);
+	info->props.power = FB_BLANK_UNBLANK;
+	info->props.brightness = machinfo->default_intensity;
+	corgibl_send_intensity(info->bd);
 
-	printk("Corgi Backlight Driver Initialized.\n");
+	platform_set_drvdata(pdev, info);
+
+	printk("Corgi Backlight Driver Initialized (%s).\n", name);
 	return 0;
+
+error:
+	kfree(info);
+	return ret;
 }
 
-static int corgibl_remove(struct platform_device *dev)
+static int corgibl_remove(struct platform_device *pdev)
 {
-	corgibl_data.power = 0;
-	corgibl_data.brightness = 0;
-	corgibl_send_intensity(corgi_backlight_device);
+	struct corgibl_info *info = platform_get_drvdata(pdev);
+	info->props.power = 0;
+	info->props.brightness = 0;
+	corgibl_send_intensity(info->bd);
+
+	backlight_device_unregister(info->bd);
 
-	backlight_device_unregister(corgi_backlight_device);
+	kfree(info);
 
 	printk("Corgi Backlight Driver Unloaded\n");
 	return 0;
diff --git a/include/linux/corgi_bl.h b/include/linux/corgi_bl.h
index 079fe65..2c6d0c4 100644
--- a/include/linux/corgi_bl.h
+++ b/include/linux/corgi_bl.h
@@ -1,10 +1,26 @@
 /*
  * Generic Backlight, from sharpsl.h
  */
+#include <linux/mutex.h>
+
 struct corgibl_machinfo {
 	int max_intensity;
 	int default_intensity;
 	int limit_mask;
 	void (*set_bl_intensity)(int intensity);
 };
-extern void corgibl_limit_intensity(int limit);
+extern void corgibl_limit_intensity(struct backlight_device *bd, int limit);
+
+struct corgibl_info {
+	int intensity;
+	struct mutex mutex;
+	struct backlight_properties props;
+	struct backlight_device *bd;
+	struct corgibl_machinfo *machinfo;
+
+	unsigned long flags;
+#define CORGIBL_SUSPENDED     0x01
+#define CORGIBL_BATTLOW       0x02
+};
+
+#define to_backlight_info(_p) container_of(_p, struct corgibl_info, props) 

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Richard Purdie]

On Thu, 2007-02-22 at 17:37 +0100, Rodolfo Giometti wrote:
> On Thu, Feb 22, 2007 at 12:33:35PM +0200, Paul Sokolovsky wrote:
> > 
> >   We in handhelds.org codebase have attached patch* to make corgi_bl
> > more suitable for general use. This patch was submitted to Richard
> > (so, more votes needed ;-) ). Otherwise, snippet I pasted is from real
> > machine implementation, HP iPaq h4000.
> 
> You have my vote! :)
> 
> So let me suggest this patch who allows multiple driver instances (I
> use it for both LCD and keypad backlighti devices).
> 
> Hope it could be merged into main line (maybe without the "corgi"
> prefix ;-).

Doing something along these lines is on my todo list but not at the top
since its 2.6.22 material now. Something should appear in -mm within a
few weeks but I don't have the time just at the moment.

Richard

Re: [PATCH 1/1] PXAFB: Support for backlight control

[Richard Purdie]

On Thu, 2007-02-22 at 17:37 +0100, Rodolfo Giometti wrote:
> On Thu, Feb 22, 2007 at 12:33:35PM +0200, Paul Sokolovsky wrote:
> > 
> >   We in handhelds.org codebase have attached patch* to make corgi_bl
> > more suitable for general use. This patch was submitted to Richard
> > (so, more votes needed ;-) ). Otherwise, snippet I pasted is from real
> > machine implementation, HP iPaq h4000.
> 
> You have my vote! :)
> 
> So let me suggest this patch who allows multiple driver instances (I
> use it for both LCD and keypad backlighti devices).
> 
> Hope it could be merged into main line (maybe without the "corgi"
> prefix ;-).

Doing something along these lines is on my todo list but not at the top
since its 2.6.22 material now. Something should appear in -mm within a
few weeks but I don't have the time just at the moment.

Richard



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [Linux-fbdev-devel] [PATCH 1/1] PXAFB: Support for backlight control

[James Simmons]

> On Thu, Feb 22, 2007 at 12:33:35PM +0200, Paul Sokolovsky wrote:
> > 
> >   We in handhelds.org codebase have attached patch* to make corgi_bl
> > more suitable for general use. This patch was submitted to Richard
> > (so, more votes needed ;-) ). Otherwise, snippet I pasted is from real
> > machine implementation, HP iPaq h4000.
> 
> You have my vote! :)
> 
> So let me suggest this patch who allows multiple driver instances (I
> use it for both LCD and keypad backlighti devices).
> 
> Hope it could be merged into main line (maybe without the "corgi"
> prefix ;-).

Nice patch. I have no problem with it.