[PATCH 0/8] arm64: dts: qcom: msm8939: Rework regulator constraints

[PATCH 0/8] arm64: dts: qcom: msm8939: Rework regulator constraints

[Stephan Gerhold]

Rework the regulator constraints for the MSM8939 device trees to be 
closer to reality. There are several mistakes in there, some of them 
taken over directly from Qualcomm's vendor kernel. Fortunately, none of 
the mistakes is absolutely critical because it turns out that the RPM 
firmware also validates the voltages and silently clamps the requests 
to a proper range. Still, this behavior should be clearly represented 
in the device tree rather than pretending to apply the wrong voltages.

Apply the same changes as previously for MSM8916 [1] and move the 
voltages for the standard components in the SoC to the shared 
msm8916-pm8939.dtsi. With this only the actual board-specific 
regulators are described in the board DT.

[1]: https://lore.kernel.org/linux-arm-msm/20230510-msm8916-regulators-v1-0-54d4960a05fc@gerhold.net/

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Stephan Gerhold (8):
      arm64: dts: qcom: msm8939-pm8916: Add missing pm8916_codec supplies
      arm64: dts: qcom: msm8939: Disable lpass_codec by default
      arm64: dts: qcom: msm8939-sony-tulip: Fix l10-l12 regulator voltages
      arm64: dts: qcom: msm8939-sony-tulip: Allow disabling pm8916_l6
      arm64: dts: qcom: msm8939: Fix regulator constraints
      arm64: dts: qcom: msm8939-pm8916: Clarify purpose
      arm64: dts: qcom: msm8939: Define regulator constraints next to usage
      arm64: dts: qcom: msm8939-pm8916: Mark always-on regulators

 arch/arm64/boot/dts/qcom/apq8039-t2.dts            | 102 +-----------------
 arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi       | 119 +++++++++++++++++----
 .../dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts  |  99 -----------------
 arch/arm64/boot/dts/qcom/msm8939.dtsi              |   1 +
 4 files changed, 104 insertions(+), 217 deletions(-)
---
base-commit: 818e1fca81bfd773144d42f4fbe52b287235e8b4
change-id: 20230530-msm8939-regulators-dd631887ff4c

Best regards,
-- 
Stephan Gerhold <stephan@gerhold.net>

[PATCH 1/8] arm64: dts: qcom: msm8939-pm8916: Add missing pm8916_codec supplies

[Stephan Gerhold]

Update for recent changes to pm8916.dtsi in commit 38218822a72f
("arm64: dts: qcom: pm8916: Move default regulator "-supply"s")
and add the now missing pm8916_codec supplies to msm8939-pm8916.dtsi
as well.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
index 33e02f42f5e4..503c2dd5fe47 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
@@ -25,6 +25,12 @@ &mpss {
 	pll-supply = <&pm8916_l7>;
 };
 
+&pm8916_codec {
+	vdd-cdc-io-supply = <&pm8916_l5>;
+	vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>;
+	vdd-micbias-supply = <&pm8916_l13>;
+};
+
 &rpm_requests {
 	smd_rpm_regulators: regulators {
 		compatible = "qcom,rpm-pm8916-regulators";

-- 
2.40.1

[PATCH 2/8] arm64: dts: qcom: msm8939: Disable lpass_codec by default

[Stephan Gerhold]

Update for recent changes to msm8916.dtsi in commit a5cf21b14666
("arm64: dts: qcom: msm8916: Disable audio codecs by default") and
make lpass_codec disabled by default for devices that are not using
the audio codec functionality inside MSM8939.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/apq8039-t2.dts | 4 ++++
 arch/arm64/boot/dts/qcom/msm8939.dtsi   | 1 +
 2 files changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/apq8039-t2.dts b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
index 2a39216ceef5..c8442242137a 100644
--- a/arch/arm64/boot/dts/qcom/apq8039-t2.dts
+++ b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
@@ -135,6 +135,10 @@ &lpass {
 	status = "okay";
 };
 
+&lpass_codec {
+	status = "okay";
+};
+
 &mdss {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi b/arch/arm64/boot/dts/qcom/msm8939.dtsi
index 3c1505b69a6b..895cafc11480 100644
--- a/arch/arm64/boot/dts/qcom/msm8939.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi
@@ -1608,6 +1608,7 @@ lpass_codec: audio-codec@771c000 {
 				 <&gcc GCC_CODEC_DIGCODEC_CLK>;
 			clock-names = "ahbix-clk", "mclk";
 			#sound-dai-cells = <1>;
+			status = "disabled";
 		};
 
 		sdhc_1: mmc@7824900 {

-- 
2.40.1

[PATCH 3/8] arm64: dts: qcom: msm8939-sony-tulip: Fix l10-l12 regulator voltages

[Stephan Gerhold]

msm8939-sony-xperia-kanuti-tulip.dts has several regulator voltages
that do not quite seem to match what is used in the vendor kernel.
In particular:

 - l10 is fixed at 2.8V [1, 2]
 - l11/l12 are 2.95V max [1]

[1]: https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/arch/arm/boot/dts/qcom/msm8939-regulator.dtsi
[2]: https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/arch/arm/boot/dts/qcom/msm8939-kanuti_tulip.dtsi#L671C1-L673

Fixes: f1134f738fad ("arm64: dts: qcom: Add msm8939 Sony Xperia M4 Aqua")
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
index a9df8dab5481..dc5b8cd5b9f4 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
@@ -110,20 +110,20 @@ pm8916_l9: l9 {
 	};
 
 	pm8916_l10: l10 {
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
 	};
 
 	pm8916_l11: l11 {
 		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
+		regulator-max-microvolt = <2950000>;
 		regulator-system-load = <200000>;
 		regulator-allow-set-load;
 	};
 
 	pm8916_l12: l12 {
 		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
+		regulator-max-microvolt = <2950000>;
 	};
 
 	pm8916_l13: l13 {

-- 
2.40.1

[PATCH 4/8] arm64: dts: qcom: msm8939-sony-tulip: Allow disabling pm8916_l6

[Stephan Gerhold]

The vendor kernel from Sony does not have regulator-always-on for
pm8916_l6, so we should be able to disable it when setting up the
display properly. Since sony-tulip does not have display set up
currently it should be fine to let the regulator disable until then.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
index dc5b8cd5b9f4..509abcdad287 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
@@ -91,7 +91,6 @@ pm8916_l5: l5 {
 	pm8916_l6: l6 {
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
-		regulator-always-on;
 	};
 
 	pm8916_l7: l7 {

-- 
2.40.1

[PATCH 5/8] arm64: dts: qcom: msm8939: Fix regulator constraints

[Stephan Gerhold]

The regulator constraints for the MSM8939 devices were originally taken
from Qualcomm's msm-3.10 vendor device tree (for lack of better
documentation). Unfortunately it turns out that Qualcomm's voltages are
slightly off as well and do not match the voltage constraints applied
by the RPM firmware.

This means that we sometimes request a specific voltage but the RPM
firmware actually applies a much lower or higher voltage. This is
particularly critical for pm8916_l11 which is used as SD card VMMC
regulator: The SD card can choose a voltage from the current range of
1.8 - 2.95V. If it chooses to run at 1.8V we pretend that this is fine
but the RPM firmware will still silently end up configuring 2.95V.
This can be easily reproduced with a multimeter or by checking the
SPMI hardware registers of the regulator.

Apply the same change as for MSM8916 in commit 355750828c55 ("arm64:
dts: qcom: msm8916: Fix regulator constraints") and make the voltages
match the actual "specified range" in the PM8916 Device Specification
which is enforced by the RPM firmware.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/apq8039-t2.dts                      | 12 ++++++------
 .../arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/apq8039-t2.dts b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
index c8442242137a..ceae83b1480b 100644
--- a/arch/arm64/boot/dts/qcom/apq8039-t2.dts
+++ b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
@@ -164,13 +164,13 @@ &smd_rpm_regulators {
 	vdd_l7-supply = <&pm8916_s4>;
 
 	pm8916_s3: s3 {
-		regulator-min-microvolt = <1200000>;
-		regulator-max-microvolt = <1300000>;
+		regulator-min-microvolt = <1250000>;
+		regulator-max-microvolt = <1350000>;
 	};
 
 	pm8916_s4: s4 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <2100000>;
+		regulator-min-microvolt = <1850000>;
+		regulator-max-microvolt = <2150000>;
 	};
 
 	/* l1 is fixed to 1225000, but not connected in schematic */
@@ -201,7 +201,7 @@ pm8916_l7: l7 {
 	};
 
 	pm8916_l8: l8 {
-		regulator-min-microvolt = <2850000>;
+		regulator-min-microvolt = <2900000>;
 		regulator-max-microvolt = <2900000>;
 	};
 
@@ -216,7 +216,7 @@ pm8916_l10: l10 {
 	};
 
 	pm8916_l11: l11 {
-		regulator-min-microvolt = <1800000>;
+		regulator-min-microvolt = <2950000>;
 		regulator-max-microvolt = <2950000>;
 	};
 
diff --git a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
index 509abcdad287..2905dd8d4bf9 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
@@ -64,13 +64,13 @@ &smd_rpm_regulators {
 	vdd_l7-supply = <&pm8916_s4>;
 
 	pm8916_s3: s3 {
-		regulator-min-microvolt = <1200000>;
-		regulator-max-microvolt = <1300000>;
+		regulator-min-microvolt = <1250000>;
+		regulator-max-microvolt = <1350000>;
 	};
 
 	pm8916_s4: s4 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <2100000>;
+		regulator-min-microvolt = <1850000>;
+		regulator-max-microvolt = <2150000>;
 	};
 
 	pm8916_l2: l2 {
@@ -99,7 +99,7 @@ pm8916_l7: l7 {
 	};
 
 	pm8916_l8: l8 {
-		regulator-min-microvolt = <2850000>;
+		regulator-min-microvolt = <2900000>;
 		regulator-max-microvolt = <2900000>;
 	};
 
@@ -114,7 +114,7 @@ pm8916_l10: l10 {
 	};
 
 	pm8916_l11: l11 {
-		regulator-min-microvolt = <1800000>;
+		regulator-min-microvolt = <2950000>;
 		regulator-max-microvolt = <2950000>;
 		regulator-system-load = <200000>;
 		regulator-allow-set-load;

-- 
2.40.1

[PATCH 6/8] arm64: dts: qcom: msm8939-pm8916: Clarify purpose

[Stephan Gerhold]

Add the same comment to msm8939-pm8916.dtsi that was added for the
MSM8916 variant in commit f193264986b5 ("arm64: dts: qcom:
msm8916-pm8916: Clarify purpose").

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
index 503c2dd5fe47..3bede1a23603 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
@@ -1,4 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-only
+/*
+ * msm8939-pm8916.dtsi describes common properties (e.g. regulator connections)
+ * that apply to most devices that make use of the MSM8939 SoC and PM8916 PMIC.
+ * Many regulators have a fixed purpose in the original reference design and
+ * were rarely re-used for different purposes. Devices that deviate from the
+ * typical reference design should not make use of this include and instead add
+ * the necessary properties in the board-specific device tree.
+ */
 
 #include "msm8939.dtsi"
 #include "pm8916.dtsi"

-- 
2.40.1

[PATCH 7/8] arm64: dts: qcom: msm8939: Define regulator constraints next to usage

[Stephan Gerhold]

Right now each MSM8939 device has a huge block of regulator constraints
with allowed voltages for each regulator. For lack of better
documentation these voltages are often copied as-is from the vendor
device tree, without much extra thought.

Unfortunately, the voltages in the vendor device trees are often
misleading or even wrong, e.g. because:

 - There is a large voltage range allowed and the actual voltage is
   only set somewhere hidden in some messy vendor driver. This is often
   the case for pm8916_{l14,l15,l16} because they have a broad range of
   1.8-3.3V by default.

 - The voltage is actually wrong but thanks to the voltage constraints
   in the RPM firmware it still ends up applying the correct voltage.

To have proper regulator constraints it is important to review them in
context of the usage. The current setup in the MSM8939 device trees
makes this quite hard because each device duplicates the standard
voltages for components of the SoC and mixes those with minor
device-specific additions and dummy voltages for completely unused
regulators.

The actual usage of the regulators for the SoC components is in
msm8939-pm8916.dtsi, so it can and should also define the related
voltage constraints. These are not board-specific but defined in the
MSM8939/PM8916 specification. There is no documentation available for
MSM8939 but in practice it's almost identical to MSM8916.

Note that this commit does not make any functional change. All used
regulators still have the same regulator constraints as before. Unused
regulators do not have regulator constraints anymore because most of
these were too broad or even entirely wrong. They should be added back
with proper voltage constraints when there is an actual usage.

The same changes were already made for MSM8916 in commit b0a8f16ae4a0
("arm64: dts: qcom: msm8916: Define regulator constraints next to usage").

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/apq8039-t2.dts            |  98 --------------------
 arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi       | 100 ++++++++++++++++-----
 .../dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts  |  98 --------------------
 3 files changed, 80 insertions(+), 216 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/apq8039-t2.dts b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
index ceae83b1480b..40644c242fb7 100644
--- a/arch/arm64/boot/dts/qcom/apq8039-t2.dts
+++ b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
@@ -158,104 +158,6 @@ &pm8916_gpios {
 		"PM_GPIO4";
 };
 
-&smd_rpm_regulators {
-	vdd_l1_l2_l3-supply = <&pm8916_s3>;
-	vdd_l4_l5_l6-supply = <&pm8916_s4>;
-	vdd_l7-supply = <&pm8916_s4>;
-
-	pm8916_s3: s3 {
-		regulator-min-microvolt = <1250000>;
-		regulator-max-microvolt = <1350000>;
-	};
-
-	pm8916_s4: s4 {
-		regulator-min-microvolt = <1850000>;
-		regulator-max-microvolt = <2150000>;
-	};
-
-	/* l1 is fixed to 1225000, but not connected in schematic */
-
-	pm8916_l2: l2 {
-		regulator-min-microvolt = <1200000>;
-		regulator-max-microvolt = <1200000>;
-	};
-
-	pm8916_l4: l4 {
-		regulator-min-microvolt = <2050000>;
-		regulator-max-microvolt = <2050000>;
-	};
-
-	pm8916_l5: l5 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-	};
-
-	pm8916_l6: l6 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-	};
-
-	pm8916_l7: l7 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-	};
-
-	pm8916_l8: l8 {
-		regulator-min-microvolt = <2900000>;
-		regulator-max-microvolt = <2900000>;
-	};
-
-	pm8916_l9: l9 {
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l10: l10 {
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l11: l11 {
-		regulator-min-microvolt = <2950000>;
-		regulator-max-microvolt = <2950000>;
-	};
-
-	pm8916_l12: l12 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <2950000>;
-	};
-
-	pm8916_l13: l13 {
-		regulator-min-microvolt = <3075000>;
-		regulator-max-microvolt = <3075000>;
-	};
-
-	pm8916_l14: l14 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l15: l15 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l16: l16 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l17: l17 {
-		regulator-min-microvolt = <2850000>;
-		regulator-max-microvolt = <2850000>;
-	};
-
-	pm8916_l18: l18 {
-		regulator-min-microvolt = <2700000>;
-		regulator-max-microvolt = <2700000>;
-	};
-};
-
 &sdhc_1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
index 3bede1a23603..12538211bc72 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
@@ -40,32 +40,92 @@ &pm8916_codec {
 };
 
 &rpm_requests {
-	smd_rpm_regulators: regulators {
+	pm8916_rpm_regulators: regulators {
 		compatible = "qcom,rpm-pm8916-regulators";
+		vdd_l1_l2_l3-supply = <&pm8916_s3>;
+		vdd_l4_l5_l6-supply = <&pm8916_s4>;
+		vdd_l7-supply = <&pm8916_s4>;
 
 		/* pm8916_s1 is managed by rpmpd (MSM8939_VDDMDCX) */
 		/* pm8916_s2 is managed by rpmpd (MSM8939_VDDCX) */
-		pm8916_s3: s3 {};
-		pm8916_s4: s4 {};
+		pm8916_s3: s3 {
+			regulator-min-microvolt = <1250000>;
+			regulator-max-microvolt = <1350000>;
+		};
+		pm8916_s4: s4 {
+			regulator-min-microvolt = <1850000>;
+			regulator-max-microvolt = <2150000>;
+		};
+
+		/*
+		 * Some of the regulators are unused or managed by another
+		 * processor (e.g. the modem). We should still define nodes for
+		 * them to ensure the vote from the application processor can be
+		 * dropped in case the regulators are already on during boot.
+		 *
+		 * The labels for these nodes are omitted on purpose because
+		 * boards should configure a proper voltage before using them.
+		 */
+		l1 {};
+
+		pm8916_l2: l2 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
 
-		pm8916_l1: l1 {};
-		pm8916_l2: l2 {};
 		/* pm8916_l3 is managed by rpmpd (MSM8939_VDDMX) */
-		pm8916_l4: l4 {};
-		pm8916_l5: l5 {};
-		pm8916_l6: l6 {};
-		pm8916_l7: l7 {};
-		pm8916_l8: l8 {};
-		pm8916_l9: l9 {};
-		pm8916_l10: l10 {};
-		pm8916_l11: l11 {};
-		pm8916_l12: l12 {};
-		pm8916_l13: l13 {};
-		pm8916_l14: l14 {};
-		pm8916_l15: l15 {};
-		pm8916_l16: l16 {};
-		pm8916_l17: l17 {};
-		pm8916_l18: l18 {};
+
+		l4 {};
+
+		pm8916_l5: l5 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		pm8916_l6: l6 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		pm8916_l7: l7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		pm8916_l8: l8 {
+			regulator-min-microvolt = <2900000>;
+			regulator-max-microvolt = <2900000>;
+		};
+
+		pm8916_l9: l9 {
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+
+		l10 {};
+
+		pm8916_l11: l11 {
+			regulator-min-microvolt = <2950000>;
+			regulator-max-microvolt = <2950000>;
+			regulator-allow-set-load;
+			regulator-system-load = <200000>;
+		};
+
+		pm8916_l12: l12 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2950000>;
+		};
+
+		pm8916_l13: l13 {
+			regulator-min-microvolt = <3075000>;
+			regulator-max-microvolt = <3075000>;
+		};
+
+		l14 {};
+		l15 {};
+		l16 {};
+		l17 {};
+		l18 {};
 	};
 };
 
diff --git a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
index 2905dd8d4bf9..8613cf93dac5 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
@@ -58,104 +58,6 @@ usb_id_default: usb-id-default-state {
 	};
 };
 
-&smd_rpm_regulators {
-	vdd_l1_l2_l3-supply = <&pm8916_s3>;
-	vdd_l4_l5_l6-supply = <&pm8916_s4>;
-	vdd_l7-supply = <&pm8916_s4>;
-
-	pm8916_s3: s3 {
-		regulator-min-microvolt = <1250000>;
-		regulator-max-microvolt = <1350000>;
-	};
-
-	pm8916_s4: s4 {
-		regulator-min-microvolt = <1850000>;
-		regulator-max-microvolt = <2150000>;
-	};
-
-	pm8916_l2: l2 {
-		regulator-min-microvolt = <1200000>;
-		regulator-max-microvolt = <1200000>;
-	};
-
-	pm8916_l4: l4 {
-		regulator-min-microvolt = <2050000>;
-		regulator-max-microvolt = <2050000>;
-	};
-
-	pm8916_l5: l5 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-	};
-
-	pm8916_l6: l6 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-	};
-
-	pm8916_l7: l7 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-	};
-
-	pm8916_l8: l8 {
-		regulator-min-microvolt = <2900000>;
-		regulator-max-microvolt = <2900000>;
-	};
-
-	pm8916_l9: l9 {
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l10: l10 {
-		regulator-min-microvolt = <2800000>;
-		regulator-max-microvolt = <2800000>;
-	};
-
-	pm8916_l11: l11 {
-		regulator-min-microvolt = <2950000>;
-		regulator-max-microvolt = <2950000>;
-		regulator-system-load = <200000>;
-		regulator-allow-set-load;
-	};
-
-	pm8916_l12: l12 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <2950000>;
-	};
-
-	pm8916_l13: l13 {
-		regulator-min-microvolt = <3075000>;
-		regulator-max-microvolt = <3075000>;
-	};
-
-	pm8916_l14: l14 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l15: l15 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l16: l16 {
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	pm8916_l17: l17 {
-		regulator-min-microvolt = <2850000>;
-		regulator-max-microvolt = <2850000>;
-	};
-
-	pm8916_l18: l18 {
-		regulator-min-microvolt = <2700000>;
-		regulator-max-microvolt = <2700000>;
-	};
-};
-
 &sdhc_1 {
 	status = "okay";
 };

-- 
2.40.1

[PATCH 8/8] arm64: dts: qcom: msm8939-pm8916: Mark always-on regulators

[Stephan Gerhold]

Some of the regulators must be always-on to ensure correct operation of
the system, e.g. PM8916 L2 for the LPDDR RAM, L5 for most digital I/O
and L7 for the CPU PLL (strictly speaking the CPU PLL might only need
an active-only vote but this is not supported for regulators in
mainline currently).

The RPM firmware seems to enforce that internally, these supplies stay
on even if we vote for them to power off (and there is no other
processor running). This means it's pointless to keep sending
enable/disable requests because they will just be ignored.
Also, drivers are much more likely to get a wrong impression of the
regulator status, because regulator_is_enabled() will return false when
there are no users, even though the regulator is always on.

Describe this properly by marking the regulators as always-on.

The same changes was already made for MSM8916 in commit 8bbd35771f90
("arm64: dts: qcom: msm8916-pm8916: Mark always-on regulators").

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
index 12538211bc72..adb96cd8d643 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
@@ -51,10 +51,12 @@ pm8916_rpm_regulators: regulators {
 		pm8916_s3: s3 {
 			regulator-min-microvolt = <1250000>;
 			regulator-max-microvolt = <1350000>;
+			regulator-always-on; /* Needed for L2 */
 		};
 		pm8916_s4: s4 {
 			regulator-min-microvolt = <1850000>;
 			regulator-max-microvolt = <2150000>;
+			regulator-always-on; /* Needed for L5/L7 */
 		};
 
 		/*
@@ -71,6 +73,7 @@ pm8916_s4: s4 {
 		pm8916_l2: l2 {
 			regulator-min-microvolt = <1200000>;
 			regulator-max-microvolt = <1200000>;
+			regulator-always-on; /* Needed for LPDDR RAM */
 		};
 
 		/* pm8916_l3 is managed by rpmpd (MSM8939_VDDMX) */
@@ -80,6 +83,7 @@ pm8916_l2: l2 {
 		pm8916_l5: l5 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-always-on; /* Needed for most digital I/O */
 		};
 
 		pm8916_l6: l6 {
@@ -90,6 +94,7 @@ pm8916_l6: l6 {
 		pm8916_l7: l7 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-always-on; /* Needed for CPU PLL */
 		};
 
 		pm8916_l8: l8 {

-- 
2.40.1

Re: [PATCH 1/8] arm64: dts: qcom: msm8939-pm8916: Add missing pm8916_codec supplies

[Bryan O'Donoghue]

On 14/06/2023 08:15, Stephan Gerhold wrote:
> Update for recent changes to pm8916.dtsi in commit 38218822a72f
> ("arm64: dts: qcom: pm8916: Move default regulator "-supply"s")
> and add the now missing pm8916_codec supplies to msm8939-pm8916.dtsi
> as well.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>   arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
> index 33e02f42f5e4..503c2dd5fe47 100644
> --- a/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8939-pm8916.dtsi
> @@ -25,6 +25,12 @@ &mpss {
>   	pll-supply = <&pm8916_l7>;
>   };
>   
> +&pm8916_codec {
> +	vdd-cdc-io-supply = <&pm8916_l5>;
> +	vdd-cdc-tx-rx-cx-supply = <&pm8916_l5>;
> +	vdd-micbias-supply = <&pm8916_l13>;
> +};
> +
>   &rpm_requests {
>   	smd_rpm_regulators: regulators {
>   		compatible = "qcom,rpm-pm8916-regulators";
> 

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 2/8] arm64: dts: qcom: msm8939: Disable lpass_codec by default

[Bryan O'Donoghue]

On 14/06/2023 08:16, Stephan Gerhold wrote:
> Update for recent changes to msm8916.dtsi in commit a5cf21b14666
> ("arm64: dts: qcom: msm8916: Disable audio codecs by default") and
> make lpass_codec disabled by default for devices that are not using
> the audio codec functionality inside MSM8939.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>   arch/arm64/boot/dts/qcom/apq8039-t2.dts | 4 ++++
>   arch/arm64/boot/dts/qcom/msm8939.dtsi   | 1 +
>   2 files changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/apq8039-t2.dts b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
> index 2a39216ceef5..c8442242137a 100644
> --- a/arch/arm64/boot/dts/qcom/apq8039-t2.dts
> +++ b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
> @@ -135,6 +135,10 @@ &lpass {
>   	status = "okay";
>   };
>   
> +&lpass_codec {
> +	status = "okay";
> +};
> +
>   &mdss {
>   	status = "okay";
>   };
> diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi b/arch/arm64/boot/dts/qcom/msm8939.dtsi
> index 3c1505b69a6b..895cafc11480 100644
> --- a/arch/arm64/boot/dts/qcom/msm8939.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi
> @@ -1608,6 +1608,7 @@ lpass_codec: audio-codec@771c000 {
>   				 <&gcc GCC_CODEC_DIGCODEC_CLK>;
>   			clock-names = "ahbix-clk", "mclk";
>   			#sound-dai-cells = <1>;
> +			status = "disabled";
>   		};
>   
>   		sdhc_1: mmc@7824900 {
> 

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 3/8] arm64: dts: qcom: msm8939-sony-tulip: Fix l10-l12 regulator voltages

[Bryan O'Donoghue]

On 14/06/2023 08:16, Stephan Gerhold wrote:
> msm8939-sony-xperia-kanuti-tulip.dts has several regulator voltages
> that do not quite seem to match what is used in the vendor kernel.
> In particular:
> 
>   - l10 is fixed at 2.8V [1, 2]
>   - l11/l12 are 2.95V max [1]
> 
> [1]: https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/arch/arm/boot/dts/qcom/msm8939-regulator.dtsi
> [2]: https://github.com/sonyxperiadev/kernel/blob/aosp/LA.BR.1.3.3_rb2.14/arch/arm/boot/dts/qcom/msm8939-kanuti_tulip.dtsi#L671C1-L673
> 
> Fixes: f1134f738fad ("arm64: dts: qcom: Add msm8939 Sony Xperia M4 Aqua")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

I think I probably just copied the Square regulator setup here, so 
thanks for doing this extra work.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 4/8] arm64: dts: qcom: msm8939-sony-tulip: Allow disabling pm8916_l6

[Bryan O'Donoghue]

On 14/06/2023 08:16, Stephan Gerhold wrote:
> The vendor kernel from Sony does not have regulator-always-on for
> pm8916_l6, so we should be able to disable it when setting up the
> display properly. Since sony-tulip does not have display set up
> currently it should be fine to let the regulator disable until then.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>   arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
> index dc5b8cd5b9f4..509abcdad287 100644
> --- a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
> @@ -91,7 +91,6 @@ pm8916_l5: l5 {
>   	pm8916_l6: l6 {
>   		regulator-min-microvolt = <1800000>;
>   		regulator-max-microvolt = <1800000>;
> -		regulator-always-on;
>   	};
>   
>   	pm8916_l7: l7 {
> 

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 5/8] arm64: dts: qcom: msm8939: Fix regulator constraints

[Bryan O'Donoghue]

On 14/06/2023 08:16, Stephan Gerhold wrote:
> The regulator constraints for the MSM8939 devices were originally taken
> from Qualcomm's msm-3.10 vendor device tree (for lack of better
> documentation). Unfortunately it turns out that Qualcomm's voltages are
> slightly off as well and do not match the voltage constraints applied
> by the RPM firmware.
> 
> This means that we sometimes request a specific voltage but the RPM
> firmware actually applies a much lower or higher voltage. This is
> particularly critical for pm8916_l11 which is used as SD card VMMC
> regulator: The SD card can choose a voltage from the current range of
> 1.8 - 2.95V. If it chooses to run at 1.8V we pretend that this is fine
> but the RPM firmware will still silently end up configuring 2.95V.
> This can be easily reproduced with a multimeter or by checking the
> SPMI hardware registers of the regulator.
> 
> Apply the same change as for MSM8916 in commit 355750828c55 ("arm64:
> dts: qcom: msm8916: Fix regulator constraints") and make the voltages
> match the actual "specified range" in the PM8916 Device Specification
> which is enforced by the RPM firmware.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Hrmm.

I recently found in the schematic the following table.

Supply | Default V | Range V     | Rated I | Default On
L1       1.2875      0.375-1.525   250       N
L2       1.2         0.375.1.525   600       Y
L3       1.15        0.375-1.525   350       Y
L4       2.05        1.75-3.337    250       N
L5       1.8         1.75-3.337    200       Y
L6       1.8         1.75-3.337    150       Y
L7       1.8         1.75-3.337    110       Y
L8       2.9         1.75-3.337    400       Y
L9       3.3         1.75-3.337    600       N
L10      2.8         1.75-3.337    150       N
L11      2.95        1.75-3.337    600       Y
L12      2.95        1.75-3.337    50        Y
L13      3.075       1.75-3.337    50        Y
L14      1.8         1.75-3.337    55        N
L15      1.8         1.75-3.337    55        N
L16      1.8         1.75-3.337    55        N
L17      2.85        1.75-3.337    450       N
L18      2.7         1.75-3.337    150       N

So let me see.

L8 2.9 = true
L11 2.95 = true

S3 just says 1v3 but, I take your word for it on the multi-meter 
measurement.

S4 says 2v1

This patch looks fine

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 7/8] arm64: dts: qcom: msm8939: Define regulator constraints next to usage

[Bryan O'Donoghue]

On 14/06/2023 08:16, Stephan Gerhold wrote:
> Right now each MSM8939 device has a huge block of regulator constraints
> with allowed voltages for each regulator. For lack of better
> documentation these voltages are often copied as-is from the vendor
> device tree, without much extra thought.
> 
> Unfortunately, the voltages in the vendor device trees are often
> misleading or even wrong, e.g. because:
> 
>   - There is a large voltage range allowed and the actual voltage is
>     only set somewhere hidden in some messy vendor driver. This is often
>     the case for pm8916_{l14,l15,l16} because they have a broad range of
>     1.8-3.3V by default.
> 
>   - The voltage is actually wrong but thanks to the voltage constraints
>     in the RPM firmware it still ends up applying the correct voltage.
> 
> To have proper regulator constraints it is important to review them in
> context of the usage. The current setup in the MSM8939 device trees
> makes this quite hard because each device duplicates the standard
> voltages for components of the SoC and mixes those with minor
> device-specific additions and dummy voltages for completely unused
> regulators.
> 
> The actual usage of the regulators for the SoC components is in
> msm8939-pm8916.dtsi, so it can and should also define the related
> voltage constraints. These are not board-specific but defined in the
> MSM8939/PM8916 specification. There is no documentation available for
> MSM8939 but in practice it's almost identical to MSM8916.
> 
> Note that this commit does not make any functional change. All used
> regulators still have the same regulator constraints as before. Unused
> regulators do not have regulator constraints anymore because most of
> these were too broad or even entirely wrong. They should be added back
> with proper voltage constraints when there is an actual usage.
> 
> The same changes were already made for MSM8916 in commit b0a8f16ae4a0
> ("arm64: dts: qcom: msm8916: Define regulator constraints next to usage").
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>


It makes sense to replicate the 8916 change.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 0/8] arm64: dts: qcom: msm8939: Rework regulator constraints

[Bjorn Andersson]

On Wed, 14 Jun 2023 09:15:58 +0200, Stephan Gerhold wrote:
> Rework the regulator constraints for the MSM8939 device trees to be
> closer to reality. There are several mistakes in there, some of them
> taken over directly from Qualcomm's vendor kernel. Fortunately, none of
> the mistakes is absolutely critical because it turns out that the RPM
> firmware also validates the voltages and silently clamps the requests
> to a proper range. Still, this behavior should be clearly represented
> in the device tree rather than pretending to apply the wrong voltages.
> 
> [...]

Applied, thanks!

[1/8] arm64: dts: qcom: msm8939-pm8916: Add missing pm8916_codec supplies
      commit: dce9254511d6c9ea0d5ed7e4f21e6206e2ca35ce
[2/8] arm64: dts: qcom: msm8939: Disable lpass_codec by default
      commit: 6002a78023cded6f02eac7c812b076046cab8060
[3/8] arm64: dts: qcom: msm8939-sony-tulip: Fix l10-l12 regulator voltages
      commit: 209aea1ad505519faf018b596e4fdca0d0569469
[4/8] arm64: dts: qcom: msm8939-sony-tulip: Allow disabling pm8916_l6
      commit: 8771308c91cefc072f36415cec0b802ee55b1d96
[5/8] arm64: dts: qcom: msm8939: Fix regulator constraints
      commit: 9187d555c4ba9544c7f117062d241aa085f59a06
[6/8] arm64: dts: qcom: msm8939-pm8916: Clarify purpose
      commit: 88028fa047fb72826dd206b51550be780777718c
[7/8] arm64: dts: qcom: msm8939: Define regulator constraints next to usage
      commit: 5cdab9a8c70c4d979909dd1bb6d1f3eacd9fa270
[8/8] arm64: dts: qcom: msm8939-pm8916: Mark always-on regulators
      commit: ecbfba694b5baf2b854689c63ef011e905810c59

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>