| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 5 Dec 2018 15:06:42 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: linux.amoon@...il.com
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>, linux-kernel@...r.kernel.org,
robh+dt@...nel.org, mark.rutland@....com, kgene@...nel.org,
Marek Szyprowski <m.szyprowski@...sung.com>,
Bartłomiej Żołnierkiewicz
<b.zolnierkie@...sung.com>
Subject: Re: [PATCH] ARM: dts: exynos: Add proper regulator states for
suspend-to-mem for odroid-u3
On Tue, 4 Dec 2018 at 20:40, Anand Moon <linux.amoon@...il.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@...il.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
>
Powered by blists - more mailing lists