linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
@ 2022-08-29 16:49 Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 1/6] arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed Douglas Anderson
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Andy Gross, AngeloGioacchino Del Regno, Bhupesh Sharma,
	Johan Hovold, Konrad Dybcio, Krzysztof Kozlowski, Rob Herring,
	Vinod Koul, devicetree, linux-arm-msm, linux-kernel

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/20220803121942.30236-1-johan+linaro@kernel.org/
[3] https://lore.kernel.org/r/YwhwIX+Ib8epUYWS@hovoldconsulting.com/

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/6] arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
@ 2022-08-29 16:49 ` Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 2/6] arm64: dts: qcom: sa8295p-adp: " Douglas Anderson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Konrad Dybcio, Andy Gross, Bhupesh Sharma, Krzysztof Kozlowski,
	Rob Herring, devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/6] arm64: dts: qcom: sa8295p-adp: Specify which LDO modes are allowed
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 1/6] arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed Douglas Anderson
@ 2022-08-29 16:49 ` Douglas Anderson
  2022-08-31  6:42   ` Johan Hovold
  2022-08-29 16:49 ` [PATCH v2 3/6] arm64: dts: qcom: sc8280xp-crd: " Douglas Anderson
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Konrad Dybcio, Andy Gross, Johan Hovold, Krzysztof Kozlowski,
	Rob Herring, devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/6] arm64: dts: qcom: sc8280xp-crd: Specify which LDO modes are allowed
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 1/6] arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 2/6] arm64: dts: qcom: sa8295p-adp: " Douglas Anderson
@ 2022-08-29 16:49 ` Douglas Anderson
  2022-08-31  6:43   ` Johan Hovold
  2022-08-29 16:49 ` [PATCH v2 4/6] arm64: dts: qcom: sm8150-xperia-kumano: " Douglas Anderson
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Konrad Dybcio, Andy Gross, Johan Hovold, Krzysztof Kozlowski,
	Rob Herring, devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 4/6] arm64: dts: qcom: sm8150-xperia-kumano: Specify which LDO modes are allowed
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
                   ` (2 preceding siblings ...)
  2022-08-29 16:49 ` [PATCH v2 3/6] arm64: dts: qcom: sc8280xp-crd: " Douglas Anderson
@ 2022-08-29 16:49 ` Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 5/6] arm64: dts: qcom: sm8250-xperia-edo: " Douglas Anderson
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Konrad Dybcio, Andy Gross, AngeloGioacchino Del Regno,
	Krzysztof Kozlowski, Rob Herring, devicetree, linux-arm-msm,
	linux-kernel

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 <dianders@chromium.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---

(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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 5/6] arm64: dts: qcom: sm8250-xperia-edo: Specify which LDO modes are allowed
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
                   ` (3 preceding siblings ...)
  2022-08-29 16:49 ` [PATCH v2 4/6] arm64: dts: qcom: sm8150-xperia-kumano: " Douglas Anderson
@ 2022-08-29 16:49 ` Douglas Anderson
  2022-08-29 16:49 ` [PATCH v2 6/6] arm64: dts: qcom: sm8350-hdk: " Douglas Anderson
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Konrad Dybcio, Andy Gross, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---

(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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 6/6] arm64: dts: qcom: sm8350-hdk: Specify which LDO modes are allowed
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
                   ` (4 preceding siblings ...)
  2022-08-29 16:49 ` [PATCH v2 5/6] arm64: dts: qcom: sm8250-xperia-edo: " Douglas Anderson
@ 2022-08-29 16:49 ` Douglas Anderson
  2022-08-31  6:47 ` [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Johan Hovold
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Douglas Anderson @ 2022-08-29 16:49 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Douglas Anderson,
	Vinod Koul, Konrad Dybcio, Andy Gross, Krzysztof Kozlowski,
	Rob Herring, devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---

(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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/6] arm64: dts: qcom: sa8295p-adp: Specify which LDO modes are allowed
  2022-08-29 16:49 ` [PATCH v2 2/6] arm64: dts: qcom: sa8295p-adp: " Douglas Anderson
@ 2022-08-31  6:42   ` Johan Hovold
  0 siblings, 0 replies; 19+ messages in thread
From: Johan Hovold @ 2022-08-31  6:42 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Konrad Dybcio,
	Andy Gross, Johan Hovold, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
> Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> ---
> 
> Changes in v2:
> - Rebased atop ("...: sa8295p-adp: disallow regulator mode switches")

Looks good: 

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

>  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>;
>  		};
>  	};

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/6] arm64: dts: qcom: sc8280xp-crd: Specify which LDO modes are allowed
  2022-08-29 16:49 ` [PATCH v2 3/6] arm64: dts: qcom: sc8280xp-crd: " Douglas Anderson
@ 2022-08-31  6:43   ` Johan Hovold
  0 siblings, 0 replies; 19+ messages in thread
From: Johan Hovold @ 2022-08-31  6:43 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Konrad Dybcio,
	Andy Gross, Johan Hovold, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-msm, linux-kernel

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 <dianders@chromium.org>
> Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> ---
> 
> Changes in v2:
> - Rebased atop ("...: sc8280xp-crd: disallow regulator mode switches")

Reviewed-by: Johan Hovold <johan+linaro@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
                   ` (5 preceding siblings ...)
  2022-08-29 16:49 ` [PATCH v2 6/6] arm64: dts: qcom: sm8350-hdk: " Douglas Anderson
@ 2022-08-31  6:47 ` Johan Hovold
  2022-08-31 14:52   ` Doug Anderson
  2022-09-08 16:04 ` Doug Anderson
  2022-10-18  3:05 ` Bjorn Andersson
  8 siblings, 1 reply; 19+ messages in thread
From: Johan Hovold @ 2022-08-31  6:47 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	devicetree, linux-arm-msm, linux-kernel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-31  6:47 ` [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Johan Hovold
@ 2022-08-31 14:52   ` Doug Anderson
  2022-08-31 19:00     ` Andrew Halaney
  2022-09-01 15:52     ` Johan Hovold
  0 siblings, 2 replies; 19+ messages in thread
From: Doug Anderson @ 2022-08-31 14:52 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Tue, Aug 30, 2022 at 11:47 PM Johan Hovold <johan@kernel.org> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-31 14:52   ` Doug Anderson
@ 2022-08-31 19:00     ` Andrew Halaney
  2022-09-02 18:59       ` Andrew Halaney
  2022-09-01 15:52     ` Johan Hovold
  1 sibling, 1 reply; 19+ messages in thread
From: Andrew Halaney @ 2022-08-31 19:00 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Johan Hovold, Bjorn Andersson, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

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 <johan@kernel.org> 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
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-31 14:52   ` Doug Anderson
  2022-08-31 19:00     ` Andrew Halaney
@ 2022-09-01 15:52     ` Johan Hovold
  2022-09-02  0:44       ` Doug Anderson
  1 sibling, 1 reply; 19+ messages in thread
From: Johan Hovold @ 2022-09-01 15:52 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

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 <johan@kernel.org> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-09-01 15:52     ` Johan Hovold
@ 2022-09-02  0:44       ` Doug Anderson
  2022-09-02  5:40         ` Johan Hovold
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2022-09-02  0:44 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Hi,

On Thu, Sep 1, 2022 at 8:52 AM Johan Hovold <johan@kernel.org> 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 <johan@kernel.org> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-09-02  0:44       ` Doug Anderson
@ 2022-09-02  5:40         ` Johan Hovold
  0 siblings, 0 replies; 19+ messages in thread
From: Johan Hovold @ 2022-09-02  5:40 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Bjorn Andersson, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

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 <johan@kernel.org> 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 <johan@kernel.org> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-31 19:00     ` Andrew Halaney
@ 2022-09-02 18:59       ` Andrew Halaney
  0 siblings, 0 replies; 19+ messages in thread
From: Andrew Halaney @ 2022-09-02 18:59 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Johan Hovold, Bjorn Andersson, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

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 <johan@kernel.org> 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/20220902185148.635292-1-ahalaney@redhat.com/

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
                   ` (6 preceding siblings ...)
  2022-08-31  6:47 ` [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Johan Hovold
@ 2022-09-08 16:04 ` Doug Anderson
  2022-09-23  0:16   ` Doug Anderson
  2022-10-18  3:05 ` Bjorn Andersson
  8 siblings, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2022-09-08 16:04 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Bjorn,

On Mon, Aug 29, 2022 at 9:50 AM Douglas Anderson <dianders@chromium.org> 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/20220803121942.30236-1-johan+linaro@kernel.org/
> [3] https://lore.kernel.org/r/YwhwIX+Ib8epUYWS@hovoldconsulting.com/
>
> 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/166181675980.322065.3918715363441736917.b4-ty@kernel.org/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-09-08 16:04 ` Doug Anderson
@ 2022-09-23  0:16   ` Doug Anderson
  0 siblings, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2022-09-23  0:16 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Johan Hovold, Andrew Halaney, Mark Brown, Andy Gross,
	AngeloGioacchino Del Regno, Bhupesh Sharma, Johan Hovold,
	Konrad Dybcio, Krzysztof Kozlowski, Rob Herring, Vinod Koul,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, LKML

Bjorn,

On Thu, Sep 8, 2022 at 9:04 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Bjorn,
>
> On Mon, Aug 29, 2022 at 9:50 AM Douglas Anderson <dianders@chromium.org> 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/20220803121942.30236-1-johan+linaro@kernel.org/
> > [3] https://lore.kernel.org/r/YwhwIX+Ib8epUYWS@hovoldconsulting.com/
> >
> > 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/166181675980.322065.3918715363441736917.b4-ty@kernel.org/

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards
  2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
                   ` (7 preceding siblings ...)
  2022-09-08 16:04 ` Doug Anderson
@ 2022-10-18  3:05 ` Bjorn Andersson
  8 siblings, 0 replies; 19+ messages in thread
From: Bjorn Andersson @ 2022-10-18  3:05 UTC (permalink / raw)
  To: Douglas Anderson, Bjorn Andersson
  Cc: angelogioacchino.delregno, johan, ahalaney, devicetree,
	linux-kernel, agross, broonie, vkoul, bhupesh.sharma, robh+dt,
	johan+linaro, linux-arm-msm, Krzysztof Kozlowski, Konrad Dybcio

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 <andersson@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-10-18  3:06 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 16:49 [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Douglas Anderson
2022-08-29 16:49 ` [PATCH v2 1/6] arm64: dts: qcom: sa8155p-adp: Specify which LDO modes are allowed Douglas Anderson
2022-08-29 16:49 ` [PATCH v2 2/6] arm64: dts: qcom: sa8295p-adp: " Douglas Anderson
2022-08-31  6:42   ` Johan Hovold
2022-08-29 16:49 ` [PATCH v2 3/6] arm64: dts: qcom: sc8280xp-crd: " Douglas Anderson
2022-08-31  6:43   ` Johan Hovold
2022-08-29 16:49 ` [PATCH v2 4/6] arm64: dts: qcom: sm8150-xperia-kumano: " Douglas Anderson
2022-08-29 16:49 ` [PATCH v2 5/6] arm64: dts: qcom: sm8250-xperia-edo: " Douglas Anderson
2022-08-29 16:49 ` [PATCH v2 6/6] arm64: dts: qcom: sm8350-hdk: " Douglas Anderson
2022-08-31  6:47 ` [PATCH v2 0/6] arm64: dts: qcom: Fix broken regulator spec on RPMH boards Johan Hovold
2022-08-31 14:52   ` Doug Anderson
2022-08-31 19:00     ` Andrew Halaney
2022-09-02 18:59       ` Andrew Halaney
2022-09-01 15:52     ` Johan Hovold
2022-09-02  0:44       ` Doug Anderson
2022-09-02  5:40         ` Johan Hovold
2022-09-08 16:04 ` Doug Anderson
2022-09-23  0:16   ` Doug Anderson
2022-10-18  3:05 ` Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).