Re: [PATCHv4] video: backlight: gpio-backlight: Add DT support.

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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

On Tue, Oct 22, 2013 at 05:34:45PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:23 Tue 22 Oct     , Thierry Reding wrote:
> > On Tue, Oct 22, 2013 at 06:58:33AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 11:13 Mon 21 Oct     , Denis Carikli wrote:
> > > > Cc: Richard Purdie <rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org>
> > > > Cc: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > Cc: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > > > Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > > > Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> > > > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > > > Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> > > > Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> > > > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> > > > Cc: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
> > > > Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > > > Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
> > > > Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > > ChangeLog v3->v4:
> > > > - The default-brightness property is now optional, it defaults to 1 if not set.
> > > by default we set OFF not ON
> > > 
> > > do not actiate driver or properti by default you can not known to consequence
> > > on the hw
> > 
> > Turning on a backlight by default is what pretty much every backlight
> > driver does. I personally think that's the wrong default, I even tried
> > to get some discussion started recently about how we could change this.
> > However, given that this has been the case for possibly as long as the
> > subsystem has existed, suddenly changing it might cause quite a few of
> > our users to boot the new kernel and not see their display come up. As
> > with any other ABI, this isn't something we can just change without a
> > very good migration path.
> 
> I'm sorry but the blacklight descibe in DT have nothing to do with the common
> pratice that the current driver have today

That's not at all what I said. What I said was that the majority of
backlight drivers currently default to turning the backlight on when
probed. Therefore I think it would be consistent if this driver did the
same.

I also said that I don't think it's a very good default, but at the same
time we can't just go and change the default behaviour at will because
people may rely on it.

> put on by default if wrong specially without the property define. Even put it
> on by default it wrong as the bootloader may have set it already for splash
> screen and to avoid glitch the drivers need to detect this.

I agree that would be preferable, but I don't know of any way to detect
what value the bootloader set a GPIO to. The GPIO API requires that you
call gpio_direction_output(), and that requires a value parameter which
will be used as the output level of the GPIO.

> For me this should not even be a property but handled by the driver them
> selves in C.

Agreed. There has been some discussion recently about whether devicetree
should be extended (or supplemented) to allow defining behaviour as well
(in addition to just hardware). But that's not immediately relevant here
at this time.

Thierry

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