| Message ID | 20190213214052.2427-2-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Suspend and Resume for Exynos4412 on | expand |
Hi Anand, Thanks for the patch. See comments below. On Wed, 13 Feb 2019 at 22:41, Anand Moon <linux.amoon@gmail.com> wrote: > > 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 <m.szyprowski@samsung.com> > Cc: Krzysztof Kozlowski <krzk@kernel.org> > Cc: Chanwoo Choi <cw00.choi@samsung.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > > 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(+) > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > index 08d3a0a7b4eb..e984461c37d9 100644 > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > @@ -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. > }; > > 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... > }; > > 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; > + }; The same... any comments? > }; > > 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; Again, you did not address my comments. > + }; > }; > > 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; > + }; The same... > }; > > 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 { > diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts > index 2bdf899df436..4ebde09fc51d 100644 > --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts > +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts > @@ -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
hi Krzysztof, Thanks fro your review comments. On Thu, 14 Feb 2019 at 18:11, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > Hi Anand, > > Thanks for the patch. See comments below. > > On Wed, 13 Feb 2019 at 22:41, Anand Moon <linux.amoon@gmail.com> wrote: > > > > 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 <m.szyprowski@samsung.com> > > Cc: Krzysztof Kozlowski <krzk@kernel.org> > > Cc: Chanwoo Choi <cw00.choi@samsung.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > > > 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(+) > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > index 08d3a0a7b4eb..e984461c37d9 100644 > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > @@ -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, 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. > > > }; > > > > 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, > > > }; > > > > 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; > > + }; > > > The same... any comments? I left this on for mshc_0 (emmc) regulator, since I could not get suspend resume working on eMMc > > > }; > > > > 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; > > 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. > > + }; > > }; > > > > 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; > > + }; > > The same... Buck3 support entering into LPM setting, But I will set this to regulator-on-in-suspend; in next patch. > > > }; > > > > 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 { > > diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > index 2bdf899df436..4ebde09fc51d 100644 > > --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts > > +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > @@ -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. > Best regards, > Krzysztof Best Regards -Anand
On Thu, 14 Feb 2019 at 19:35, Anand Moon <linux.amoon@gmail.com> wrote: > > hi Krzysztof, > > Thanks fro your review comments. > > On Thu, 14 Feb 2019 at 18:11, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > Hi Anand, > > > > Thanks for the patch. See comments below. > > > > On Wed, 13 Feb 2019 at 22:41, Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > 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 <m.szyprowski@samsung.com> > > > Cc: Krzysztof Kozlowski <krzk@kernel.org> > > > Cc: Chanwoo Choi <cw00.choi@samsung.com> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > > > > 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(+) > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > index 08d3a0a7b4eb..e984461c37d9 100644 > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > @@ -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, 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. > > 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. > > > > > > }; > > > > > > 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, The same - why changing this to on or off? I need explanation why this should be in first place. > > > > > }; > > > > > > 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; > > > + }; > > > > > > The same... any comments? > > 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. > > > > > }; > > > > > > 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; > > > > 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. Why? > > > + }; > > > }; > > > > > > 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; > > > + }; > > > > The same... > > 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? > > > > > }; > > > > > > 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 { > > > diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > index 2bdf899df436..4ebde09fc51d 100644 > > > --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > @@ -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. 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
Hi Krzysztof, On Fri, 15 Feb 2019 at 13:01, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Thu, 14 Feb 2019 at 19:35, Anand Moon <linux.amoon@gmail.com> wrote: > > > > hi Krzysztof, > > > > Thanks fro your review comments. > > > > On Thu, 14 Feb 2019 at 18:11, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > Hi Anand, > > > > > > Thanks for the patch. See comments below. > > > > > > On Wed, 13 Feb 2019 at 22:41, Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > 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 <m.szyprowski@samsung.com> > > > > Cc: Krzysztof Kozlowski <krzk@kernel.org> > > > > Cc: Chanwoo Choi <cw00.choi@samsung.com> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > > > > > 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(+) > > > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > index 08d3a0a7b4eb..e984461c37d9 100644 > > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > @@ -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, > > 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. > 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. > > > > 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. > > > > > > > > > }; > > > > > > > > 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, > > 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. > > > > > > > > }; > > > > > > > > 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; > > > > + }; > > > > > > > > > The same... any comments? > > > > 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. > > > > > > > > }; > > > > > > > > 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; > > > > > > 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. > > Why? I will check in the driver code. > > > > > + }; > > > > }; > > > > > > > > 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; > > > > + }; > > > > > > The same... > > > > 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? > > > > > > > > }; > > > > > > > > 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 { > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > > index 2bdf899df436..4ebde09fc51d 100644 > > > > --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > > +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > > @@ -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. > > 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. > Best regards, > Krzysztof *Sorry for wasting your time* I know I have the improve a lot of things. Best regards -Anand
Hi Krzysztof, On Fri, 15 Feb 2019 at 13:01, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Thu, 14 Feb 2019 at 19:35, Anand Moon <linux.amoon@gmail.com> wrote: > > > > hi Krzysztof, > > > > Thanks fro your review comments. > > > > On Thu, 14 Feb 2019 at 18:11, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > Hi Anand, > > > > > > Thanks for the patch. See comments below. > > > > > > On Wed, 13 Feb 2019 at 22:41, Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > 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 <m.szyprowski@samsung.com> > > > > Cc: Krzysztof Kozlowski <krzk@kernel.org> > > > > Cc: Chanwoo Choi <cw00.choi@samsung.com> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > > > > > 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(+) > > > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > index 08d3a0a7b4eb..e984461c37d9 100644 > > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > @@ -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, > > 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. > ------------------------------------------------------------------------------------------------ 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] > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > > index 2bdf899df436..4ebde09fc51d 100644 > > > > --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > > +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts > > > > @@ -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. > > 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
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi index 08d3a0a7b4eb..e984461c37d9 100644 --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi @@ -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 { diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts index 2bdf899df436..4ebde09fc51d 100644 --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts @@ -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 {
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 <m.szyprowski@samsung.com> Cc: Krzysztof Kozlowski <krzk@kernel.org> Cc: Chanwoo Choi <cw00.choi@samsung.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- 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(+)