Re: [PATCH 1/1] pwm_bl: Add support for backlight enable GPIO

[Thierry Reding]

On Tue, Mar 05, 2013 at 01:59:06PM +0900, Alex Courbot wrote:
> On 03/05/2013 01:48 PM, Andrew Chew wrote:
> >I sent out a new patch that enables/disables the backlight enable gpio.
> >
> >>On 03/05/2013 01:00 PM, Andrew Chew wrote:
> >>>I did come to the same conclusion regarding the platform data breakage.
> >>>I'm expecting that the use of platform data will go away, at least on
> >>>ARM, since we are all aggressively moving what used to be in platform
> >>>data into the device tree.  Do other platforms use this driver?
> >>
> >>I can see at least 29 users of platform_pwm_backlight_data, all ARM with the
> >>exception of one unicore32. I guess at least for the foreseeable future
> >>platform data will remain.
> >
> >I'm not sure how to solve this, then.  Any suggestions?
> 
> In one of my (many) attempts to add power sequencing to
> pwm-backlight, I just added a boolean to the platform data that must
> be explicitly set in order to enable control by GPIO. I.e.
> 
> 	bool			use_enable_gpio
> 	int			enable_gpio;
> 	unsigned int		enable_gpio_flags;
> 
> enable_gpio and enable_gpio_flags would then only be considered if
> use_enable_gpio is true. Granted, it's not the best solution here
> but that's the only way to handle this correctly with integer GPIOS,
> and it does not pollute the DT anyway (use_enable_gpio will only be
> set by pwm_backlight_parse_dt() if of_get_named_gpio() returned a
> valid GPIO. Btw, you also want to check if the enable-gpio property
> exists first because otherwise probe() will fails if no GPIO is
> specified).

There are two more options that I can see. The first involves updating
all users to initialize the GPIO to -1 in the platform data, which will
remove the requirement for an extra flag field. Another option would be
to make this feature DT only, so that the GPIO can always be assumed to
be -1 for non-DT and DT without an enable-gpio property.

There's a third alternative, namely using a regulator for this, which
has better lookup support and therefore should be easier to make
optional. And as Alex mentioned it already has support for always-on
functionality and such.

Thierry

Attachment: pgp00000.pgp
Description: PGP signature