[RFC/PATCH 0/2] Backlight support

[RFC/PATCH 0/2] Backlight support

[Gregory CLEMENT]

This patch set is a RFC about a backlight framework. The purpose of
this framework is mainly to allow to add easily a support for a
backlight with the possibility of setting brightness directly from the
barebox shell using the brightness parameter.

An implementation is provided for i.MX23 by using the PWM. It was
tested on a custom i.MX23 base board.

All this commits are done to be applied on top of this git repository :

  git://git.pengutronix.de/git/barebox.git master

The HEAD of this git repository, when I created this patch, was:
commit 71e806048e5a370fbcabf5441de33bd3f083903a
login: correct a typo in command description

Gregory CLEMENT (2):
  backlight: Add backlight framework to support backlight driver
  backlight: mxs: Add backlight support for i.MX23

 arch/arm/mach-mxs/include/mach/backlight.h |   19 ++++
 drivers/video/Kconfig                      |    3 +-
 drivers/video/Makefile                     |    2 +
 drivers/video/backlight/Kconfig            |   16 ++++
 drivers/video/backlight/Makefile           |    3 +
 drivers/video/backlight/backlight-mxs.c    |  136 ++++++++++++++++++++++++++++
 drivers/video/backlight/backlight.c        |   85 +++++++++++++++++
 include/backlight.h                        |   21 +++++
 8 files changed, 284 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mxs/include/mach/backlight.h
 create mode 100644 drivers/video/backlight/Kconfig
 create mode 100644 drivers/video/backlight/Makefile
 create mode 100644 drivers/video/backlight/backlight-mxs.c
 create mode 100644 drivers/video/backlight/backlight.c
 create mode 100644 include/backlight.h

-- 
1.7.4.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC/PATCH 0/2] Backlight support

[Sascha Hauer]

Hi Gregory,

On Thu, Oct 06, 2011 at 10:05:07AM +0200, Gregory CLEMENT wrote:
> This patch set is a RFC about a backlight framework. The purpose of
> this framework is mainly to allow to add easily a support for a
> backlight with the possibility of setting brightness directly from the
> barebox shell using the brightness parameter.
> 
> An implementation is provided for i.MX23 by using the PWM. It was
> tested on a custom i.MX23 base board.

Two things that bother me in this series.

First thing is that I wonder if it would be better to not
register a seperate device for backlight. How about a call
like this:

fb_register_backlight(const char *fb,
	void (*set_brightness)(int brightness, void *priv));

The core would only need a function to find the struct device_d
by the corresponding "fbx" string. This way we could add the
brightness variable to the framebuffer and not a seperate device,
so fb0.brightness=50.

The second thing is that the pwm you use for the mxs backlight
is a generic pwm which not necessarily drives a backlight. We
should have a generic pwm api for this. Otherwise we end up
with different drivers for the same pwm.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC/PATCH 0/2] Backlight support

[Gregory CLEMENT]

On 10/07/2011 03:15 PM, Sascha Hauer wrote:
> Hi Gregory,
> 
Hi Sasha,

thanks to have taken some time to have a look on my series.

> On Thu, Oct 06, 2011 at 10:05:07AM +0200, Gregory CLEMENT wrote:
>> This patch set is a RFC about a backlight framework. The purpose of
>> this framework is mainly to allow to add easily a support for a
>> backlight with the possibility of setting brightness directly from the
>> barebox shell using the brightness parameter.
>>
>> An implementation is provided for i.MX23 by using the PWM. It was
>> tested on a custom i.MX23 base board.
> 
> Two things that bother me in this series.
> 
> First thing is that I wonder if it would be better to not
> register a seperate device for backlight. How about a call
> like this:
> 
> fb_register_backlight(const char *fb,
> 	void (*set_brightness)(int brightness, void *priv));
> 
> The core would only need a function to find the struct device_d
> by the corresponding "fbx" string. This way we could add the
> brightness variable to the framebuffer and not a seperate device,
> so fb0.brightness=50.
> 

I wasn't entirely convinced by having a driver with a single
function. I came to this by several iterations and it didn't lead me
in the best direction. That's why I called this series a RFC. I will
take in account your idea and propose a new version.

> The second thing is that the pwm you use for the mxs backlight
> is a generic pwm which not necessarily drives a backlight. We
> should have a generic pwm api for this. Otherwise we end up
> with different drivers for the same pwm.

As I didn't see any pwm framework or API I wasn't sure it was planned
or needed to have it in barebox. But I volunteer to work on it. So how
do you see it: as an API or as complete driver ?

Gregory

-- 
Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC/PATCH 0/2] Backlight support

[Sascha Hauer]

On Fri, Oct 07, 2011 at 10:47:34PM +0200, Gregory CLEMENT wrote:
> On 10/07/2011 03:15 PM, Sascha Hauer wrote:
> > Hi Gregory,
> > 
> Hi Sasha,
> 
> thanks to have taken some time to have a look on my series.
> 
> > On Thu, Oct 06, 2011 at 10:05:07AM +0200, Gregory CLEMENT wrote:
> >> This patch set is a RFC about a backlight framework. The purpose of
> >> this framework is mainly to allow to add easily a support for a
> >> backlight with the possibility of setting brightness directly from the
> >> barebox shell using the brightness parameter.
> >>
> >> An implementation is provided for i.MX23 by using the PWM. It was
> >> tested on a custom i.MX23 base board.
> > 
> > Two things that bother me in this series.
> > 
> > First thing is that I wonder if it would be better to not
> > register a seperate device for backlight. How about a call
> > like this:
> > 
> > fb_register_backlight(const char *fb,
> > 	void (*set_brightness)(int brightness, void *priv));
> > 
> > The core would only need a function to find the struct device_d
> > by the corresponding "fbx" string. This way we could add the
> > brightness variable to the framebuffer and not a seperate device,
> > so fb0.brightness=50.
> > 
> 
> I wasn't entirely convinced by having a driver with a single
> function. I came to this by several iterations and it didn't lead me
> in the best direction. That's why I called this series a RFC. I will
> take in account your idea and propose a new version.
> 
> > The second thing is that the pwm you use for the mxs backlight
> > is a generic pwm which not necessarily drives a backlight. We
> > should have a generic pwm api for this. Otherwise we end up
> > with different drivers for the same pwm.
> 
> As I didn't see any pwm framework or API I wasn't sure it was planned
> or needed to have it in barebox. But I volunteer to work on it. So how
> do you see it: as an API or as complete driver ?

Maybe you can use https://lkml.org/lkml/2011/6/30/163 as a template.
It should be easy to convert to barebox and also gives us the chance
that we can easily adopt drivers from the kernel.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox