| Message ID | 20180901221953.21666-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | arm64: dts: qcom: sdm845-mtp: pm8998 and pmi8998 regulators | expand |
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
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
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 --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 {
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(+)