Skip to contentSkip to conversation

Add support for Suspend and Resume for Exynos4412 on

RFC In review Anand Moon 2 participants 12 messages Feb 13, 2019 – Feb 15, 2019

Subsystems (2)
lore
Share & tools
lore

Anand Moon

author
#0 lore raw

cover Add support for Suspend and Resume for Exynos4412 on

Add support to set the Odroid U3+ device into DEEP sleep mode and is able to resume from suspend state.

Changes base on Krxk/next/dt and worked on 5.0.0-rc6+
TODO: fix the eMMC failuer.

Any input are welcome.

Best Regards
-Anand

Anand Moon (2):
ARM: dts: exynos: Add proper regulator states for suspend-to-mem for
odroid-u3
soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power
state

 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 ++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 drivers/soc/samsung/exynos4-pmu.c             | 83 +++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h   | 21 +++++
 4 files changed, 170 insertions(+)
Signature · 1 line
2.20.1
author
#1 lore raw

RFC 1/2 ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3

Add suspend-to-mem node to regulator core to be enabled or disabled during system suspend and also support changing the regulator operating mode during runtime and when the system enter sleep mode.

Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previos patch
[0] https://patchwork.kernel.org/patch/10712549/

Set all the WAKEUP source regulator in suspend-on state. LD04, LD012, LD015, LD020, LD022
Set all the non used regulator in suspend-odd state LD02, LD03, LD05, LD06, LD07, LD011, LD013, LDO14, LD016

BUCK5, BUCK6, BUCK7 and not confirable as per driver max77686-regulator

Tested on microSD card and it resumes correcly after suspend. eMMC is not able to resume after entering into suspend state, which need to be investigated and how to debug more.

---
 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 +++++++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 2 files changed, 66 insertions(+)
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +63 -0
index 08d3a0a7b4eb..e984461c37d9 100644
@@ -288,6 +288,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo3_reg: LDO3 {
@@ -295,6 +298,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo4_reg: LDO4 {
@@ -302,6 +308,9 @@
 				regulator-min-microvolt = <2800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo5_reg: LDO5 {
@@ -310,6 +319,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo6_reg: LDO6 {
@@ -317,6 +329,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo7_reg: LDO7 {
@@ -324,18 +339,27 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo8_reg: LDO8 {
 				regulator-name = "VDD10_HDMI_1.0V";
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo10_reg: LDO10 {
 				regulator-name = "VDDQ_MIPIHSI_1.8V";
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo11_reg: LDO11 {
@@ -343,6 +367,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo12_reg: LDO12 {
@@ -351,6 +378,9 @@
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo13_reg: LDO13 {
@@ -359,6 +389,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo14_reg: LDO14 {
@@ -367,6 +400,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo15_reg: LDO15 {
@@ -375,6 +411,9 @@
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo16_reg: LDO16 {
@@ -383,6 +422,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo20_reg: LDO20 {
@@ -396,6 +438,9 @@
 				regulator-min-microvolt = <2800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo22_reg: LDO22 {
@@ -405,6 +450,9 @@
 				 */
 				regulator-name = "LDO22";
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo25_reg: LDO25 {
@@ -413,6 +461,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck1_reg: BUCK1 {
@@ -421,6 +472,9 @@
 				regulator-max-microvolt = <1100000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck2_reg: BUCK2 {
@@ -429,6 +483,9 @@
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck3_reg: BUCK3 {
@@ -437,6 +494,9 @@
 				regulator-max-microvolt = <1050000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck4_reg: BUCK4 {
@@ -444,6 +504,9 @@
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1100000>;
 				regulator-microvolt-offset = <50000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck5_reg: BUCK5 {
arch/arm/boot/dts/exynos4412-odroidu3.dts +3 -0
index 2bdf899df436..4ebde09fc51d 100644
@@ -82,6 +82,9 @@
 	regulator-name = "LDO22_VDDQ_MMC4_2.8V";
 	regulator-min-microvolt = <2800000>;
 	regulator-max-microvolt = <2800000>;
+	regulator-state-mem {
+		regulator-on-in-suspend;
+	};
 };
 
 &mshc_0 {
Signature · 1 line
2.20.1
#2 lore raw

Re: [RFC 1/2] ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3

Hi Anand,

Thanks for the patch. See comments below.

Quoted 34 lines · Anand Moon

Add suspend-to-mem node to regulator core to be enabled or disabled during system suspend and also support changing the regulator operating mode during runtime and when the system enter sleep mode.

Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previos patch
[0] https://patchwork.kernel.org/patch/10712549/

Set all the WAKEUP source regulator in suspend-on state. LD04, LD012, LD015, LD020, LD022
Set all the non used regulator in suspend-odd state LD02, LD03, LD05, LD06, LD07, LD011, LD013, LDO14, LD016

BUCK5, BUCK6, BUCK7 and not confirable as per driver max77686-regulator

Tested on microSD card and it resumes correcly after suspend. eMMC is not able to resume after entering into suspend state, which need to be investigated and how to debug more.

---
 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 +++++++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 2 files changed, 66 insertions(+)
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +0 -0
index 08d3a0a7b4eb..e984461c37d9 100644

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 293

Quoted 6 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
@@ -288,6 +288,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

I see my comment from previous patch was not addressed.

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 303

Quoted 10 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        ldo3_reg: LDO3 {
@@ -295,6 +298,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Quoted 122 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +36 -0
                        };
 
                        ldo4_reg: LDO4 {
@@ -302,6 +308,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo5_reg: LDO5 {
@@ -310,6 +319,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo6_reg: LDO6 {
@@ -317,6 +329,9 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo7_reg: LDO7 {
@@ -324,18 +339,27 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo8_reg: LDO8 {
                                regulator-name = "VDD10_HDMI_1.0V";
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo10_reg: LDO10 {
                                regulator-name = "VDDQ_MIPIHSI_1.8V";
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo11_reg: LDO11 {
@@ -343,6 +367,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo12_reg: LDO12 {
@@ -351,6 +378,9 @@
                                regulator-max-microvolt = <3300000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo13_reg: LDO13 {
@@ -359,6 +389,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo14_reg: LDO14 {
@@ -367,6 +400,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo15_reg: LDO15 {
@@ -375,6 +411,9 @@
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo16_reg: LDO16 {
@@ -383,6 +422,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo20_reg: LDO20 {

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 443

Quoted 6 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
@@ -396,6 +438,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };

The same... any comments?

Quoted 24 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +6 -0
                        };
 
                        ldo22_reg: LDO22 {
@@ -405,6 +450,9 @@
                                 */
                                regulator-name = "LDO22";
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo25_reg: LDO25 {
@@ -413,6 +461,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        buck1_reg: BUCK1 {

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 476

Quoted 5 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +2 -0
@@ -421,6 +472,9 @@
                                regulator-max-microvolt = <1100000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;

Again, you did not address my comments.

Quoted 15 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +4 -0
+                               };
                        };
 
                        buck2_reg: BUCK2 {
@@ -429,6 +483,9 @@
                                regulator-max-microvolt = <1350000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        buck3_reg: BUCK3 {

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 499

Quoted 6 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
@@ -437,6 +494,9 @@
                                regulator-max-microvolt = <1050000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Quoted 18 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        buck4_reg: BUCK4 {
@@ -444,6 +504,9 @@
                                regulator-min-microvolt = <900000>;
                                regulator-max-microvolt = <1100000>;
                                regulator-microvolt-offset = <50000>;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        buck5_reg: BUCK5 {
arch/arm/boot/dts/exynos4412-odroidu3.dts +0 -0
index 2bdf899df436..4ebde09fc51d 100644

arch/arm/boot/dts/exynos4412-odroidu3.dts line 86

Quoted 5 lines
arch/arm/boot/dts/exynos4412-odroidu3.dts +2 -0
@@ -82,6 +82,9 @@
        regulator-name = "LDO22_VDDQ_MMC4_2.8V";
        regulator-min-microvolt = <2800000>;
        regulator-max-microvolt = <2800000>;
+       regulator-state-mem {
+               regulator-on-in-suspend;

Why?

Best regards,
Krzysztof

author
#3 lore raw

Re: [RFC 1/2] ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3

hi Krzysztof,

Thanks fro your review comments.

Quoted 35 lines · Krzysztof Kozlowski

Hi Anand,

Thanks for the patch. See comments below.

Quoted 29 lines · Anand Moon

Add suspend-to-mem node to regulator core to be enabled or disabled during system suspend and also support changing the regulator operating mode during runtime and when the system enter sleep mode.

Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previos patch
[0] https://patchwork.kernel.org/patch/10712549/

Set all the WAKEUP source regulator in suspend-on state. LD04, LD012, LD015, LD020, LD022
Set all the non used regulator in suspend-odd state LD02, LD03, LD05, LD06, LD07, LD011, LD013, LDO14, LD016

BUCK5, BUCK6, BUCK7 and not confirable as per driver max77686-regulator

Tested on microSD card and it resumes correcly after suspend. eMMC is not able to resume after entering into suspend state, which need to be investigated and how to debug more.

---
 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 +++++++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 2 files changed, 66 insertions(+)
Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 293

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
index 08d3a0a7b4eb..e984461c37d9 100644
@@ -288,6 +288,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

I see my comment from previous patch was not addressed.

I left this unchanged since this regulator is not active linked and used. Ok I will change this to regulator-on-in-suspend,

Quoted 1 line

From documentation device tree binding regulator.txt

"- regulator-always-on: boolean, regulator should never be disabled"

But some regulator switches to a low power mode under light current loads and the device automatically switches back to a fast response mode as the output load increases.

Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 303

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        ldo3_reg: LDO3 {
@@ -295,6 +298,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Ok I will change this to regulator-on-in-suspend,

Quoted 121 lines
Quoted 121 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +36 -0
                        };
 
                        ldo4_reg: LDO4 {
@@ -302,6 +308,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo5_reg: LDO5 {
@@ -310,6 +319,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo6_reg: LDO6 {
@@ -317,6 +329,9 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo7_reg: LDO7 {
@@ -324,18 +339,27 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo8_reg: LDO8 {
                                regulator-name = "VDD10_HDMI_1.0V";
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo10_reg: LDO10 {
                                regulator-name = "VDDQ_MIPIHSI_1.8V";
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo11_reg: LDO11 {
@@ -343,6 +367,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo12_reg: LDO12 {
@@ -351,6 +378,9 @@
                                regulator-max-microvolt = <3300000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo13_reg: LDO13 {
@@ -359,6 +389,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo14_reg: LDO14 {
@@ -367,6 +400,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo15_reg: LDO15 {
@@ -375,6 +411,9 @@
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo16_reg: LDO16 {
@@ -383,6 +422,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
Quoted 11 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 443

Quoted 8 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        ldo20_reg: LDO20 {
@@ -396,6 +438,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };

The same... any comments?

I left this on for mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMc

Quoted 23 lines
Quoted 23 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +6 -0
                        };
 
                        ldo22_reg: LDO22 {
@@ -405,6 +450,9 @@
                                 */
                                regulator-name = "LDO22";
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo25_reg: LDO25 {
@@ -413,6 +461,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
Quoted 10 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 476

Quoted 7 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +2 -0
                        buck1_reg: BUCK1 {
@@ -421,6 +472,9 @@
                                regulator-max-microvolt = <1100000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;

Again, you did not address my comments.

Buck1 support entering into LPM setting,
But I will set this to regulator-on-in-suspend; in next patch.

Quoted 13 lines
Quoted 13 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +4 -0
+                               };
                        };
 
                        buck2_reg: BUCK2 {
@@ -429,6 +483,9 @@
                                regulator-max-microvolt = <1350000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
Quoted 10 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 499

Quoted 8 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        buck3_reg: BUCK3 {
@@ -437,6 +494,9 @@
                                regulator-max-microvolt = <1050000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Buck3 support entering into LPM setting,
But I will set this to regulator-on-in-suspend; in next patch.

Quoted 13 lines
Quoted 13 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        buck4_reg: BUCK4 {
@@ -444,6 +504,9 @@
                                regulator-min-microvolt = <900000>;
                                regulator-max-microvolt = <1100000>;
                                regulator-microvolt-offset = <50000>;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
Quoted 14 lines

arch/arm/boot/dts/exynos4412-odroidu3.dts line 86

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +0 -0
                        buck5_reg: BUCK5 {
arch/arm/boot/dts/exynos4412-odroidu3.dts +2 -0
index 2bdf899df436..4ebde09fc51d 100644
@@ -82,6 +82,9 @@
        regulator-name = "LDO22_VDDQ_MMC4_2.8V";
        regulator-min-microvolt = <2800000>;
        regulator-max-microvolt = <2800000>;
+       regulator-state-mem {
+               regulator-on-in-suspend;

Why?

I chose not to disable mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMC.

Quoted 2 lines

Best regards,
Krzysztof

Best Regards

-Anand

#4 lore raw

Re: [RFC 1/2] ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3

Quoted 55 lines · Anand Moon

hi Krzysztof,

Thanks fro your review comments.

Quoted 35 lines · Krzysztof Kozlowski

Hi Anand,

Thanks for the patch. See comments below.

Quoted 29 lines · Anand Moon

Add suspend-to-mem node to regulator core to be enabled or disabled during system suspend and also support changing the regulator operating mode during runtime and when the system enter sleep mode.

Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previos patch
[0] https://patchwork.kernel.org/patch/10712549/

Set all the WAKEUP source regulator in suspend-on state. LD04, LD012, LD015, LD020, LD022
Set all the non used regulator in suspend-odd state LD02, LD03, LD05, LD06, LD07, LD011, LD013, LDO14, LD016

BUCK5, BUCK6, BUCK7 and not confirable as per driver max77686-regulator

Tested on microSD card and it resumes correcly after suspend. eMMC is not able to resume after entering into suspend state, which need to be investigated and how to debug more.

---
 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 +++++++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 2 files changed, 66 insertions(+)
Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 293

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
index 08d3a0a7b4eb..e984461c37d9 100644
@@ -288,6 +288,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

I see my comment from previous patch was not addressed.

Quoted 2 lines

I left this unchanged since this regulator is not active linked and used. Ok I will change this to regulator-on-in-suspend,

Why?

Previous patch was setting this to "on". I said that this is noop and if you want to add it, I need explanation why this regulator has to stay on during suspend.

So you changed to "off"... which is still noop... and you did not provide explanation. Now you replied that you will change to "on"... so this will be circle. Still without explanation.

Quoted 22 lines

From documentation device tree binding regulator.txt
"- regulator-always-on: boolean, regulator should never be disabled"

But some regulator switches to a low power mode under light current loads and the device automatically switches back to a fast response mode as the output load increases.

Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 303

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        ldo3_reg: LDO3 {
@@ -295,6 +298,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Quoted 1 line

Ok I will change this to regulator-on-in-suspend,

The same - why changing this to on or off? I need explanation why this should be in first place.

Quoted 133 lines
Quoted 121 lines
Quoted 121 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +36 -0
                        };
 
                        ldo4_reg: LDO4 {
@@ -302,6 +308,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo5_reg: LDO5 {
@@ -310,6 +319,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo6_reg: LDO6 {
@@ -317,6 +329,9 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo7_reg: LDO7 {
@@ -324,18 +339,27 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo8_reg: LDO8 {
                                regulator-name = "VDD10_HDMI_1.0V";
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo10_reg: LDO10 {
                                regulator-name = "VDDQ_MIPIHSI_1.8V";
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo11_reg: LDO11 {
@@ -343,6 +367,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo12_reg: LDO12 {
@@ -351,6 +378,9 @@
                                regulator-max-microvolt = <3300000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo13_reg: LDO13 {
@@ -359,6 +389,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo14_reg: LDO14 {
@@ -367,6 +400,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo15_reg: LDO15 {
@@ -375,6 +411,9 @@
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo16_reg: LDO16 {
@@ -383,6 +422,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
Quoted 11 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 443

Quoted 8 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        ldo20_reg: LDO20 {
@@ -396,6 +438,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };

The same... any comments?

Quoted 2 lines

I left this on for mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMc

In general eMMC can be turned off during suspend. However eMMC layer may be handling this (manually turning off/on). So after checking and confirming it, please document it.

Quoted 34 lines
Quoted 23 lines
Quoted 23 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +6 -0
                        };
 
                        ldo22_reg: LDO22 {
@@ -405,6 +450,9 @@
                                 */
                                regulator-name = "LDO22";
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo25_reg: LDO25 {
@@ -413,6 +461,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
Quoted 10 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 476

Quoted 7 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +2 -0
                        buck1_reg: BUCK1 {
@@ -421,6 +472,9 @@
                                regulator-max-microvolt = <1100000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;

Again, you did not address my comments.

Quoted 2 lines

Buck1 support entering into LPM setting,
But I will set this to regulator-on-in-suspend; in next patch.

Why?

Quoted 24 lines
Quoted 13 lines
Quoted 13 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +4 -0
+                               };
                        };
 
                        buck2_reg: BUCK2 {
@@ -429,6 +483,9 @@
                                regulator-max-microvolt = <1350000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
Quoted 10 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 499

Quoted 8 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        buck3_reg: BUCK3 {
@@ -437,6 +494,9 @@
                                regulator-max-microvolt = <1050000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Quoted 2 lines

Buck3 support entering into LPM setting,
But I will set this to regulator-on-in-suspend; in next patch.

Again - why this should be on or off?

Quoted 28 lines
Quoted 13 lines
Quoted 13 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        buck4_reg: BUCK4 {
@@ -444,6 +504,9 @@
                                regulator-min-microvolt = <900000>;
                                regulator-max-microvolt = <1100000>;
                                regulator-microvolt-offset = <50000>;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
Quoted 14 lines

arch/arm/boot/dts/exynos4412-odroidu3.dts line 86

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +0 -0
                        buck5_reg: BUCK5 {
arch/arm/boot/dts/exynos4412-odroidu3.dts +2 -0
index 2bdf899df436..4ebde09fc51d 100644
@@ -82,6 +82,9 @@
        regulator-name = "LDO22_VDDQ_MMC4_2.8V";
        regulator-min-microvolt = <2800000>;
        regulator-max-microvolt = <2800000>;
+       regulator-state-mem {
+               regulator-on-in-suspend;

Why?

Quoted 2 lines

I chose not to disable mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMC.

Having "regulator-on-in-suspend" is not the same as not disabling regulator during suspend. This property means you will explicitly enable it during suspend. To me it looks wrong so I would be happy to see explanations.

Best regards,
Krzysztof

author
#5 lore raw

Re: [RFC 1/2] ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3

Hi Krzysztof,

Quoted 62 lines · Krzysztof Kozlowski
Quoted 55 lines · Anand Moon

hi Krzysztof,

Thanks fro your review comments.

Quoted 35 lines · Krzysztof Kozlowski

Hi Anand,

Thanks for the patch. See comments below.

Quoted 29 lines · Anand Moon

Add suspend-to-mem node to regulator core to be enabled or disabled during system suspend and also support changing the regulator operating mode during runtime and when the system enter sleep mode.

Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previos patch
[0] https://patchwork.kernel.org/patch/10712549/

Set all the WAKEUP source regulator in suspend-on state. LD04, LD012, LD015, LD020, LD022
Set all the non used regulator in suspend-odd state LD02, LD03, LD05, LD06, LD07, LD011, LD013, LDO14, LD016

BUCK5, BUCK6, BUCK7 and not confirable as per driver max77686-regulator

Tested on microSD card and it resumes correcly after suspend. eMMC is not able to resume after entering into suspend state, which need to be investigated and how to debug more.

---
 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 +++++++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 2 files changed, 66 insertions(+)
Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 293

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
index 08d3a0a7b4eb..e984461c37d9 100644
@@ -288,6 +288,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

I see my comment from previous patch was not addressed.

Quoted 2 lines

I left this unchanged since this regulator is not active linked and used. Ok I will change this to regulator-on-in-suspend,

Why?

Quoted 8 lines

Previous patch was setting this to "on". I said that this is noop and if you want to add it, I need explanation why this regulator has to stay on during suspend.

So you changed to "off"... which is still noop... and you did not provide explanation. Now you replied that you will change to "on"... so this will be circle. Still without explanation.

I will re check with my previous patch and the driver code which support suspend state.
Ok check the regulator driver max77686-regulator and enable the logs their.

Quoted 22 lines
Quoted 22 lines

From documentation device tree binding regulator.txt
"- regulator-always-on: boolean, regulator should never be disabled"

But some regulator switches to a low power mode under light current loads and the device automatically switches back to a fast response mode as the output load increases.

Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 303

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        ldo3_reg: LDO3 {
@@ -295,6 +298,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Quoted 4 lines
Quoted 1 line

Ok I will change this to regulator-on-in-suspend,

The same - why changing this to on or off? I need explanation why this should be in first place.

I will check in the driver code.

Quoted 134 lines
Quoted 134 lines
Quoted 133 lines
Quoted 124 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +36 -0
                        };
 
                        ldo4_reg: LDO4 {
@@ -302,6 +308,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo5_reg: LDO5 {
@@ -310,6 +319,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo6_reg: LDO6 {
@@ -317,6 +329,9 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo7_reg: LDO7 {
@@ -324,18 +339,27 @@
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo8_reg: LDO8 {
                                regulator-name = "VDD10_HDMI_1.0V";
                                regulator-min-microvolt = <1000000>;
                                regulator-max-microvolt = <1000000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo10_reg: LDO10 {
                                regulator-name = "VDDQ_MIPIHSI_1.8V";
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo11_reg: LDO11 {
@@ -343,6 +367,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo12_reg: LDO12 {
@@ -351,6 +378,9 @@
                                regulator-max-microvolt = <3300000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo13_reg: LDO13 {
@@ -359,6 +389,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo14_reg: LDO14 {
@@ -367,6 +400,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo15_reg: LDO15 {
@@ -375,6 +411,9 @@
                                regulator-max-microvolt = <1000000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo16_reg: LDO16 {
@@ -383,6 +422,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        ldo20_reg: LDO20 {

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 443

Quoted 6 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
@@ -396,6 +438,9 @@
                                regulator-min-microvolt = <2800000>;
                                regulator-max-microvolt = <2800000>;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };

The same... any comments?

Quoted 6 lines
Quoted 2 lines

I left this on for mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMc

In general eMMC can be turned off during suspend. However eMMC layer may be handling this (manually turning off/on). So after checking and confirming it, please document it.

I will check in the driver code.

Quoted 35 lines
Quoted 35 lines
Quoted 34 lines
Quoted 26 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +6 -0
                        };
 
                        ldo22_reg: LDO22 {
@@ -405,6 +450,9 @@
                                 */
                                regulator-name = "LDO22";
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
                        ldo25_reg: LDO25 {
@@ -413,6 +461,9 @@
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        buck1_reg: BUCK1 {

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 476

Quoted 5 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +2 -0
@@ -421,6 +472,9 @@
                                regulator-max-microvolt = <1100000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;

Again, you did not address my comments.

Quoted 4 lines
Quoted 2 lines

Buck1 support entering into LPM setting,
But I will set this to regulator-on-in-suspend; in next patch.

Why?

I will check in the driver code.

Quoted 58 lines
Quoted 25 lines
Quoted 14 lines
Quoted 14 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +4 -0
+                               };
                        };
 
                        buck2_reg: BUCK2 {
@@ -429,6 +483,9 @@
                                regulator-max-microvolt = <1350000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-on-in-suspend;
+                               };
                        };
 
Quoted 10 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 499

Quoted 8 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        buck3_reg: BUCK3 {
@@ -437,6 +494,9 @@
                                regulator-max-microvolt = <1050000>;
                                regulator-always-on;
                                regulator-boot-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

The same...

Quoted 2 lines

Buck3 support entering into LPM setting,
But I will set this to regulator-on-in-suspend; in next patch.

Again - why this should be on or off?

Quoted 28 lines
Quoted 27 lines
Quoted 19 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
                        };
 
                        buck4_reg: BUCK4 {
@@ -444,6 +504,9 @@
                                regulator-min-microvolt = <900000>;
                                regulator-max-microvolt = <1100000>;
                                regulator-microvolt-offset = <50000>;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };
                        };
 
                        buck5_reg: BUCK5 {
arch/arm/boot/dts/exynos4412-odroidu3.dts +0 -0
index 2bdf899df436..4ebde09fc51d 100644

arch/arm/boot/dts/exynos4412-odroidu3.dts line 86

Quoted 5 lines
arch/arm/boot/dts/exynos4412-odroidu3.dts +2 -0
@@ -82,6 +82,9 @@
        regulator-name = "LDO22_VDDQ_MMC4_2.8V";
        regulator-min-microvolt = <2800000>;
        regulator-max-microvolt = <2800000>;
+       regulator-state-mem {
+               regulator-on-in-suspend;

Why?

Quoted 8 lines
Quoted 2 lines

I chose not to disable mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMC.

Having "regulator-on-in-suspend" is not the same as not disabling regulator during suspend. This property means you will explicitly enable it during suspend. To me it looks wrong so I would be happy to see explanations.

Ok I will disable this and check this help me fix the issue I am observing.

Quoted 2 lines

Best regards,
Krzysztof

*Sorry for wasting your time*
I know I have the improve a lot of things.

Best regards

-Anand

author
#6 lore raw

Re: [RFC 1/2] ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3

In reply to Krzysztof Kozlowski

Hi Krzysztof,

Quoted 62 lines · Krzysztof Kozlowski
Quoted 55 lines · Anand Moon

hi Krzysztof,

Thanks fro your review comments.

Quoted 35 lines · Krzysztof Kozlowski

Hi Anand,

Thanks for the patch. See comments below.

Quoted 29 lines · Anand Moon

Add suspend-to-mem node to regulator core to be enabled or disabled during system suspend and also support changing the regulator operating mode during runtime and when the system enter sleep mode.

Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previos patch
[0] https://patchwork.kernel.org/patch/10712549/

Set all the WAKEUP source regulator in suspend-on state. LD04, LD012, LD015, LD020, LD022
Set all the non used regulator in suspend-odd state LD02, LD03, LD05, LD06, LD07, LD011, LD013, LDO14, LD016

BUCK5, BUCK6, BUCK7 and not confirable as per driver max77686-regulator

Tested on microSD card and it resumes correcly after suspend. eMMC is not able to resume after entering into suspend state, which need to be investigated and how to debug more.

---
 .../boot/dts/exynos4412-odroid-common.dtsi    | 63 +++++++++++++++++++
 arch/arm/boot/dts/exynos4412-odroidu3.dts     |  3 +
 2 files changed, 66 insertions(+)
Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroid-common.dtsi line 293

Quoted 11 lines
arch/arm/boot/dts/exynos4412-odroid-common.dtsi +3 -0
index 08d3a0a7b4eb..e984461c37d9 100644
@@ -288,6 +288,9 @@
                                regulator-min-microvolt = <1800000>;
                                regulator-max-microvolt = <1800000>;
                                regulator-always-on;
+                               regulator-state-mem {
+                                       regulator-off-in-suspend;
+                               };

I see my comment from previous patch was not addressed.

Quoted 2 lines

I left this unchanged since this regulator is not active linked and used. Ok I will change this to regulator-on-in-suspend,

Why?

Quoted 8 lines

Previous patch was setting this to "on". I said that this is noop and if you want to add it, I need explanation why this regulator has to stay on during suspend.

So you changed to "off"... which is still noop... and you did not provide explanation. Now you replied that you will change to "on"... so this will be circle. Still without explanation.

------------------------------------------------------------------------------------------------
I have added some debug prints to get more inputs.

Here is the boot logs for sdcard suspend resume

[1] https://pastebin.com/wvJvJidp

[root@archl-u3m ~]# rtcwake -d /dev/rtc0 -m mem -s 10
rtcwake: assuming RTC uses UTC ...
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Feb 15 13:28:06 2019

[   97.165349] PM: suspend entry (deep)
[   97.165846] PM: Syncing filesystems ... done.
[   97.344436] Freezing user space processes ... (elapsed 0.006 seconds) done.
[   97.354738] OOM killer disabled.
[   97.356028] Freezing remaining freezable tasks ... (elapsed 0.004
seconds) done.
[   97.413453] smsc95xx 1-2:1.0 eth0: entering SUSPEND2 mode
[   97.423227] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[   97.444346] dwc2 12480000.hsotg: suspending usb gadget g_ether
[   97.446728] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   97.451287] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   97.457742] dwc2 12480000.hsotg: new device is full-speed
[   97.465766] wake enabled for irq 119
[   97.466979] BUCK9: No configuration
[   97.470417] BUCK8_P3V3: No configuration
[   97.474131] BUCK7_2.0V: No configuration
[   97.477954] BUCK6_1.35V: No configuration
[   97.481946] VDDQ_CKEM1_2_1.2V: No configuration
[   97.487930] BUCK4: regulator suspend disable supported
[   97.491964] BUCK3: regulator suspend disable supported
[   97.497184] BUCK1: regulator suspend disable supported
[   97.501838] LDO26: No configuration
[   97.505383] LDO24: No configuration
[   97.508761] LDO23: No configuration
[   97.512368] LDO19: No configuration
[   97.515706] LDO18: No configuration
[   97.519178] LDO17: No configuration
[   97.523076] LDO16: regulator suspend disable supported
[   97.528154] LDO15: regulator suspend disable supported
[   97.533367] LDO14: regulator suspend disable supported
[   97.538447] LDO12: regulator suspend disable supported
[   97.543603] LDO11: regulator suspend disable supported
[   97.548308] LDO9: No configuration
[   97.552107] LDO7: regulator suspend disable supported
[   97.557064] LDO6: regulator suspend disable supported
[   97.562251] LDO2: regulator suspend disable supported
[   97.566769] VDD_ALIVE_1.0V: No configuration
[   97.581717] usb3503 0-0008: switched to STANDBY mode
[   97.582496] wake enabled for irq 123
[   97.602770] samsung-pinctrl 11000000.pinctrl: Setting external
wakeup interrupt mask: 0xfbfff7ff
[   97.623616] Disabling non-boot CPUs ...
[   97.681453] s3c2410-wdt 10060000.watchdog: watchdog disabled
[   97.681743] wake disabled for irq 123
[   97.692569] usb3503 0-0008: switched to HUB mode
[   97.801553] wake disabled for irq 119
[   97.802087] dwc2 12480000.hsotg: resuming usb gadget g_ether
[   98.080458] usb 1-2: reset high-speed USB device number 2 using exynos-ehci
[   98.432321] usb 1-3: reset high-speed USB device number 3 using exynos-ehci
[   98.970441] usb 1-3.3: reset high-speed USB device number 4 using exynos-ehci
[   99.135835] OOM killer enabled.
[   99.135940] Restarting tasks ... done.
[   99.140760] PM: suspend exit

[root@archl-u3m ~]# [ 99.877843] smsc95xx 1-2:1.0 eth0: link up,
100Mbps, full-duplex, lpa 0xC5E1

------------------------------------------------
Here is the boot log for emmc suspend resume. after disable of emmc regulator.

[2] https://pastebin.com/Z05K3kj2

rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Feb 15 13:27:26 2019

[   79.775108] PM: suspend entry (deep)
[   79.775462] PM: Syncing filesystems ... done.
[   79.819009] Freezing user space processes ... (elapsed 0.004 seconds) done.
[   79.826300] OOM killer disabled.
[   79.828603] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[   79.875967] smsc95xx 1-2:1.0 eth0: entering SUSPEND2 mode
[   79.959495] dwc2 12480000.hsotg: suspending usb gadget g_ether
[   79.960480] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   79.966431] dwc2 12480000.hsotg: new device is full-speed
[   79.971805] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0
[   79.980568] wake enabled for irq 119
[   79.982195] BUCK9: No configuration
[   79.985540] BUCK8_P3V3: No configuration
[   79.989201] BUCK7_2.0V: No configuration
[   79.993175] BUCK6_1.35V: No configuration
[   79.997099] VDDQ_CKEM1_2_1.2V: No configuration
[   80.002765] BUCK4: regulator suspend disable supported
[   80.007115] BUCK3: regulator suspend disable supported
[   80.012346] BUCK1: regulator suspend disable supported
[   80.016991] LDO26: No configuration
[   80.020467] LDO24: No configuration
[   80.023991] LDO23: No configuration
[   80.027456] LDO19: No configuration
[   80.030858] LDO18: No configuration
[   80.034388] LDO17: No configuration
[   80.038172] LDO16: regulator suspend disable supported
[   80.043410] LDO15: regulator suspend disable supported
[   80.048429] LDO14: regulator suspend disable supported
[   80.053658] LDO12: regulator suspend disable supported
[   80.058670] LDO11: regulator suspend disable supported
[   80.063522] LDO9: No configuration
[   80.067187] LDO7: regulator suspend disable supported
[   80.072290] LDO6: regulator suspend disable supported
[   80.077321] LDO2: regulator suspend disable supported
[   80.081995] VDD_ALIVE_1.0V: No configuration
[   80.096885] usb3503 0-0008: switched to STANDBY mode
[   80.097568] wake enabled for irq 123
[   80.116002] samsung-pinctrl 11000000.pinctrl: Setting external
wakeup interrupt mask: 0xfbfff7ff
[   80.134695] Disabling non-boot CPUs ...

[snip]

Quoted 14 lines
Quoted 14 lines
Quoted 13 lines

arch/arm/boot/dts/exynos4412-odroidu3.dts line 86

Quoted 10 lines
arch/arm/boot/dts/exynos4412-odroidu3.dts +2 -0
index 2bdf899df436..4ebde09fc51d 100644
@@ -82,6 +82,9 @@
        regulator-name = "LDO22_VDDQ_MMC4_2.8V";
        regulator-min-microvolt = <2800000>;
        regulator-max-microvolt = <2800000>;
+       regulator-state-mem {
+               regulator-on-in-suspend;

Why?

Quoted 8 lines
Quoted 2 lines

I chose not to disable mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMC.

Having "regulator-on-in-suspend" is not the same as not disabling regulator during suspend. This property means you will explicitly enable it during suspend. To me it looks wrong so I would be happy to see explanations.

During suspend the driver set the regulator in standby mode until resume. Opps I understood the regulator in suspend mode enter into

REGULATOR_MODE_STANDBY : MAX77686_OFF_PWRREQ;

[   79.997099] VDDQ_CKEM1_2_1.2V: No configuration
[   80.002765] BUCK4: regulator suspend disable supported
[   80.007115] BUCK3: regulator suspend disable supported
[   80.012346] BUCK1: regulator suspend disable supported
[   80.016991] LDO26: No configuration
[   80.020467] LDO24: No configuration
[   80.023991] LDO23: No configuration
[   80.027456] LDO19: No configuration
[   80.030858] LDO18: No configuration
[   80.034388] LDO17: No configuration
[   80.038172] LDO16: regulator suspend disable supported
[   80.043410] LDO15: regulator suspend disable supported
[   80.048429] LDO14: regulator suspend disable supported
[   80.053658] LDO12: regulator suspend disable supported
[   80.058670] LDO11: regulator suspend disable supported
[   80.063522] LDO9: No configuration
[   80.067187] LDO7: regulator suspend disable supported
[   80.072290] LDO6: regulator suspend disable supported
[   80.077321] LDO2: regulator suspend disable supported

So as per the logs BUCK1 / BUCK3 / BUCK4 successfully entered into suspend mode.

Where as suspend is disabled in groups as per the documentation. (Group 1)
Buck6-->Buck7-->LD01-->LD011-->LD014
Group 2)
Buck2-->Buck1-->Buck3-->Buck4
Group 3> LD06-->LD013-->LD08-->LD015-->LD010-->LD016-->LD05-->LD012

Best Regards
-Anand

author
#7 lore raw

RFC 2/2 soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

In reply to Anand Moon

This patch adds configration for PMU (Power Management Unit) state tuning for exynos4412 SoC in order to enter low-power mode during suspend power modes and help resume from suspend state.

Fixes bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")
Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previous patch.
New patch to this series to support suspend and resume state

Changes have been tested on microSD card but fails to resume on cMMC. It need to be investigated and more debuging

---
 drivers/soc/samsung/exynos4-pmu.c           | 83 +++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++++++
 2 files changed, 104 insertions(+)
drivers/soc/samsung/exynos4-pmu.c +83 -0
index a7cdbf1aac0c..d261a0d2371e 100644
@@ -200,10 +200,93 @@ static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
 	{ PMU_TABLE_END,},
 };
 
+static unsigned int const exynos4412_list_feed[] = {
+	EXYNOS4_ARM_CORE0_OPTION,
+	EXYNOS4_ARM_CORE1_OPTION,
+	EXYNOS4_ARM_CORE2_OPTION,
+	EXYNOS4_ARM_CORE3_OPTION,
+	EXYNOS4_ARM_COMMON_OPTION,
+	EXYNOS4_CAM_OPTION,
+	EXYNOS4_TV_OPTION,
+	EXYNOS4_MFC_OPTION,
+	EXYNOS4_G3D_OPTION,
+	EXYNOS4_LCD0_OPTION,
+	EXYNOS4_ISP_OPTION,
+	EXYNOS4_MAUDIO_OPTION,
+	EXYNOS4_GPS_OPTION,
+	EXYNOS4_GPS_ALIVE_OPTION,
+};
+
+static void exynos4412_pmu_central_seq(bool enable)
+{
+	unsigned int value;
+
+	value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+	if (enable)
+		value &= ~S5P_CENTRAL_LOWPWR_CFG;
+	else
+		value |= S5P_CENTRAL_LOWPWR_CFG;
+	pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);
+
+	value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+	if (enable)
+		value &= ~S5P_CENTRAL_LOWPWR_CFG;
+	else
+		value |= S5P_CENTRAL_LOWPWR_CFG;
+	pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+}
+
+static void exynos4412_pmu_init(void)
+{
+	unsigned int value;
+	int i;
+
+	/* Enable USE_STANDBY_WFI for all CORE */
+	pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);
+
+	/* Decides whether to use retention capability */
+	value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
+	value &= ~EXYNOS_L2_USE_RETENTION;
+	pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
+
+	value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
+	value &= ~EXYNOS_L2_USE_RETENTION;
+	pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
+
+	/* Set PSHOLD port for output high */
+	value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+	value |= S5P_PS_HOLD_OUTPUT_HIGH;
+	pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+
+	/* Enable signal for PSHOLD port */
+	value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+	value |= S5P_PS_HOLD_EN;
+	pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+
+	/* Enable only SC_FEEDBACK */
+	for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
+		value =  pmu_raw_readl(exynos4412_list_feed[i]);
+		value &= ~(EXYNOS_USE_SC_COUNTER);
+		value |= EXYNOS_USE_SC_FEEDBACK;
+		pmu_raw_writel(value, exynos4412_list_feed[i]);
+	}
+
+	exynos4412_pmu_central_seq(false);
+
+	pr_info("EXYNOS4x12 PMU Initialize\n");
+}
+
+static void exynos4412_powerdown_conf(enum sys_powerdown mode)
+{
+	exynos4412_pmu_central_seq(true);
+}
+
 const struct exynos_pmu_data exynos4210_pmu_data = {
 	.pmu_config	= exynos4210_pmu_config,
 };
 
 const struct exynos_pmu_data exynos4412_pmu_data = {
 	.pmu_config		= exynos4412_pmu_config,
+	.pmu_init		= exynos4412_pmu_init,
+	.powerdown_conf		= exynos4412_powerdown_conf,
 };
include/linux/soc/samsung/exynos-regs-pmu.h +21 -0
index 5addaf5ccbce..6beed3e669d2 100644
@@ -16,6 +16,8 @@
 
 #define S5P_CENTRAL_SEQ_CONFIGURATION		0x0200
 
+#define S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK   0x0240
+
 #define S5P_CENTRAL_LOWPWR_CFG			(1 << 16)
 
 #define S5P_CENTRAL_SEQ_OPTION			0x0208
@@ -347,6 +349,25 @@
 #define EXYNOS3_OPTION_USE_SC_FEEDBACK			(1 << 1)
 #define EXYNOS3_OPTION_SKIP_DEACTIVATE_ACEACP_IN_PWDN	(1 << 7)
 
+/* EXYNOS4 */
+#define EXYNOS_USE_SC_FEEDBACK					BIT(1)
+#define EXYNOS_USE_SC_COUNTER					BIT(0)
+
+#define EXYNOS4_ARM_CORE0_OPTION				0x2008
+#define EXYNOS4_ARM_CORE1_OPTION				0x2088
+#define EXYNOS4_ARM_CORE2_OPTION				0x2108
+#define EXYNOS4_ARM_CORE3_OPTION				0x2188
+#define EXYNOS4_ARM_COMMON_OPTION				0x2408
+#define EXYNOS4_CAM_OPTION					0x3C08
+#define EXYNOS4_TV_OPTION					0x3C28
+#define EXYNOS4_MFC_OPTION					0x3C48
+#define EXYNOS4_G3D_OPTION					0x3C68
+#define EXYNOS4_LCD0_OPTION					0x3C88
+#define EXYNOS4_ISP_OPTION					0x3CA8
+#define EXYNOS4_MAUDIO_OPTION					0x3CC8
+#define EXYNOS4_GPS_OPTION					0x3CE8
+#define EXYNOS4_GPS_ALIVE_OPTION				0x3D08
+
 /* For EXYNOS5 */
 
 #define EXYNOS5_AUTO_WDTRESET_DISABLE				0x0408
Signature · 1 line
2.20.1
#8 lore raw

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

Quoted 4 lines · Anand Moon

This patch adds configration for PMU (Power Management Unit) state tuning for exynos4412 SoC in order to enter low-power mode during suspend power modes and help resume from suspend state.

The U3 and Trats2 already enter STOP/S2R so please describe what exactly you change.

Quoted 1 line
Fixes bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")

How it fixes it? What was broken in that commit?

Quoted 32 lines
Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previous patch.
New patch to this series to support suspend and resume state

Changes have been tested on microSD card but fails to resume on cMMC. It need to be investigated and more debuging

---
 drivers/soc/samsung/exynos4-pmu.c           | 83 +++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++++++
 2 files changed, 104 insertions(+)
drivers/soc/samsung/exynos4-pmu.c +8 -0
index a7cdbf1aac0c..d261a0d2371e 100644
@@ -200,10 +200,93 @@ static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
        { PMU_TABLE_END,},
 };
 
+static unsigned int const exynos4412_list_feed[] = {
+       EXYNOS4_ARM_CORE0_OPTION,
+       EXYNOS4_ARM_CORE1_OPTION,
+       EXYNOS4_ARM_CORE2_OPTION,
+       EXYNOS4_ARM_CORE3_OPTION,
+       EXYNOS4_ARM_COMMON_OPTION,
+       EXYNOS4_CAM_OPTION,
+       EXYNOS4_TV_OPTION,

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 220

Quoted 10 lines
drivers/soc/samsung/exynos4-pmu.c +10 -0
+       EXYNOS4_MFC_OPTION,
+       EXYNOS4_G3D_OPTION,
+       EXYNOS4_LCD0_OPTION,
+       EXYNOS4_ISP_OPTION,
+       EXYNOS4_MAUDIO_OPTION,
+       EXYNOS4_GPS_OPTION,
+       EXYNOS4_GPS_ALIVE_OPTION,
+};
+
+static void exynos4412_pmu_central_seq(bool enable)

You name the argument as "enable" but during initialization and system running you pass here false. It confuses me. What do you enable here?

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 229

Quoted 9 lines
drivers/soc/samsung/exynos4-pmu.c +9 -0
+{
+       unsigned int value;
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);

You duplicate exynos_pm_central_suspend() without removing the original code.

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 235

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;

As manual says - set this register only if you disable C2C. Our entire low power configuration for STOP mode is for C2C enabled case so you add inconsistent configuration.

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 245

Quoted 10 lines
drivers/soc/samsung/exynos4-pmu.c +10 -0
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+}
+
+static void exynos4412_pmu_init(void)
+{
+       unsigned int value;
+       int i;
+
+       /* Enable USE_STANDBY_WFI for all CORE */
+       pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);

This does not look related to improving suspend... it looks unrelated.

Quoted 10 lines
drivers/soc/samsung/exynos4-pmu.c +10 -0
+
+       /* Decides whether to use retention capability */
+       value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
+
+       value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
+

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 265

Quoted 10 lines
drivers/soc/samsung/exynos4-pmu.c +10 -0
+       /* Set PSHOLD port for output high */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_OUTPUT_HIGH;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+
+       /* Enable signal for PSHOLD port */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_EN;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+

The same - not related.

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 271

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+       /* Enable only SC_FEEDBACK */
+       for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
+               value =  pmu_raw_readl(exynos4412_list_feed[i]);
+               value &= ~(EXYNOS_USE_SC_COUNTER);
+               value |= EXYNOS_USE_SC_FEEDBACK;
+               pmu_raw_writel(value, exynos4412_list_feed[i]);

Why do you prefer to use feedback instead of counter?

Quoted 35 lines
drivers/soc/samsung/exynos4-pmu.c +14 -0
+       }
+
+       exynos4412_pmu_central_seq(false);
+
+       pr_info("EXYNOS4x12 PMU Initialize\n");
+}
+
+static void exynos4412_powerdown_conf(enum sys_powerdown mode)
+{
+       exynos4412_pmu_central_seq(true);
+}
+
 const struct exynos_pmu_data exynos4210_pmu_data = {
        .pmu_config     = exynos4210_pmu_config,
 };
 
 const struct exynos_pmu_data exynos4412_pmu_data = {
        .pmu_config             = exynos4412_pmu_config,
+       .pmu_init               = exynos4412_pmu_init,
+       .powerdown_conf         = exynos4412_powerdown_conf,
 };
include/linux/soc/samsung/exynos-regs-pmu.h +2 -0
index 5addaf5ccbce..6beed3e669d2 100644
@@ -16,6 +16,8 @@
 
 #define S5P_CENTRAL_SEQ_CONFIGURATION          0x0200
 
+#define S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK   0x0240
+
 #define S5P_CENTRAL_LOWPWR_CFG                 (1 << 16)
 
 #define S5P_CENTRAL_SEQ_OPTION                 0x0208

include/linux/soc/samsung/exynos-regs-pmu.h line 354

Quoted 6 lines
include/linux/soc/samsung/exynos-regs-pmu.h +3 -0
@@ -347,6 +349,25 @@
 #define EXYNOS3_OPTION_USE_SC_FEEDBACK                 (1 << 1)
 #define EXYNOS3_OPTION_SKIP_DEACTIVATE_ACEACP_IN_PWDN  (1 << 7)
 
+/* EXYNOS4 */
+#define EXYNOS_USE_SC_FEEDBACK                                 BIT(1)
+#define EXYNOS_USE_SC_COUNTER                                  BIT(0)

Instead of adding third set of these defines, I would prefer to cleanup existing and squash all into one (exynos3 and exynos5).

include/linux/soc/samsung/exynos-regs-pmu.h line 359

Quoted 5 lines
include/linux/soc/samsung/exynos-regs-pmu.h +5 -0
+
+#define EXYNOS4_ARM_CORE0_OPTION                               0x2008
+#define EXYNOS4_ARM_CORE1_OPTION                               0x2088
+#define EXYNOS4_ARM_CORE2_OPTION                               0x2108
+#define EXYNOS4_ARM_CORE3_OPTION                               0x2188

The same.

include/linux/soc/samsung/exynos-regs-pmu.h line 369

Quoted 10 lines
include/linux/soc/samsung/exynos-regs-pmu.h +10 -0
+#define EXYNOS4_ARM_COMMON_OPTION                              0x2408
+#define EXYNOS4_CAM_OPTION                                     0x3C08
+#define EXYNOS4_TV_OPTION                                      0x3C28
+#define EXYNOS4_MFC_OPTION                                     0x3C48
+#define EXYNOS4_G3D_OPTION                                     0x3C68
+#define EXYNOS4_LCD0_OPTION                                    0x3C88
+#define EXYNOS4_ISP_OPTION                                     0x3CA8
+#define EXYNOS4_MAUDIO_OPTION                                  0x3CC8
+#define EXYNOS4_GPS_OPTION                                     0x3CE8
+#define EXYNOS4_GPS_ALIVE_OPTION                               0x3D08

Do you need them? I think the are already defined and used by Exynos3.

Best regards,
Krzysztof

Quoted 7 lines
include/linux/soc/samsung/exynos-regs-pmu.h +1 -1
+
 /* For EXYNOS5 */
 
 #define EXYNOS5_AUTO_WDTRESET_DISABLE                          0x0408
--

2.20.1

author
#9 lore raw

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

Hi Krzysztof,

Thanks for your review comments.

Quoted 7 lines · Krzysztof Kozlowski
Quoted 4 lines · Anand Moon

This patch adds configration for PMU (Power Management Unit) state tuning for exynos4412 SoC in order to enter low-power mode during suspend power modes and help resume from suspend state.

Quoted 6 lines

The U3 and Trats2 already enter STOP/S2R so please describe what exactly you change.

Quoted 1 line
Fixes bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")

How it fixes it? What was broken in that commit?

* I was not aware on their is common framework for suspend and resume other than setting this here.I only look in to some the other exynos pmu architecture
and referring 3.10.x kernel to model my changes.*

Quoted 39 lines
Quoted 39 lines
Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previous patch.
New patch to this series to support suspend and resume state

Changes have been tested on microSD card but fails to resume on cMMC. It need to be investigated and more debuging

---
 drivers/soc/samsung/exynos4-pmu.c           | 83 +++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++++++
 2 files changed, 104 insertions(+)
drivers/soc/samsung/exynos4-pmu.c +14 -0
index a7cdbf1aac0c..d261a0d2371e 100644
@@ -200,10 +200,93 @@ static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
        { PMU_TABLE_END,},
 };
 
+static unsigned int const exynos4412_list_feed[] = {
+       EXYNOS4_ARM_CORE0_OPTION,
+       EXYNOS4_ARM_CORE1_OPTION,
+       EXYNOS4_ARM_CORE2_OPTION,
+       EXYNOS4_ARM_CORE3_OPTION,
+       EXYNOS4_ARM_COMMON_OPTION,
+       EXYNOS4_CAM_OPTION,
+       EXYNOS4_TV_OPTION,
+       EXYNOS4_MFC_OPTION,
+       EXYNOS4_G3D_OPTION,
+       EXYNOS4_LCD0_OPTION,
+       EXYNOS4_ISP_OPTION,
+       EXYNOS4_MAUDIO_OPTION,
+       EXYNOS4_GPS_OPTION,
Quoted 9 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 220

Quoted 4 lines
drivers/soc/samsung/exynos4-pmu.c +4 -0
+       EXYNOS4_GPS_ALIVE_OPTION,
+};
+
+static void exynos4412_pmu_central_seq(bool enable)

You name the argument as "enable" but during initialization and system running you pass here false. It confuses me. What do you enable here?

Yep your are correct need to drop this function as already done in common frame work.

Quoted 19 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 229

Quoted 9 lines
drivers/soc/samsung/exynos4-pmu.c +9 -0
+{
+       unsigned int value;
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);

You duplicate exynos_pm_central_suspend() without removing the original code.

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 235

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
Quoted 3 lines

As manual says - set this register only if you disable C2C. Our entire low power configuration for STOP mode is for C2C enabled case so you add inconsistent configuration.

Ok Sorry I overlook this code change.

Enable system power down. Set only CENTRAL_SEQ_CONFIGURATION register if you disable C2C.
Set both CENTRAL_SEQ_CONFIGURATION and CENTRAL_SEQ_CONFIGURATION_COREBLK registers if you enable C2C.

Quoted 4 lines
Quoted 4 lines
drivers/soc/samsung/exynos4-pmu.c +4 -0
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+}
+
+static void exynos4412_pmu_init(void)
Quoted 9 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 245

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+{
+       unsigned int value;
+       int i;
+
+       /* Enable USE_STANDBY_WFI for all CORE */
+       pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);

This does not look related to improving suspend... it looks unrelated.

Ok yes your are correct all ready done in comment frame work.

"Execute WFI/WFE for all CPU cores. As soon as all the CPU cores in Exynos 4412 SCP enter STANDBY mode"

Quoted 14 lines
Quoted 14 lines
drivers/soc/samsung/exynos4-pmu.c +14 -0
+
+       /* Decides whether to use retention capability */
+       value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
+
+       value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
+
+       /* Set PSHOLD port for output high */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_OUTPUT_HIGH;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
Quoted 8 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 265

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       /* Enable signal for PSHOLD port */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_EN;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+

The same - not related.

PS_HOLD_CONTROL need to be set under following set
Hardware reset Reset
Watchdog timer reset Keep its value
Software reset Keep its value
Warm reset Keep its value
Wakeup reset Keep its value

Quoted 10 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 271

Quoted 7 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+       /* Enable only SC_FEEDBACK */
+       for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
+               value =  pmu_raw_readl(exynos4412_list_feed[i]);
+               value &= ~(EXYNOS_USE_SC_COUNTER);
+               value |= EXYNOS_USE_SC_FEEDBACK;
+               pmu_raw_writel(value, exynos4412_list_feed[i]);

Why do you prefer to use feedback instead of counter?

Their is a note below if we use this setting.
NOTE: Either one of USE_SC_FEEDBACK and USE_SC_COUNTER should be activated.

Quoted 38 lines
Quoted 38 lines
drivers/soc/samsung/exynos4-pmu.c +14 -0
+       }
+
+       exynos4412_pmu_central_seq(false);
+
+       pr_info("EXYNOS4x12 PMU Initialize\n");
+}
+
+static void exynos4412_powerdown_conf(enum sys_powerdown mode)
+{
+       exynos4412_pmu_central_seq(true);
+}
+
 const struct exynos_pmu_data exynos4210_pmu_data = {
        .pmu_config     = exynos4210_pmu_config,
 };
 
 const struct exynos_pmu_data exynos4412_pmu_data = {
        .pmu_config             = exynos4412_pmu_config,
+       .pmu_init               = exynos4412_pmu_init,
+       .powerdown_conf         = exynos4412_powerdown_conf,
 };
include/linux/soc/samsung/exynos-regs-pmu.h +2 -0
index 5addaf5ccbce..6beed3e669d2 100644
@@ -16,6 +16,8 @@
 
 #define S5P_CENTRAL_SEQ_CONFIGURATION          0x0200
 
+#define S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK   0x0240
+
 #define S5P_CENTRAL_LOWPWR_CFG                 (1 << 16)
 
 #define S5P_CENTRAL_SEQ_OPTION                 0x0208
@@ -347,6 +349,25 @@
 #define EXYNOS3_OPTION_USE_SC_FEEDBACK                 (1 << 1)
 #define EXYNOS3_OPTION_SKIP_DEACTIVATE_ACEACP_IN_PWDN  (1 << 7)
 
Quoted 7 lines

include/linux/soc/samsung/exynos-regs-pmu.h line 354

Quoted 3 lines
include/linux/soc/samsung/exynos-regs-pmu.h +3 -0
+/* EXYNOS4 */
+#define EXYNOS_USE_SC_FEEDBACK                                 BIT(1)
+#define EXYNOS_USE_SC_COUNTER                                  BIT(0)

Instead of adding third set of these defines, I would prefer to cleanup existing and squash all into one (exynos3 and exynos5).

Ok I will try to do this changes or use the existing one.

Quoted 8 lines

include/linux/soc/samsung/exynos-regs-pmu.h line 359

Quoted 5 lines
include/linux/soc/samsung/exynos-regs-pmu.h +5 -0
+
+#define EXYNOS4_ARM_CORE0_OPTION                               0x2008
+#define EXYNOS4_ARM_CORE1_OPTION                               0x2088
+#define EXYNOS4_ARM_CORE2_OPTION                               0x2108
+#define EXYNOS4_ARM_CORE3_OPTION                               0x2188

The same.

Quoted 12 lines

include/linux/soc/samsung/exynos-regs-pmu.h line 369

Quoted 10 lines
include/linux/soc/samsung/exynos-regs-pmu.h +10 -0
+#define EXYNOS4_ARM_COMMON_OPTION                              0x2408
+#define EXYNOS4_CAM_OPTION                                     0x3C08
+#define EXYNOS4_TV_OPTION                                      0x3C28
+#define EXYNOS4_MFC_OPTION                                     0x3C48
+#define EXYNOS4_G3D_OPTION                                     0x3C68
+#define EXYNOS4_LCD0_OPTION                                    0x3C88
+#define EXYNOS4_ISP_OPTION                                     0x3CA8
+#define EXYNOS4_MAUDIO_OPTION                                  0x3CC8
+#define EXYNOS4_GPS_OPTION                                     0x3CE8
+#define EXYNOS4_GPS_ALIVE_OPTION                               0x3D08

Do you need them? I think the are already defined and used by Exynos3.

Ok I will uses these form Exynos3.

Quoted 11 lines

Best regards,
Krzysztof

Quoted 7 lines
include/linux/soc/samsung/exynos-regs-pmu.h +1 -1
+
 /* For EXYNOS5 */
 
 #define EXYNOS5_AUTO_WDTRESET_DISABLE                          0x0408
--

2.20.1

Best Regards

-Anand

#10 lore raw

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

Quoted 16 lines · Anand Moon

Hi Krzysztof,

Thanks for your review comments.

Quoted 10 lines · Krzysztof Kozlowski
Quoted 4 lines · Anand Moon

This patch adds configration for PMU (Power Management Unit) state tuning for exynos4412 SoC in order to enter low-power mode during suspend power modes and help resume from suspend state.

The U3 and Trats2 already enter STOP/S2R so please describe what exactly you change.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Fixes bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")

How it fixes it? What was broken in that commit?

* I was not aware on their is common framework for suspend and resume other than setting this here.I only look in to some the other exynos pmu architecture
and referring 3.10.x kernel to model my changes.*

Suspend in general should be working already so adding this code just because is not a valid reason. Please specify what is not working first.

Quoted 123 lines
Quoted 39 lines
Quoted 39 lines
Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previous patch.
New patch to this series to support suspend and resume state

Changes have been tested on microSD card but fails to resume on cMMC. It need to be investigated and more debuging

---
 drivers/soc/samsung/exynos4-pmu.c           | 83 +++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++++++
 2 files changed, 104 insertions(+)
drivers/soc/samsung/exynos4-pmu.c +14 -0
index a7cdbf1aac0c..d261a0d2371e 100644
@@ -200,10 +200,93 @@ static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
        { PMU_TABLE_END,},
 };
 
+static unsigned int const exynos4412_list_feed[] = {
+       EXYNOS4_ARM_CORE0_OPTION,
+       EXYNOS4_ARM_CORE1_OPTION,
+       EXYNOS4_ARM_CORE2_OPTION,
+       EXYNOS4_ARM_CORE3_OPTION,
+       EXYNOS4_ARM_COMMON_OPTION,
+       EXYNOS4_CAM_OPTION,
+       EXYNOS4_TV_OPTION,
+       EXYNOS4_MFC_OPTION,
+       EXYNOS4_G3D_OPTION,
+       EXYNOS4_LCD0_OPTION,
+       EXYNOS4_ISP_OPTION,
+       EXYNOS4_MAUDIO_OPTION,
+       EXYNOS4_GPS_OPTION,
Quoted 9 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 220

Quoted 4 lines
drivers/soc/samsung/exynos4-pmu.c +4 -0
+       EXYNOS4_GPS_ALIVE_OPTION,
+};
+
+static void exynos4412_pmu_central_seq(bool enable)

You name the argument as "enable" but during initialization and system running you pass here false. It confuses me. What do you enable here?

Yep your are correct need to drop this function as already done in common frame work.

Quoted 19 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 229

Quoted 9 lines
drivers/soc/samsung/exynos4-pmu.c +9 -0
+{
+       unsigned int value;
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);

You duplicate exynos_pm_central_suspend() without removing the original code.

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 235

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
Quoted 3 lines

As manual says - set this register only if you disable C2C. Our entire low power configuration for STOP mode is for C2C enabled case so you add inconsistent configuration.

Ok Sorry I overlook this code change.

Enable system power down. Set only CENTRAL_SEQ_CONFIGURATION register if you disable C2C.
Set both CENTRAL_SEQ_CONFIGURATION and CENTRAL_SEQ_CONFIGURATION_COREBLK registers if you enable C2C.

Quoted 4 lines
Quoted 4 lines
drivers/soc/samsung/exynos4-pmu.c +4 -0
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+}
+
+static void exynos4412_pmu_init(void)
Quoted 9 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 245

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+{
+       unsigned int value;
+       int i;
+
+       /* Enable USE_STANDBY_WFI for all CORE */
+       pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);

This does not look related to improving suspend... it looks unrelated.

Ok yes your are correct all ready done in comment frame work.

"Execute WFI/WFE for all CPU cores. As soon as all the CPU cores in Exynos 4412 SCP enter STANDBY mode"

Quoted 14 lines
Quoted 14 lines
drivers/soc/samsung/exynos4-pmu.c +14 -0
+
+       /* Decides whether to use retention capability */
+       value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
+
+       value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
+
+       /* Set PSHOLD port for output high */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_OUTPUT_HIGH;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
Quoted 8 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 265

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       /* Enable signal for PSHOLD port */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_EN;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+

The same - not related.

Quoted 6 lines

PS_HOLD_CONTROL need to be set under following set
Hardware reset Reset
Watchdog timer reset Keep its value
Software reset Keep its value
Warm reset Keep its value
Wakeup reset Keep its value

Which is not related to suspend...

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+       /* Enable only SC_FEEDBACK */
+       for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
+               value =  pmu_raw_readl(exynos4412_list_feed[i]);
+               value &= ~(EXYNOS_USE_SC_COUNTER);
+               value |= EXYNOS_USE_SC_FEEDBACK;
+               pmu_raw_writel(value, exynos4412_list_feed[i]);
Quoted 5 lines
Quoted 2 lines

Why do you prefer to use feedback instead of counter?

Their is a note below if we use this setting.
NOTE: Either one of USE_SC_FEEDBACK and USE_SC_COUNTER should be activated.

This is not the answer to my question at all.

Best regards,
Krzysztof

author
#11 lore raw

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

Hi Krzysztof,

Quoted 22 lines · Krzysztof Kozlowski
Quoted 20 lines · Anand Moon

Hi Krzysztof,

Thanks for your review comments.

Quoted 13 lines · Krzysztof Kozlowski
Quoted 4 lines · Anand Moon

This patch adds configration for PMU (Power Management Unit) state tuning for exynos4412 SoC in order to enter low-power mode during suspend power modes and help resume from suspend state.

The U3 and Trats2 already enter STOP/S2R so please describe what exactly you change.

Quoted 1 line
Fixes bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")

How it fixes it? What was broken in that commit?

Quoted 9 lines
Quoted 4 lines

* I was not aware on their is common framework for suspend and resume other than setting this here.I only look in to some the other exynos pmu architecture
and referring 3.10.x kernel to model my changes.*

Suspend in general should be working already so adding this code just because is not a valid reason. Please specify what is not working first.

Please accept *my apology* for not studying the code changes required. Sorry for wasting your precious time in review.

My only intention with this patch was to correctly initialize some PMU parameters.
which might be wrong. so lets drop this.

Quoted 144 lines
Quoted 123 lines
Quoted 39 lines
Quoted 39 lines
Cc Marek Szyprowski <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Chanwoo Choi <[email protected]>

---

Changes from previous patch.
New patch to this series to support suspend and resume state

Changes have been tested on microSD card but fails to resume on cMMC. It need to be investigated and more debuging

---
 drivers/soc/samsung/exynos4-pmu.c           | 83 +++++++++++++++++++++
 include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++++++
 2 files changed, 104 insertions(+)
drivers/soc/samsung/exynos4-pmu.c +14 -0
index a7cdbf1aac0c..d261a0d2371e 100644
@@ -200,10 +200,93 @@ static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
        { PMU_TABLE_END,},
 };
 
+static unsigned int const exynos4412_list_feed[] = {
+       EXYNOS4_ARM_CORE0_OPTION,
+       EXYNOS4_ARM_CORE1_OPTION,
+       EXYNOS4_ARM_CORE2_OPTION,
+       EXYNOS4_ARM_CORE3_OPTION,
+       EXYNOS4_ARM_COMMON_OPTION,
+       EXYNOS4_CAM_OPTION,
+       EXYNOS4_TV_OPTION,
+       EXYNOS4_MFC_OPTION,
+       EXYNOS4_G3D_OPTION,
+       EXYNOS4_LCD0_OPTION,
+       EXYNOS4_ISP_OPTION,
+       EXYNOS4_MAUDIO_OPTION,
+       EXYNOS4_GPS_OPTION,
Quoted 9 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 220

Quoted 4 lines
drivers/soc/samsung/exynos4-pmu.c +4 -0
+       EXYNOS4_GPS_ALIVE_OPTION,
+};
+
+static void exynos4412_pmu_central_seq(bool enable)

You name the argument as "enable" but during initialization and system running you pass here false. It confuses me. What do you enable here?

Yep your are correct need to drop this function as already done in common frame work.

Quoted 19 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 229

Quoted 9 lines
drivers/soc/samsung/exynos4-pmu.c +9 -0
+{
+       unsigned int value;
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);

You duplicate exynos_pm_central_suspend() without removing the original code.

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 235

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+       if (enable)
+               value &= ~S5P_CENTRAL_LOWPWR_CFG;
+       else
+               value |= S5P_CENTRAL_LOWPWR_CFG;
Quoted 3 lines

As manual says - set this register only if you disable C2C. Our entire low power configuration for STOP mode is for C2C enabled case so you add inconsistent configuration.

Ok Sorry I overlook this code change.

Enable system power down. Set only CENTRAL_SEQ_CONFIGURATION register if you disable C2C.
Set both CENTRAL_SEQ_CONFIGURATION and CENTRAL_SEQ_CONFIGURATION_COREBLK registers if you enable C2C.

Quoted 4 lines
Quoted 4 lines
drivers/soc/samsung/exynos4-pmu.c +4 -0
+       pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+}
+
+static void exynos4412_pmu_init(void)
Quoted 9 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 245

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+{
+       unsigned int value;
+       int i;
+
+       /* Enable USE_STANDBY_WFI for all CORE */
+       pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);

This does not look related to improving suspend... it looks unrelated.

Ok yes your are correct all ready done in comment frame work.

"Execute WFI/WFE for all CPU cores. As soon as all the CPU cores in Exynos 4412 SCP enter STANDBY mode"

Quoted 14 lines
Quoted 14 lines
drivers/soc/samsung/exynos4-pmu.c +14 -0
+
+       /* Decides whether to use retention capability */
+       value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
+
+       value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
+       value &= ~EXYNOS_L2_USE_RETENTION;
+       pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
+
+       /* Set PSHOLD port for output high */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_OUTPUT_HIGH;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
Quoted 8 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 265

Quoted 6 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+
+       /* Enable signal for PSHOLD port */
+       value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+       value |= S5P_PS_HOLD_EN;
+       pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+

The same - not related.

Quoted 6 lines

PS_HOLD_CONTROL need to be set under following set
Hardware reset Reset
Watchdog timer reset Keep its value
Software reset Keep its value
Warm reset Keep its value
Wakeup reset Keep its value

Which is not related to suspend...

Quoted 12 lines
Quoted 11 lines

drivers/soc/samsung/exynos4-pmu.c static const struct exynos_pmu_conf exynos4412_pmu_config[] = { · line 271

Quoted 8 lines
drivers/soc/samsung/exynos4-pmu.c +6 -0
+       /* Enable only SC_FEEDBACK */
+       for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
+               value =  pmu_raw_readl(exynos4412_list_feed[i]);
+               value &= ~(EXYNOS_USE_SC_COUNTER);
+               value |= EXYNOS_USE_SC_FEEDBACK;
+               pmu_raw_writel(value, exynos4412_list_feed[i]);

Why do you prefer to use feedback instead of counter?

Quoted 7 lines
Quoted 2 lines

Their is a note below if we use this setting.
NOTE: Either one of USE_SC_FEEDBACK and USE_SC_COUNTER should be activated.

This is not the answer to my question at all.

Best regards,
Krzysztof

Once again sorry for wasting your time.
I know I have the improve a lot of things.

Best Regards

-Anand