| Message ID | 20181204194025.2719-1-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ARM: dts: exynos: Add proper regulator states for suspend-to-mem for odroid-u3 | expand |
On Tue, 4 Dec 2018 at 20:40, 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. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > Tested on Odroid U3+ > --- > .../boot/dts/exynos4412-odroid-common.dtsi | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > index 2caa3132f34e..837713a2ec3b 100644 > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > @@ -285,6 +285,9 @@ > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > regulator-always-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; Driver does not support suspend_enable so this will be noop. We could add this for DT-correctness (and full description of HW) but then I need explanation why this regulator has to stay on during suspend. > }; > > ldo3_reg: LDO3 { > @@ -292,6 +295,9 @@ > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > regulator-always-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; The same but with suspend_disable. I am not saying that these are wrong but I would be happy to see explanations why you chosen these particular properties. > }; > > ldo4_reg: LDO4 { > @@ -299,6 +305,9 @@ > regulator-min-microvolt = <2800000>; > regulator-max-microvolt = <2800000>; > regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > }; > > ldo5_reg: LDO5 { > @@ -307,6 +316,9 @@ > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > }; > > ldo6_reg: LDO6 { > @@ -314,6 +326,9 @@ > regulator-min-microvolt = <1000000>; > regulator-max-microvolt = <1000000>; > regulator-always-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > }; > > ldo7_reg: LDO7 { > @@ -321,18 +336,27 @@ > regulator-min-microvolt = <1000000>; > regulator-max-microvolt = <1000000>; > regulator-always-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > }; > > ldo8_reg: LDO8 { > regulator-name = "VDD10_HDMI_1.0V"; > regulator-min-microvolt = <1000000>; > regulator-max-microvolt = <1000000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo10_reg: LDO10 { > regulator-name = "VDDQ_MIPIHSI_1.8V"; > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo11_reg: LDO11 { > @@ -340,6 +364,9 @@ > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > regulator-always-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo12_reg: LDO12 { > @@ -348,6 +375,9 @@ > regulator-max-microvolt = <3300000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo13_reg: LDO13 { > @@ -356,6 +386,9 @@ > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo14_reg: LDO14 { > @@ -364,6 +397,9 @@ > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo15_reg: LDO15 { > @@ -372,6 +408,9 @@ > regulator-max-microvolt = <1000000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo16_reg: LDO16 { > @@ -380,6 +419,9 @@ > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > }; > > ldo20_reg: LDO20 { > @@ -387,6 +429,9 @@ > regulator-min-microvolt = <1800000>; > regulator-max-microvolt = <1800000>; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > ldo21_reg: LDO21 { > @@ -394,6 +439,9 @@ > regulator-min-microvolt = <2800000>; > regulator-max-microvolt = <2800000>; > regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; That's unusual setting... enabled in suspend but not necessarily during work (no always-on). > + }; > }; > > ldo22_reg: LDO22 { > @@ -411,6 +459,9 @@ > regulator-max-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > buck1_reg: BUCK1 { > @@ -419,6 +470,9 @@ > regulator-max-microvolt = <1100000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; This is seriously wrong so I have doubts whether you tested the changes or you know what is happening with them. If you turn off memory bus regulator off, how the memory will work in Suspend-to-Memory mode? > }; > > buck2_reg: BUCK2 { > @@ -427,6 +481,9 @@ > regulator-max-microvolt = <1350000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-on-in-suspend; > + }; > }; > > buck3_reg: BUCK3 { > @@ -435,6 +492,9 @@ > regulator-max-microvolt = <1050000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; Looks suspicious as well. Best regards, Krzysztof > + }; > }; > > buck4_reg: BUCK4 { > @@ -442,6 +502,9 @@ > regulator-min-microvolt = <900000>; > regulator-max-microvolt = <1100000>; > regulator-microvolt-offset = <50000>; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > buck5_reg: BUCK5 { > @@ -450,6 +513,9 @@ > regulator-max-microvolt = <1200000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > buck6_reg: BUCK6 { > @@ -458,6 +524,9 @@ > regulator-max-microvolt = <1350000>; > regulator-always-on; > regulator-boot-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > buck7_reg: BUCK7 { > @@ -465,6 +534,9 @@ > regulator-min-microvolt = <2000000>; > regulator-max-microvolt = <2000000>; > regulator-always-on; > + regulator-state-mem { > + regulator-off-in-suspend; > + }; > }; > > buck8_reg: BUCK8 { > -- > 2.19.2 >
Hi Krzysztof, Thanks for your review. . On Wed, 5 Dec 2018 at 19:36, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Tue, 4 Dec 2018 at 20:40, 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. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > Tested on Odroid U3+ > > --- > > .../boot/dts/exynos4412-odroid-common.dtsi | 72 +++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > index 2caa3132f34e..837713a2ec3b 100644 > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > @@ -285,6 +285,9 @@ > > regulator-min-microvolt = <1800000>; > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > Driver does not support suspend_enable so this will be noop. We could > add this for DT-correctness (and full description of HW) but then I > need explanation why this regulator has to stay on during suspend. > Well I tried to study the suspend/resume feature from *arch/arm/boot/dts/exynos4412-midas.dtsi* and them I tried to apply the same with this on Odroid-U3 boards and test these changes. > > }; > > > > ldo3_reg: LDO3 { > > @@ -292,6 +295,9 @@ > > regulator-min-microvolt = <1800000>; > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > The same but with suspend_disable. > > I am not saying that these are wrong but I would be happy to see > explanations why you chosen these particular properties. > Well I was not aware on the regulator-always-on cannot be set to off in suspend mode. Will drop this in the future patch where regulator-always-on; is set. > > }; > > > > ldo4_reg: LDO4 { > > @@ -299,6 +305,9 @@ > > regulator-min-microvolt = <2800000>; > > regulator-max-microvolt = <2800000>; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > }; > > > > ldo5_reg: LDO5 { > > @@ -307,6 +316,9 @@ > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > }; > > > > ldo6_reg: LDO6 { > > @@ -314,6 +326,9 @@ > > regulator-min-microvolt = <1000000>; > > regulator-max-microvolt = <1000000>; > > regulator-always-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > }; > > > > ldo7_reg: LDO7 { > > @@ -321,18 +336,27 @@ > > regulator-min-microvolt = <1000000>; > > regulator-max-microvolt = <1000000>; > > regulator-always-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > }; > > > > ldo8_reg: LDO8 { > > regulator-name = "VDD10_HDMI_1.0V"; > > regulator-min-microvolt = <1000000>; > > regulator-max-microvolt = <1000000>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo10_reg: LDO10 { > > regulator-name = "VDDQ_MIPIHSI_1.8V"; > > regulator-min-microvolt = <1800000>; > > regulator-max-microvolt = <1800000>; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo11_reg: LDO11 { > > @@ -340,6 +364,9 @@ > > regulator-min-microvolt = <1800000>; > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo12_reg: LDO12 { > > @@ -348,6 +375,9 @@ > > regulator-max-microvolt = <3300000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo13_reg: LDO13 { > > @@ -356,6 +386,9 @@ > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo14_reg: LDO14 { > > @@ -364,6 +397,9 @@ > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo15_reg: LDO15 { > > @@ -372,6 +408,9 @@ > > regulator-max-microvolt = <1000000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo16_reg: LDO16 { > > @@ -380,6 +419,9 @@ > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > }; > > > > ldo20_reg: LDO20 { > > @@ -387,6 +429,9 @@ > > regulator-min-microvolt = <1800000>; > > regulator-max-microvolt = <1800000>; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > ldo21_reg: LDO21 { > > @@ -394,6 +439,9 @@ > > regulator-min-microvolt = <2800000>; > > regulator-max-microvolt = <2800000>; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > That's unusual setting... enabled in suspend but not necessarily > during work (no always-on). > I kept the regulator required for emmc/sd card in regulator-on-in-suspend in suspend mode. > > + }; > > }; > > > > ldo22_reg: LDO22 { > > @@ -411,6 +459,9 @@ > > regulator-max-microvolt = <1800000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > }; > > > > buck1_reg: BUCK1 { > > @@ -419,6 +470,9 @@ > > regulator-max-microvolt = <1100000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > + }; > > This is seriously wrong so I have doubts whether you tested the > changes or you know what is happening with them. If you turn off > memory bus regulator off, how the memory will work in > Suspend-to-Memory mode? > Well I did not find any issue in my testing but I might be wrong. Ok I will drop this changes in the next version of the patch where regulator-always-on is set. > > }; > > > > buck2_reg: BUCK2 { > > @@ -427,6 +481,9 @@ > > regulator-max-microvolt = <1350000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-on-in-suspend; > > + }; > > }; > > > > buck3_reg: BUCK3 { > > @@ -435,6 +492,9 @@ > > regulator-max-microvolt = <1050000>; > > regulator-always-on; > > regulator-boot-on; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > Looks suspicious as well. Ok I will drop this changes in the next version of the patch where regulator-always-on is set. > > Best regards, > Krzysztof > Well I have tested this patch as following with only one issue, before enable suspend number of On-line cpu is 4 after resume number of On-line cpu is 1. # lscpu Architecture: armv7l Byte Order: Little Endian CPU(s): 4 On-line CPU(s) list: 0-3 Thread(s) per core: 1 Core(s) per socket: 4 Socket(s): 1 Vendor ID: ARM Model: 0 Model name: Cortex-A9 Stepping: r3p0 CPU max MHz: 1704.0000 CPU min MHz: 200.0000 BogoMIPS: 48.00 Flags: half thumb fastmult vfp edsp neon vfpv3 tls vfpd32 # # echo 1 > /sys/power/pm_debug_messages # echo +30 > /sys/class/rtc/rtc0/wakealarm # echo -n mem > /sys/power/state [ 86.584147] PM: suspend entry (deep) [ 86.584684] PM: Syncing filesystems ... done. [ 86.735819] Freezing user space processes ... (elapsed 0.003 seconds) done. [ 86.742319] OOM killer disabled. [ 86.744018] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 86.751479] printk: Suspending console(s) (use no_console_suspend to debug) [ 86.792770] smsc95xx 1-2:1.0 eth0: entering SUSPEND2 mode [ 86.821751] dwc2 12480000.hsotg: suspending usb gadget g_ether [ 86.822560] dwc2 12480000.hsotg: new device is full-speed [ 86.822666] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0 [ 86.822682] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0 [ 86.824507] wake enabled for irq 119 [ 86.824592] BUCK9: No configuration [ 86.824672] BUCK8_P3V3: No configuration [ 86.824706] BUCK7_2.0V: No configuration [ 86.824738] BUCK6_1.35V: No configuration [ 86.824770] VDDQ_CKEM1_2_1.2V: No configuration [ 86.826766] LDO26: No configuration [ 86.826802] VDDQ_LCD_1.8V: No configuration [ 86.826834] LDO24: No configuration [ 86.826866] LDO23: No configuration [ 86.826898] LDO22_VDDQ_MMC4_2.8V: No configuration [ 86.826930] TFLASH_2.8V: No configuration [ 86.826962] LDO20_1.8V: No configuration [ 86.826994] LDO19: No configuration [ 86.827026] LDO18: No configuration [ 86.827057] LDO17: No configuration [ 86.827516] VDDQ_C2C_W_1.8V: No configuration [ 86.828324] VDDQ_MIPIHSI_1.8V: No configuration [ 86.828359] LDO9: No configuration [ 86.828818] VDDQ_MMC1_3_1.8V: No configuration [ 86.828851] VDDQ_MMC2_2.8V: No configuration [ 86.828884] VDDQ_EXT_1.8V: No configuration [ 86.828935] VDD_ALIVE_1.0V: No configuration [ 86.839760] usb3503 0-0008: switched to STANDBY mode [ 86.840336] wake enabled for irq 123 [ 86.855834] samsung-pinctrl 11000000.pinctrl: Setting external wakeup interrupt mask: 0xfbfff7ff [ 86.871661] Disabling non-boot CPUs ... [ 87.016430] Enabling non-boot CPUs ... [ 88.009557] CPU1: failed to boot: -110 [ 88.010324] Error taking CPU1 up: -110 [ 89.009841] CPU2: failed to boot: -110 [ 89.011290] Error taking CPU2 up: -110 [ 90.009615] CPU3: failed to boot: -110 [ 90.010258] Error taking CPU3 up: -110 [ 90.012152] s3c-i2c 13860000.i2c: slave address 0x00 [ 90.012174] s3c-i2c 13860000.i2c: bus frequency set to 390 KHz [ 90.012208] s3c-i2c 13870000.i2c: slave address 0x00 [ 90.012225] s3c-i2c 13870000.i2c: bus frequency set to 97 KHz [ 90.012257] s3c-i2c 13880000.i2c: slave address 0x00 [ 90.012274] s3c-i2c 13880000.i2c: bus frequency set to 97 KHz [ 90.012306] s3c-i2c 138e0000.i2c: slave address 0x00 [ 90.012323] s3c-i2c 138e0000.i2c: bus frequency set to 97 KHz [ 90.028694] s3c-rtc 10070000.rtc: rtc disabled, re-enabling [ 90.029174] usb usb1: root hub lost power or was reset [ 90.032911] s3c2410-wdt 10060000.watchdog: watchdog disabled [ 90.033064] wake disabled for irq 123 [ 90.041886] usb3503 0-0008: switched to HUB mode [ 90.127583] wake disabled for irq 119 [ 90.127865] dwc2 12480000.hsotg: resuming usb gadget g_ether [ 90.429505] usb 1-2: reset high-speed USB device number 2 using exynos-ehci [ 90.781458] usb 1-3: reset high-speed USB device number 3 using exynos-ehci [ 91.366725] OOM killer enabled. [ 91.369869] Restarting tasks ... done. [ 91.419515] PM: suspend exit # [ 92.229649] smsc95xx 1-2:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1 # lscpu Architecture: armv7l Byte Order: Little Endian CPU(s): 4 On-line CPU(s) list: 0 Off-line CPU(s) list: 1-3 Thread(s) per core: 1 Core(s) per socket: 1 Socket(s): 1 Vendor ID: ARM Model: 0 Model name: Cortex-A9 Stepping: r3p0 CPU max MHz: 1704.0000 CPU min MHz: 200.0000 BogoMIPS: 24.00 Flags: half thumb fastmult vfp edsp neon vfpv3 tls vfpd32 Best Regards -Anand
On Wed, 5 Dec 2018 at 17:11, Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Krzysztof, > > Thanks for your review. > . > On Wed, 5 Dec 2018 at 19:36, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > On Tue, 4 Dec 2018 at 20:40, 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. > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > Tested on Odroid U3+ > > > --- > > > .../boot/dts/exynos4412-odroid-common.dtsi | 72 +++++++++++++++++++ > > > 1 file changed, 72 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > index 2caa3132f34e..837713a2ec3b 100644 > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > @@ -285,6 +285,9 @@ > > > regulator-min-microvolt = <1800000>; > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > > Driver does not support suspend_enable so this will be noop. We could > > add this for DT-correctness (and full description of HW) but then I > > need explanation why this regulator has to stay on during suspend. > > > > Well I tried to study the suspend/resume feature from > *arch/arm/boot/dts/exynos4412-midas.dtsi* > and them I tried to apply the same with this on Odroid-U3 boards and > test these changes. Midas DTSI clearly has bugs then. > > > > }; > > > > > > ldo3_reg: LDO3 { > > > @@ -292,6 +295,9 @@ > > > regulator-min-microvolt = <1800000>; > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > > The same but with suspend_disable. > > > > I am not saying that these are wrong but I would be happy to see > > explanations why you chosen these particular properties. > > > > Well I was not aware on the regulator-always-on cannot be set to off > in suspend mode. > Will drop this in the future patch where regulator-always-on; is set. > > > > }; > > > > > > ldo4_reg: LDO4 { > > > @@ -299,6 +305,9 @@ > > > regulator-min-microvolt = <2800000>; > > > regulator-max-microvolt = <2800000>; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > }; > > > > > > ldo5_reg: LDO5 { > > > @@ -307,6 +316,9 @@ > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > }; > > > > > > ldo6_reg: LDO6 { > > > @@ -314,6 +326,9 @@ > > > regulator-min-microvolt = <1000000>; > > > regulator-max-microvolt = <1000000>; > > > regulator-always-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > }; > > > > > > ldo7_reg: LDO7 { > > > @@ -321,18 +336,27 @@ > > > regulator-min-microvolt = <1000000>; > > > regulator-max-microvolt = <1000000>; > > > regulator-always-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > }; > > > > > > ldo8_reg: LDO8 { > > > regulator-name = "VDD10_HDMI_1.0V"; > > > regulator-min-microvolt = <1000000>; > > > regulator-max-microvolt = <1000000>; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo10_reg: LDO10 { > > > regulator-name = "VDDQ_MIPIHSI_1.8V"; > > > regulator-min-microvolt = <1800000>; > > > regulator-max-microvolt = <1800000>; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo11_reg: LDO11 { > > > @@ -340,6 +364,9 @@ > > > regulator-min-microvolt = <1800000>; > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo12_reg: LDO12 { > > > @@ -348,6 +375,9 @@ > > > regulator-max-microvolt = <3300000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo13_reg: LDO13 { > > > @@ -356,6 +386,9 @@ > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo14_reg: LDO14 { > > > @@ -364,6 +397,9 @@ > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo15_reg: LDO15 { > > > @@ -372,6 +408,9 @@ > > > regulator-max-microvolt = <1000000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo16_reg: LDO16 { > > > @@ -380,6 +419,9 @@ > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > }; > > > > > > ldo20_reg: LDO20 { > > > @@ -387,6 +429,9 @@ > > > regulator-min-microvolt = <1800000>; > > > regulator-max-microvolt = <1800000>; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > ldo21_reg: LDO21 { > > > @@ -394,6 +439,9 @@ > > > regulator-min-microvolt = <2800000>; > > > regulator-max-microvolt = <2800000>; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > > That's unusual setting... enabled in suspend but not necessarily > > during work (no always-on). > > > > I kept the regulator required for emmc/sd card in > regulator-on-in-suspend in suspend mode. > > > > + }; > > > }; > > > > > > ldo22_reg: LDO22 { > > > @@ -411,6 +459,9 @@ > > > regulator-max-microvolt = <1800000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > }; > > > > > > buck1_reg: BUCK1 { > > > @@ -419,6 +470,9 @@ > > > regulator-max-microvolt = <1100000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > + }; > > > > This is seriously wrong so I have doubts whether you tested the > > changes or you know what is happening with them. If you turn off > > memory bus regulator off, how the memory will work in > > Suspend-to-Memory mode? > > > > Well I did not find any issue in my testing but I might be wrong. > Ok I will drop this changes in the next version of the patch where > regulator-always-on is set. It worked fine because regulator-off-in-suspend is noop for buck1 but it is clearly wrong from logical point of view. Do you think that memory should be off in Suspend to RAM? > > > }; > > > > > > buck2_reg: BUCK2 { > > > @@ -427,6 +481,9 @@ > > > regulator-max-microvolt = <1350000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-on-in-suspend; > > > + }; > > > }; > > > > > > buck3_reg: BUCK3 { > > > @@ -435,6 +492,9 @@ > > > regulator-max-microvolt = <1050000>; > > > regulator-always-on; > > > regulator-boot-on; > > > + regulator-state-mem { > > > + regulator-off-in-suspend; > > > > Looks suspicious as well. > > Ok I will drop this changes in the next version of the patch where > regulator-always-on is set. > > > > > Best regards, > > Krzysztof > > > > Well I have tested this patch as following > with only one issue, before enable suspend number of On-line cpu is 4 > after resume number of On-line cpu is 1. If before your change number of resumed CPUs was 4, then you apparently break some things. Best regards, Krzysztof
Hi Krzysztof, On Wed, 5 Dec 2018 at 21:49, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Wed, 5 Dec 2018 at 17:11, Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Krzysztof, > > > > Thanks for your review. > > . > > On Wed, 5 Dec 2018 at 19:36, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > On Tue, 4 Dec 2018 at 20:40, 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. > > > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > Tested on Odroid U3+ > > > > --- > > > > .../boot/dts/exynos4412-odroid-common.dtsi | 72 +++++++++++++++++++ > > > > 1 file changed, 72 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > index 2caa3132f34e..837713a2ec3b 100644 > > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > @@ -285,6 +285,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > > > Driver does not support suspend_enable so this will be noop. We could > > > add this for DT-correctness (and full description of HW) but then I > > > need explanation why this regulator has to stay on during suspend. > > > > > > > Well I tried to study the suspend/resume feature from > > *arch/arm/boot/dts/exynos4412-midas.dtsi* > > and them I tried to apply the same with this on Odroid-U3 boards and > > test these changes. > > Midas DTSI clearly has bugs then. > > > > > > > }; > > > > > > > > ldo3_reg: LDO3 { > > > > @@ -292,6 +295,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > > > The same but with suspend_disable. > > > > > > I am not saying that these are wrong but I would be happy to see > > > explanations why you chosen these particular properties. > > > > > > > Well I was not aware on the regulator-always-on cannot be set to off > > in suspend mode. > > Will drop this in the future patch where regulator-always-on; is set. > > > > > > }; > > > > > > > > ldo4_reg: LDO4 { > > > > @@ -299,6 +305,9 @@ > > > > regulator-min-microvolt = <2800000>; > > > > regulator-max-microvolt = <2800000>; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo5_reg: LDO5 { > > > > @@ -307,6 +316,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo6_reg: LDO6 { > > > > @@ -314,6 +326,9 @@ > > > > regulator-min-microvolt = <1000000>; > > > > regulator-max-microvolt = <1000000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo7_reg: LDO7 { > > > > @@ -321,18 +336,27 @@ > > > > regulator-min-microvolt = <1000000>; > > > > regulator-max-microvolt = <1000000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo8_reg: LDO8 { > > > > regulator-name = "VDD10_HDMI_1.0V"; > > > > regulator-min-microvolt = <1000000>; > > > > regulator-max-microvolt = <1000000>; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo10_reg: LDO10 { > > > > regulator-name = "VDDQ_MIPIHSI_1.8V"; > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo11_reg: LDO11 { > > > > @@ -340,6 +364,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo12_reg: LDO12 { > > > > @@ -348,6 +375,9 @@ > > > > regulator-max-microvolt = <3300000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo13_reg: LDO13 { > > > > @@ -356,6 +386,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo14_reg: LDO14 { > > > > @@ -364,6 +397,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo15_reg: LDO15 { > > > > @@ -372,6 +408,9 @@ > > > > regulator-max-microvolt = <1000000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo16_reg: LDO16 { > > > > @@ -380,6 +419,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo20_reg: LDO20 { > > > > @@ -387,6 +429,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo21_reg: LDO21 { > > > > @@ -394,6 +439,9 @@ > > > > regulator-min-microvolt = <2800000>; > > > > regulator-max-microvolt = <2800000>; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > > > That's unusual setting... enabled in suspend but not necessarily > > > during work (no always-on). > > > > > > > I kept the regulator required for emmc/sd card in > > regulator-on-in-suspend in suspend mode. > > > > > > + }; > > > > }; > > > > > > > > ldo22_reg: LDO22 { > > > > @@ -411,6 +459,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > buck1_reg: BUCK1 { > > > > @@ -419,6 +470,9 @@ > > > > regulator-max-microvolt = <1100000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > > > This is seriously wrong so I have doubts whether you tested the > > > changes or you know what is happening with them. If you turn off > > > memory bus regulator off, how the memory will work in > > > Suspend-to-Memory mode? > > > > > > > Well I did not find any issue in my testing but I might be wrong. > > Ok I will drop this changes in the next version of the patch where > > regulator-always-on is set. > > It worked fine because regulator-off-in-suspend is noop for buck1 but > it is clearly wrong from logical point of view. Do you think that > memory should be off in Suspend to RAM? > My bad setting this for buck setting I will drop this in the future patch. > > > > }; > > > > > > > > buck2_reg: BUCK2 { > > > > @@ -427,6 +481,9 @@ > > > > regulator-max-microvolt = <1350000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > buck3_reg: BUCK3 { > > > > @@ -435,6 +492,9 @@ > > > > regulator-max-microvolt = <1050000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > > > Looks suspicious as well. > > > > Ok I will drop this changes in the next version of the patch where > > regulator-always-on is set. > > > > > > > > Best regards, > > > Krzysztof > > > > > > > Well I have tested this patch as following > > with only one issue, before enable suspend number of On-line cpu is 4 > > after resume number of On-line cpu is 1. > > If before your change number of resumed CPUs was 4, then you > apparently break some things. > > Best regards, > Krzysztof Ok I will double check that I do not break any feature. Best Regards -Anand
Hi Krzysztof, On Wed, 5 Dec 2018 at 21:49, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Wed, 5 Dec 2018 at 17:11, Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Krzysztof, > > > > Thanks for your review. > > . > > On Wed, 5 Dec 2018 at 19:36, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > On Tue, 4 Dec 2018 at 20:40, 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. > > > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > Tested on Odroid U3+ > > > > --- > > > > .../boot/dts/exynos4412-odroid-common.dtsi | 72 +++++++++++++++++++ > > > > 1 file changed, 72 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > index 2caa3132f34e..837713a2ec3b 100644 > > > > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > > > > @@ -285,6 +285,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > > > Driver does not support suspend_enable so this will be noop. We could > > > add this for DT-correctness (and full description of HW) but then I > > > need explanation why this regulator has to stay on during suspend. > > > > > > > Well I tried to study the suspend/resume feature from > > *arch/arm/boot/dts/exynos4412-midas.dtsi* > > and them I tried to apply the same with this on Odroid-U3 boards and > > test these changes. > > Midas DTSI clearly has bugs then. > > > > > > > }; > > > > > > > > ldo3_reg: LDO3 { > > > > @@ -292,6 +295,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > > > The same but with suspend_disable. > > > > > > I am not saying that these are wrong but I would be happy to see > > > explanations why you chosen these particular properties. > > > > > > > Well I was not aware on the regulator-always-on cannot be set to off > > in suspend mode. > > Will drop this in the future patch where regulator-always-on; is set. > > > > > > }; > > > > > > > > ldo4_reg: LDO4 { > > > > @@ -299,6 +305,9 @@ > > > > regulator-min-microvolt = <2800000>; > > > > regulator-max-microvolt = <2800000>; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo5_reg: LDO5 { > > > > @@ -307,6 +316,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo6_reg: LDO6 { > > > > @@ -314,6 +326,9 @@ > > > > regulator-min-microvolt = <1000000>; > > > > regulator-max-microvolt = <1000000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo7_reg: LDO7 { > > > > @@ -321,18 +336,27 @@ > > > > regulator-min-microvolt = <1000000>; > > > > regulator-max-microvolt = <1000000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo8_reg: LDO8 { > > > > regulator-name = "VDD10_HDMI_1.0V"; > > > > regulator-min-microvolt = <1000000>; > > > > regulator-max-microvolt = <1000000>; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo10_reg: LDO10 { > > > > regulator-name = "VDDQ_MIPIHSI_1.8V"; > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo11_reg: LDO11 { > > > > @@ -340,6 +364,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo12_reg: LDO12 { > > > > @@ -348,6 +375,9 @@ > > > > regulator-max-microvolt = <3300000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo13_reg: LDO13 { > > > > @@ -356,6 +386,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo14_reg: LDO14 { > > > > @@ -364,6 +397,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo15_reg: LDO15 { > > > > @@ -372,6 +408,9 @@ > > > > regulator-max-microvolt = <1000000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo16_reg: LDO16 { > > > > @@ -380,6 +419,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo20_reg: LDO20 { > > > > @@ -387,6 +429,9 @@ > > > > regulator-min-microvolt = <1800000>; > > > > regulator-max-microvolt = <1800000>; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > ldo21_reg: LDO21 { > > > > @@ -394,6 +439,9 @@ > > > > regulator-min-microvolt = <2800000>; > > > > regulator-max-microvolt = <2800000>; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > > > That's unusual setting... enabled in suspend but not necessarily > > > during work (no always-on). > > > > > > > I kept the regulator required for emmc/sd card in > > regulator-on-in-suspend in suspend mode. > > > > > > + }; > > > > }; > > > > > > > > ldo22_reg: LDO22 { > > > > @@ -411,6 +459,9 @@ > > > > regulator-max-microvolt = <1800000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > }; > > > > > > > > buck1_reg: BUCK1 { > > > > @@ -419,6 +470,9 @@ > > > > regulator-max-microvolt = <1100000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > + }; > > > > > > This is seriously wrong so I have doubts whether you tested the > > > changes or you know what is happening with them. If you turn off > > > memory bus regulator off, how the memory will work in > > > Suspend-to-Memory mode? > > > > > > > Well I did not find any issue in my testing but I might be wrong. > > Ok I will drop this changes in the next version of the patch where > > regulator-always-on is set. > > It worked fine because regulator-off-in-suspend is noop for buck1 but > it is clearly wrong from logical point of view. Do you think that > memory should be off in Suspend to RAM? > > > > > }; > > > > > > > > buck2_reg: BUCK2 { > > > > @@ -427,6 +481,9 @@ > > > > regulator-max-microvolt = <1350000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-on-in-suspend; > > > > + }; > > > > }; > > > > > > > > buck3_reg: BUCK3 { > > > > @@ -435,6 +492,9 @@ > > > > regulator-max-microvolt = <1050000>; > > > > regulator-always-on; > > > > regulator-boot-on; > > > > + regulator-state-mem { > > > > + regulator-off-in-suspend; > > > > > > Looks suspicious as well. > > > > Ok I will drop this changes in the next version of the patch where > > regulator-always-on is set. > > > > > > > > Best regards, > > > Krzysztof > > > > > > > Well I have tested this patch as following > > with only one issue, before enable suspend number of On-line cpu is 4 > > after resume number of On-line cpu is 1. > > If before your change number of resumed CPUs was 4, then you > apparently break some things. > No bring of secondary cpu's is broken. I have check this without my dts changes. [root@archlinux-u3 alarm]# echo 1 > /sys/power/pm_debug_messages [root@archlinux-u3 alarm]# echo +30 > /sys/class/rtc/rtc0/wakealarm [root@archlinux-u3 alarm]# echo -n mem > /sys/power/state [ 86.594308] PM: suspend entry (deep) [ 86.594762] PM: Syncing filesystems ... done. [ 86.899480] Freezing user space processes ... (elapsed 0.009 seconds) done. [ 86.911830] OOM killer disabled. [ 86.914121] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 86.921412] printk: Suspending console(s) (use no_console_suspend to debug) [ 86.971761] smsc95xx 1-2:1.0 eth0: entering SUSPEND2 mode [ 87.003552] dwc2 12480000.hsotg: suspending usb gadget g_ether [ 87.004504] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0 [ 87.004521] dwc2 12480000.hsotg: dwc2_hsotg_ep_disable: called for ep0 [ 87.004594] dwc2 12480000.hsotg: new device is full-speed [ 87.006402] wake enabled for irq 119 [ 87.006503] BUCK9: No configuration [ 87.006592] BUCK8_P3V3: No configuration [ 87.006631] BUCK7_2.0V: No configuration [ 87.006667] BUCK6_1.35V: No configuration [ 87.006703] VDDQ_CKEM1_2_1.2V: No configuration [ 87.006830] LDO26: No configuration [ 87.006868] VDDQ_LCD_1.8V: No configuration [ 87.006905] LDO24: No configuration [ 87.006939] LDO23: No configuration [ 87.006977] LDO22_VDDQ_MMC4_2.8V: No configuration [ 87.007013] TFLASH_2.8V: No configuration [ 87.007048] LDO20_1.8V: No configuration [ 87.007083] LDO19: No configuration [ 87.007119] LDO18: No configuration [ 87.007154] LDO17: No configuration [ 87.007190] VDD18_HSIC_1.8V: No configuration [ 87.007226] VDD10_HSIC_1.0V: No configuration [ 87.007262] VDD18_ABB0_2_1.8V: No configuration [ 87.007298] VDDQ_C2C_W_1.8V: No configuration [ 87.007333] VDD33_USB_3.3V: No configuration [ 87.007369] VDD18_ABB1_1.8V: No configuration [ 87.007405] VDDQ_MIPIHSI_1.8V: No configuration [ 87.007441] LDO9: No configuration [ 87.007477] VDD10_HDMI_1.0V: No configuration [ 87.007512] VDD10_XPLL_1.0V: No configuration [ 87.007548] VDD10_MPLL_1.0V: No configuration [ 87.007583] VDDQ_MMC1_3_1.8V: No configuration [ 87.007619] VDDQ_MMC2_2.8V: No configuration [ 87.007655] VDDQ_EXT_1.8V: No configuration [ 87.007691] VDDQ_M1_2_1.8V: No configuration [ 87.007727] VDD_ALIVE_1.0V: No configuration [ 87.014494] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 87.024071] usb3503 0-0008: switched to STANDBY mode [ 87.024739] wake enabled for irq 123 [ 87.066437] samsung-pinctrl 11000000.pinctrl: Setting external wakeup interrupt mask: 0xfbfff7ff [ 87.086663] Disabling non-boot CPUs ... [ 87.230744] Enabling non-boot CPUs ... [ 88.229226] CPU1: failed to boot: -110 [ 88.231113] Error taking CPU1 up: -110 [ 88.234223] ------------[ cut here ]------------ [ 88.234267] WARNING: CPU: 2 PID: 0 at arch/arm/include/asm/proc-fns.h:126 secondary_start_kernel+0x214/0x270 [ 88.234273] Modules linked in: [ 89.229219] CPU2: failed to boot: -110 [ 89.230012] Error taking CPU2 up: -110 [ 90.229071] CPU3: failed to boot: -110 [ 90.229823] Error taking CPU3 up: -110 [ 90.234887] s3c-i2c 13860000.i2c: slave address 0x00 [ 90.234917] s3c-i2c 13860000.i2c: bus frequency set to 390 KHz [ 90.234955] s3c-i2c 13870000.i2c: slave address 0x00 [ 90.234975] s3c-i2c 13870000.i2c: bus frequency set to 97 KHz [ 90.235012] s3c-i2c 13880000.i2c: slave address 0x00 [ 90.235031] s3c-i2c 13880000.i2c: bus frequency set to 97 KHz [ 90.235067] s3c-i2c 138e0000.i2c: slave address 0x00 [ 90.235086] s3c-i2c 138e0000.i2c: bus frequency set to 97 KHz [ 90.255430] s3c-rtc 10070000.rtc: rtc disabled, re-enabling [ 90.255985] usb usb1: root hub lost power or was reset [ 90.261948] s3c2410-wdt 10060000.watchdog: watchdog disabled [ 90.262139] wake disabled for irq 123 [ 90.273868] usb3503 0-0008: switched to HUB mode [ 90.364530] wake disabled for irq 119 [ 90.364688] dwc2 12480000.hsotg: resuming usb gadget g_ether [ 90.649045] usb 1-2: reset high-speed USB device number 2 using exynos-ehci [ 91.001232] usb 1-3: reset high-speed USB device number 3 using exynos-ehci [ 91.539004] usb 1-3.3: reset high-speed USB device number 4 using exynos-ehci [ 92.027260] OOM killer enabled. [ 92.030413] Restarting tasks ... done. [ 92.078154] PM: suspend exit [root@archlinux-u3 alarm]# [ 92.451624] smsc95xx 1-2:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1 Here is the summary of the regulator to be turned off during suspend regulator-off-in-suspend LDO1, LDO2, LDO3, LDO6, LDO7, LDO8, LDO10, LDO11, LDO12, LDO13, LDO14, LDO15, LDO16, LDO20, LDO21, LDO22, LDO25,BUCK4, BUCK5, BUCK6, BUCK7, regulator-on-in-suspend LDO4, LDO5, LD021, BUCK1, BUCK2, BUCK3. Please let me know if this would be fine with you, so that I could send patch v2. Best Regards -Anand
Hi All, On 2018-12-05 17:11, Anand Moon wrote: > Hi Krzysztof, > > Thanks for your review. > . > On Wed, 5 Dec 2018 at 19:36, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> On Tue, 4 Dec 2018 at 20:40, 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. >>> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> Tested on Odroid U3+ >>> ... > Well I have tested this patch as following > with only one issue, before enable suspend number of On-line cpu is 4 > after resume number of On-line cpu is 1. This seems to be a regression in v4.20-rc1, not related to dts changes at all. I'm investigating this now... > ... Best regards
Hi All, On 2018-12-06 09:25, Marek Szyprowski wrote: > On 2018-12-05 17:11, Anand Moon wrote: >> On Wed, 5 Dec 2018 at 19:36, Krzysztof Kozlowski <krzk@kernel.org> wrote: >>> On Tue, 4 Dec 2018 at 20:40, 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. >>>> >>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>>> --- >>>> Tested on Odroid U3+ >>>> >>>> ... >> Well I have tested this patch as following >> with only one issue, before enable suspend number of On-line cpu is 4 >> after resume number of On-line cpu is 1. > This seems to be a regression in v4.20-rc1, not related to dts changes > at all. I'm investigating this now... Okay, I mixed kernel versions a bit. To be precise, this regression is between v4.20-rc2 and v4.20-rc3. Bisecting pointed commit 383fb3ee8024 ("ARM: spectre-v2: per-CPU vtables to work around big.Little systems"). I will post a bug report about the regression. Best regards
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi index 2caa3132f34e..837713a2ec3b 100644 --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi @@ -285,6 +285,9 @@ regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; regulator-always-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo3_reg: LDO3 { @@ -292,6 +295,9 @@ regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; regulator-always-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo4_reg: LDO4 { @@ -299,6 +305,9 @@ regulator-min-microvolt = <2800000>; regulator-max-microvolt = <2800000>; regulator-boot-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo5_reg: LDO5 { @@ -307,6 +316,9 @@ regulator-max-microvolt = <1800000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo6_reg: LDO6 { @@ -314,6 +326,9 @@ regulator-min-microvolt = <1000000>; regulator-max-microvolt = <1000000>; regulator-always-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo7_reg: LDO7 { @@ -321,18 +336,27 @@ regulator-min-microvolt = <1000000>; regulator-max-microvolt = <1000000>; regulator-always-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo8_reg: LDO8 { regulator-name = "VDD10_HDMI_1.0V"; regulator-min-microvolt = <1000000>; regulator-max-microvolt = <1000000>; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo10_reg: LDO10 { regulator-name = "VDDQ_MIPIHSI_1.8V"; regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo11_reg: LDO11 { @@ -340,6 +364,9 @@ regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; regulator-always-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo12_reg: LDO12 { @@ -348,6 +375,9 @@ regulator-max-microvolt = <3300000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo13_reg: LDO13 { @@ -356,6 +386,9 @@ regulator-max-microvolt = <1800000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo14_reg: LDO14 { @@ -364,6 +397,9 @@ regulator-max-microvolt = <1800000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo15_reg: LDO15 { @@ -372,6 +408,9 @@ regulator-max-microvolt = <1000000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo16_reg: LDO16 { @@ -380,6 +419,9 @@ regulator-max-microvolt = <1800000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo20_reg: LDO20 { @@ -387,6 +429,9 @@ regulator-min-microvolt = <1800000>; regulator-max-microvolt = <1800000>; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; ldo21_reg: LDO21 { @@ -394,6 +439,9 @@ regulator-min-microvolt = <2800000>; regulator-max-microvolt = <2800000>; regulator-boot-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; ldo22_reg: LDO22 { @@ -411,6 +459,9 @@ regulator-max-microvolt = <1800000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck1_reg: BUCK1 { @@ -419,6 +470,9 @@ regulator-max-microvolt = <1100000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck2_reg: BUCK2 { @@ -427,6 +481,9 @@ regulator-max-microvolt = <1350000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-on-in-suspend; + }; }; buck3_reg: BUCK3 { @@ -435,6 +492,9 @@ regulator-max-microvolt = <1050000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck4_reg: BUCK4 { @@ -442,6 +502,9 @@ regulator-min-microvolt = <900000>; regulator-max-microvolt = <1100000>; regulator-microvolt-offset = <50000>; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck5_reg: BUCK5 { @@ -450,6 +513,9 @@ regulator-max-microvolt = <1200000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck6_reg: BUCK6 { @@ -458,6 +524,9 @@ regulator-max-microvolt = <1350000>; regulator-always-on; regulator-boot-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck7_reg: BUCK7 { @@ -465,6 +534,9 @@ regulator-min-microvolt = <2000000>; regulator-max-microvolt = <2000000>; regulator-always-on; + regulator-state-mem { + regulator-off-in-suspend; + }; }; buck8_reg: BUCK8 {
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. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- Tested on Odroid U3+ --- .../boot/dts/exynos4412-odroid-common.dtsi | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+)