Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") several boards were able to
change their regulator mode even though they had nothing listed in
"regulator-allowed-modes". After that commit (and fixes [1]) we'll be
stuck at the initial mode. Discussion of this (again, see [1]) has
resulted in the decision that the old dts files were wrong and should
be fixed to fully restore old functionality.
This series attempts to fix everyone. I've kept each board in a
separate patch to make stable / backports work easier.
Affected boards were found with:
rpmh_users=$(git grep -l -i rpmh -- arch/arm*/boot/dts/qcom)
set_modes=$(grep -l regulator-allow-set-load ${rpmh_users})
but_no_allowed_modes=$(grep -l -v regulator-allowed-modes ${set_modes})
Fix was applied with:
for f in ${but_no_allowed_modes}; do
sed -i~ -e \
's/^\(\s*\)regulator-allow-set-load;/\1regulator-allow-set-load;\n\1regulator-allowed-modes =\n\1 <RPMH_REGULATOR_MODE_LPM\n\1 RPMH_REGULATOR_MODE_HPM>;/'\
$f
done
Then results were manually inspected. In one board I removed a
"regulator-allow-set-load" from a fixed regulator since that was
clearly wrong.
v2 of this series adds tags and also rebases atop Johan's series [2]
as requested [3]. This ends up turning the series into a 6-part series
instead of a 7-part one.
It should also be noted that as of the v2 posting that the related
regulator fixes have all landed in the regulator tree.
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
[2] https://lore.kernel.org/r/[email protected]/
[3] https://lore.kernel.org/r/[email protected]/
Changes in v2:
- Added note about removing regulator-allow-set-load from vreg_s4a_1p8
- Rebased atop ("...: sa8295p-adp: disallow regulator mode switches")
- Rebased atop ("...: sc8280xp-crd: disallow regulator mode switches")
Douglas Anderson (6):
arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed
arm64: dts: qcom: sa8295p-adp: Specify which LDO modes are allowed
arm64: dts: qcom: sc8280xp-crd: Specify which LDO modes are allowed
arm64: dts: qcom: sm8150-xperia-kumano: Specify which LDO modes are
allowed
arm64: dts: qcom: sm8250-xperia-edo: Specify which LDO modes are
allowed
arm64: dts: qcom: sm8350-hdk: Specify which LDO modes are allowed
arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 13 ++++++++++++-
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 12 ++++++++++++
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 6 ++++++
.../boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi | 6 ++++++
.../arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi | 6 ++++++
arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 12 ++++++++++++
6 files changed, 54 insertions(+), 1 deletion(-)
--
2.37.2.672.g94769d06f0-goog
This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes".
Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") the above meant that we were able
to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
be stuck at the initial mode. Discussion of this has resulted in the
decision that the old dts files were wrong and should be fixed to
fully restore old functionality.
Let's re-enable the old functionality by fixing the dts.
NOTE: while here, let's also remove the nonsensical
"regulator-allow-set-load" on the fixed regulator "vreg_s4a_1p8".
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
Fixes: 5b85e8f2225c ("arm64: dts: qcom: sa8155p-adp: Add base dts file")
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
Changes in v2:
- Added note about removing regulator-allow-set-load from vreg_s4a_1p8
arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
index ba547ca9fc6b..ddb9cb182152 100644
--- a/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8155p-adp.dts
@@ -43,7 +43,6 @@ vreg_s4a_1p8: smps4 {
regulator-always-on;
regulator-boot-on;
- regulator-allow-set-load;
vin-supply = <&vreg_3p3>;
};
@@ -137,6 +136,9 @@ vreg_l5a_0p88: ldo5 {
regulator-max-microvolt = <880000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7a_1p8: ldo7 {
@@ -152,6 +154,9 @@ vreg_l10a_2p96: ldo10 {
regulator-max-microvolt = <2960000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l11a_0p8: ldo11 {
@@ -258,6 +263,9 @@ vreg_l5c_1p2: ldo5 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7c_1p8: ldo7 {
@@ -273,6 +281,9 @@ vreg_l8c_1p2: ldo8 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l10c_3p3: ldo10 {
--
2.37.2.672.g94769d06f0-goog
This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes".
Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") the above meant that we were able
to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
be stuck at the initial mode. Discussion of this has resulted in the
decision that the old dts files were wrong and should be fixed to
fully restore old functionality.
Let's re-enable the old functionality by fixing the dts.
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
Fixes: 519183af39b2 ("arm64: dts: qcom: add SA8540P and ADP")
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
Changes in v2:
- Rebased atop ("...: sa8295p-adp: disallow regulator mode switches")
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
index ca5f5ad32ce5..5b16ac76fefb 100644
--- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
@@ -83,6 +83,9 @@ vreg_l3c: ldo3 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l4c: ldo4 {
@@ -98,6 +101,9 @@ vreg_l6c: ldo6 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7c: ldo7 {
@@ -113,6 +119,9 @@ vreg_l10c: ldo10 {
regulator-max-microvolt = <2504000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l17c: ldo17 {
@@ -121,6 +130,9 @@ vreg_l17c: ldo17 {
regulator-max-microvolt = <2504000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
};
--
2.37.2.672.g94769d06f0-goog
This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes".
Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") the above meant that we were able
to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
be stuck at the initial mode. Discussion of this has resulted in the
decision that the old dts files were wrong and should be fixed to
fully restore old functionality.
Let's re-enable the old functionality by fixing the dts.
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
Fixes: 9208c19f2124 ("arm64: dts: qcom: Introduce SM8350 HDK")
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Vinod Koul <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
index 0fcf5bd88fc7..69ae6503c2f6 100644
--- a/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8350-hdk.dts
@@ -107,6 +107,9 @@ vreg_l5b_0p88: ldo5 {
regulator-max-microvolt = <888000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l6b_1p2: ldo6 {
@@ -115,6 +118,9 @@ vreg_l6b_1p2: ldo6 {
regulator-max-microvolt = <1208000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7b_2p96: ldo7 {
@@ -123,6 +129,9 @@ vreg_l7b_2p96: ldo7 {
regulator-max-microvolt = <2504000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l9b_1p2: ldo9 {
@@ -131,6 +140,9 @@ vreg_l9b_1p2: ldo9 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
};
--
2.37.2.672.g94769d06f0-goog
This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes".
Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") the above meant that we were able
to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
be stuck at the initial mode. Discussion of this has resulted in the
decision that the old dts files were wrong and should be fixed to
fully restore old functionality.
Let's re-enable the old functionality by fixing the dts.
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
Changes in v2:
- Rebased atop ("...: sc8280xp-crd: disallow regulator mode switches")
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index fea7d8273ccd..5e30349efd20 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -124,6 +124,9 @@ vreg_l7c: ldo7 {
regulator-max-microvolt = <2504000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l13c: ldo13 {
@@ -146,6 +149,9 @@ vreg_l3d: ldo3 {
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l4d: ldo4 {
--
2.37.2.672.g94769d06f0-goog
This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes".
Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") the above meant that we were able
to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
be stuck at the initial mode. Discussion of this has resulted in the
decision that the old dts files were wrong and should be fixed to
fully restore old functionality.
Let's re-enable the old functionality by fixing the dts.
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
Fixes: d0a6ce59ea4e ("arm64: dts: qcom: sm8150: Add support for SONY Xperia 1 / 5 (Kumano platform)")
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi b/arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi
index 014fe3a31548..fb6e5a140c9f 100644
--- a/arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi
@@ -348,6 +348,9 @@ vreg_l6c_2p9: ldo6 {
regulator-max-microvolt = <2960000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7c_3p0: ldo7 {
@@ -367,6 +370,9 @@ vreg_l9c_2p9: ldo9 {
regulator-max-microvolt = <2960000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l10c_3p3: ldo10 {
--
2.37.2.672.g94769d06f0-goog
This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
but doesn't specify any modes with "regulator-allowed-modes".
Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()") the above meant that we were able
to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
be stuck at the initial mode. Discussion of this has resulted in the
decision that the old dts files were wrong and should be fixed to
fully restore old functionality.
Let's re-enable the old functionality by fixing the dts.
[1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
Fixes: 69cdb97ef652 ("arm64: dts: qcom: sm8250: Add support for SONY Xperia 1 II / 5 II (Edo platform)")
Signed-off-by: Douglas Anderson <[email protected]>
Reviewed-by: Andrew Halaney <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi b/arch/arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi
index 549e0a2aa9fe..5428aab3058d 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi
@@ -317,6 +317,9 @@ vreg_l6c_2p9: ldo6 {
regulator-max-microvolt = <2960000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l7c_2p85: ldo7 {
@@ -339,6 +342,9 @@ vreg_l9c_2p9: ldo9 {
regulator-max-microvolt = <2960000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
regulator-allow-set-load;
+ regulator-allowed-modes =
+ <RPMH_REGULATOR_MODE_LPM
+ RPMH_REGULATOR_MODE_HPM>;
};
vreg_l10c_3p3: ldo10 {
--
2.37.2.672.g94769d06f0-goog
On Mon, Aug 29, 2022 at 09:49:48AM -0700, Douglas Anderson wrote:
> This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
> but doesn't specify any modes with "regulator-allowed-modes".
>
> Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") the above meant that we were able
> to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
> be stuck at the initial mode. Discussion of this has resulted in the
> decision that the old dts files were wrong and should be fixed to
> fully restore old functionality.
>
> Let's re-enable the old functionality by fixing the dts.
>
> [1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
>
> Fixes: 519183af39b2 ("arm64: dts: qcom: add SA8540P and ADP")
> Signed-off-by: Douglas Anderson <[email protected]>
> Reviewed-by: Andrew Halaney <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
> ---
>
> Changes in v2:
> - Rebased atop ("...: sa8295p-adp: disallow regulator mode switches")
Looks good:
Reviewed-by: Johan Hovold <[email protected]>
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index ca5f5ad32ce5..5b16ac76fefb 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -83,6 +83,9 @@ vreg_l3c: ldo3 {
> regulator-max-microvolt = <1200000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> regulator-allow-set-load;
> + regulator-allowed-modes =
> + <RPMH_REGULATOR_MODE_LPM
> + RPMH_REGULATOR_MODE_HPM>;
> };
>
> vreg_l4c: ldo4 {
> @@ -98,6 +101,9 @@ vreg_l6c: ldo6 {
> regulator-max-microvolt = <1200000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> regulator-allow-set-load;
> + regulator-allowed-modes =
> + <RPMH_REGULATOR_MODE_LPM
> + RPMH_REGULATOR_MODE_HPM>;
> };
>
> vreg_l7c: ldo7 {
> @@ -113,6 +119,9 @@ vreg_l10c: ldo10 {
> regulator-max-microvolt = <2504000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> regulator-allow-set-load;
> + regulator-allowed-modes =
> + <RPMH_REGULATOR_MODE_LPM
> + RPMH_REGULATOR_MODE_HPM>;
> };
>
> vreg_l17c: ldo17 {
> @@ -121,6 +130,9 @@ vreg_l17c: ldo17 {
> regulator-max-microvolt = <2504000>;
> regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> regulator-allow-set-load;
> + regulator-allowed-modes =
> + <RPMH_REGULATOR_MODE_LPM
> + RPMH_REGULATOR_MODE_HPM>;
> };
> };
On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") several boards were able to
> change their regulator mode even though they had nothing listed in
> "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> stuck at the initial mode. Discussion of this (again, see [1]) has
> resulted in the decision that the old dts files were wrong and should
> be fixed to fully restore old functionality.
>
> This series attempts to fix everyone. I've kept each board in a
> separate patch to make stable / backports work easier.
Should you also update the bindings so that this can be caught during
devicetree validation? That is, to always require
"regulator-allowed-modes" when "regulator-allow-set-load" is specified.
Perhaps at least for RPMh as it seemed you found some cases were this
wasn't currently needed (even if that sounded like an Linux-specific
implementation detail).
Johan
On Mon, Aug 29, 2022 at 09:49:49AM -0700, Douglas Anderson wrote:
> This board uses RPMH, specifies "regulator-allow-set-load" for LDOs,
> but doesn't specify any modes with "regulator-allowed-modes".
>
> Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") the above meant that we were able
> to set either LPM or HPM mode. After that commit (and fixes [1]) we'll
> be stuck at the initial mode. Discussion of this has resulted in the
> decision that the old dts files were wrong and should be fixed to
> fully restore old functionality.
>
> Let's re-enable the old functionality by fixing the dts.
>
> [1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
>
> Fixes: ccd3517faf18 ("arm64: dts: qcom: sc8280xp: Add reference device")
> Signed-off-by: Douglas Anderson <[email protected]>
> Reviewed-by: Andrew Halaney <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
> ---
>
> Changes in v2:
> - Rebased atop ("...: sc8280xp-crd: disallow regulator mode switches")
Reviewed-by: Johan Hovold <[email protected]>
Hi,
On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <[email protected]> wrote:
>
> On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > get_optimum_mode(), not set_load()") several boards were able to
> > change their regulator mode even though they had nothing listed in
> > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > stuck at the initial mode. Discussion of this (again, see [1]) has
> > resulted in the decision that the old dts files were wrong and should
> > be fixed to fully restore old functionality.
> >
> > This series attempts to fix everyone. I've kept each board in a
> > separate patch to make stable / backports work easier.
>
> Should you also update the bindings so that this can be caught during
> devicetree validation? That is, to always require
> "regulator-allowed-modes" when "regulator-allow-set-load" is specified.
Yeah, it's probably a good idea. I'm happy to review a patch that does
that. I'm already quite a few patches deep of submitting random
cleanups because someone mentioned it in a code review. ;-) That's
actually how I got in this mess to begin with. The RPMH change was in
response to a request in a different code review. ...and that came
about in a code review that was posted in response to a comment about
how awkward setting regulator loads was... Need to get back to my day
job.
In any case, I think these dts patches are ready to land now.
> Perhaps at least for RPMh as it seemed you found some cases were this
> wasn't currently needed (even if that sounded like an Linux-specific
> implementation detail).
I think you're talking about the RPM vs. RPMH difference? It's
actually not Linux specific. In RPM the API to the "hardware"
(actually a remote processor) is to pass the load. In RPMH the API to
the hardware is to pass a mode. This is why RPMH has
"regulator-allowed-modes" and "regulator-initial-mode". Both RPM and
RPMH have "regulator-allow-set-load" though...
-Doug
On Wed, Aug 31, 2022 at 07:52:52AM -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <[email protected]> wrote:
> >
> > On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> > > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > > get_optimum_mode(), not set_load()") several boards were able to
> > > change their regulator mode even though they had nothing listed in
> > > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > > stuck at the initial mode. Discussion of this (again, see [1]) has
> > > resulted in the decision that the old dts files were wrong and should
> > > be fixed to fully restore old functionality.
> > >
> > > This series attempts to fix everyone. I've kept each board in a
> > > separate patch to make stable / backports work easier.
> >
> > Should you also update the bindings so that this can be caught during
> > devicetree validation? That is, to always require
> > "regulator-allowed-modes" when "regulator-allow-set-load" is specified.
>
> Yeah, it's probably a good idea. I'm happy to review a patch that does
> that. I'm already quite a few patches deep of submitting random
> cleanups because someone mentioned it in a code review. ;-) That's
> actually how I got in this mess to begin with. The RPMH change was in
> response to a request in a different code review. ...and that came
> about in a code review that was posted in response to a comment about
> how awkward setting regulator loads was... Need to get back to my day
> job.
I can take a stab at this during the week here I hope.. I owe Doug for
the slew of patches and have wanted to peek at how all the dt-binding
validation stuff works anyways.
Thanks,
Andrew
>
> In any case, I think these dts patches are ready to land now.
>
>
> > Perhaps at least for RPMh as it seemed you found some cases were this
> > wasn't currently needed (even if that sounded like an Linux-specific
> > implementation detail).
>
> I think you're talking about the RPM vs. RPMH difference? It's
> actually not Linux specific. In RPM the API to the "hardware"
> (actually a remote processor) is to pass the load. In RPMH the API to
> the hardware is to pass a mode. This is why RPMH has
> "regulator-allowed-modes" and "regulator-initial-mode". Both RPM and
> RPMH have "regulator-allow-set-load" though...
>
> -Doug
>
On Wed, Aug 31, 2022 at 07:52:52AM -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <[email protected]> wrote:
> >
> > On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> > > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > > get_optimum_mode(), not set_load()") several boards were able to
> > > change their regulator mode even though they had nothing listed in
> > > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > > stuck at the initial mode. Discussion of this (again, see [1]) has
> > > resulted in the decision that the old dts files were wrong and should
> > > be fixed to fully restore old functionality.
> > >
> > > This series attempts to fix everyone. I've kept each board in a
> > > separate patch to make stable / backports work easier.
> >
> > Should you also update the bindings so that this can be caught during
> > devicetree validation? That is, to always require
> > "regulator-allowed-modes" when "regulator-allow-set-load" is specified.
>
> Yeah, it's probably a good idea. I'm happy to review a patch that does
> that. I'm already quite a few patches deep of submitting random
> cleanups because someone mentioned it in a code review. ;-) That's
> actually how I got in this mess to begin with. The RPMH change was in
> response to a request in a different code review. ...and that came
> about in a code review that was posted in response to a comment about
> how awkward setting regulator loads was... Need to get back to my day
> job.
Heh.
> In any case, I think these dts patches are ready to land now.
Yeah, as the old dtbs are now broken with newer kernels these are indeed
needed.
But regardless of the question of backwards compatibility, it seems that
the bindings should at least reflect that the old DTs are now considered
malformed.
> > Perhaps at least for RPMh as it seemed you found some cases were this
> > wasn't currently needed (even if that sounded like an Linux-specific
> > implementation detail).
>
> I think you're talking about the RPM vs. RPMH difference? It's
> actually not Linux specific. In RPM the API to the "hardware"
> (actually a remote processor) is to pass the load. In RPMH the API to
> the hardware is to pass a mode. This is why RPMH has
> "regulator-allowed-modes" and "regulator-initial-mode". Both RPM and
> RPMH have "regulator-allow-set-load" though...
Ah, ok. And this was only an issue for Qualcomm DTs, which are the only
users of "regulator-allow-set-load" in mainline.
Johan
Hi,
On Thu, Sep 1, 2022 at 8:52 AM Johan Hovold <[email protected]> wrote:
>
> On Wed, Aug 31, 2022 at 07:52:52AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <[email protected]> wrote:
> > >
> > > On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> > > > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > > > get_optimum_mode(), not set_load()") several boards were able to
> > > > change their regulator mode even though they had nothing listed in
> > > > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > > > stuck at the initial mode. Discussion of this (again, see [1]) has
> > > > resulted in the decision that the old dts files were wrong and should
> > > > be fixed to fully restore old functionality.
> > > >
> > > > This series attempts to fix everyone. I've kept each board in a
> > > > separate patch to make stable / backports work easier.
> > >
> > > Should you also update the bindings so that this can be caught during
> > > devicetree validation? That is, to always require
> > > "regulator-allowed-modes" when "regulator-allow-set-load" is specified.
> >
> > Yeah, it's probably a good idea. I'm happy to review a patch that does
> > that. I'm already quite a few patches deep of submitting random
> > cleanups because someone mentioned it in a code review. ;-) That's
> > actually how I got in this mess to begin with. The RPMH change was in
> > response to a request in a different code review. ...and that came
> > about in a code review that was posted in response to a comment about
> > how awkward setting regulator loads was... Need to get back to my day
> > job.
>
> Heh.
>
> > In any case, I think these dts patches are ready to land now.
>
> Yeah, as the old dtbs are now broken with newer kernels these are indeed
> needed.
With the latest patches in the regulator tree things shouldn't be
_too_ broken even without the dts files. Essentially things will get
stuck at their initial mode (HPM). So without these patches things
should all still boot but could possibly end up at a higher power
state.
-Doug
On Thu, Sep 01, 2022 at 05:44:03PM -0700, Doug Anderson wrote:
> Hi,
>
> On Thu, Sep 1, 2022 at 8:52 AM Johan Hovold <[email protected]> wrote:
> >
> > On Wed, Aug 31, 2022 at 07:52:52AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <[email protected]> wrote:
> > > >
> > > > On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> > > > > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > > > > get_optimum_mode(), not set_load()") several boards were able to
> > > > > change their regulator mode even though they had nothing listed in
> > > > > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > > > > stuck at the initial mode. Discussion of this (again, see [1]) has
> > > > > resulted in the decision that the old dts files were wrong and should
> > > > > be fixed to fully restore old functionality.
> > > > >
> > > > > This series attempts to fix everyone. I've kept each board in a
> > > > > separate patch to make stable / backports work easier.
> > > >
> > > > Should you also update the bindings so that this can be caught during
> > > > devicetree validation? That is, to always require
> > > > "regulator-allowed-modes" when "regulator-allow-set-load" is specified.
> > >
> > > Yeah, it's probably a good idea. I'm happy to review a patch that does
> > > that. I'm already quite a few patches deep of submitting random
> > > cleanups because someone mentioned it in a code review. ;-) That's
> > > actually how I got in this mess to begin with. The RPMH change was in
> > > response to a request in a different code review. ...and that came
> > > about in a code review that was posted in response to a comment about
> > > how awkward setting regulator loads was... Need to get back to my day
> > > job.
> >
> > Heh.
> >
> > > In any case, I think these dts patches are ready to land now.
> >
> > Yeah, as the old dtbs are now broken with newer kernels these are indeed
> > needed.
>
> With the latest patches in the regulator tree things shouldn't be
> _too_ broken even without the dts files. Essentially things will get
> stuck at their initial mode (HPM). So without these patches things
> should all still boot but could possibly end up at a higher power
> state.
Ok, and there's also a warning during boot when that happens so that
it's not a silent regression?
Johan
On Wed, Aug 31, 2022 at 02:00:18PM -0500, Andrew Halaney wrote:
> On Wed, Aug 31, 2022 at 07:52:52AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <[email protected]> wrote:
> > >
> > > On Mon, Aug 29, 2022 at 09:49:46AM -0700, Douglas Anderson wrote:
> > > > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > > > get_optimum_mode(), not set_load()") several boards were able to
> > > > change their regulator mode even though they had nothing listed in
> > > > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > > > stuck at the initial mode. Discussion of this (again, see [1]) has
> > > > resulted in the decision that the old dts files were wrong and should
> > > > be fixed to fully restore old functionality.
> > > >
> > > > This series attempts to fix everyone. I've kept each board in a
> > > > separate patch to make stable / backports work easier.
> > >
> > > Should you also update the bindings so that this can be caught during
> > > devicetree validation? That is, to always require
> > > "regulator-allowed-modes" when "regulator-allow-set-load" is specified.
> >
> > Yeah, it's probably a good idea. I'm happy to review a patch that does
> > that. I'm already quite a few patches deep of submitting random
> > cleanups because someone mentioned it in a code review. ;-) That's
> > actually how I got in this mess to begin with. The RPMH change was in
> > response to a request in a different code review. ...and that came
> > about in a code review that was posted in response to a comment about
> > how awkward setting regulator loads was... Need to get back to my day
> > job.
>
> I can take a stab at this during the week here I hope.. I owe Doug for
> the slew of patches and have wanted to peek at how all the dt-binding
> validation stuff works anyways.
>
Here's my attempt after a couple hours of banging the head on the wall:
https://lore.kernel.org/all/[email protected]/
Thanks,
Andrew
Bjorn,
On Mon, Aug 29, 2022 at 9:50 AM Douglas Anderson <[email protected]> wrote:
>
> Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") several boards were able to
> change their regulator mode even though they had nothing listed in
> "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> stuck at the initial mode. Discussion of this (again, see [1]) has
> resulted in the decision that the old dts files were wrong and should
> be fixed to fully restore old functionality.
>
> This series attempts to fix everyone. I've kept each board in a
> separate patch to make stable / backports work easier.
>
> Affected boards were found with:
> rpmh_users=$(git grep -l -i rpmh -- arch/arm*/boot/dts/qcom)
> set_modes=$(grep -l regulator-allow-set-load ${rpmh_users})
> but_no_allowed_modes=$(grep -l -v regulator-allowed-modes ${set_modes})
>
> Fix was applied with:
> for f in ${but_no_allowed_modes}; do
> sed -i~ -e \
> 's/^\(\s*\)regulator-allow-set-load;/\1regulator-allow-set-load;\n\1regulator-allowed-modes =\n\1 <RPMH_REGULATOR_MODE_LPM\n\1 RPMH_REGULATOR_MODE_HPM>;/'\
> $f
> done
>
> Then results were manually inspected. In one board I removed a
> "regulator-allow-set-load" from a fixed regulator since that was
> clearly wrong.
>
> v2 of this series adds tags and also rebases atop Johan's series [2]
> as requested [3]. This ends up turning the series into a 6-part series
> instead of a 7-part one.
>
> It should also be noted that as of the v2 posting that the related
> regulator fixes have all landed in the regulator tree.
>
> [1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
> [2] https://lore.kernel.org/r/[email protected]/
> [3] https://lore.kernel.org/r/[email protected]/
>
> Changes in v2:
> - Added note about removing regulator-allow-set-load from vreg_s4a_1p8
> - Rebased atop ("...: sa8295p-adp: disallow regulator mode switches")
> - Rebased atop ("...: sc8280xp-crd: disallow regulator mode switches")
>
> Douglas Anderson (6):
> arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed
> arm64: dts: qcom: sa8295p-adp: Specify which LDO modes are allowed
> arm64: dts: qcom: sc8280xp-crd: Specify which LDO modes are allowed
> arm64: dts: qcom: sm8150-xperia-kumano: Specify which LDO modes are
> allowed
> arm64: dts: qcom: sm8250-xperia-edo: Specify which LDO modes are
> allowed
> arm64: dts: qcom: sm8350-hdk: Specify which LDO modes are allowed
>
> arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 13 ++++++++++++-
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 12 ++++++++++++
> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 6 ++++++
> .../boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi | 6 ++++++
> .../arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi | 6 ++++++
> arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 12 ++++++++++++
> 6 files changed, 54 insertions(+), 1 deletion(-)
I think this series is ready to land if it's a good time now. It looks
like you've already applied the series that this depends on [1] and
this one is all reviewed and ready to go. Thanks!
[1] https://lore.kernel.org/all/[email protected]/
Bjorn,
On Thu, Sep 8, 2022 at 9:04 AM Doug Anderson <[email protected]> wrote:
>
> Bjorn,
>
> On Mon, Aug 29, 2022 at 9:50 AM Douglas Anderson <[email protected]> wrote:
> >
> > Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> > get_optimum_mode(), not set_load()") several boards were able to
> > change their regulator mode even though they had nothing listed in
> > "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> > stuck at the initial mode. Discussion of this (again, see [1]) has
> > resulted in the decision that the old dts files were wrong and should
> > be fixed to fully restore old functionality.
> >
> > This series attempts to fix everyone. I've kept each board in a
> > separate patch to make stable / backports work easier.
> >
> > Affected boards were found with:
> > rpmh_users=$(git grep -l -i rpmh -- arch/arm*/boot/dts/qcom)
> > set_modes=$(grep -l regulator-allow-set-load ${rpmh_users})
> > but_no_allowed_modes=$(grep -l -v regulator-allowed-modes ${set_modes})
> >
> > Fix was applied with:
> > for f in ${but_no_allowed_modes}; do
> > sed -i~ -e \
> > 's/^\(\s*\)regulator-allow-set-load;/\1regulator-allow-set-load;\n\1regulator-allowed-modes =\n\1 <RPMH_REGULATOR_MODE_LPM\n\1 RPMH_REGULATOR_MODE_HPM>;/'\
> > $f
> > done
> >
> > Then results were manually inspected. In one board I removed a
> > "regulator-allow-set-load" from a fixed regulator since that was
> > clearly wrong.
> >
> > v2 of this series adds tags and also rebases atop Johan's series [2]
> > as requested [3]. This ends up turning the series into a 6-part series
> > instead of a 7-part one.
> >
> > It should also be noted that as of the v2 posting that the related
> > regulator fixes have all landed in the regulator tree.
> >
> > [1] https://lore.kernel.org/r/20220824142229.RFT.v2.2.I6f77860e5cd98bf5c67208fa9edda4a08847c304@changeid
> > [2] https://lore.kernel.org/r/[email protected]/
> > [3] https://lore.kernel.org/r/[email protected]/
> >
> > Changes in v2:
> > - Added note about removing regulator-allow-set-load from vreg_s4a_1p8
> > - Rebased atop ("...: sa8295p-adp: disallow regulator mode switches")
> > - Rebased atop ("...: sc8280xp-crd: disallow regulator mode switches")
> >
> > Douglas Anderson (6):
> > arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed
> > arm64: dts: qcom: sa8295p-adp: Specify which LDO modes are allowed
> > arm64: dts: qcom: sc8280xp-crd: Specify which LDO modes are allowed
> > arm64: dts: qcom: sm8150-xperia-kumano: Specify which LDO modes are
> > allowed
> > arm64: dts: qcom: sm8250-xperia-edo: Specify which LDO modes are
> > allowed
> > arm64: dts: qcom: sm8350-hdk: Specify which LDO modes are allowed
> >
> > arch/arm64/boot/dts/qcom/sa8155p-adp.dts | 13 ++++++++++++-
> > arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 12 ++++++++++++
> > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 6 ++++++
> > .../boot/dts/qcom/sm8150-sony-xperia-kumano.dtsi | 6 ++++++
> > .../arm64/boot/dts/qcom/sm8250-sony-xperia-edo.dtsi | 6 ++++++
> > arch/arm64/boot/dts/qcom/sm8350-hdk.dts | 12 ++++++++++++
> > 6 files changed, 54 insertions(+), 1 deletion(-)
>
> I think this series is ready to land if it's a good time now. It looks
> like you've already applied the series that this depends on [1] and
> this one is all reviewed and ready to go. Thanks!
>
> [1] https://lore.kernel.org/all/[email protected]/
I saw you sent out a pull request, but it didn't seem to include these
patches. Were they missing anything? Maybe you're still planning on a
"Fixes" pull request?
-Doug
On Mon, 29 Aug 2022 09:49:46 -0700, Douglas Anderson wrote:
> Prior to commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
> get_optimum_mode(), not set_load()") several boards were able to
> change their regulator mode even though they had nothing listed in
> "regulator-allowed-modes". After that commit (and fixes [1]) we'll be
> stuck at the initial mode. Discussion of this (again, see [1]) has
> resulted in the decision that the old dts files were wrong and should
> be fixed to fully restore old functionality.
>
> [...]
Applied, thanks!
[1/6] arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed
commit: bd9f3dcf42d943b53190f99bcdbcfe98a56ac4cd
[2/6] arm64: dts: qcom: sa8295p-adp: Specify which LDO modes are allowed
commit: 09a1710b3e12e7ac8d871506bc395a9e8a0f63d6
[3/6] arm64: dts: qcom: sc8280xp-crd: Specify which LDO modes are allowed
commit: a4543e21ae363f4f094fb3c3503d77029e0c5d90
[4/6] arm64: dts: qcom: sm8150-xperia-kumano: Specify which LDO modes are allowed
commit: aa30e786202e4ed1df980442d305658441f65859
[5/6] arm64: dts: qcom: sm8250-xperia-edo: Specify which LDO modes are allowed
commit: b7870d460c05ce31e2311036d91de1e2e0b32cea
[6/6] arm64: dts: qcom: sm8350-hdk: Specify which LDO modes are allowed
commit: 1ce8aaf6abdc35cde555924418b3d4516b4ec871
Best regards,
--
Bjorn Andersson <[email protected]>