diff mbox

[4/4] ARM: tegra: correct Colibri T20 regulator settings

Message ID 1374442132-24040-5-git-send-email-dev@lynxeye.de
State Superseded, archived
Headers show

Commit Message

Lucas Stach July 21, 2013, 9:28 p.m. UTC
Core and CPU voltage settings were a bit on the safe side. The actually
used chips on the Colibri allow for lower voltages and work just fine
this way.

SM2 is not a the parent of LDO regs, but actually the DDR regulator. The
Colibri uses a different version of the TPS with other voltage mapping
tables for SM2, currently we cheat by setting a fake 3,2V which results
in 1,8V physical.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
The issue with the used version of the PMIC having a different voltage
mapping for SM2 should be resolved properly. As this needs some bigger
adjustments at the regulator driver this quick fix is just aimed at
stopping slight overvolting of the ram with 3.11 kernels. A proper fix
should land in time for 3.12.
---
 arch/arm/boot/dts/tegra20-colibri-512.dtsi | 33 ++++++++++++++++--------------
 1 file changed, 18 insertions(+), 15 deletions(-)

Comments

Stephen Warren July 23, 2013, 6:07 p.m. UTC | #1
On 07/21/2013 02:28 PM, Lucas Stach wrote:
> Core and CPU voltage settings were a bit on the safe side. The actually
> used chips on the Colibri allow for lower voltages and work just fine
> this way.

That sounds like a non-critical issue. Shouldn't that part be separated
out into a patch for 3.12?

> SM2 is not a the parent of LDO regs, but actually the DDR regulator. The
> Colibri uses a different version of the TPS with other voltage mapping
> tables for SM2, currently we cheat by setting a fake 3,2V which results
> in 1,8V physical.

That's quite unfortunate. Since DT is supposed to be an ABI, the
existing DT should continue to work for arbitrary kernels, and the
modified DT should also work for arbitrary kernels. Clearly that isn't
possible if we start putting incorrect voltage values into the DT. Isn't
there some way to make an isolated fix to the PMIC driver itself so that
it actually programs the HW correctly? Even if that patch is larger than
this patch, it still seems more likely to be acceptable for 3.11.

But is this a regression? If not, how far back in CC: stable should this
change go?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach July 23, 2013, 8:35 p.m. UTC | #2
Am Dienstag, den 23.07.2013, 11:07 -0700 schrieb Stephen Warren:
> On 07/21/2013 02:28 PM, Lucas Stach wrote:
> > Core and CPU voltage settings were a bit on the safe side. The actually
> > used chips on the Colibri allow for lower voltages and work just fine
> > this way.
> 
> That sounds like a non-critical issue. Shouldn't that part be separated
> out into a patch for 3.12?
Hm, yes.

> 
> > SM2 is not a the parent of LDO regs, but actually the DDR regulator. The
> > Colibri uses a different version of the TPS with other voltage mapping
> > tables for SM2, currently we cheat by setting a fake 3,2V which results
> > in 1,8V physical.
> 
> That's quite unfortunate. Since DT is supposed to be an ABI, the
> existing DT should continue to work for arbitrary kernels, and the
> modified DT should also work for arbitrary kernels. Clearly that isn't
> possible if we start putting incorrect voltage values into the DT. Isn't
> there some way to make an isolated fix to the PMIC driver itself so that
> it actually programs the HW correctly? Even if that patch is larger than
> this patch, it still seems more likely to be acceptable for 3.11.
> 
I'm a bit one the fence here. One the one hand it's a severe issue as
the DDR ram gets overvolted by a fair bit and should be fixed by putting
the correct voltage in the DT and getting this into stable. On the other
hand this will prevent older kernels from working correctly as with 1,8V
set on this rail the regulator driver will just plainly refuse to probe.

Thinking again about this maybe it really the best thing to get the DT
right first with CC stable and then changing the regulator driver and
try and see if stable also accepts this patch. I don't really want to
leave people with a supposedly working system, but constantly
overvolting their RAM.

> But is this a regression? If not, how far back in CC: stable should this
> change go?

This is not a regression. It was introduced with the original Colibri
T20 commit and was caused by Toradex not providing any schematics for
the Module plus me not physically checking this voltage rail before
pushing things out.

Regards,
Lucas

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren July 23, 2013, 8:53 p.m. UTC | #3
On 07/23/2013 01:35 PM, Lucas Stach wrote:
> Am Dienstag, den 23.07.2013, 11:07 -0700 schrieb Stephen Warren:
>> On 07/21/2013 02:28 PM, Lucas Stach wrote:
...
>>> SM2 is not a the parent of LDO regs, but actually the DDR regulator. The
>>> Colibri uses a different version of the TPS with other voltage mapping
>>> tables for SM2, currently we cheat by setting a fake 3,2V which results
>>> in 1,8V physical.
...
>> But is this a regression? If not, how far back in CC: stable should this
>> change go?
> 
> This is not a regression. It was introduced with the original Colibri
> T20 commit and was caused by Toradex not providing any schematics for
> the Module plus me not physically checking this voltage rail before
> pushing things out.

FYI, the reason I ask is that there's more push-back on adding patches
late in the rc cycle (or after release, to -stable) for things that are
not regressions. A safety issue like over-voltage might well still
qualify to be accepted, but fixes for regressions are even more likely
to be accepted.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Agner July 28, 2013, 10:06 p.m. UTC | #4
Hi,

I ran the mainline Kernel the first time on my Colibri T20. I compiled
3.11-rc2 without any patch, but the kernel refused to boot, the boot
process freezed just while probing SM2:
[    0.420075] vdd_sm2,vdd_ddr: 3700 mV

I then applied this patch, but without luck, the freeze happend even
earlier:
[    0.413314] vdd_sm1,vdd_cpu: supplied by vdd_5v0

However, using 3.8 works for me:
[    0.419848] vdd_sm2,vdd_ddr: 3800 mV
[    0.423851] vdd_sm2,vdd_ddr: supplied by vdd_5v0

My Module Version is 1.2a with TPS658643 PMIC on it.


Am 2013-07-21 23:28, schrieb Lucas Stach:
> Core and CPU voltage settings were a bit on the safe side. The actually
> used chips on the Colibri allow for lower voltages and work just fine
> this way.
This voltages seem to work for me too...

 
> SM2 is not a the parent of LDO regs, but actually the DDR regulator. The
> Colibri uses a different version of the TPS with other voltage mapping
> tables for SM2, currently we cheat by setting a fake 3,2V which results
> in 1,8V physical.
I could successfully boot with 1.8V, but with messages complaining that
probe failed... 

--
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Aug. 15, 2013, 11:05 a.m. UTC | #5
On Sun, Jul 21, 2013 at 11:28:52PM +0200, Lucas Stach wrote:
> Core and CPU voltage settings were a bit on the safe side. The actually
> used chips on the Colibri allow for lower voltages and work just fine
> this way.
> 
> SM2 is not a the parent of LDO regs, but actually the DDR regulator. The
> Colibri uses a different version of the TPS with other voltage mapping
> tables for SM2, currently we cheat by setting a fake 3,2V which results
> in 1,8V physical.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
> The issue with the used version of the PMIC having a different voltage
> mapping for SM2 should be resolved properly. As this needs some bigger
> adjustments at the regulator driver this quick fix is just aimed at
> stopping slight overvolting of the ram with 3.11 kernels. A proper fix
> should land in time for 3.12.

This wouldn't happen to be the TPS658640? I remember a similar problem
with a special version of Tamonten that had this version of the PMU
which supposedly is pin and register-compatible. Well, it is register
compatible alright, and the pins are the same as well, but the voltage
tables are slightly different.

I did attempt to work the changes into the tps6586x driver at some point
but there were a few things that made it much more difficult than I had
imagined.

Thierry
diff mbox

Patch

diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
index 92551ba..9b56dcb 100644
--- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
+++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
@@ -226,14 +226,14 @@ 
 			gpio-controller;
 
 			sys-supply = <&vdd_5v0_reg>;
-			vin-sm0-supply = <&sys_reg>;
-			vin-sm1-supply = <&sys_reg>;
-			vin-sm2-supply = <&sys_reg>;
-			vinldo01-supply = <&sm2_reg>;
-			vinldo23-supply = <&sm2_reg>;
-			vinldo4-supply = <&sm2_reg>;
-			vinldo678-supply = <&sm2_reg>;
-			vinldo9-supply = <&sm2_reg>;
+			vin-sm0-supply = <&vdd_5v0_reg>;
+			vin-sm1-supply = <&vdd_5v0_reg>;
+			vin-sm2-supply = <&vdd_5v0_reg>;
+			vinldo01-supply = <&vdd_5v0_reg>;
+			vinldo23-supply = <&vdd_5v0_reg>;
+			vinldo4-supply = <&vdd_5v0_reg>;
+			vinldo678-supply = <&vdd_5v0_reg>;
+			vinldo9-supply = <&vdd_5v0_reg>;
 
 			regulators {
 				#address-cells = <1>;
@@ -250,8 +250,8 @@ 
 					reg = <1>;
 					regulator-compatible = "sm0";
 					regulator-name = "vdd_sm0,vdd_core";
-					regulator-min-microvolt = <1275000>;
-					regulator-max-microvolt = <1275000>;
+					regulator-min-microvolt = <1225000>;
+					regulator-max-microvolt = <1225000>;
 					regulator-always-on;
 				};
 
@@ -259,17 +259,20 @@ 
 					reg = <2>;
 					regulator-compatible = "sm1";
 					regulator-name = "vdd_sm1,vdd_cpu";
-					regulator-min-microvolt = <1100000>;
-					regulator-max-microvolt = <1100000>;
+					regulator-min-microvolt = <1025000>;
+					regulator-max-microvolt = <1025000>;
 					regulator-always-on;
 				};
 
 				sm2_reg: regulator@3 {
 					reg = <3>;
 					regulator-compatible = "sm2";
-					regulator-name = "vdd_sm2,vin_ldo*";
-					regulator-min-microvolt = <3700000>;
-					regulator-max-microvolt = <3700000>;
+					regulator-name = "vdd_sm2,vdd_ddr";
+					/* The voltage is lying, but results
+					in the desired 1,8V on the TPS version
+					used on the Colibri */
+					regulator-min-microvolt = <3200000>;
+					regulator-max-microvolt = <3200000>;
 					regulator-always-on;
 				};