[PATCH V2] ARM: dt: tegra: paz00: add regulators

[PATCH V2] ARM: dt: tegra: paz00: add regulators

[Stephen Warren]

From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The Toshiba AC100/PAZ00 uses a TPS6586x regulator. Instantiate this.

Three data sources were used for the data encoded here:
* The HW defaults, as extracted from real HW.
* The schematic, which specifies a voltage for each rail in the signal
  names.
* The AC100 kernel used by the Ubuntu port:

  repo git://gitorious.org/~marvin24/ac100/marvin24s-kernel.git
  branch chromeos-ac100-3.0
  file arch/arm/mach-tegra/board-paz00-power.c

  For many rails, the constraints in that tree specified differing min
  and max voltages. In all cases, the min value was ignored, since
  there's no need currently to vary any of the voltages at run-time.
  DVFS might change this in the future.

In most cases these sources all matched. Differences are:

sm0: HW defaults and schematic match at 1.2v. marvin24's kernel had a max
of 1.3v, but this wasn't applied since apply_uV wasn't set.

sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a max
of 1.125v, but this wasn't applied since apply_uV wasn't set.

ldo0: The HW default is 1.2v. The schematic and marvin24's kernel state
this should be 3.3v. On similar board designs, this rail is typically
used for PCIe clock which requires 3.3v. Note that this rail isn't
actually used on this board.

ldo3: The HW default is on. marvin24's kernel didn't specify always-on,
but since the board wasn't marked as having fully constrained regulators,
the rail was not turned off, so the difference had no effect. The rail
is needed for USB.

Cc: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2:
* Made all constraints specify a single voltage rather than different
  min/max, per description above.
* Removed vin-supply properties from LDO nodes; the driver and binding
  need to be updated to support specifying the parent regulators before
  we can put these into DT.
---
 arch/arm/boot/dts/tegra20-paz00.dts |  130 +++++++++++++++++++++++++++++++++++
 1 files changed, 130 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-paz00.dts b/arch/arm/boot/dts/tegra20-paz00.dts
index 684a9e1..dccc8b4 100644
--- a/arch/arm/boot/dts/tegra20-paz00.dts
+++ b/arch/arm/boot/dts/tegra20-paz00.dts
@@ -272,12 +272,142 @@
 		status = "okay";
 		clock-frequency = <400000>;
 
+		pmic: tps6586x@34 {
+			compatible = "ti,tps6586x";
+			reg = <0x34>;
+			interrupts = <0 86 0x4>;
+
+			#gpio-cells = <2>;
+			gpio-controller;
+
+			regulators {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				regulator@0 {
+					reg = <0>;
+					regulator-compatible = "sm0";
+					regulator-name = "+1.2vs_sm0";
+					regulator-min-microvolt = <1200000>;
+					regulator-max-microvolt = <1200000>;
+					regulator-always-on;
+				};
+
+				regulator@1 {
+					reg = <1>;
+					regulator-compatible = "sm1";
+					regulator-name = "+1.0vs_sm1";
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1000000>;
+					regulator-always-on;
+				};
+
+				regulator@2 {
+					reg = <2>;
+					regulator-compatible = "sm2";
+					regulator-name = "+3.7vs_sm2";
+					regulator-min-microvolt = <3700000>;
+					regulator-max-microvolt = <3700000>;
+					regulator-always-on;
+				};
+
+				regulator@3 {
+					reg = <3>;
+					regulator-compatible = "ldo0";
+					regulator-name = "+3.3vs_ldo0";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+				};
+
+				regulator@4 {
+					reg = <4>;
+					regulator-compatible = "ldo1";
+					regulator-name = "+1.1vs_ldo1";
+					regulator-min-microvolt = <1100000>;
+					regulator-max-microvolt = <1100000>;
+					regulator-always-on;
+				};
+
+				regulator@5 {
+					reg = <5>;
+					regulator-compatible = "ldo2";
+					regulator-name = "+1.2vs_ldo2";
+					regulator-min-microvolt = <1200000>;
+					regulator-max-microvolt = <1200000>;
+				};
+
+				regulator@6 {
+					reg = <6>;
+					regulator-compatible = "ldo3";
+					regulator-name = "+3.3vs_ldo3";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+
+				regulator@7 {
+					reg = <7>;
+					regulator-compatible = "ldo4";
+					regulator-name = "+1.8vs_ldo4";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				regulator@8 {
+					reg = <8>;
+					regulator-compatible = "ldo5";
+					regulator-name = "+2.85vs_ldo5";
+					regulator-min-microvolt = <2850000>;
+					regulator-max-microvolt = <2850000>;
+					regulator-always-on;
+				};
+
+				regulator@9 {
+					reg = <9>;
+					regulator-compatible = "ldo6";
+					regulator-name = "+2.85vs_ldo6";
+					regulator-min-microvolt = <2850000>;
+					regulator-max-microvolt = <2850000>;
+				};
+
+				regulator@10 {
+					reg = <10>;
+					regulator-compatible = "ldo7";
+					regulator-name = "+3.3vs_ldo7";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+				};
+
+				regulator@11 {
+					reg = <11>;
+					regulator-compatible = "ldo8";
+					regulator-name = "+1.8vs_ldo8";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+				};
+
+				regulator@12 {
+					reg = <12>;
+					regulator-compatible = "ldo9";
+					regulator-name = "+2.85vs_ldo9";
+					regulator-min-microvolt = <2850000>;
+					regulator-max-microvolt = <2850000>;
+					regulator-always-on;
+				};
+			};
+		};
+
 		adt7461@4c {
 			compatible = "adi,adt7461";
 			reg = <0x4c>;
 		};
 	};
 
+	pmc {
+		nvidia,invert-interrupt;
+	};
+
 	usb@c5000000 {
 		status = "okay";
 	};
-- 
1.7.0.4

Re: [PATCH V2] ARM: dt: tegra: paz00: add regulators

[Mark Brown]

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

On Tue, Jun 26, 2012 at 06:31:00PM -0600, Stephen Warren wrote:

> sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a max
> of 1.125v, but this wasn't applied since apply_uV wasn't set.

apply_uV is only valid if a single voltage is specified.  If a voltage
range were specified and it were acted on we'd take the lowest (not
highest) voltage allowed.

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

Re: [PATCH V2] ARM: dt: tegra: paz00: add regulators

[Stephen Warren]

On 06/27/2012 05:31 AM, Mark Brown wrote:
> On Tue, Jun 26, 2012 at 06:31:00PM -0600, Stephen Warren wrote:
> 
>> sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a max
>> of 1.125v, but this wasn't applied since apply_uV wasn't set.
> 
> apply_uV is only valid if a single voltage is specified.  If a voltage
> range were specified and it were acted on we'd take the lowest (not
> highest) voltage allowed.

Ah right. When I apply this, I'll reword that to something more like:

sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a
max of 1.125v, but this higher voltage was only applied to HW by DVFS
code, which isn't currently supported in mainline.

Re: [PATCH V2] ARM: dt: tegra: paz00: add regulators

[Marc Dietrich]

On Wednesday 27 June 2012 12:31:01 Mark Brown wrote:
> On Tue, Jun 26, 2012 at 06:31:00PM -0600, Stephen Warren wrote:
> > sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a max
> > of 1.125v, but this wasn't applied since apply_uV wasn't set.
> 
> apply_uV is only valid if a single voltage is specified.  

yes, that's why there is a ".apply_uV = (_minmv == _maxmv)" in the regulator 
macro.

> If a voltage
> range were specified and it were acted on we'd take the lowest (not
> highest) voltage allowed.

Sorry, I don't get it. In this case, the board wouldn't boot at all because 
nearly all supplies would be undervoltaged. I just checked and all voltages 
are actually set to the *highest* (max) value. Maybe they aren't changed at 
all?

Marc

Re: [PATCH V2] ARM: dt: tegra: paz00: add regulators

[Stephen Warren]

On 06/27/2012 12:50 PM, Marc Dietrich wrote:
> On Wednesday 27 June 2012 12:31:01 Mark Brown wrote:
>> On Tue, Jun 26, 2012 at 06:31:00PM -0600, Stephen Warren wrote:
>>> sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a max
>>> of 1.125v, but this wasn't applied since apply_uV wasn't set.
>>
>> apply_uV is only valid if a single voltage is specified.  
> 
> yes, that's why there is a ".apply_uV = (_minmv == _maxmv)" in the regulator 
> macro.
> 
>> If a voltage
>> range were specified and it were acted on we'd take the lowest (not
>> highest) voltage allowed.
> 
> Sorry, I don't get it. In this case, the board wouldn't boot at all because 
> nearly all supplies would be undervoltaged. I just checked and all voltages 
> are actually set to the *highest* (max) value. Maybe they aren't changed at 
> all?

Yes, in the absence of any explicit action (i.e. a call to
regulator_set_voltage() elsewhere), the regulator core doesn't reprogram
the regulator at registration time, except for a few specific conditions
e.g. something like when min==max and apply_uV is set.

I imagine the DVFS code in your downstream kernel /is/ calling
regulator_set_voltage() later, assuming that config option is enabled
anyway. See arch/arm/mach-tegra/{dvfs.c,tegra2_dvfs.c}.

Re: [PATCH V2] ARM: dt: tegra: paz00: add regulators

[Marc Dietrich]

On Wednesday 27 June 2012 12:56:01 Stephen Warren wrote:
> On 06/27/2012 12:50 PM, Marc Dietrich wrote:
> > On Wednesday 27 June 2012 12:31:01 Mark Brown wrote:
> >> On Tue, Jun 26, 2012 at 06:31:00PM -0600, Stephen Warren wrote:
> >>> sm1: HW defaults and schematic match at 1.0v. marvin24's kernel had a
> >>> max
> >>> of 1.125v, but this wasn't applied since apply_uV wasn't set.
> >> 
> >> apply_uV is only valid if a single voltage is specified.
> > 
> > yes, that's why there is a ".apply_uV = (_minmv == _maxmv)" in the
> > regulator macro.
> > 
> >> If a voltage
> >> range were specified and it were acted on we'd take the lowest (not
> >> highest) voltage allowed.
> > 
> > Sorry, I don't get it. In this case, the board wouldn't boot at all
> > because
> > nearly all supplies would be undervoltaged. I just checked and all
> > voltages
> > are actually set to the *highest* (max) value. Maybe they aren't changed
> > at
> > all?
> 
> Yes, in the absence of any explicit action (i.e. a call to
> regulator_set_voltage() elsewhere), the regulator core doesn't reprogram
> the regulator at registration time, except for a few specific conditions
> e.g. something like when min==max and apply_uV is set.
> 
> I imagine the DVFS code in your downstream kernel /is/ calling
> regulator_set_voltage() later, assuming that config option is enabled
> anyway. See arch/arm/mach-tegra/{dvfs.c,tegra2_dvfs.c}.

Great, so all the tables (except sm0/1) were moot :-( At least I learned 
something again ;-)

Stephen, I'm going to test your patch, just a few minutes ...

Thanks

Marc

Re: [PATCH V2] ARM: dt: tegra: paz00: add regulators

[Marc Dietrich]

On Tuesday 26 June 2012 18:31:00 Stephen Warren wrote:
> From: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> The Toshiba AC100/PAZ00 uses a TPS6586x regulator. Instantiate this.
> 
> [...]
> 
> Cc: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

I tested this on paz00 using linux-next and found no issues, so feel free to 
add ...

Tested-By: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>

> ---
> v2:
> * Made all constraints specify a single voltage rather than different
>   min/max, per description above.
> * Removed vin-supply properties from LDO nodes; the driver and binding
>   need to be updated to support specifying the parent regulators before
>   we can put these into DT.
> ---
>  arch/arm/boot/dts/tegra20-paz00.dts |  130
> +++++++++++++++++++++++++++++++++++ 1 files changed, 130 insertions(+), 0
> deletions(-)
> 
> diff --git a/arch/arm/boot/dts/tegra20-paz00.dts
> b/arch/arm/boot/dts/tegra20-paz00.dts index 684a9e1..dccc8b4 100644
> --- a/arch/arm/boot/dts/tegra20-paz00.dts
> +++ b/arch/arm/boot/dts/tegra20-paz00.dts
> @@ -272,12 +272,142 @@
>  		status = "okay";
>  		clock-frequency = <400000>;
> 
> +		pmic: tps6586x@34 {
> +			compatible = "ti,tps6586x";
> +			reg = <0x34>;
> +			interrupts = <0 86 0x4>;
> +
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +
> +			regulators {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				regulator@0 {
> +					reg = <0>;
> +					regulator-compatible = "sm0";
> +					regulator-name = "+1.2vs_sm0";
> +					regulator-min-microvolt = <1200000>;
> +					regulator-max-microvolt = <1200000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@1 {
> +					reg = <1>;
> +					regulator-compatible = "sm1";
> +					regulator-name = "+1.0vs_sm1";
> +					regulator-min-microvolt = <1000000>;
> +					regulator-max-microvolt = <1000000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@2 {
> +					reg = <2>;
> +					regulator-compatible = "sm2";
> +					regulator-name = "+3.7vs_sm2";
> +					regulator-min-microvolt = <3700000>;
> +					regulator-max-microvolt = <3700000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@3 {
> +					reg = <3>;
> +					regulator-compatible = "ldo0";
> +					regulator-name = "+3.3vs_ldo0";
> +					regulator-min-microvolt = <3300000>;
> +					regulator-max-microvolt = <3300000>;
> +				};
> +
> +				regulator@4 {
> +					reg = <4>;
> +					regulator-compatible = "ldo1";
> +					regulator-name = "+1.1vs_ldo1";
> +					regulator-min-microvolt = <1100000>;
> +					regulator-max-microvolt = <1100000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@5 {
> +					reg = <5>;
> +					regulator-compatible = "ldo2";
> +					regulator-name = "+1.2vs_ldo2";
> +					regulator-min-microvolt = <1200000>;
> +					regulator-max-microvolt = <1200000>;
> +				};
> +
> +				regulator@6 {
> +					reg = <6>;
> +					regulator-compatible = "ldo3";
> +					regulator-name = "+3.3vs_ldo3";
> +					regulator-min-microvolt = <3300000>;
> +					regulator-max-microvolt = <3300000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@7 {
> +					reg = <7>;
> +					regulator-compatible = "ldo4";
> +					regulator-name = "+1.8vs_ldo4";
> +					regulator-min-microvolt = <1800000>;
> +					regulator-max-microvolt = <1800000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@8 {
> +					reg = <8>;
> +					regulator-compatible = "ldo5";
> +					regulator-name = "+2.85vs_ldo5";
> +					regulator-min-microvolt = <2850000>;
> +					regulator-max-microvolt = <2850000>;
> +					regulator-always-on;
> +				};
> +
> +				regulator@9 {
> +					reg = <9>;
> +					regulator-compatible = "ldo6";
> +					regulator-name = "+2.85vs_ldo6";
> +					regulator-min-microvolt = <2850000>;
> +					regulator-max-microvolt = <2850000>;
> +				};
> +
> +				regulator@10 {
> +					reg = <10>;
> +					regulator-compatible = "ldo7";
> +					regulator-name = "+3.3vs_ldo7";
> +					regulator-min-microvolt = <3300000>;
> +					regulator-max-microvolt = <3300000>;
> +				};
> +
> +				regulator@11 {
> +					reg = <11>;
> +					regulator-compatible = "ldo8";
> +					regulator-name = "+1.8vs_ldo8";
> +					regulator-min-microvolt = <1800000>;
> +					regulator-max-microvolt = <1800000>;
> +				};
> +
> +				regulator@12 {
> +					reg = <12>;
> +					regulator-compatible = "ldo9";
> +					regulator-name = "+2.85vs_ldo9";
> +					regulator-min-microvolt = <2850000>;
> +					regulator-max-microvolt = <2850000>;
> +					regulator-always-on;
> +				};
> +			};
> +		};
> +
>  		adt7461@4c {
>  			compatible = "adi,adt7461";
>  			reg = <0x4c>;
>  		};
>  	};
> 
> +	pmc {
> +		nvidia,invert-interrupt;
> +	};
> +
>  	usb@c5000000 {
>  		status = "okay";
>  	};