Re: [PATCH] pwm-backlight: add support for device tree gpio control

[Thierry Reding]

On Tue, Sep 03, 2013 at 12:26:12PM -0700, Mike Dunn wrote:
> This patch adds support for controlling an arbitrary number of gpios to the
> pwm-backlight driver.  This was left as a TODO when initial device tree support
> was added by Thierry a while back.  This functionality replaces the callbacks
> that are passed in the platform data for non-DT cases.  Users can avail
> themselves of this feature by adding a 'gpios' property to the 'backlight' node.
> When the update_status() callback in backlight_ops runs, the gpios listed in the
> property are asserted/deasserted if the specified brightness is non-zero/zero.
> 
> Tested on a pxa270-based Palm Treo 680.
> 
> Signed-off-by: Mike Dunn <mikedunn@xxxxxxxxxxx>
> ---
> 
> Thanks for looking!
> 
>  .../bindings/video/backlight/pwm-backlight.txt     |   4 +
>  drivers/video/backlight/pwm_bl.c                   | 128 ++++++++++++++++++---
>  2 files changed, 113 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> index 1e4fc72..4583e68 100644
> --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> @@ -14,6 +14,9 @@ Required properties:
>  Optional properties:
>    - pwm-names: a list of names for the PWM devices specified in the
>                 "pwms" property (see PWM binding[0])
> +  - gpios:     An arbitrary number of gpios that must be asserted when the
> +               backlight is on, and de-asserted when off.  They will be asserted
> +               in the order they appear, and de-asserted in reverse order.

Do you have a real setup that actually needs multiple GPIOs? Usually
such a setup requires some kind of timing or other additional constraint
which can't be represented by this simple binding.

Looking at the Palm Treo code it seems like the reason why multiple
GPIOs are needed is because one is to enable the backlight, while the
other is in fact used to enable the LCD panel. I have been working on a
patch series to provide simple panel support, specifically to allow the
separation of backlight and panel power sequencing. Would such a method
work for your use-case as well? The work that I'm doing is somewhat DRM
centric, and I don't think there's a DRM driver for PXA, but perhaps it
would be a good occasion to look at converting the PXA display drivers
to DRM... =)

That said, I've had a patch similar to yours in a local tree (in fact in
the same branch as the panel work I've been doing) for quite some time,
which allows a single "enable" GPIO to be specified. I haven't gotten
around to sending it out yet, but I'll do that shortly. The patch is a
little more involved because it exposes the GPIO via platform data as
well and therefore has to update a number of board files, too. While
going over the various board files I found only a single board which can
actually make use of the new functionality (which I also converted in
the patch). Any other cases couldn't be implemented by the simple change
so I suspect they can't either using your proposed binding.

I've been trying for a while to come up with a way to support more use-
cases, and I keep coming back to the same solution. Since the DT binding
for power sequences was shot down some time ago, we probably have to
represent any kind of sequencing in C code. So instead of trying to fit
everything into a single binding, I think a more maintainable solution
would be to create separate drivers for the more complex use-cases. That
could either be done by using separate compatible values within the
pwm-backlight driver or via completely different drivers. In the latter
case we should probably think about exporting some of the pwm-backlight
functionality so that drivers can easily reuse some of the code.

Thierry

Attachment: pgp00000.pgp
Description: PGP signature