| Message ID | 1363719573-20926-7-git-send-email-achew@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear 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> Do we really need a mandatory regulator? Why can't it be optional? Best regards, Marek Vasut
> Dear 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> > > Do we really need a mandatory regulator? Why can't it be optional? > > Best regards, > Marek Vasut Because for those using board setup code (and not devicetree), it was not possible to distinguish between opting out of the regulator, and deferred probe, without adding something to the platform data that board setup code needed to fill in anyway.
Dear Andrew Chew, > > Dear 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> > > > > Do we really need a mandatory regulator? Why can't it be optional? > > > > Best regards, > > Marek Vasut > > Because for those using board setup code (and not devicetree), it was > not possible to distinguish between opting out of the regulator, and > deferred probe, without adding something to the platform data that > board setup code needed to fill in anyway. Sure, but this adds a bit of overhead for the DT platforms, no? Other than this curiosity, I have no problems with this stuff. Best regards, Marek Vasut
On 03/19/2013 03:27 PM, Marek Vasut wrote: > Dear 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> > > Do we really need a mandatory regulator? Why can't it be optional? IIRC, the previous advice I've seen is that if a device (driver) uses a regulator, it must /require/ a regulator, and if a particular board doesn't actually have a SW-controlled regulator, then a fixed- or dummy- regulator should be provided to satisfy this requirement. CC'ing Mark Brown to make sure I really do Recall Correctly.
On Tue, Mar 19, 2013 at 11:59:30AM -0700, Andrew Chew wrote: > 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> > --- > arch/arm/boot/dts/imx23-evk.dts | 6 ++++++ > arch/arm/boot/dts/imx28-apf28dev.dts | 6 ++++++ > arch/arm/boot/dts/imx28-cfa10049.dts | 6 ++++++ > arch/arm/boot/dts/imx28-evk.dts | 6 ++++++ > arch/arm/boot/dts/imx28-tx28.dts | 6 ++++++ > 5 files changed, 30 insertions(+) > > diff --git a/arch/arm/boot/dts/imx23-evk.dts b/arch/arm/boot/dts/imx23-evk.dts > index 035c13f..e48e36c 100644 > --- a/arch/arm/boot/dts/imx23-evk.dts > +++ b/arch/arm/boot/dts/imx23-evk.dts > @@ -97,10 +97,16 @@ > }; > }; > > + bl_en: fixed-regulator { The node name is too generic. Very likely we have other fixed regulator in a dts. For mxs, we have all such board level regulators defined in node "regulators" as children. Shawn > + compatible = "regulator-fixed"; > + regulator-name = "bl-en-supply"; > + }; > + > backlight { > compatible = "pwm-backlight"; > pwms = <&pwm 2 5000000>; > brightness-levels = <0 4 8 16 32 64 128 255>; > default-brightness-level = <6>; > + enable-supply = <&bl_en>; > }; > };
On Tue, Mar 19, 2013 at 04:10:26PM -0600, Stephen Warren wrote: > On 03/19/2013 03:27 PM, Marek Vasut wrote: > > Do we really need a mandatory regulator? Why can't it be optional? > IIRC, the previous advice I've seen is that if a device (driver) uses a > regulator, it must /require/ a regulator, and if a particular board > doesn't actually have a SW-controlled regulator, then a fixed- or dummy- > regulator should be provided to satisfy this requirement. > CC'ing Mark Brown to make sure I really do Recall Correctly. Yes, and it should be fixed rather than dummy. The issue is partly that it's probably important that the device has power so we don't want to just ignore errors and partly that this is something which applies to essentially all devices so whatever we do for this case ought to be done by the core so all devices can benefit and we don't have to duplicate lots of code in individual drivers.
Hi Mark, > On Tue, Mar 19, 2013 at 04:10:26PM -0600, Stephen Warren wrote: > > On 03/19/2013 03:27 PM, Marek Vasut wrote: > > > Do we really need a mandatory regulator? Why can't it be optional? > > > > IIRC, the previous advice I've seen is that if a device (driver) uses a > > regulator, it must /require/ a regulator, and if a particular board > > doesn't actually have a SW-controlled regulator, then a fixed- or dummy- > > regulator should be provided to satisfy this requirement. > > > > CC'ing Mark Brown to make sure I really do Recall Correctly. > > Yes, and it should be fixed rather than dummy. The issue is partly that > it's probably important that the device has power so we don't want to > just ignore errors and partly that this is something which applies to > essentially all devices so whatever we do for this case ought to be done > by the core so all devices can benefit and we don't have to duplicate > lots of code in individual drivers. Thanks for clearing this! Best regards, Marek Vasut
diff --git a/arch/arm/boot/dts/imx23-evk.dts b/arch/arm/boot/dts/imx23-evk.dts index 035c13f..e48e36c 100644 --- a/arch/arm/boot/dts/imx23-evk.dts +++ b/arch/arm/boot/dts/imx23-evk.dts @@ -97,10 +97,16 @@ }; }; + bl_en: fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "bl-en-supply"; + }; + backlight { compatible = "pwm-backlight"; pwms = <&pwm 2 5000000>; brightness-levels = <0 4 8 16 32 64 128 255>; default-brightness-level = <6>; + enable-supply = <&bl_en>; }; }; diff --git a/arch/arm/boot/dts/imx28-apf28dev.dts b/arch/arm/boot/dts/imx28-apf28dev.dts index 6d8865b..866e26f 100644 --- a/arch/arm/boot/dts/imx28-apf28dev.dts +++ b/arch/arm/boot/dts/imx28-apf28dev.dts @@ -144,11 +144,17 @@ }; }; + bl_en: fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "bl-en-supply"; + }; + backlight { compatible = "pwm-backlight"; pwms = <&pwm 3 191000>; brightness-levels = <0 4 8 16 32 64 128 255>; default-brightness-level = <6>; + enable-supply = <&bl_en>; }; }; diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts index a0d3e9f..a8477b6 100644 --- a/arch/arm/boot/dts/imx28-cfa10049.dts +++ b/arch/arm/boot/dts/imx28-cfa10049.dts @@ -299,10 +299,16 @@ rotary-encoder,relative-axis; }; + bl_en: fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "bl-en-supply"; + }; + backlight { compatible = "pwm-backlight"; pwms = <&pwm 3 5000000>; brightness-levels = <0 4 8 16 32 64 128 255>; default-brightness-level = <6>; + enable-supply = <&bl_en>; }; }; diff --git a/arch/arm/boot/dts/imx28-evk.dts b/arch/arm/boot/dts/imx28-evk.dts index 2da316e..810f1e4 100644 --- a/arch/arm/boot/dts/imx28-evk.dts +++ b/arch/arm/boot/dts/imx28-evk.dts @@ -307,10 +307,16 @@ }; }; + bl_en: fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "bl-en-supply"; + }; + backlight { compatible = "pwm-backlight"; pwms = <&pwm 2 5000000>; brightness-levels = <0 4 8 16 32 64 128 255>; default-brightness-level = <6>; + enable-supply = <&bl_en>; }; }; diff --git a/arch/arm/boot/dts/imx28-tx28.dts b/arch/arm/boot/dts/imx28-tx28.dts index 37be532..ea3c88a 100644 --- a/arch/arm/boot/dts/imx28-tx28.dts +++ b/arch/arm/boot/dts/imx28-tx28.dts @@ -107,10 +107,16 @@ }; }; + bl_en: fixed-regulator { + compatible = "regulator-fixed"; + regulator-name = "bl-en-supply"; + }; + 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>; }; };
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> --- arch/arm/boot/dts/imx23-evk.dts | 6 ++++++ arch/arm/boot/dts/imx28-apf28dev.dts | 6 ++++++ arch/arm/boot/dts/imx28-cfa10049.dts | 6 ++++++ arch/arm/boot/dts/imx28-evk.dts | 6 ++++++ arch/arm/boot/dts/imx28-tx28.dts | 6 ++++++ 5 files changed, 30 insertions(+)