diff mbox series

[V2,3/5] arm64: dts: qcom: sc7280: Add RPMh regulators for sc7280-idp

Message ID 1615816454-1733-4-git-send-email-skakit@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Add PM7325/PM8350C/PMR735A regulator support | expand

Commit Message

Satya Priya March 15, 2021, 1:54 p.m. UTC
Add regulator devices for SC7280 as RPMh regulators. This ensures
that consumers are able to modify the physical state of PMIC
regulators.

Signed-off-by: satya priya <skakit@codeaurora.org>
---
Changes in V2:
 - Corrected the indentation for "compatible" and "qcom,pmic-id" under
   pm8350c-regulators as per Konrad's comment.

 arch/arm64/boot/dts/qcom/sc7280-idp.dts | 212 ++++++++++++++++++++++++++++++++
 1 file changed, 212 insertions(+)

Comments

Matthias Kaehlcke March 16, 2021, 11:26 p.m. UTC | #1
On Mon, Mar 15, 2021 at 07:24:12PM +0530, satya priya wrote:
> Add regulator devices for SC7280 as RPMh regulators. This ensures
> that consumers are able to modify the physical state of PMIC
> regulators.
> 
> Signed-off-by: satya priya <skakit@codeaurora.org>
> ---
> Changes in V2:
>  - Corrected the indentation for "compatible" and "qcom,pmic-id" under
>    pm8350c-regulators as per Konrad's comment.
> 
>  arch/arm64/boot/dts/qcom/sc7280-idp.dts | 212 ++++++++++++++++++++++++++++++++
>  1 file changed, 212 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> index 428f863..78effe5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> @@ -22,6 +22,218 @@
>  	};
>  };
>  
> +&apps_rsc {
> +	pm7325-regulators {
> +		compatible = "qcom,pm7325-rpmh-regulators";
> +		qcom,pmic-id = "b";
> +
> +		vreg_s1b_1p8: smps1 {
> +			regulator-min-microvolt = <1856000>;

For most LDOs their 'Active minimum voltage' is specified as their
minimum, however for S1B and S8B it's the 'Nominal voltage. Is that
intentional?

There might be a misunderstanding on my side what the values in the
datasheet actually mean, see my comment at the end.

> +			regulator-max-microvolt = <2040000>;
> +		};
> +
> +		vreg_s7b_0p9: smps7 {
> +			regulator-min-microvolt = <535000>;

According to the datasheet the minimum voltage of the S7B regulator
is 904 mV.

> +			regulator-max-microvolt = <1120000>;
> +		};
> +
> +		vreg_s8b_1p2: smps8 {
> +			regulator-min-microvolt = <1256000>;
> +			regulator-max-microvolt = <1500000>;
> +		};
> +
> +		vreg_l1b_0p8: ldo1 {
> +			regulator-min-microvolt = <825000>;
> +			regulator-max-microvolt = <925000>;
> +		};
> +
> +		vreg_l2b_3p0: ldo2 {
> +			regulator-min-microvolt = <2700000>;
> +			regulator-max-microvolt = <3544000>;
> +		};

Another question that came up for sc7180-trogdor regulators,
whose core regulator config was derived from sc7180-idp: the
label suggests that this regulator is supposed to supply 3V,
however the range spans from 2.7 to 3.54V. Shouldn't it be
narrower around 3V? Same for other some regulators.

> +
> +		vreg_l6b_1p2: ldo6 {
> +			regulator-min-microvolt = <1140000>;

The datasheet says the minimum for L6B is 1.2V.

> +			regulator-max-microvolt = <1260000>;
> +		};
> +
> +		vreg_l7b_2p9: ldo7 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2960000>;
> +		};

This regulator has a fixed voltage in difference to the others, why
is that?

> +
> +		vreg_l8b_0p9: ldo8 {
> +			regulator-min-microvolt = <870000>;
> +			regulator-max-microvolt = <970000>;
> +		};
> +
> +		vreg_l9b_1p2: ldo9 {
> +			regulator-min-microvolt = <1080000>;
> +			regulator-max-microvolt = <1304000>;
> +		};
> +
> +		vreg_l11b_1p7: ldo11 {
> +			regulator-min-microvolt = <1504000>;

The datasheet says the mininum voltage for L11B is 1.776V.

> +			regulator-max-microvolt = <2000000>;
> +		};
> +
> +		vreg_l12b_0p8: ldo12 {
> +			regulator-min-microvolt = <751000>;
> +			regulator-max-microvolt = <824000>;
> +		};
> +
> +		vreg_l13b_0p8: ldo13 {
> +			regulator-min-microvolt = <530000>;
> +			regulator-max-microvolt = <824000>;

The max for L13B is 880mV, is this a copy and paste from L12B?

> +		};
> +
> +		vreg_l14b_1p2: ldo14 {
> +			regulator-min-microvolt = <1080000>;

The datasheet says the mininum voltage for L14B is 1.2V.

> +			regulator-max-microvolt = <1304000>;
> +		};
> +
> +		vreg_l15b_0p8: ldo15 {
> +			regulator-min-microvolt = <765000>;
> +			regulator-max-microvolt = <1020000>;
> +		};
> +
> +		vreg_l16b_1p2: ldo16 {
> +			regulator-min-microvolt = <1100000>;

The datasheet says the mininum voltage for L16B is 1.2V.

> +			regulator-max-microvolt = <1300000>;
> +		};
> +
> +		vreg_l17b_1p8: ldo17 {
> +			regulator-min-microvolt = <1700000>;

The datasheet says the mininum voltage for L17B is 1.8V.

> +			regulator-max-microvolt = <1900000>;
> +		};
> +
> +		vreg_l18b_1p8: ldo18 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <2000000>;
> +		};
> +
> +		vreg_l19b_1p8: ldo19 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;

Is a fixed voltage intentional here?

> +		};
> +	};
> +
> +	pm8350c-regulators {
> +		compatible = "qcom,pm8350c-rpmh-regulators";

I can't find the datasheet for this chip, skipping this part.


> +	pmr735a-regulators {
> +		compatible = "qcom,pmr735a-rpmh-regulators";
> +		qcom,pmic-id = "e";
> +
> +		vreg_l2e_1p2: ldo2 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +		};
> +
> +		vreg_l3e_0p9: ldo3 {
> +			regulator-min-microvolt = <912000>;
> +			regulator-max-microvolt = <1020000>;

According to the datasheet min and max for L3E is 1.2V. The
datasheet lists different voltages for 'SM8350 lineup' and
'SM8xyz' lineup though, does that mean that the voltages
aren't limitations of what the regulators can provide but
what their consumers support?

There are also deltas for the remaining regulators, but now
I'm in doubt about what the info in the datasheet actually
means.

> +		};
> +
> +		vreg_l4e_1p7: ldo4 {
> +			regulator-min-microvolt = <1776000>;
> +			regulator-max-microvolt = <1890000>;
> +		};
> +
> +		vreg_l5e_0p8: ldo5 {
> +			regulator-min-microvolt = <800000>;
> +			regulator-max-microvolt = <800000>;
> +		};
> +
> +		vreg_l6e_0p8: ldo6 {
> +			regulator-min-microvolt = <480000>;
> +			regulator-max-microvolt = <904000>;
> +		};
> +	};
> +};
> +
>  &qupv3_id_0 {
>  	status = "okay";
>  };
Satya Priya March 19, 2021, 8 a.m. UTC | #2
On 2021-03-17 04:56, Matthias Kaehlcke wrote:
> On Mon, Mar 15, 2021 at 07:24:12PM +0530, satya priya wrote:
>> Add regulator devices for SC7280 as RPMh regulators. This ensures
>> that consumers are able to modify the physical state of PMIC
>> regulators.
>> 
>> Signed-off-by: satya priya <skakit@codeaurora.org>
>> ---
>> Changes in V2:
>>  - Corrected the indentation for "compatible" and "qcom,pmic-id" under
>>    pm8350c-regulators as per Konrad's comment.
>> 
>>  arch/arm64/boot/dts/qcom/sc7280-idp.dts | 212 
>> ++++++++++++++++++++++++++++++++
>>  1 file changed, 212 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> index 428f863..78effe5 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> @@ -22,6 +22,218 @@
>>  	};
>>  };
>> 
>> +&apps_rsc {
>> +	pm7325-regulators {
>> +		compatible = "qcom,pm7325-rpmh-regulators";
>> +		qcom,pmic-id = "b";
>> +
>> +		vreg_s1b_1p8: smps1 {
>> +			regulator-min-microvolt = <1856000>;
> 
> For most LDOs their 'Active minimum voltage' is specified as their
> minimum, however for S1B and S8B it's the 'Nominal voltage. Is that
> intentional?
> 
> There might be a misunderstanding on my side what the values in the
> datasheet actually mean, see my comment at the end.
> 
>> +			regulator-max-microvolt = <2040000>;
>> +		};
>> +
>> +		vreg_s7b_0p9: smps7 {
>> +			regulator-min-microvolt = <535000>;
> 
> According to the datasheet the minimum voltage of the S7B regulator
> is 904 mV.
> 
>> +			regulator-max-microvolt = <1120000>;
>> +		};
>> +
>> +		vreg_s8b_1p2: smps8 {
>> +			regulator-min-microvolt = <1256000>;
>> +			regulator-max-microvolt = <1500000>;
>> +		};
>> +
>> +		vreg_l1b_0p8: ldo1 {
>> +			regulator-min-microvolt = <825000>;
>> +			regulator-max-microvolt = <925000>;
>> +		};
>> +
>> +		vreg_l2b_3p0: ldo2 {
>> +			regulator-min-microvolt = <2700000>;
>> +			regulator-max-microvolt = <3544000>;
>> +		};
> 
> Another question that came up for sc7180-trogdor regulators,
> whose core regulator config was derived from sc7180-idp: the
> label suggests that this regulator is supposed to supply 3V,
> however the range spans from 2.7 to 3.54V. Shouldn't it be
> narrower around 3V? Same for other some regulators.
> 

The label names are given based on the default voltage value(a typical 
value supported by any usecase) which is specified in the Powergrid. For 
this regulator the default voltage is 3.072V, whereas the range is 2.7 
to 3.5V

>> +
>> +		vreg_l6b_1p2: ldo6 {
>> +			regulator-min-microvolt = <1140000>;
> 
> The datasheet says the minimum for L6B is 1.2V.
> 
>> +			regulator-max-microvolt = <1260000>;
>> +		};
>> +
>> +		vreg_l7b_2p9: ldo7 {
>> +			regulator-min-microvolt = <2960000>;
>> +			regulator-max-microvolt = <2960000>;
>> +		};
> 
> This regulator has a fixed voltage in difference to the others, why
> is that?
> 

L7B and L19B regulators are used for EMMC. EMMC probe is failing if the 
value is not fixed to default voltage(2.96V for L7B). Similar issue was 
seen on Rennell as well, [1] is the upstream change for the same posted 
by storage team.

[1] https://lore.kernel.org/patchwork/patch/1176993/

>> +
>> +		vreg_l8b_0p9: ldo8 {
>> +			regulator-min-microvolt = <870000>;
>> +			regulator-max-microvolt = <970000>;
>> +		};
>> +
>> +		vreg_l9b_1p2: ldo9 {
>> +			regulator-min-microvolt = <1080000>;
>> +			regulator-max-microvolt = <1304000>;
>> +		};
>> +
>> +		vreg_l11b_1p7: ldo11 {
>> +			regulator-min-microvolt = <1504000>;
> 
> The datasheet says the mininum voltage for L11B is 1.776V.
> 
>> +			regulator-max-microvolt = <2000000>;
>> +		};
>> +
>> +		vreg_l12b_0p8: ldo12 {
>> +			regulator-min-microvolt = <751000>;
>> +			regulator-max-microvolt = <824000>;
>> +		};
>> +
>> +		vreg_l13b_0p8: ldo13 {
>> +			regulator-min-microvolt = <530000>;
>> +			regulator-max-microvolt = <824000>;
> 
> The max for L13B is 880mV, is this a copy and paste from L12B?
> 
>> +		};
>> +
>> +		vreg_l14b_1p2: ldo14 {
>> +			regulator-min-microvolt = <1080000>;
> 
> The datasheet says the mininum voltage for L14B is 1.2V.
> 
>> +			regulator-max-microvolt = <1304000>;
>> +		};
>> +
>> +		vreg_l15b_0p8: ldo15 {
>> +			regulator-min-microvolt = <765000>;
>> +			regulator-max-microvolt = <1020000>;
>> +		};
>> +
>> +		vreg_l16b_1p2: ldo16 {
>> +			regulator-min-microvolt = <1100000>;
> 
> The datasheet says the mininum voltage for L16B is 1.2V.
> 
>> +			regulator-max-microvolt = <1300000>;
>> +		};
>> +
>> +		vreg_l17b_1p8: ldo17 {
>> +			regulator-min-microvolt = <1700000>;
> 
> The datasheet says the mininum voltage for L17B is 1.8V.
> 
>> +			regulator-max-microvolt = <1900000>;
>> +		};
>> +
>> +		vreg_l18b_1p8: ldo18 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <2000000>;
>> +		};
>> +
>> +		vreg_l19b_1p8: ldo19 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
> 
> Is a fixed voltage intentional here?
> 

Same explanation as l7b regulator(see above).

>> +		};
>> +	};
>> +
>> +	pm8350c-regulators {
>> +		compatible = "qcom,pm8350c-rpmh-regulators";
> 
> I can't find the datasheet for this chip, skipping this part.
> 
> 
>> +	pmr735a-regulators {
>> +		compatible = "qcom,pmr735a-rpmh-regulators";
>> +		qcom,pmic-id = "e";
>> +
>> +		vreg_l2e_1p2: ldo2 {
>> +			regulator-min-microvolt = <1200000>;
>> +			regulator-max-microvolt = <1200000>;
>> +		};
>> +
>> +		vreg_l3e_0p9: ldo3 {
>> +			regulator-min-microvolt = <912000>;
>> +			regulator-max-microvolt = <1020000>;
> 
> According to the datasheet min and max for L3E is 1.2V. The
> datasheet lists different voltages for 'SM8350 lineup' and
> 'SM8xyz' lineup though, does that mean that the voltages
> aren't limitations of what the regulators can provide but
> what their consumers support?
> 
> There are also deltas for the remaining regulators, but now
> I'm in doubt about what the info in the datasheet actually
> means.
> 

The min/max voltages here are as per min/max values mentioned in AOP, 
which are finalized after multiple discussions with chipset architecture 
team. The same is followed for all chipsets.

>> +		};
>> +
>> +		vreg_l4e_1p7: ldo4 {
>> +			regulator-min-microvolt = <1776000>;
>> +			regulator-max-microvolt = <1890000>;
>> +		};
>> +
>> +		vreg_l5e_0p8: ldo5 {
>> +			regulator-min-microvolt = <800000>;
>> +			regulator-max-microvolt = <800000>;
>> +		};
>> +
>> +		vreg_l6e_0p8: ldo6 {
>> +			regulator-min-microvolt = <480000>;
>> +			regulator-max-microvolt = <904000>;
>> +		};
>> +	};
>> +};
>> +
>>  &qupv3_id_0 {
>>  	status = "okay";
>>  };
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 428f863..78effe5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -22,6 +22,218 @@ 
 	};
 };
 
+&apps_rsc {
+	pm7325-regulators {
+		compatible = "qcom,pm7325-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vreg_s1b_1p8: smps1 {
+			regulator-min-microvolt = <1856000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		vreg_s7b_0p9: smps7 {
+			regulator-min-microvolt = <535000>;
+			regulator-max-microvolt = <1120000>;
+		};
+
+		vreg_s8b_1p2: smps8 {
+			regulator-min-microvolt = <1256000>;
+			regulator-max-microvolt = <1500000>;
+		};
+
+		vreg_l1b_0p8: ldo1 {
+			regulator-min-microvolt = <825000>;
+			regulator-max-microvolt = <925000>;
+		};
+
+		vreg_l2b_3p0: ldo2 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+		};
+
+		vreg_l6b_1p2: ldo6 {
+			regulator-min-microvolt = <1140000>;
+			regulator-max-microvolt = <1260000>;
+		};
+
+		vreg_l7b_2p9: ldo7 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+		};
+
+		vreg_l8b_0p9: ldo8 {
+			regulator-min-microvolt = <870000>;
+			regulator-max-microvolt = <970000>;
+		};
+
+		vreg_l9b_1p2: ldo9 {
+			regulator-min-microvolt = <1080000>;
+			regulator-max-microvolt = <1304000>;
+		};
+
+		vreg_l11b_1p7: ldo11 {
+			regulator-min-microvolt = <1504000>;
+			regulator-max-microvolt = <2000000>;
+		};
+
+		vreg_l12b_0p8: ldo12 {
+			regulator-min-microvolt = <751000>;
+			regulator-max-microvolt = <824000>;
+		};
+
+		vreg_l13b_0p8: ldo13 {
+			regulator-min-microvolt = <530000>;
+			regulator-max-microvolt = <824000>;
+		};
+
+		vreg_l14b_1p2: ldo14 {
+			regulator-min-microvolt = <1080000>;
+			regulator-max-microvolt = <1304000>;
+		};
+
+		vreg_l15b_0p8: ldo15 {
+			regulator-min-microvolt = <765000>;
+			regulator-max-microvolt = <1020000>;
+		};
+
+		vreg_l16b_1p2: ldo16 {
+			regulator-min-microvolt = <1100000>;
+			regulator-max-microvolt = <1300000>;
+		};
+
+		vreg_l17b_1p8: ldo17 {
+			regulator-min-microvolt = <1700000>;
+			regulator-max-microvolt = <1900000>;
+		};
+
+		vreg_l18b_1p8: ldo18 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2000000>;
+		};
+
+		vreg_l19b_1p8: ldo19 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+	};
+
+	pm8350c-regulators {
+		compatible = "qcom,pm8350c-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		vreg_s1c_2p2: smps1 {
+			regulator-min-microvolt = <2190000>;
+			regulator-max-microvolt = <2210000>;
+		};
+
+		vreg_s9c_1p0: smps9 {
+			regulator-min-microvolt = <1010000>;
+			regulator-max-microvolt = <1170000>;
+		};
+
+		vreg_l1c_1p8: ldo1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1980000>;
+		};
+
+		vreg_l2c_1p8: ldo2 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <1980000>;
+		};
+
+		vreg_l3c_3p0: ldo3 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3540000>;
+		};
+
+		vreg_l4c_1p8: ldo4 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <3300000>;
+		};
+
+		vreg_l5c_1p8: ldo5 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <3300000>;
+		};
+
+		vreg_l6c_2p9: ldo6 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2950000>;
+		};
+
+		vreg_l7c_3p0: ldo7 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3544000>;
+		};
+
+		vreg_l8c_1p8: ldo8 {
+			regulator-min-microvolt = <1620000>;
+			regulator-max-microvolt = <2000000>;
+		};
+
+		vreg_l9c_2p9: ldo9 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2960000>;
+		};
+
+		vreg_l10c_0p8: ldo10 {
+			regulator-min-microvolt = <720000>;
+			regulator-max-microvolt = <1050000>;
+		};
+
+		vreg_l11c_2p8: ldo11 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3544000>;
+		};
+
+		vreg_l12c_1p8: ldo12 {
+			regulator-min-microvolt = <1650000>;
+			regulator-max-microvolt = <2000000>;
+		};
+
+		vreg_l13c_3p0: ldo13 {
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3544000>;
+		};
+
+		vreg_bob: bob {
+			regulator-min-microvolt = <3008000>;
+			regulator-max-microvolt = <3960000>;
+		};
+	};
+
+	pmr735a-regulators {
+		compatible = "qcom,pmr735a-rpmh-regulators";
+		qcom,pmic-id = "e";
+
+		vreg_l2e_1p2: ldo2 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+
+		vreg_l3e_0p9: ldo3 {
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <1020000>;
+		};
+
+		vreg_l4e_1p7: ldo4 {
+			regulator-min-microvolt = <1776000>;
+			regulator-max-microvolt = <1890000>;
+		};
+
+		vreg_l5e_0p8: ldo5 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <800000>;
+		};
+
+		vreg_l6e_0p8: ldo6 {
+			regulator-min-microvolt = <480000>;
+			regulator-max-microvolt = <904000>;
+		};
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };