diff mbox

[5/5] ARM: dts: Add initial regulator mode on exynos Peach boards

Message ID 1412775847-15213-6-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Oct. 8, 2014, 1:44 p.m. UTC 
The regulator core now has support to choose a default initial
operating mode for regulators from DT. Set the initial opmode
for the max77802 PMIC regulators with the same modes that are
used in the downstream ChromeOS kernel, in order to allow the
system to lower power at suspend time.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/exynos5420-peach-pit.dts | 26 ++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 26 ++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

Comments

Mark Brown Oct. 8, 2014, 2:56 p.m. UTC | #1 
On Wed, Oct 08, 2014 at 03:44:07PM +0200, Javier Martinez Canillas wrote:

> The regulator core now has support to choose a default initial
> operating mode for regulators from DT. Set the initial opmode
> for the max77802 PMIC regulators with the same modes that are
> used in the downstream ChromeOS kernel, in order to allow the
> system to lower power at suspend time.

The stated goal of this change doesn't appear to correspond to what it
actually does.  It's saying that we want to be able to set the regulator
into low power modes on suspend which is a sensible and useful thing to
want to do (especially for regulators which don't have physical suspend
modes which we currently make any effort to handle) but what the change
actually does is cause us to set the default state of supplies on boot.

The device tree should describe what it's trying to achieve, otherwise
even if things happen to work right now it's going to be vulnerable to
being broken by future kernel changes which take what it's saying at
face value.

>  			buck2_reg: BUCK2 {
> @@ -201,6 +203,7 @@
>  				regulator-always-on;
>  				regulator-boot-on;
>  				regulator-ramp-delay = <12500>;
> +				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
>  			};

This appears to set the supply which is labelled as VDD_ARM into standby
mode by default (from a first glance the change appears to set all
supplies into standby mode) and of course we currently have no way of
changing the mode at runtime in DT systems.  Are you *really* sure this
is a good idea of which an electrical engineer would approve, CPU cores
are typically one of the most demanding workloads available for a
regulator?
Javier Martinez Canillas Oct. 8, 2014, 4:25 p.m. UTC | #2 
On 10/08/2014 04:56 PM, Mark Brown wrote:
> On Wed, Oct 08, 2014 at 03:44:07PM +0200, Javier Martinez Canillas wrote:
> 
>> The regulator core now has support to choose a default initial
>> operating mode for regulators from DT. Set the initial opmode
>> for the max77802 PMIC regulators with the same modes that are
>> used in the downstream ChromeOS kernel, in order to allow the
>> system to lower power at suspend time.
> 
> The stated goal of this change doesn't appear to correspond to what it
> actually does.  It's saying that we want to be able to set the regulator
> into low power modes on suspend which is a sensible and useful thing to
> want to do (especially for regulators which don't have physical suspend
> modes which we currently make any effort to handle) but what the change
> actually does is cause us to set the default state of supplies on boot.
> 

Well, setting a regulator into low power mode on suspend is something
that Chanwoo's series tried to address. At least on v3 since he removed
regulator-mode property for the regulator-state-{standby,mem,disk} on v4.

What this series tried to solve is when you have to set a opmode on boot
to change how the physical suspend is managed by the PMIC.

In the Maxim77802 PMIC is even trickier since not all the modes have the
semantics. That is the value 0x1 could have different meanings depending
of the regulator.

> The device tree should describe what it's trying to achieve, otherwise
> even if things happen to work right now it's going to be vulnerable to
> being broken by future kernel changes which take what it's saying at
> face value.
> 
>>  			buck2_reg: BUCK2 {
>> @@ -201,6 +203,7 @@
>>  				regulator-always-on;
>>  				regulator-boot-on;
>>  				regulator-ramp-delay = <12500>;
>> +				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
>>  			};
> 
> This appears to set the supply which is labelled as VDD_ARM into standby
> mode by default (from a first glance the change appears to set all
> supplies into standby mode) and of course we currently have no way of
> changing the mode at runtime in DT systems.  Are you *really* sure this
> is a good idea of which an electrical engineer would approve, CPU cores
> are typically one of the most demanding workloads available for a
> regulator?
> 

Well, the standby mode for this regulator is actually:

Output ON/OFF Controlled by PWRREQ
	PWRREQ=HIGH (1): Output ON in Normal Mode
	PWRREQ=LOW (0): Output OFF

this means that when the Soc enters into suspend mode a pin is toggled
and that pin is connected to the PWRREQ line in the PMIC to signal that
the SoC has entered in sleep mode so the regulator has to be OFF.

When the SoC leaves sleep mode and is resumed again, the pin is toggled
so the PMIC puts the regulator in ON again.

There is other mode that instead of turning off the regulator, keeps it
enabled but in low power mode.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a050e1..f7eb70d 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -13,6 +13,7 @@ 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/regulator/regulator.h>
 #include "exynos5420.dtsi"
 
 / {
@@ -192,6 +193,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck2_reg: BUCK2 {
@@ -201,6 +203,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck3_reg: BUCK3 {
@@ -210,6 +213,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck4_reg: BUCK4 {
@@ -219,6 +223,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck5_reg: BUCK5 {
@@ -227,6 +232,7 @@ 
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck6_reg: BUCK6 {
@@ -236,6 +242,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck7_reg: BUCK7 {
@@ -244,6 +251,7 @@ 
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_NORMAL>;
 			};
 
 			buck8_reg: BUCK8 {
@@ -252,6 +260,7 @@ 
 				regulator-max-microvolt = <2850000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck9_reg: BUCK9 {
@@ -260,6 +269,7 @@ 
 				regulator-max-microvolt = <2000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_NORMAL>;
 			};
 
 			buck10_reg: BUCK10 {
@@ -268,6 +278,7 @@ 
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_NORMAL>;
 			};
 
 			ldo1_reg: LDO1 {
@@ -275,6 +286,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo2_reg: LDO2 {
@@ -288,6 +300,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			vqmmc_sdcard: ldo4_reg: LDO4 {
@@ -295,6 +308,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo5_reg: LDO5 {
@@ -302,6 +316,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo6_reg: LDO6 {
@@ -309,6 +324,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo7_reg: LDO7 {
@@ -322,6 +338,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo9_reg: LDO9 {
@@ -329,6 +346,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo10_reg: LDO10 {
@@ -336,6 +354,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo11_reg: LDO11 {
@@ -343,6 +362,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo12_reg: LDO12 {
@@ -350,6 +370,7 @@ 
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo13_reg: LDO13 {
@@ -357,6 +378,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo14_reg: LDO14 {
@@ -364,6 +386,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo15_reg: LDO15 {
@@ -371,6 +394,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo17_reg: LDO17 {
@@ -378,6 +402,7 @@ 
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo18_reg: LDO18 {
@@ -451,6 +476,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo32_reg: LDO32 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e8fdda8..3da8084 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -13,6 +13,7 @@ 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/regulator/regulator.h>
 #include "exynos5800.dtsi"
 
 / {
@@ -191,6 +192,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck2_reg: BUCK2 {
@@ -200,6 +202,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck3_reg: BUCK3 {
@@ -209,6 +212,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck4_reg: BUCK4 {
@@ -218,6 +222,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck5_reg: BUCK5 {
@@ -226,6 +231,7 @@ 
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck6_reg: BUCK6 {
@@ -235,6 +241,7 @@ 
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck7_reg: BUCK7 {
@@ -243,6 +250,7 @@ 
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_NORMAL>;
 			};
 
 			buck8_reg: BUCK8 {
@@ -251,6 +259,7 @@ 
 				regulator-max-microvolt = <2850000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			buck9_reg: BUCK9 {
@@ -259,6 +268,7 @@ 
 				regulator-max-microvolt = <2000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_NORMAL>;
 			};
 
 			buck10_reg: BUCK10 {
@@ -267,6 +277,7 @@ 
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-initial-mode = <REGULATOR_MODE_NORMAL>;
 			};
 
 			ldo1_reg: LDO1 {
@@ -274,6 +285,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo2_reg: LDO2 {
@@ -287,6 +299,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			vqmmc_sdcard: ldo4_reg: LDO4 {
@@ -294,6 +307,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo5_reg: LDO5 {
@@ -301,6 +315,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo6_reg: LDO6 {
@@ -308,6 +323,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo7_reg: LDO7 {
@@ -321,6 +337,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo9_reg: LDO9 {
@@ -328,6 +345,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo10_reg: LDO10 {
@@ -335,6 +353,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo11_reg: LDO11 {
@@ -342,6 +361,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo12_reg: LDO12 {
@@ -349,6 +369,7 @@ 
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo13_reg: LDO13 {
@@ -356,6 +377,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_IDLE>;
 			};
 
 			ldo14_reg: LDO14 {
@@ -363,6 +385,7 @@ 
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo15_reg: LDO15 {
@@ -370,6 +393,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo17_reg: LDO17 {
@@ -377,6 +401,7 @@ 
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo18_reg: LDO18 {
@@ -450,6 +475,7 @@ 
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
 			};
 
 			ldo32_reg: LDO32 {