lkml.org 
[lkml]   [2019]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC 1/2] ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3
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

\
 
 \ /
  Last update: 2019-02-14 19:36    [W:0.109 / U:3.916 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site