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

[Thierry Reding]

On Fri, Nov 01, 2013 at 08:37:23AM +0900, Jingoo Han wrote:
> On Wednesday, October 23, 2013 5:02 AM, Thierry Reding wrote:
> > 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 at rpsys.net>
> > > > > > Cc: Jingoo Han <jg1.han at samsung.com>
> > > > > > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > > > Cc: Rob Herring <rob.herring at calxeda.com>
> > > > > > Cc: Pawel Moll <pawel.moll at arm.com>
> > > > > > Cc: Mark Rutland <mark.rutland at arm.com>
> > > > > > Cc: Stephen Warren <swarren at wwwdotorg.org>
> > > > > > Cc: Ian Campbell <ijc+devicetree at hellion.org.uk>
> > > > > > Cc: devicetree at vger.kernel.org
> > > > > > Cc: Sascha Hauer <kernel at pengutronix.de>
> > > > > > Cc: linux-arm-kernel at lists.infradead.org
> > > > > > Cc: Lothar Waßmann <LW at KARO-electronics.de>
> > > > > > Cc: Jean-Christophe Plagniol-Villard <plagnioj at jcrosoft.com>
> > > > > > Cc: Eric Bénard <eric at eukrea.com>
> > > > > > Signed-off-by: Denis Carikli <denis at eukrea.com>
> > > > > > ---
> > > > > > 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.
> 
> I agree with your opinion.
> But, I can't decide how to change it.

One solution that Stephen proposed was to make all DT-based setups use a
new default (off). An advantage of that is that such setups are still
fairly actively being worked on, so we can probably get early adopters
to cope with the new default.

Looking through some DTS files it seems that many use the pwm-backlight
driver. So if we can get some concensus from all the users that changing
the default would be okay, then I think we could reasonably change it.

Another solution perhaps would be to add a property to DT which encodes
the default state of the backlight on boot. This used to be impossible
because the general concensus was that DT should describe hardware only
and not software policy. During the kernel summit this requirement was
somewhat relaxed and it should now be okay to describe system
configuration data, and I think this would be a good match. If the
system is designed to keep the backlight off by default because some
other component (display panel driver) is meant to turn it on later,
that's system configuration data, right?

Perhaps a boolean property could be used:

	backlight {
		compatible = "pwm-backlight";

		...

		backlight-default-off;
	};

> > > 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.
> 
> Jean-Christophe's point is right.
> 
> We may need to discuss 'the way to detect what value the bootloader
> set a GPIO to'.

Since some GPIO hardware simply can't do it, I guess we could resolve to
passing such information via DT. That obviously won't work for non-DT
setups, but I guess that's something we could live with.

One possibility would be to supplement the backlight-default-off
property with another property (backlight-boot-on) that can be passed on
to the kernel from the bootloader to signal that it has turned the
backlight on.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131101/72ee32dc/attachment-0001.sig>