diff mbox series

arm64: dts: qcom: sdm845-mtp: pm8998 and pmi8998 regulators

Message ID 20180901221953.21666-1-bjorn.andersson@linaro.org
State Under Review
Delegated to: Andy Gross
Headers show
Series arm64: dts: qcom: sdm845-mtp: pm8998 and pmi8998 regulators | expand

Commit Message

Bjorn Andersson Sept. 1, 2018, 10:19 p.m. UTC
Add regulator definitions for pm8998 and pmi8998 regulators on the MTP.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 216 ++++++++++++++++++++++++
 1 file changed, 216 insertions(+)

Comments

Doug Anderson Sept. 6, 2018, 5:52 p.m. UTC | #1
Bjorn,

On Sat, Sep 1, 2018 at 3:19 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> Add regulator definitions for pm8998 and pmi8998 regulators on the MTP.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 216 ++++++++++++++++++++++++
>  1 file changed, 216 insertions(+)

I'm curious why you chose to post this instead of reviewing and/or
building upon the patch I sent up at
<https://lkml.kernel.org/r/20180822173629.202885-3-dianders@chromium.org>.
Compared to mine, yours:

* Seems to have a few rails named differently LDO22 is named
"vreg_l22a_2p95" in your DTS but "vreg_l22a_2p85" in mine as one
example.  The schematics I have (from Dec 5, 2017) show it as 2p85.

* Is missing "regulator-initial-mode".  We can debate this if you want.

* Is lacking many alternate names for rails.  We can debate this also
if you want.

* Have a few voltage values different.  If you have better info than
me we should update to yours.  Diffing against yours does make me
believe that perhaps LDO14 should be listed as 1.88 V in my patch.

* Is lacking pm8005.

Anyway, let me know.  If you provide a review of my patch I'm happy to
spin it with your feedback.


Thanks!

-Doug
Bjorn Andersson Sept. 6, 2018, 8:12 p.m. UTC | #2
On Thu 06 Sep 10:52 PDT 2018, Doug Anderson wrote:

> Bjorn,
> 
> On Sat, Sep 1, 2018 at 3:19 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > Add regulator definitions for pm8998 and pmi8998 regulators on the MTP.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 216 ++++++++++++++++++++++++
> >  1 file changed, 216 insertions(+)
> 
> I'm curious why you chose to post this instead of reviewing and/or
> building upon the patch I sent up at
> <https://lkml.kernel.org/r/20180822173629.202885-3-dianders@chromium.org>.

Sorry about that, my search skills failed me.

> Compared to mine, yours:
> 
> * Seems to have a few rails named differently LDO22 is named
> "vreg_l22a_2p95" in your DTS but "vreg_l22a_2p85" in mine as one
> example.  The schematics I have (from Dec 5, 2017) show it as 2p85.
> 

Looks like a typo on my part, yours matches the schematics I have as
well.

> * Is missing "regulator-initial-mode".  We can debate this if you want.
> 

Either we start by making them all HPM or we tune as we debug, I'm fine
with your suggestion.

> * Is lacking many alternate names for rails.  We can debate this also
> if you want.
> 

Afaict names such as "vdda_pcie_1p2" is the name of the pin on the
SDM845, while the thing that comes out of the regulator is named
vreg_l26a_1p2.

So I believe this name should be used in the pcie node as:

  vdda_pcie_1p2-supply = <&vreg_l26a_1p2>;

> * Have a few voltage values different.  If you have better info than
> me we should update to yours.  Diffing against yours does make me
> believe that perhaps LDO14 should be listed as 1.88 V in my patch.
> 

Downstream it's listed as min: 1.8V max: 1.88V init: 1.8V.

> * Is lacking pm8005.
> 

I didn't need this, yet...

> Anyway, let me know.  If you provide a review of my patch I'm happy to
> spin it with your feedback.
> 

I will review your patches.

Regards,
Bjorn
Doug Anderson Sept. 6, 2018, 8:20 p.m. UTC | #3
Hi,

On Thu, Sep 6, 2018 at 1:12 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>> * Seems to have a few rails named differently LDO22 is named
>> "vreg_l22a_2p95" in your DTS but "vreg_l22a_2p85" in mine as one
>> example.  The schematics I have (from Dec 5, 2017) show it as 2p85.
>>
>
> Looks like a typo on my part, yours matches the schematics I have as
> well.

OK.  That was just one example.  Maybe do a diff of the two and see if
you find any where you believe your patch was right and mine was
wrong.


>> * Is lacking many alternate names for rails.  We can debate this also
>> if you want.
>>
>
> Afaict names such as "vdda_pcie_1p2" is the name of the pin on the
> SDM845, while the thing that comes out of the regulator is named
> vreg_l26a_1p2.
>
> So I believe this name should be used in the pcie node as:
>
>   vdda_pcie_1p2-supply = <&vreg_l26a_1p2>;

Adding labels for things like "vdda_pcie_1p2" is based on a suggestion
Stephen Boyd made to me that he felt it would be nice to be able to
refer to things by the name of the supply pin on the SoC.  This could
possibly enable better sharing of device tree snippets between
different boards using the same SoC and the idea seemed sane to me, so
I implemented it.  I described it in my commit message like this:

- Regulators that are hooked up to supply pins on the SoC are given
  an alias matching the name of that pin (pin name comes from the
  Qualcomm SoC "device specification" doc).

If folks don't like it, though, it's easy to remove.

>> * Have a few voltage values different.  If you have better info than
>> me we should update to yours.  Diffing against yours does make me
>> believe that perhaps LDO14 should be listed as 1.88 V in my patch.
>>
>
> Downstream it's listed as min: 1.8V max: 1.88V init: 1.8V.

I can change mine to that.  It's a very strange specification though.
Is it really 1.80 - 1.88 V?  Does something out there actually need
the voltage to be variable in this small range?


>> * Is lacking pm8005.
>>
>
> I didn't need this, yet...

OK.  I can certainly remove it as needed--the downstream patch I
started with had it so I figured I'd keep it in.


-Doug
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 6d651f314193..d9fbdcb39a4d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -7,6 +7,7 @@ 
 
 /dts-v1/;
 
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sdm845.dtsi"
 
 / {
@@ -20,6 +21,221 @@ 
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
+	/* S4 is always-on 1.8V and not controllable through RPMh */
+	vreg_s4a_1p8: vreg-s4a-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vreg_s4a_1p8";
+
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+
+		regulator-always-on;
+		regulator-boot-on;
+
+		vin-supply = <&vph_pwr>;
+	};
+};
+
+&apps_rsc {
+	pm8998-rpmh-regulators {
+		compatible = "qcom,pm8998-rpmh-regulators";
+		qcom,pmic-id = "a";
+
+		vdd-s3-supply = <&vph_pwr>;
+		vdd-s5-supply = <&vph_pwr>;
+		vdd-s7-supply = <&vph_pwr>;
+		vdd-l1-l27-supply = <&vreg_s7a_1p025>;
+		vdd-l10-l23-l25-supply = <&vreg_bob>;
+		vdd-l13-l19-l21-supply = <&vreg_bob>;
+		vdd-l16-l28-supply = <&vreg_bob>;
+		vdd-l18-l22-supply = <&vreg_bob>;
+		vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
+		vdd-l20-l24-supply = <&vreg_bob>;
+		vdd-l26-supply = <&vreg_s3a_1p35>;
+		vdd-l3-l11-supply = <&vreg_s7a_1p025>;
+		vdd-l4-l5-supply = <&vreg_s7a_1p025>;
+		vdd-l6-supply = <&vph_pwr>;
+		vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
+		vdd-l9-supply = <&vreg_bob>;
+		vin-lvs-1-2-supply = <&vreg_s4a_1p8>;
+
+		vreg_s3a_1p35: smps3 {
+			regulator-min-microvolt = <1352000>;
+			regulator-max-microvolt = <1352000>;
+		};
+
+		vreg_s5a_2p04: smps5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		vreg_s7a_1p025: smps7 {
+			regulator-min-microvolt = <900000>;
+			regulator-max-microvolt = <1028000>;
+		};
+
+		vreg_l1a_0p875: ldo1 {
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+			regulator-allow-set-load;
+		};
+
+		vreg_l2a_1p2: ldo2 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-always-on;
+		};
+
+		vreg_l3a_1p0: ldo3 {
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1000000>;
+		};
+
+		vreg_l5a_0p8: ldo5 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <800000>;
+		};
+
+		vreg_l6a_1p8: ldo6 {
+			regulator-min-microvolt = <1856000>;
+			regulator-max-microvolt = <1856000>;
+		};
+
+		vreg_l7a_1p8: ldo7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_l8a_1p2: ldo8 {
+			 regulator-min-microvolt = <1200000>;
+			 regulator-max-microvolt = <1248000>;
+		};
+
+		vreg_l9a_1p8: ldo9 {
+			regulator-min-microvolt = <1704000>;
+			regulator-max-microvolt = <2928000>;
+		};
+
+		vreg_l10a_1p8: ldo10 {
+			regulator-min-microvolt = <1704000>;
+			regulator-max-microvolt = <2928000>;
+		};
+
+		vreg_l11a_1p0: ldo11 {
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1048000>;
+		};
+
+		vreg_12a_1p8: ldo12 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_l13a_2p95: ldo13 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2960000>;
+		};
+
+		vreg_l14a_1p88: ldo14 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1880000>;
+		};
+
+		vreg_l15a_1p8: ldo15 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_l16a_2p7: ldo16 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2704000>;
+		};
+
+		vreg_l17a_1p3: ldo17 {
+			regulator-min-microvolt = <1304000>;
+			regulator-max-microvolt = <1304000>;
+		};
+
+		vreg_l18a_2p7: ldo18 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2960000>;
+		};
+
+		vreg_l19a_3p0: ldo19 {
+			regulator-min-microvolt = <2856000>;
+			regulator-max-microvolt = <3104000>;
+		};
+
+		vreg_l20a_2p95: ldo20 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2960000>;
+		};
+
+		vreg_l21a_2p95: ldo21 {
+			regulator-min-microvolt = <2704000>;
+			regulator-max-microvolt = <2960000>;
+		};
+
+		vreg_l22a_2p95: ldo22 {
+			regulator-min-microvolt = <2864000>;
+			regulator-max-microvolt = <3312000>;
+		};
+
+		vreg_l23a_3p3: ldo23 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3312000>;
+		};
+
+		vreg_l24a_3p075: ldo24 {
+			regulator-min-microvolt = <3088000>;
+			regulator-max-microvolt = <3088000>;
+		};
+
+		vreg_l25_3p075: ldo25 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3312000>;
+		};
+
+		vreg_l26a_1p2: ldo26 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+		};
+
+		vreg_l28a_3p0: ldo28 {
+			regulator-min-microvolt = <2856000>;
+			regulator-max-microvolt = <3008000>;
+		};
+
+		vreg_lvs1a_1p8: lvs1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+
+		vreg_lvs2a_1p8: lvs2 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+	};
+
+	pmi8998-rpmh-regulators {
+		compatible = "qcom,pmi8998-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vdd-bob-supply = <&vph_pwr>;
+
+		vreg_bob: bob {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3600000>;
+		};
+	};
 };
 
 &i2c10 {