diff mbox series

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

Message ID 20230530-msm8939-regulators-v1-7-a3c3ac833567@gerhold.net (mailing list archive)
State Accepted
Commit 5cdab9a8c70c4d979909dd1bb6d1f3eacd9fa270
Headers show
Series arm64: dts: qcom: msm8939: Rework regulator constraints | expand

Commit Message

Stephan Gerhold June 14, 2023, 7:16 a.m. UTC 
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(-)

Comments

Bryan O'Donoghue June 14, 2023, 12:51 p.m. UTC | #1 
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>
diff mbox series

Patch

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