[PATCH V3 0/2] Add mandatory regulator for all users of pwm-backlight.

[PATCH V3 0/2] Add mandatory regulator for all users of pwm-backlight.

[Andrew Chew]

Many backlights are enabled via GPIO.  We can generalize the GPIO to a
fixed regulator.

The enable regulator needs to be mandatory because there was no good way
to determine the difference between opting out of the regulator, and probe
deferral.

This series of patches is intended to add a dummy regulator (or a GPIO
regulator) for all users of the pwm-backlight.

The last patch in the series will always be the pwm-backlight change to add
this mandatory regulator.  Patches following up to that patch add the
mandatory regulator on a per mach family basis.  Once all users of
pwm-backlight have been patched, this series can be applied in order to
maintain bisectability.

Andrew Chew (2):
  ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight
  pwm_bl: Add mandatory backlight enable regulator

 .../bindings/video/backlight/pwm-backlight.txt     |   14 +++++
 arch/arm/mach-omap2/board-4430sdp.c                |    5 ++
 drivers/video/backlight/pwm_bl.c                   |   55 ++++++++++++++++----
 3 files changed, 64 insertions(+), 10 deletions(-)

-- 
1.7.9.5

[PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Andrew Chew]

The pwm-backlight driver now takes a mandatory regulator that is gotten
during driver probe.  Initialize a dummy regulator to satisfy this
requirement.

Signed-off-by: Andrew Chew <achew@nvidia.com>
---
Changed the device name of the backlight regulator supply to "pwm-backlight",
per Peter's comment.

Changed the name of the regulator to "backlight-enable", per Thierry's
suggestion.

 arch/arm/mach-omap2/board-4430sdp.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 35f3ad0..a01a39a 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -291,6 +291,10 @@ static struct platform_device sdp4430_leds_pwm = {
 	},
 };
 
+/* Dummy regulator for pwm-backlight driver */
+static struct regulator_consumer_supply backlight_supply =
+	REGULATOR_SUPPLY("enable", "pwm-backlight");
+
 static struct platform_pwm_backlight_data sdp4430_backlight_data = {
 	.max_brightness = 127,
 	.dft_brightness = 127,
@@ -718,6 +722,8 @@ static void __init omap_4430sdp_init(void)
 
 	omap4_i2c_init();
 	omap_sfh7741prox_init();
+	regulator_register_always_on(-1, "backlight-enable",
+				     &backlight_supply, 1, 0);
 	platform_add_devices(sdp4430_devices, ARRAY_SIZE(sdp4430_devices));
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
-- 
1.7.9.5

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Tony Lindgren]

* Andrew Chew <achew@nvidia.com> [130313 15:37]:
> The pwm-backlight driver now takes a mandatory regulator that is gotten
> during driver probe.  Initialize a dummy regulator to satisfy this
> requirement.
> 
> Signed-off-by: Andrew Chew <achew@nvidia.com>
> ---
> Changed the device name of the backlight regulator supply to "pwm-backlight",
> per Peter's comment.
> 
> Changed the name of the regulator to "backlight-enable", per Thierry's
> suggestion.

Thanks applying into omap-for-v3.10/board. Note that I'm not taking the
second one, that should be resent to the related driver maintainers.
You can get that list by running scripts/get_maintainer.pl against it.

Regards,

Tony
 
>  arch/arm/mach-omap2/board-4430sdp.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 35f3ad0..a01a39a 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -291,6 +291,10 @@ static struct platform_device sdp4430_leds_pwm = {
>  	},
>  };
>  
> +/* Dummy regulator for pwm-backlight driver */
> +static struct regulator_consumer_supply backlight_supply =
> +	REGULATOR_SUPPLY("enable", "pwm-backlight");
> +
>  static struct platform_pwm_backlight_data sdp4430_backlight_data = {
>  	.max_brightness = 127,
>  	.dft_brightness = 127,
> @@ -718,6 +722,8 @@ static void __init omap_4430sdp_init(void)
>  
>  	omap4_i2c_init();
>  	omap_sfh7741prox_init();
> +	regulator_register_always_on(-1, "backlight-enable",
> +				     &backlight_supply, 1, 0);
>  	platform_add_devices(sdp4430_devices, ARRAY_SIZE(sdp4430_devices));
>  	omap_serial_init();
>  	omap_sdrc_init(NULL, NULL);
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Thierry Reding]

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

On Mon, Apr 08, 2013 at 02:46:24PM -0700, Tony Lindgren wrote:
> * Andrew Chew <achew@nvidia.com> [130313 15:37]:
> > The pwm-backlight driver now takes a mandatory regulator that is gotten
> > during driver probe.  Initialize a dummy regulator to satisfy this
> > requirement.
> > 
> > Signed-off-by: Andrew Chew <achew@nvidia.com>
> > ---
> > Changed the device name of the backlight regulator supply to "pwm-backlight",
> > per Peter's comment.
> > 
> > Changed the name of the regulator to "backlight-enable", per Thierry's
> > suggestion.
> 
> Thanks applying into omap-for-v3.10/board. Note that I'm not taking the
> second one, that should be resent to the related driver maintainers.
> You can get that list by running scripts/get_maintainer.pl against it.

The plan was to take these all through one tree, preferably the PWM tree
because it's all PWM related and the pwm-backlight change will go
through that tree as well. Technically these individual patches can be
applied as is and aren't harmful, but keeping track of the dependencies
might be difficult if they go in via separate trees.

Thierry

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

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Tony Lindgren]

* Thierry Reding <thierry.reding@avionic-design.de> [130408 15:01]:
> On Mon, Apr 08, 2013 at 02:46:24PM -0700, Tony Lindgren wrote:
> > * Andrew Chew <achew@nvidia.com> [130313 15:37]:
> > > The pwm-backlight driver now takes a mandatory regulator that is gotten
> > > during driver probe.  Initialize a dummy regulator to satisfy this
> > > requirement.
> > > 
> > > Signed-off-by: Andrew Chew <achew@nvidia.com>
> > > ---
> > > Changed the device name of the backlight regulator supply to "pwm-backlight",
> > > per Peter's comment.
> > > 
> > > Changed the name of the regulator to "backlight-enable", per Thierry's
> > > suggestion.
> > 
> > Thanks applying into omap-for-v3.10/board. Note that I'm not taking the
> > second one, that should be resent to the related driver maintainers.
> > You can get that list by running scripts/get_maintainer.pl against it.
> 
> The plan was to take these all through one tree, preferably the PWM tree
> because it's all PWM related and the pwm-backlight change will go
> through that tree as well. Technically these individual patches can be
> applied as is and aren't harmful, but keeping track of the dependencies
> might be difficult if they go in via separate trees.

Registering the regulator alone should not do anything. Also the driver
should do the right thing if the regulator is not yet registered.

Can you please check your driver patch so the driver won't do anything
if the regulator is not (yet) registered and repost it alone as I've
already applied the board-*.c change.

The reason why we want to do queue these patches separately is to cut
away the dependency between drivers and the core arch/arm changes. 
Otherwise we'll end up with pointless merge conflicts as we've seen
earlier several times with the USB patches for example.

Regards,

Tony

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Thierry Reding]

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

On Mon, Apr 08, 2013 at 03:16:57PM -0700, Tony Lindgren wrote:
> * Thierry Reding <thierry.reding@avionic-design.de> [130408 15:01]:
> > On Mon, Apr 08, 2013 at 02:46:24PM -0700, Tony Lindgren wrote:
> > > * Andrew Chew <achew@nvidia.com> [130313 15:37]:
> > > > The pwm-backlight driver now takes a mandatory regulator that is gotten
> > > > during driver probe.  Initialize a dummy regulator to satisfy this
> > > > requirement.
> > > > 
> > > > Signed-off-by: Andrew Chew <achew@nvidia.com>
> > > > ---
> > > > Changed the device name of the backlight regulator supply to "pwm-backlight",
> > > > per Peter's comment.
> > > > 
> > > > Changed the name of the regulator to "backlight-enable", per Thierry's
> > > > suggestion.
> > > 
> > > Thanks applying into omap-for-v3.10/board. Note that I'm not taking the
> > > second one, that should be resent to the related driver maintainers.
> > > You can get that list by running scripts/get_maintainer.pl against it.
> > 
> > The plan was to take these all through one tree, preferably the PWM tree
> > because it's all PWM related and the pwm-backlight change will go
> > through that tree as well. Technically these individual patches can be
> > applied as is and aren't harmful, but keeping track of the dependencies
> > might be difficult if they go in via separate trees.
> 
> Registering the regulator alone should not do anything. Also the driver
> should do the right thing if the regulator is not yet registered.
> 
> Can you please check your driver patch so the driver won't do anything
> if the regulator is not (yet) registered and repost it alone as I've
> already applied the board-*.c change.

That's not the way that the regulator subsystem works, unfortunately.
There's no way you can distinguish between the case where a regulator
just hasn't been registered yet, or whether it's missing. That's the
whole reason why we need to add the dummy regulators in the first
place.

> The reason why we want to do queue these patches separately is to cut
> away the dependency between drivers and the core arch/arm changes. 
> Otherwise we'll end up with pointless merge conflicts as we've seen
> earlier several times with the USB patches for example.

We could possibly postpone merging the pwm-backlight changes until all
other patches have been merged. If you have this in you for-3.10 branch
I guess it will go into linux-next when the 3.9 merge window closes? If
so I could possibly base my for-3.10 branch on your branch if you can
provide a stable one that you can guarantee not to rebase. There are
other alternatives too, but certainly the easiest would've been to take
all patches through the PWM tree. The potential for merge conflicts
should be rather minimal.

Thierry

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

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Tony Lindgren]

* Thierry Reding <thierry.reding@avionic-design.de> [130409 01:00]:
> On Mon, Apr 08, 2013 at 03:16:57PM -0700, Tony Lindgren wrote:
> > * Thierry Reding <thierry.reding@avionic-design.de> [130408 15:01]:
> > > On Mon, Apr 08, 2013 at 02:46:24PM -0700, Tony Lindgren wrote:
> > > > * Andrew Chew <achew@nvidia.com> [130313 15:37]:
> > > > > The pwm-backlight driver now takes a mandatory regulator that is gotten
> > > > > during driver probe.  Initialize a dummy regulator to satisfy this
> > > > > requirement.
> > > > > 
> > > > > Signed-off-by: Andrew Chew <achew@nvidia.com>
> > > > > ---
> > > > > Changed the device name of the backlight regulator supply to "pwm-backlight",
> > > > > per Peter's comment.
> > > > > 
> > > > > Changed the name of the regulator to "backlight-enable", per Thierry's
> > > > > suggestion.
> > > > 
> > > > Thanks applying into omap-for-v3.10/board. Note that I'm not taking the
> > > > second one, that should be resent to the related driver maintainers.
> > > > You can get that list by running scripts/get_maintainer.pl against it.
> > > 
> > > The plan was to take these all through one tree, preferably the PWM tree
> > > because it's all PWM related and the pwm-backlight change will go
> > > through that tree as well. Technically these individual patches can be
> > > applied as is and aren't harmful, but keeping track of the dependencies
> > > might be difficult if they go in via separate trees.
> > 
> > Registering the regulator alone should not do anything. Also the driver
> > should do the right thing if the regulator is not yet registered.
> > 
> > Can you please check your driver patch so the driver won't do anything
> > if the regulator is not (yet) registered and repost it alone as I've
> > already applied the board-*.c change.
> 
> That's not the way that the regulator subsystem works, unfortunately.
> There's no way you can distinguish between the case where a regulator
> just hasn't been registered yet, or whether it's missing. That's the
> whole reason why we need to add the dummy regulators in the first
> place.

But then the regulator is not found and the driver should just exit,
or do nothing. If this is an optional regulator, then that should be
indicated in some platform data flags?
 
> > The reason why we want to do queue these patches separately is to cut
> > away the dependency between drivers and the core arch/arm changes. 
> > Otherwise we'll end up with pointless merge conflicts as we've seen
> > earlier several times with the USB patches for example.
> 
> We could possibly postpone merging the pwm-backlight changes until all
> other patches have been merged. If you have this in you for-3.10 branch
> I guess it will go into linux-next when the 3.9 merge window closes? If
> so I could possibly base my for-3.10 branch on your branch if you can
> provide a stable one that you can guarantee not to rebase. There are
> other alternatives too, but certainly the easiest would've been to take
> all patches through the PWM tree. The potential for merge conflicts
> should be rather minimal.

The driver parts really must be done in independently from any platform
data or .dts changes. The only common part needed should be changes
to include/linux/platform_data/*.h files.

Regards,

Tony

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Thierry Reding]

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

On Tue, Apr 09, 2013 at 09:40:04AM -0700, Tony Lindgren wrote:
[...]
> But then the regulator is not found and the driver should just exit,
> or do nothing. If this is an optional regulator, then that should be
> indicated in some platform data flags?

Yes, if the regulator isn't found then the driver fails. However the
goal was to maintain bisectability. If we apply them in the wrong order
we can't guarantee that because pwm-backlight will fail to work between
both patches.

> The driver parts really must be done in independently from any platform
> data or .dts changes. The only common part needed should be changes
> to include/linux/platform_data/*.h files.

We don't even need to touch platform data because the regulators are
looked up via a global table. And the changes are all done independently
but as I mentioned above, bisectability isn't maintained, so the
preferred patch order is the one in which pwm-backlight keeps working at
each point in the commit history.

Thierry

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

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Tony Lindgren]

* Thierry Reding <thierry.reding@avionic-design.de> [130409 12:45]:
> On Tue, Apr 09, 2013 at 09:40:04AM -0700, Tony Lindgren wrote:
> [...]
> > But then the regulator is not found and the driver should just exit,
> > or do nothing. If this is an optional regulator, then that should be
> > indicated in some platform data flags?
> 
> Yes, if the regulator isn't found then the driver fails. However the
> goal was to maintain bisectability. If we apply them in the wrong order
> we can't guarantee that because pwm-backlight will fail to work between
> both patches.

But it's fixing something that's not working anyways for board-4430sdp.c,
It seems so as these patches just add new features?
 
> > The driver parts really must be done in independently from any platform
> > data or .dts changes. The only common part needed should be changes
> > to include/linux/platform_data/*.h files.
> 
> We don't even need to touch platform data because the regulators are
> looked up via a global table. And the changes are all done independently
> but as I mentioned above, bisectability isn't maintained, so the
> preferred patch order is the one in which pwm-backlight keeps working at
> each point in the commit history.

Bisectability is a good point. But in the 4430sdp case I'm sure it's enough
that it builds and boots no matter how the patches get merged :)

Regards,

Tony

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Thierry Reding]

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

On Tue, Apr 09, 2013 at 01:17:46PM -0700, Tony Lindgren wrote:
> * Thierry Reding <thierry.reding@avionic-design.de> [130409 12:45]:
> > On Tue, Apr 09, 2013 at 09:40:04AM -0700, Tony Lindgren wrote:
> > [...]
> > > But then the regulator is not found and the driver should just exit,
> > > or do nothing. If this is an optional regulator, then that should be
> > > indicated in some platform data flags?
> > 
> > Yes, if the regulator isn't found then the driver fails. However the
> > goal was to maintain bisectability. If we apply them in the wrong order
> > we can't guarantee that because pwm-backlight will fail to work between
> > both patches.
> 
> But it's fixing something that's not working anyways for board-4430sdp.c,
> It seems so as these patches just add new features?

Not quite. It's adding a dummy regulator to represent hardware where the
enable pin is always high. So I think pwm-backlight will work in the
current state, but if we make the pwm-backlight driver change without
adding the dummy regulator, then pwm-backlight will fail to probe and
therefore the PWM won't be enabled either and the display will stay
black.

> > > The driver parts really must be done in independently from any platform
> > > data or .dts changes. The only common part needed should be changes
> > > to include/linux/platform_data/*.h files.
> > 
> > We don't even need to touch platform data because the regulators are
> > looked up via a global table. And the changes are all done independently
> > but as I mentioned above, bisectability isn't maintained, so the
> > preferred patch order is the one in which pwm-backlight keeps working at
> > each point in the commit history.
> 
> Bisectability is a good point. But in the 4430sdp case I'm sure it's enough
> that it builds and boots no matter how the patches get merged :)

If you don't mind some intermediary breakage, I will no longer object.

Thierry

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

Re: [PATCH V3 1/2] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

[Tony Lindgren]

* Thierry Reding <thierry.reding@avionic-design.de> [130409 14:01]:
> On Tue, Apr 09, 2013 at 01:17:46PM -0700, Tony Lindgren wrote:
> > * Thierry Reding <thierry.reding@avionic-design.de> [130409 12:45]:
> > > On Tue, Apr 09, 2013 at 09:40:04AM -0700, Tony Lindgren wrote:
> > > [...]
> > > > But then the regulator is not found and the driver should just exit,
> > > > or do nothing. If this is an optional regulator, then that should be
> > > > indicated in some platform data flags?
> > > 
> > > Yes, if the regulator isn't found then the driver fails. However the
> > > goal was to maintain bisectability. If we apply them in the wrong order
> > > we can't guarantee that because pwm-backlight will fail to work between
> > > both patches.
> > 
> > But it's fixing something that's not working anyways for board-4430sdp.c,
> > It seems so as these patches just add new features?
> 
> Not quite. It's adding a dummy regulator to represent hardware where the
> enable pin is always high. So I think pwm-backlight will work in the
> current state, but if we make the pwm-backlight driver change without
> adding the dummy regulator, then pwm-backlight will fail to probe and
> therefore the PWM won't be enabled either and the display will stay
> black.

OK
 
> > > > The driver parts really must be done in independently from any platform
> > > > data or .dts changes. The only common part needed should be changes
> > > > to include/linux/platform_data/*.h files.
> > > 
> > > We don't even need to touch platform data because the regulators are
> > > looked up via a global table. And the changes are all done independently
> > > but as I mentioned above, bisectability isn't maintained, so the
> > > preferred patch order is the one in which pwm-backlight keeps working at
> > > each point in the commit history.
> > 
> > Bisectability is a good point. But in the 4430sdp case I'm sure it's enough
> > that it builds and boots no matter how the patches get merged :)
> 
> If you don't mind some intermediary breakage, I will no longer object.

In this case it should be fine. If you are worried about it, you could
add something that enables the new features in the driver only in
a follow-up patch after the merge window. But I doubt that it's needed.

Regards,

Tony

[PATCH V3 2/2] pwm_bl: Add mandatory backlight enable regulator

[Andrew Chew]

Many backlights need to be explicitly enabled.  Typically, this is done
with a GPIO.  For flexibility, we generalize the enable mechanism to a
regulator.

If an enable regulator is not needed, then a dummy regulator can be given
to the backlight driver.  If a GPIO is used to enable the backlight,
then a fixed regulator can be instantiated to control the GPIO.

The backlight enable regulator can be specified in the device tree node
for the backlight, or can be done with legacy board setup code in the
usual way.

Signed-off-by: Andrew Chew <achew@nvidia.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
Minor modification to pwm_backlight_enable() and pwm_backlight_disable(),
to keep the error value of the regulator call and report the error value
in the case of an error, from Peter and Thierry's feedback.  Used "ret"
to be consistent with the rest of the driver.

 .../bindings/video/backlight/pwm-backlight.txt     |   14 +++++
 drivers/video/backlight/pwm_bl.c                   |   59 ++++++++++++++++----
 2 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..7e2e089 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -10,6 +10,11 @@ Required properties:
       last value in the array represents a 100% duty cycle (brightest).
   - default-brightness-level: the default brightness level (index into the
       array defined by the "brightness-levels" property)
+  - enable-supply: A phandle to the regulator device tree node. This
+      regulator will be turned on and off as the pwm is enabled and disabled.
+      Many backlights are enabled via a GPIO. In this case, we instantiate
+      a fixed regulator and give that to enable-supply. If a regulator
+      is not needed, then provide a dummy fixed regulator.
 
 Optional properties:
   - pwm-names: a list of names for the PWM devices specified in the
@@ -19,10 +24,19 @@ Optional properties:
 
 Example:
 
+	bl_en: fixed-regulator {
+                compatible = "regulator-fixed";
+                regulator-name = "bl-en-supply";
+                regulator-boot-on;
+                gpio = <&some_gpio>;
+                enable-active-high;
+	};
+
 	backlight {
 		compatible = "pwm-backlight";
 		pwms = <&pwm 0 5000000>;
 
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
+		enable-supply = <&bl_en>;
 	};
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1fea627..e4922f5 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -20,10 +20,13 @@
 #include <linux/pwm.h>
 #include <linux/pwm_backlight.h>
 #include <linux/slab.h>
+#include <linux/regulator/consumer.h>
 
 struct pwm_bl_data {
 	struct pwm_device	*pwm;
 	struct device		*dev;
+	bool			enabled;
+	struct regulator	*enable_supply;
 	unsigned int		period;
 	unsigned int		lth_brightness;
 	unsigned int		*levels;
@@ -35,6 +38,42 @@ struct pwm_bl_data {
 	void			(*exit)(struct device *);
 };
 
+static void pwm_backlight_enable(struct backlight_device *bl)
+{
+	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+	int ret;
+
+	/* Bail if we are already enabled. */
+	if (pb->enabled)
+		return;
+
+	pwm_enable(pb->pwm);
+
+	ret = regulator_enable(pb->enable_supply);
+	if (ret)
+		dev_warn(&bl->dev, "Failed to enable regulator: %d", ret);
+
+	pb->enabled = true;
+}
+
+static void pwm_backlight_disable(struct backlight_device *bl)
+{
+	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+	int ret;
+
+	/* Bail if we are already disabled. */
+	if (!pb->enabled)
+		return;
+
+	ret = regulator_disable(pb->enable_supply);
+	if (ret)
+		dev_warn(&bl->dev, "Failed to disable regulator: %d", ret);
+
+	pwm_disable(pb->pwm);
+
+	pb->enabled = false;
+}
+
 static int pwm_backlight_update_status(struct backlight_device *bl)
 {
 	struct pwm_bl_data *pb = bl_get_data(bl);
@@ -51,7 +90,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 
 	if (brightness == 0) {
 		pwm_config(pb->pwm, 0, pb->period);
-		pwm_disable(pb->pwm);
+		pwm_backlight_disable(bl);
 	} else {
 		int duty_cycle;
 
@@ -65,7 +104,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 		duty_cycle = pb->lth_brightness +
 		     (duty_cycle * (pb->period - pb->lth_brightness) / max);
 		pwm_config(pb->pwm, duty_cycle, pb->period);
-		pwm_enable(pb->pwm);
+		pwm_backlight_enable(bl);
 	}
 
 	if (pb->notify_after)
@@ -138,12 +177,6 @@ static int pwm_backlight_parse_dt(struct device *dev,
 		data->max_brightness--;
 	}
 
-	/*
-	 * TODO: Most users of this driver use a number of GPIOs to control
-	 *       backlight power. Support for specifying these needs to be
-	 *       added.
-	 */
-
 	return 0;
 }
 
@@ -206,6 +239,12 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	pb->exit = data->exit;
 	pb->dev = &pdev->dev;
 
+	pb->enable_supply = devm_regulator_get(&pdev->dev, "enable");
+	if (IS_ERR(pb->enable_supply)) {
+		ret = PTR_ERR(pb->enable_supply);
+		goto err_alloc;
+	}
+
 	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
 	if (IS_ERR(pb->pwm)) {
 		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
@@ -268,7 +307,7 @@ static int pwm_backlight_remove(struct platform_device *pdev)
 
 	backlight_device_unregister(bl);
 	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
+	pwm_backlight_disable(bl);
 	if (pb->exit)
 		pb->exit(&pdev->dev);
 	return 0;
@@ -283,7 +322,7 @@ static int pwm_backlight_suspend(struct device *dev)
 	if (pb->notify)
 		pb->notify(pb->dev, 0);
 	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
+	pwm_backlight_disable(bl);
 	if (pb->notify_after)
 		pb->notify_after(pb->dev, 0);
 	return 0;
-- 
1.7.9.5

Re: [PATCH V3 0/2] Add mandatory regulator for all users of pwm-backlight.

[Peter Ujfalusi]

On 03/13/2013 11:33 PM, Andrew Chew wrote:
> Many backlights are enabled via GPIO.  We can generalize the GPIO to a
> fixed regulator.
> 
> The enable regulator needs to be mandatory because there was no good way
> to determine the difference between opting out of the regulator, and probe
> deferral.
> 
> This series of patches is intended to add a dummy regulator (or a GPIO
> regulator) for all users of the pwm-backlight.
> 
> The last patch in the series will always be the pwm-backlight change to add
> this mandatory regulator.  Patches following up to that patch add the
> mandatory regulator on a per mach family basis.  Once all users of
> pwm-backlight have been patched, this series can be applied in order to
> maintain bisectability.

I'm not really happy with the mandatory regulator since for example the
SDP4430 should not need one... Other than that the backlight works after the
series.

To both patch:

Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>