2023-05-17 18:52:51

by Stephan Gerhold

[permalink] [raw]
Subject: [PATCH 3/8] arm64: dts: qcom: msm8916: Fix regulator constraints

The regulator constraints for most MSM8916 devices (except DB410c) 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.

Fix this by making the voltages match the actual "specified range" in
the PM8916 Device Specification which is enforced by the RPM firmware.

Signed-off-by: Stephan Gerhold <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts | 12 ++++++------
arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi | 14 +++++++-------
arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts | 12 ++++++------
13 files changed, 89 insertions(+), 89 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts b/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
index 13cd9ad167df..0d517804e44e 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
@@ -159,13 +159,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -199,7 +199,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -209,12 +209,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-system-load = <200000>;
regulator-allow-set-load;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
index fecb69944cfa..ddd64cc46998 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
@@ -201,13 +201,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -241,7 +241,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -251,12 +251,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts b/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
index 91284a1d0966..982457503a3c 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
@@ -169,13 +169,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -209,7 +209,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -219,12 +219,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts b/arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts
index 525ec76efeeb..9584d271c526 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts
@@ -159,13 +159,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -199,7 +199,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -209,12 +209,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-system-load = <200000>;
regulator-allow-set-load;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts b/arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts
index 5b1bac8f5122..baa7bb86cdd5 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts
@@ -322,13 +322,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -372,12 +372,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
index f1dd625e1822..68d1b76aaf77 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
@@ -273,13 +273,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -313,7 +313,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -323,12 +323,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
index b79e80913af9..5ef51d3e9098 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
@@ -140,13 +140,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -180,7 +180,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -190,12 +190,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
index 16d67749960e..b362a76eebc9 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
@@ -285,13 +285,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -325,7 +325,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -335,12 +335,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
index 74ffd04db8d8..4464beeeaab1 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
@@ -168,13 +168,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -208,7 +208,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -218,12 +218,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-system-load = <200000>;
regulator-allow-set-load;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
index adeee0830e76..6e231e92e675 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
@@ -134,13 +134,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -174,7 +174,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -184,12 +184,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
index 1a41a4db874d..fa5b330aaeae 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
@@ -326,13 +326,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -366,7 +366,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -376,12 +376,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-allow-set-load;
regulator-system-load = <200000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
index 50bae6f214f1..b27896e83a0e 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
@@ -132,13 +132,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -172,7 +172,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -182,12 +182,12 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};

l11 {
- regulator-min-microvolt = <1800000>;
+ regulator-min-microvolt = <2950000>;
regulator-max-microvolt = <2950000>;
regulator-system-load = <200000>;
regulator-allow-set-load;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts b/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
index ac56c7595f78..78020a0db4e4 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
@@ -194,13 +194,13 @@ &smd_rpm_regulators {
vdd_l7-supply = <&pm8916_s4>;

s3 {
- regulator-min-microvolt = <1200000>;
- regulator-max-microvolt = <1300000>;
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <1350000>;
};

s4 {
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <2100000>;
+ regulator-min-microvolt = <1850000>;
+ regulator-max-microvolt = <2150000>;
};

l1 {
@@ -234,7 +234,7 @@ l7 {
};

l8 {
- regulator-min-microvolt = <2850000>;
+ regulator-min-microvolt = <2900000>;
regulator-max-microvolt = <2900000>;
};

@@ -244,7 +244,7 @@ l9 {
};

l10 {
- regulator-min-microvolt = <2700000>;
+ regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
};


--
2.40.1



2023-05-26 13:56:21

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 3/8] arm64: dts: qcom: msm8916: Fix regulator constraints

On 17/05/2023 19:48, Stephan Gerhold wrote:
> The regulator constraints for most MSM8916 devices (except DB410c) 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.
>
> Fix this by making the voltages match the actual "specified range" in
> the PM8916 Device Specification which is enforced by the RPM firmware.
>
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts | 12 ++++++------
> arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts | 12 ++++++------
> 13 files changed, 89 insertions(+), 89 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts b/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
> index 13cd9ad167df..0d517804e44e 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
> @@ -159,13 +159,13 @@ &smd_rpm_regulators {
> vdd_l7-supply = <&pm8916_s4>;
>
> s3 {
> - regulator-min-microvolt = <1200000>;
> - regulator-max-microvolt = <1300000>;
> + regulator-min-microvolt = <1250000>;
> + regulator-max-microvolt = <1350000>;

Where are you getting these 5s from ?

> };
>
> s4 {
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <2100000>;
> + regulator-min-microvolt = <1850000>;
> + regulator-max-microvolt = <2150000>;
> };
>
> l1 {
> @@ -199,7 +199,7 @@ l7 {
> };
>
> l8 {
> - regulator-min-microvolt = <2850000>;
> + regulator-min-microvolt = <2900000>;
> regulator-max-microvolt = <2900000>;
> };
>
> @@ -209,12 +209,12 @@ l9 {
> };
>
> l10 {
> - regulator-min-microvolt = <2700000>;
> + regulator-min-microvolt = <2800000>;
> regulator-max-microvolt = <2800000>;
> };
>
> l11 {
> - regulator-min-microvolt = <1800000>;
> + regulator-min-microvolt = <2950000>;

Wouldn't 1v8 be the right voltage for eMMC !SD though have you tested
eMMC instead of SD ?

---
bod


2023-05-26 14:08:40

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 3/8] arm64: dts: qcom: msm8916: Fix regulator constraints

On Fri, May 26, 2023 at 02:38:01PM +0100, Bryan O'Donoghue wrote:
> On 17/05/2023 19:48, Stephan Gerhold wrote:
> > The regulator constraints for most MSM8916 devices (except DB410c) 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.
> >
> > Fix this by making the voltages match the actual "specified range" in
> > the PM8916 Device Specification which is enforced by the RPM firmware.
> >
> > Signed-off-by: Stephan Gerhold <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts | 12 ++++++------
> > arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi | 14 +++++++-------
> > arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts | 12 ++++++------
> > 13 files changed, 89 insertions(+), 89 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts b/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
> > index 13cd9ad167df..0d517804e44e 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts
> > @@ -159,13 +159,13 @@ &smd_rpm_regulators {
> > vdd_l7-supply = <&pm8916_s4>;
> > s3 {
> > - regulator-min-microvolt = <1200000>;
> > - regulator-max-microvolt = <1300000>;
> > + regulator-min-microvolt = <1250000>;
> > + regulator-max-microvolt = <1350000>;
>
> Where are you getting these 5s from ?
>

I have two explanations for this, one is "documentation", the other is
"experimental testing". You can choose the one you find more convincing. :)

### Documentation ###

For documentation, the S3 range is defined in "PM8916/PM8916-1 Power
Management ICs - Device Specification - LM80-P0436-35 Rev.C - Table 3-16
Regulator high-level summary". The "Specified Range" for this regulator
is 1.25–1.35V.

Also, if you look at typical schematics (e.g. DB410c) you can see that
PM8916 S3 supplies the VDD_xxx_1P3 rails of the WCN3620/WCN3660/WCN3680
chip. Looking at the defined "Operating conditions" in the datasheets of
those they define "Min: 1.25V, Typ: 1.3V, Max: 1.38V". As such, 1.2V is
not even a valid voltage for the actual usage of this regulator.

### Experimental Testing ###

The reason why it still works in practice is that the RPM firmware does
not let you apply invalid voltages here. With experimental testing
I observed that it keeps the voltage always in the "Specified Range"
of 1.25–1.35V.

You can reproduce this easily by adding the SPMI regulators in
pm8916.dtsi. These represent the actual regulator hardware registers:

diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
index 864bb1cd68db..22ab0f59be4a 100644
--- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
@@ -177,5 +177,9 @@ wcd_codec: audio-codec@f000 {
#sound-dai-cells = <1>;
status = "disabled";
};
+
+ regulators {
+ compatible = "qcom,pm8916-regulators";
+ };
};
};

With this each of the regulators will show up twice in /sys/class/regulator.
Once the RPM version and once the SPMI version. If you check the
/sys/class/regulator/.../microvolts of the SPMI variant you can see the
actual voltage that was applied by the RPM firmware.

For example, if you think that 1.2V is really possible then try:

s3 {
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
};

In sysfs you will almost certainly see that the SPMI regulator is
actually configured with 1.25V by the RPM firmware. It does not allow
setting anything lower.

> > };
> > s4 {
> > - regulator-min-microvolt = <1800000>;
> > - regulator-max-microvolt = <2100000>;
> > + regulator-min-microvolt = <1850000>;
> > + regulator-max-microvolt = <2150000>;
> > };
> > l1 {
> > @@ -199,7 +199,7 @@ l7 {
> > };
> > l8 {
> > - regulator-min-microvolt = <2850000>;
> > + regulator-min-microvolt = <2900000>;
> > regulator-max-microvolt = <2900000>;
> > };
> > @@ -209,12 +209,12 @@ l9 {
> > };
> > l10 {
> > - regulator-min-microvolt = <2700000>;
> > + regulator-min-microvolt = <2800000>;
> > regulator-max-microvolt = <2800000>;
> > };
> > l11 {
> > - regulator-min-microvolt = <1800000>;
> > + regulator-min-microvolt = <2950000>;
>
> Wouldn't 1v8 be the right voltage for eMMC !SD though have you tested eMMC
> instead of SD ?
>

This is the supply voltage (not I/O voltage) which is also 2.9V for the
eMMC typically. But the point here is that only 2.95V can be set for
this regulator on most RPM firmware versions.

It does not matter what you connect there. I tried setting

l11 {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
};

but the RPM firmware still applies 2.95V. In this case I even verified
this with a multimeter. The regulator is always 2.95V, even if you ask
the RPM firmware to apply 1.8V.

Thanks,
Stephan

2023-05-26 15:55:18

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 3/8] arm64: dts: qcom: msm8916: Fix regulator constraints

On 17/05/2023 19:48, Stephan Gerhold wrote:
> The regulator constraints for most MSM8916 devices (except DB410c) 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.
>
> Fix this by making the voltages match the actual "specified range" in
> the PM8916 Device Specification which is enforced by the RPM firmware.
>
> Signed-off-by: Stephan Gerhold <[email protected]>

Reviewed-by: Bryan O'Donoghue <[email protected]>


2023-05-26 16:27:41

by Bryan O'Donoghue

[permalink] [raw]
Subject: Re: [PATCH 3/8] arm64: dts: qcom: msm8916: Fix regulator constraints

On 17/05/2023 19:48, Stephan Gerhold wrote:
> The regulator constraints for most MSM8916 devices (except DB410c) 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.
>
> Fix this by making the voltages match the actual "specified range" in
> the PM8916 Device Specification which is enforced by the RPM firmware.
>
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8916-acer-a1-724.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-gplus-fl8005a.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-huawei-g7.dts | 12 ++++++------
> arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi | 14 +++++++-------
> arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts | 12 ++++++------
> 13 files changed, 89 insertions(+), 89 deletions(-)

Reviewed-by: Bryan O'Donoghue <[email protected]>