diff mbox

[v5,1/4] regulator: helper routine to extract regulator_init_data

Message ID CAJuYYwRO+AS=WmQfNAe8L6Gi5Dm4VsCuVqk3X5Nb8uUYAdLnJQ@mail.gmail.com
State New
Headers show

Commit Message

thomas.abraham@linaro.org Dec. 4, 2011, 1:21 p.m. UTC 
On 18 November 2011 16:47, Rajendra Nayak <rnayak@ti.com> wrote:
> The helper routine is meant to be used by the regulator drivers
> to extract the regulator_init_data structure from the data
> that is passed from device tree.
> 'consumer_supplies' which is part of regulator_init_data is not extracted
> as the regulator consumer mappings are passed through DT differently,
> implemented in subsequent patches.
> Similarly the regulator<-->parent/supply mapping is handled in
> subsequent patches.
>
> Also add documentation for regulator bindings to be used to pass
> regulator_init_data struct information from device tree.
>
> Some of the regulator properties which are linux and board specific,
> are left out since its not clear if they can
> be in someway embedded into the kernel or passed in from DT.
> They will be revisited later.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  .../devicetree/bindings/regulator/regulator.txt    |   54 +++++++++++++
>  drivers/regulator/Makefile                         |    1 +
>  drivers/regulator/of_regulator.c                   |   81 ++++++++++++++++++++
>  include/linux/regulator/of_regulator.h             |   20 +++++
>  4 files changed, 156 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
>  create mode 100644 drivers/regulator/of_regulator.c
>  create mode 100644 include/linux/regulator/of_regulator.h
>
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> new file mode 100644
> index 0000000..82bef20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -0,0 +1,54 @@
> +Voltage/Current Regulators
> +
> +Optional properties:
> +- regulator-name: A string used as a descriptive name for regulator outputs
> +- regulator-min-microvolt: smallest voltage consumers may set
> +- regulator-max-microvolt: largest voltage consumers may set
> +- regulator-microvolt-offset: Offset applied to voltages to compensate for voltage drops
> +- regulator-min-microamp: smallest current consumers may set
> +- regulator-max-microamp: largest current consumers may set
> +- regulator-always-on: boolean, regulator should never be disabled
> +- regulator-boot-on: bootloader/firmware enabled regulator
> +- <name>-supply: phandle to the parent supply/regulator node

For regulators that are not turned on by bootloader, and which require
'apply_uV' constraint, is there any alternative for turning on the
regulator when using dt?

Or, how about adding a additional check as below.

 						rdev->constraints->max_uV);

Thanks,
Thomas.

Comments

Mark Brown Dec. 4, 2011, 3:54 p.m. UTC | #1 
On Sun, Dec 04, 2011 at 06:51:23PM +0530, Thomas Abraham wrote:

> For regulators that are not turned on by bootloader, and which require
> 'apply_uV' constraint, is there any alternative for turning on the
> regulator when using dt?

If the regulator isn't software managed then always_on covers this - the
regulator core will enable any always_on regulators that haven't been
enabled already.

>  	/* do we need to apply the constraint voltage */
> -	if (rdev->constraints->apply_uV &&
> -	    rdev->constraints->min_uV == rdev->constraints->max_uV) {
> +	if ((rdev->constraints->apply_uV &&
> +	    rdev->constraints->min_uV == rdev->constraints->max_uV) ||
> +		(!rdev->constraints->boot_on && rdev->constraints->always_on)) {
>  		ret = _regulator_do_set_voltage(rdev,
>  						rdev->constraints->min_uV,
>  						rdev->constraints->max_uV);

I'm not sure I understand the intended logic there.  Voltage constraints
and enable/disable constraints are orthogonal here.
thomas.abraham@linaro.org Dec. 5, 2011, 9:10 a.m. UTC | #2 
Hi Mark,

On 4 December 2011 21:24, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sun, Dec 04, 2011 at 06:51:23PM +0530, Thomas Abraham wrote:
>
>> For regulators that are not turned on by bootloader, and which require
>> 'apply_uV' constraint, is there any alternative for turning on the
>> regulator when using dt?
>
> If the regulator isn't software managed then always_on covers this - the
> regulator core will enable any always_on regulators that haven't been
> enabled already.

Thanks for the hint. I was trying to deal with a regulator that was
not software managed but also required the voltage level to be set to
certain level. That was possible with 'apply_uV' constraint in non-dt
case. Anyway, I have modified the code to manage the regulator and
this works fine in dt case as well without the 'apply_uV' constraint.

>
>>       /* do we need to apply the constraint voltage */
>> -     if (rdev->constraints->apply_uV &&
>> -         rdev->constraints->min_uV == rdev->constraints->max_uV) {
>> +     if ((rdev->constraints->apply_uV &&
>> +         rdev->constraints->min_uV == rdev->constraints->max_uV) ||
>> +             (!rdev->constraints->boot_on && rdev->constraints->always_on)) {
>>               ret = _regulator_do_set_voltage(rdev,
>>                                               rdev->constraints->min_uV,
>>                                               rdev->constraints->max_uV);
>
> I'm not sure I understand the intended logic there.  Voltage constraints
> and enable/disable constraints are orthogonal here.

Ok. I guess the above change is incorrect then.

Thanks,
Thomas.
Mark Brown Dec. 5, 2011, 10:34 a.m. UTC | #3 
On Mon, Dec 05, 2011 at 02:40:50PM +0530, Thomas Abraham wrote:
> On 4 December 2011 21:24, Mark Brown

> > If the regulator isn't software managed then always_on covers this - the
> > regulator core will enable any always_on regulators that haven't been
> > enabled already.

> Thanks for the hint. I was trying to deal with a regulator that was
> not software managed but also required the voltage level to be set to
> certain level. That was possible with 'apply_uV' constraint in non-dt
> case. Anyway, I have modified the code to manage the regulator and
> this works fine in dt case as well without the 'apply_uV' constraint.

With the regulator device tree bindings if the regulator is configured
to run a single voltage the bindings will set apply_uV unconditionally
so there's no need for a separate constraint.
thomas.abraham@linaro.org Dec. 5, 2011, 10:44 a.m. UTC | #4 
On 5 December 2011 16:04, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Dec 05, 2011 at 02:40:50PM +0530, Thomas Abraham wrote:
>> On 4 December 2011 21:24, Mark Brown
>
>> > If the regulator isn't software managed then always_on covers this - the
>> > regulator core will enable any always_on regulators that haven't been
>> > enabled already.
>
>> Thanks for the hint. I was trying to deal with a regulator that was
>> not software managed but also required the voltage level to be set to
>> certain level. That was possible with 'apply_uV' constraint in non-dt
>> case. Anyway, I have modified the code to manage the regulator and
>> this works fine in dt case as well without the 'apply_uV' constraint.
>
> With the regulator device tree bindings if the regulator is configured
> to run a single voltage the bindings will set apply_uV unconditionally
> so there's no need for a separate constraint.
>

Sorry if I have missed this, but I could not find 'apply_uV' being set
as you described in the v5 of the regulator-dt patchset.

Thanks,
Thomas.
Mark Brown Dec. 5, 2011, 10:57 a.m. UTC | #5 
On Mon, Dec 05, 2011 at 04:14:40PM +0530, Thomas Abraham wrote:
> On 5 December 2011 16:04, Mark Brown

> > With the regulator device tree bindings if the regulator is configured
> > to run a single voltage the bindings will set apply_uV unconditionally
> > so there's no need for a separate constraint.

> Sorry if I have missed this, but I could not find 'apply_uV' being set
> as you described in the v5 of the regulator-dt patchset.

Yes, looks like Rajendra missed that - just fixed it.
thomas.abraham@linaro.org Dec. 5, 2011, 11:09 a.m. UTC | #6 
On 5 December 2011 16:27, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Dec 05, 2011 at 04:14:40PM +0530, Thomas Abraham wrote:
>> On 5 December 2011 16:04, Mark Brown
>
>> > With the regulator device tree bindings if the regulator is configured
>> > to run a single voltage the bindings will set apply_uV unconditionally
>> > so there's no need for a separate constraint.
>
>> Sorry if I have missed this, but I could not find 'apply_uV' being set
>> as you described in the v5 of the regulator-dt patchset.
>
> Yes, looks like Rajendra missed that - just fixed it.
>

Thanks Mark. Could you please push your change.

Regards,
Thomas.
Rajendra Nayak Dec. 6, 2011, 6:54 a.m. UTC | #7 
On Monday 05 December 2011 04:27 PM, Mark Brown wrote:
> On Mon, Dec 05, 2011 at 04:14:40PM +0530, Thomas Abraham wrote:
>> On 5 December 2011 16:04, Mark Brown
>
>>> With the regulator device tree bindings if the regulator is configured
>>> to run a single voltage the bindings will set apply_uV unconditionally
>>> so there's no need for a separate constraint.
>
>> Sorry if I have missed this, but I could not find 'apply_uV' being set
>> as you described in the v5 of the regulator-dt patchset.
>
> Yes, looks like Rajendra missed that - just fixed it.

Thanks Mark, I certainly seem to have missed it.
diff mbox

Patch

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5baa196..25a6781 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -816,8 +816,9 @@  static int machine_constraints_voltage(struct
regulator_dev *rdev,
 	int ret;

 	/* do we need to apply the constraint voltage */
-	if (rdev->constraints->apply_uV &&
-	    rdev->constraints->min_uV == rdev->constraints->max_uV) {
+	if ((rdev->constraints->apply_uV &&
+	    rdev->constraints->min_uV == rdev->constraints->max_uV) ||
+		(!rdev->constraints->boot_on && rdev->constraints->always_on)) {
 		ret = _regulator_do_set_voltage(rdev,
 						rdev->constraints->min_uV,