| Message ID | 20220419205854.1269922-1-bhupesh.sharma@linaro.org |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2,1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply' | expand |
On Tue 19 Apr 15:58 CDT 2022, Bhupesh Sharma wrote: How about making the subject: "arm64: dts: qcom: sm8350-sagami: add supplies to USB phy node" It still says the same thing, but in much less characters. > As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & > 'vdda-phy-supply' supplies denote the power for the bus and the > clock of the usb qmp phy and are used by the qcom qmp phy driver. > > So, its safe to assume that the two regulators are the same as on > the MTP. So let's wire them up in the same way as the MTP. > I'm not sure it's "safe to assume", so I would like to get Konrad's input before merging this. > In absence of the same 'make dtbs_check' leads to following warnings: > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > phy-wrapper@88e9000: 'vdda-phy-supply' is a required property > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > phy-wrapper@88e9000: 'vdda-pll-supply' is a required property > This is good! Thanks for the patch Bhupesh, Bjorn > [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: konrad.dybcio@somainline.org > Cc: Vinod Koul <vkoul@kernel.org> > Cc: Marijn Suijten <marijn.suijten@somainline.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > Changes since v1: > ----------------- > - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html > - Fixed the commit message to read usb qmp phy instead of ufs phy (which > was introduced erroraneously in the commit log). > > .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > index 90b13cbe2fa6..238ac9380ca2 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > @@ -3,6 +3,7 @@ > * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> > */ > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > #include "sm8350.dtsi" > #include "pm8350.dtsi" > #include "pm8350b.dtsi" > @@ -75,6 +76,27 @@ ramoops@ffc00000 { > }; > }; > > +&apps_rsc { > + pm8350-rpmh-regulators { > + compatible = "qcom,pm8350-rpmh-regulators"; > + qcom,pmic-id = "b"; > + > + vreg_l1b_0p88: ldo1 { > + regulator-name = "vreg_l1b_0p88"; > + regulator-min-microvolt = <912000>; > + regulator-max-microvolt = <920000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; > + > + vreg_l6b_1p2: ldo6 { > + regulator-name = "vreg_l6b_1p2"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1208000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; > + }; > +}; > + > &adsp { > status = "okay"; > firmware-name = "qcom/adsp.mbn"; > @@ -256,4 +278,7 @@ &usb_1_hsphy { > > &usb_1_qmpphy { > status = "okay"; > + > + vdda-phy-supply = <&vreg_l6b_1p2>; > + vdda-pll-supply = <&vreg_l1b_0p88>; > }; > -- > 2.35.1 >
Hi Bjorn, On Wed, 20 Apr 2022 at 08:11, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 19 Apr 15:58 CDT 2022, Bhupesh Sharma wrote: > > How about making the subject: > > "arm64: dts: qcom: sm8350-sagami: add supplies to USB phy node" > > It still says the same thing, but in much less characters. Sure, this wording seems better to me. Will fix this in v3. > > As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & > > 'vdda-phy-supply' supplies denote the power for the bus and the > > clock of the usb qmp phy and are used by the qcom qmp phy driver. > > > > So, its safe to assume that the two regulators are the same as on > > the MTP. So let's wire them up in the same way as the MTP. > > > > I'm not sure it's "safe to assume", so I would like to get Konrad's > input before merging this. Right. @Konrad Dybcio , @Marijn Suijten - Any comments on this fix? Please share your thoughts. Thanks, Bhupesh > > In absence of the same 'make dtbs_check' leads to following warnings: > > > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > > phy-wrapper@88e9000: 'vdda-phy-supply' is a required property > > > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > > phy-wrapper@88e9000: 'vdda-pll-supply' is a required property > > > > This is good! > > Thanks for the patch Bhupesh, > Bjorn > > > [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: konrad.dybcio@somainline.org > > Cc: Vinod Koul <vkoul@kernel.org> > > Cc: Marijn Suijten <marijn.suijten@somainline.org> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > Changes since v1: > > ----------------- > > - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html > > - Fixed the commit message to read usb qmp phy instead of ufs phy (which > > was introduced erroraneously in the commit log). > > > > .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > index 90b13cbe2fa6..238ac9380ca2 100644 > > --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> > > */ > > > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > #include "sm8350.dtsi" > > #include "pm8350.dtsi" > > #include "pm8350b.dtsi" > > @@ -75,6 +76,27 @@ ramoops@ffc00000 { > > }; > > }; > > > > +&apps_rsc { > > + pm8350-rpmh-regulators { > > + compatible = "qcom,pm8350-rpmh-regulators"; > > + qcom,pmic-id = "b"; > > + > > + vreg_l1b_0p88: ldo1 { > > + regulator-name = "vreg_l1b_0p88"; > > + regulator-min-microvolt = <912000>; > > + regulator-max-microvolt = <920000>; > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > + }; > > + > > + vreg_l6b_1p2: ldo6 { > > + regulator-name = "vreg_l6b_1p2"; > > + regulator-min-microvolt = <1200000>; > > + regulator-max-microvolt = <1208000>; > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > + }; > > + }; > > +}; > > + > > &adsp { > > status = "okay"; > > firmware-name = "qcom/adsp.mbn"; > > @@ -256,4 +278,7 @@ &usb_1_hsphy { > > > > &usb_1_qmpphy { > > status = "okay"; > > + > > + vdda-phy-supply = <&vreg_l6b_1p2>; > > + vdda-pll-supply = <&vreg_l1b_0p88>; > > }; > > -- > > 2.35.1 > >
On 2022-04-20 12:28:24, Bhupesh Sharma wrote: > Hi Bjorn, > > On Wed, 20 Apr 2022 at 08:11, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Tue 19 Apr 15:58 CDT 2022, Bhupesh Sharma wrote: > > > > How about making the subject: > > > > "arm64: dts: qcom: sm8350-sagami: add supplies to USB phy node" > > > > It still says the same thing, but in much less characters. > > Sure, this wording seems better to me. Will fix this in v3. > > > > As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & > > > 'vdda-phy-supply' supplies denote the power for the bus and the > > > clock of the usb qmp phy and are used by the qcom qmp phy driver. > > > > > > So, its safe to assume that the two regulators are the same as on > > > the MTP. So let's wire them up in the same way as the MTP. > > > > > > > I'm not sure it's "safe to assume", so I would like to get Konrad's > > input before merging this. > > Right. @Konrad Dybcio , @Marijn Suijten - Any comments on this fix? > Please share your thoughts. All we can do is read downstream sources (and/or check compiled DTS after all overrides have been flattened together), or read sysfs to validate values where applicable. These are indeed the two regulators used by USB in: https://github.com/sonyxperiadev/kernel/blob/7378fb627b546e0eae24cccd3ab37fa9e0802f95/arch/arm64/boot/dts/qcom/lahaina-usb.dtsi#L146-L149 And the voltages are correct for our board too as per: https://github.com/sonyxperiadev/kernel/blob/7378fb627b546e0eae24cccd3ab37fa9e0802f95/arch/arm64/boot/dts/qcom/lahaina-regulators.dtsi It appears only L6 is used by other hardware nodes, but that should be fine. As such, with the suggestions from Bjorn applied: Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> - Marijn > Thanks, > Bhupesh > > > > In absence of the same 'make dtbs_check' leads to following warnings: > > > > > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > > > phy-wrapper@88e9000: 'vdda-phy-supply' is a required property > > > > > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > > > phy-wrapper@88e9000: 'vdda-pll-supply' is a required property > > > > > > > This is good! > > > > Thanks for the patch Bhupesh, > > Bjorn > > > > > [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ > > > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > > Cc: konrad.dybcio@somainline.org > > > Cc: Vinod Koul <vkoul@kernel.org> > > > Cc: Marijn Suijten <marijn.suijten@somainline.org> > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > > --- > > > Changes since v1: > > > ----------------- > > > - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html > > > - Fixed the commit message to read usb qmp phy instead of ufs phy (which > > > was introduced erroraneously in the commit log). > > > > > > .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ > > > 1 file changed, 25 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > > index 90b13cbe2fa6..238ac9380ca2 100644 > > > --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > > @@ -3,6 +3,7 @@ > > > * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> > > > */ > > > > > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > > #include "sm8350.dtsi" > > > #include "pm8350.dtsi" > > > #include "pm8350b.dtsi" > > > @@ -75,6 +76,27 @@ ramoops@ffc00000 { > > > }; > > > }; > > > > > > +&apps_rsc { > > > + pm8350-rpmh-regulators { > > > + compatible = "qcom,pm8350-rpmh-regulators"; > > > + qcom,pmic-id = "b"; > > > + > > > + vreg_l1b_0p88: ldo1 { > > > + regulator-name = "vreg_l1b_0p88"; > > > + regulator-min-microvolt = <912000>; > > > + regulator-max-microvolt = <920000>; > > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > > + }; > > > + > > > + vreg_l6b_1p2: ldo6 { > > > + regulator-name = "vreg_l6b_1p2"; > > > + regulator-min-microvolt = <1200000>; > > > + regulator-max-microvolt = <1208000>; > > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > > + }; > > > + }; > > > +}; > > > + > > > &adsp { > > > status = "okay"; > > > firmware-name = "qcom/adsp.mbn"; > > > @@ -256,4 +278,7 @@ &usb_1_hsphy { > > > > > > &usb_1_qmpphy { > > > status = "okay"; > > > + > > > + vdda-phy-supply = <&vreg_l6b_1p2>; > > > + vdda-pll-supply = <&vreg_l1b_0p88>; > > > }; > > > -- > > > 2.35.1 > > >
On 19/04/2022 22:58, Bhupesh Sharma wrote: > As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & > 'vdda-phy-supply' supplies denote the power for the bus and the > clock of the usb qmp phy and are used by the qcom qmp phy driver. > > So, its safe to assume that the two regulators are the same as on > the MTP. So let's wire them up in the same way as the MTP. > > In absence of the same 'make dtbs_check' leads to following warnings: > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > phy-wrapper@88e9000: 'vdda-phy-supply' is a required property > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > phy-wrapper@88e9000: 'vdda-pll-supply' is a required property > > [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: konrad.dybcio@somainline.org > Cc: Vinod Koul <vkoul@kernel.org> > Cc: Marijn Suijten <marijn.suijten@somainline.org> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > Changes since v1: > ----------------- > - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html > - Fixed the commit message to read usb qmp phy instead of ufs phy (which > was introduced erroraneously in the commit log). > > .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ > 1 file changed, 25 insertions(+) Hi, I actually threw together a patch with [almost] all RPMh regulators on Sagami quite a while ago (if thunderbird doesn't explode, you should find it as an attachment to this message), but I haven't sent it, as pm8350c_l3 and pmr735a_s3 are broken on the .c driver side (the step numbers and/or values are not correct), but Qualcomm in their infinite Qualcommery will not let us, mere mortals, access the PDFs that contain the correct specifications, so I have to wait for somebody with access to them to pick them up. That said, the values you supplied seem correct (say for lack of allow-set-load which will bite when (if?) UFS is ever functional upstream on this piece of Japanese engineering), but adding the configuration of all regulators at once just seems more complete to me.. On a note, USB - among other more or less necessary peripherals - along with its supplies, is brought up in XBL (or bootrom if you jump to EDL), so not setting these supplies in Linux does not bite in any way, shape or form other than "make dtbs_check" screaming. I'm not against this patch, but once again, even though this point may not sound very convincing to you all, I think it would be "nice" to configure all regulators in one go. Konrad > diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > index 90b13cbe2fa6..238ac9380ca2 100644 > --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > @@ -3,6 +3,7 @@ > * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> > */ > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > #include "sm8350.dtsi" > #include "pm8350.dtsi" > #include "pm8350b.dtsi" > @@ -75,6 +76,27 @@ ramoops@ffc00000 { > }; > }; > > +&apps_rsc { > + pm8350-rpmh-regulators { > + compatible = "qcom,pm8350-rpmh-regulators"; > + qcom,pmic-id = "b"; > + > + vreg_l1b_0p88: ldo1 { > + regulator-name = "vreg_l1b_0p88"; > + regulator-min-microvolt = <912000>; > + regulator-max-microvolt = <920000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; > + > + vreg_l6b_1p2: ldo6 { > + regulator-name = "vreg_l6b_1p2"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1208000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + }; > + }; > +}; > + > &adsp { > status = "okay"; > firmware-name = "qcom/adsp.mbn"; > @@ -256,4 +278,7 @@ &usb_1_hsphy { > > &usb_1_qmpphy { > status = "okay"; > + > + vdda-phy-supply = <&vreg_l6b_1p2>; > + vdda-pll-supply = <&vreg_l1b_0p88>; > }; From da20c2afa2c0715ef78136cd670ee9c1db3adf6c Mon Sep 17 00:00:00 2001 From: Konrad Dybcio <konrad.dybcio@somainline.org> Date: Sat, 5 Mar 2022 17:08:58 +0100 Subject: [PATCH] arm64: dts: qcom: sm8350-sagami: Add most RPMh regulators Configure most RPMh-controlled regulators on SoMC Sagami. The missing ones (on pm8350b and pm8008[ij]) will be configured when driver support is added. Thankfully, it looks like PDX215 and PDX214 don't have any differences when it comes to PM8350/PM8350C/PMR735a. Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org> --- .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 350 ++++++++++++++++++ 1 file changed, 350 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi index 90b13cbe2fa63..81363f367d452 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi @@ -3,6 +3,7 @@ * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> */ +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> #include "sm8350.dtsi" #include "pm8350.dtsi" #include "pm8350b.dtsi" @@ -73,6 +74,16 @@ no-map; }; }; + + vph_pwr: vph-pwr-regulator { + compatible = "regulator-fixed"; + regulator-name = "vph_pwr"; + regulator-min-microvolt = <3700000>; + regulator-max-microvolt = <3700000>; + + regulator-always-on; + regulator-boot-on; + }; }; &adsp { @@ -80,6 +91,345 @@ firmware-name = "qcom/adsp.mbn"; }; +&apps_rsc { + pm8350-rpmh-regulators { + compatible = "qcom,pm8350-rpmh-regulators"; + qcom,pmic-id = "b"; + + vdd-s1-supply = <&vph_pwr>; + vdd-s2-supply = <&vph_pwr>; + vdd-s3-supply = <&vph_pwr>; + vdd-s4-supply = <&vph_pwr>; + vdd-s5-supply = <&vph_pwr>; + vdd-s6-supply = <&vph_pwr>; + vdd-s7-supply = <&vph_pwr>; + vdd-s8-supply = <&vph_pwr>; + vdd-s9-supply = <&vph_pwr>; + vdd-s10-supply = <&vph_pwr>; + vdd-s11-supply = <&vph_pwr>; + vdd-s12-supply = <&vph_pwr>; + + vdd-l1-l4-supply = <&pm8350_s11>; + vdd-l2-l7-supply = <&vreg_bob>; + vdd-l3-l5-supply = <&vreg_bob>; + vdd-l6-l9-l10-supply = <&pm8350_s11>; + + /* + * ARC regulators: + * S5 - mx.lvl + * S6 - gfx.lvl + * S9 - mxc.lvl + */ + + pm8350_s10: smps10 { + regulator-name = "pm8350_s10"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350_s11: smps11 { + regulator-name = "pm8350_s11"; + regulator-min-microvolt = <752000>; + regulator-max-microvolt = <1000000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350_s12: smps12 { + regulator-name = "pm8350_s12"; + regulator-min-microvolt = <1224000>; + regulator-max-microvolt = <1360000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350_l1: ldo1 { + regulator-name = "pm8350_l1"; + regulator-min-microvolt = <912000>; + regulator-max-microvolt = <920000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350_l2: ldo2 { + regulator-name = "pm8350_l2"; + regulator-min-microvolt = <3072000>; + regulator-max-microvolt = <3072000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350_l3: ldo3 { + regulator-name = "pm8350_l3"; + regulator-min-microvolt = <904000>; + regulator-max-microvolt = <904000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + /* L4 - lmx.lvl (ARC) */ + + pm8350_l5: ldo5 { + regulator-name = "pm8350_l5"; + regulator-min-microvolt = <880000>; + regulator-max-microvolt = <888000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + regulator-allow-set-load; + }; + + pm8350_l6: ldo6 { + regulator-name = "pm8350_l6"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1208000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + regulator-allow-set-load; + }; + + pm8350_l7: ldo7 { + regulator-name = "pm8350_l7"; + regulator-min-microvolt = <2400000>; + regulator-max-microvolt = <3008000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + regulator-allow-set-load; + }; + + /* L8 - lcx.lvl (ARC) */ + + pm8350_l9: ldo9 { + regulator-name = "pm8350_l9"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + regulator-allow-set-load; + }; + }; + + pm8350c-rpmh-regulators { + compatible = "qcom,pm8350c-rpmh-regulators"; + qcom,pmic-id = "c"; + + vdd-s1-supply = <&vph_pwr>; + vdd-s2-supply = <&vph_pwr>; + vdd-s3-supply = <&vph_pwr>; + vdd-s4-supply = <&vph_pwr>; + vdd-s5-supply = <&vph_pwr>; + vdd-s6-supply = <&vph_pwr>; + vdd-s7-supply = <&vph_pwr>; + vdd-s8-supply = <&vph_pwr>; + vdd-s9-supply = <&vph_pwr>; + vdd-s10-supply = <&vph_pwr>; + + vdd-l1-l12-supply = <&pm8350c_s1>; + vdd-l2-l8-supply = <&pm8350c_s1>; + vdd-l3-l4-l5-l7-l13-supply = <&vreg_bob>; + vdd-l6-l9-l11-supply = <&vreg_bob>; + vdd-l10-supply = <&pm8350_s12>; + + vdd-bob-supply = <&vph_pwr>; + + pm8350c_s1: smps1 { + regulator-name = "pm8350c_s1"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1952000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + /* S2 - ebi.lvl (ARC) */ + + pm8350c_s3: smps3 { + regulator-name = "pm8350c_s3"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <704000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + /* + * ARC regulators: + * S4 - mss.lvl + * S6 - cx.lvl + * S8 - mmcx.lvl + */ + + pm8350c_s10: smps10 { + regulator-name = "pm8350c_s10"; + regulator-min-microvolt = <1048000>; + regulator-max-microvolt = <1128000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l1: ldo1 { + regulator-name = "pm8350c_l1"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l2: ldo2 { + regulator-name = "pm8350c_l2"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l3: ldo3 { + regulator-name = "pm8350c_l3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l4: ldo4 { + regulator-name = "pm8350c_l4"; + regulator-min-microvolt = <1704000>; + regulator-max-microvolt = <3000000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l5: ldo5 { + regulator-name = "pm8350c_l5"; + regulator-min-microvolt = <1704000>; + regulator-max-microvolt = <3000000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l6: ldo6 { + regulator-name = "pm8350c_l6"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2960000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l7: ldo7 { + regulator-name = "pm8350c_l7"; + regulator-min-microvolt = <3008000>; + regulator-max-microvolt = <3008000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l8: ldo8 { + regulator-name = "pm8350c_l8"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l9: ldo9 { + regulator-name = "pm8350c_l9"; + regulator-min-microvolt = <2960000>; + regulator-max-microvolt = <3008000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l10: ldo10 { + regulator-name = "pm8350c_l10"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l11: ldo11 { + regulator-name = "pm8350c_l11"; + regulator-min-microvolt = <2400000>; + regulator-max-microvolt = <3008000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l12: ldo12 { + regulator-name = "pm8350c_l12"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2000000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + pm8350c_l13: ldo13 { + regulator-name = "pm8350c_l13"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + vreg_bob: bob { + regulator-name = "vreg_bob"; + regulator-min-microvolt = <3400000>; + regulator-max-microvolt = <3960000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>; + }; + }; + + /* TODO: Add pm8350b (just one ldo) once the driver part is in */ + + pmr735a-rpmh-regulators { + compatible = "qcom,pmr735a-rpmh-regulators"; + qcom,pmic-id = "e"; + + vdd-s1-supply = <&vph_pwr>; + vdd-s2-supply = <&vph_pwr>; + vdd-s3-supply = <&vph_pwr>; + + vdd-l1-l2-supply = <&pmr735a_s2>; + vdd-l3-supply = <&pmr735a_s1>; + vdd-l4-supply = <&pm8350c_s1>; + vdd-l5-l6-supply = <&pm8350c_s1>; + vdd-l7-bob-supply = <&vreg_bob>; + + pmr735a_s1: smps1 { + regulator-name = "pmr735a_s1"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1280000>; + }; + + pmr735a_s2: smps2 { + regulator-name = "pmr735a_s2"; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <976000>; + }; + + pmr735a_s3: smps3 { + regulator-name = "pmr735a_s3"; + regulator-min-microvolt = <2200000>; + regulator-max-microvolt = <2352000>; + }; + + pmr735a_l1: ldo1 { + regulator-name = "pmr735a_l1"; + regulator-min-microvolt = <912000>; + regulator-max-microvolt = <912000>; + }; + + pmr735a_l2: ldo2 { + regulator-name = "pmr735a_l2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + pmr735a_l3: ldo3 { + regulator-name = "pmr735a_l3"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + }; + + pmr735a_l4: ldo4 { + regulator-name = "pmr735a_l4"; + regulator-min-microvolt = <1776000>; + regulator-max-microvolt = <1872000>; + }; + + pmr735a_l5: ldo5 { + regulator-name = "pmr735a_l5"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <800000>; + }; + + pmr735a_l6: ldo6 { + regulator-name = "pmr735a_l6"; + regulator-min-microvolt = <480000>; + regulator-max-microvolt = <904000>; + }; + + pmr735a_l7: ldo7 { + regulator-name = "pmr735a_l7"; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + }; + }; +}; + &cdsp { status = "okay"; firmware-name = "qcom/cdsp.mbn";
Hi Konrad, Thanks for your comments. Please see my comments below: On Wed, 20 Apr 2022 at 22:30, Konrad Dybcio <konrad.dybcio@somainline.org> wrote: > > > On 19/04/2022 22:58, Bhupesh Sharma wrote: > > As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & > > 'vdda-phy-supply' supplies denote the power for the bus and the > > clock of the usb qmp phy and are used by the qcom qmp phy driver. > > > > So, its safe to assume that the two regulators are the same as on > > the MTP. So let's wire them up in the same way as the MTP. > > > > In absence of the same 'make dtbs_check' leads to following warnings: > > > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > > phy-wrapper@88e9000: 'vdda-phy-supply' is a required property > > > > arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: > > phy-wrapper@88e9000: 'vdda-pll-supply' is a required property > > > > [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: konrad.dybcio@somainline.org > > Cc: Vinod Koul <vkoul@kernel.org> > > Cc: Marijn Suijten <marijn.suijten@somainline.org> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > Changes since v1: > > ----------------- > > - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html > > - Fixed the commit message to read usb qmp phy instead of ufs phy (which > > was introduced erroraneously in the commit log). > > > > .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > Hi, I actually threw together a patch with [almost] all RPMh regulators > on Sagami quite a while ago (if thunderbird doesn't explode, you should > find it as an attachment to this message), but I haven't sent it, as > pm8350c_l3 and pmr735a_s3 are broken on the .c driver side (the step > numbers and/or values are not correct), but Qualcomm in their infinite > Qualcommery will not let us, mere mortals, access the PDFs that contain > the correct specifications, so I have to wait for somebody with access > to them to pick them up. That said, the values you supplied seem correct > (say for lack of allow-set-load which will bite when (if?) UFS is ever > functional upstream on this piece of Japanese engineering), but adding > the configuration of all regulators at once just seems more complete to me.. The complete set of regulator enablement was something on my mind and which I tried before sending out the smaller (only relevant to USB) patch. My main concern was leaving the upstream kernel on the board in an unbootable form, as I don't have the sagami board to test the changes on. That said, if you intend to send the complete regulator enablement patch for the sagami board, it would be probably better in the longer run (as currently any change to the common binding, dts or driver files is held up by "make dtbs_check" warnings during upstream review). > On a note, USB - among other more or less necessary peripherals - along > with its supplies, is brought up in XBL (or bootrom if you jump to EDL), > so not setting these supplies in Linux does not bite in any way, shape > or form other than "make dtbs_check" screaming. I agree. If you want, you can include my simple patch as the followup (just enabling the right regulators for the usb qmp phy), to your big patch (which enables the complete regulator support). It should be fine as well, as we eventually want to use the regulators defined in the 'big' patch for some 'real' peripheral nodes like the usb qmp phy. Please let me know your views. Regards, Bhupesh > I'm not against this patch, but once again, even though this point may > not sound very convincing to you all, I think it would be "nice" to > configure all regulators in one go. > > > Konrad > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > index 90b13cbe2fa6..238ac9380ca2 100644 > > --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> > > */ > > > > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > > #include "sm8350.dtsi" > > #include "pm8350.dtsi" > > #include "pm8350b.dtsi" > > @@ -75,6 +76,27 @@ ramoops@ffc00000 { > > }; > > }; > > > > +&apps_rsc { > > + pm8350-rpmh-regulators { > > + compatible = "qcom,pm8350-rpmh-regulators"; > > + qcom,pmic-id = "b"; > > + > > + vreg_l1b_0p88: ldo1 { > > + regulator-name = "vreg_l1b_0p88"; > > + regulator-min-microvolt = <912000>; > > + regulator-max-microvolt = <920000>; > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > + }; > > + > > + vreg_l6b_1p2: ldo6 { > > + regulator-name = "vreg_l6b_1p2"; > > + regulator-min-microvolt = <1200000>; > > + regulator-max-microvolt = <1208000>; > > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > > + }; > > + }; > > +}; > > + > > &adsp { > > status = "okay"; > > firmware-name = "qcom/adsp.mbn"; > > @@ -256,4 +278,7 @@ &usb_1_hsphy { > > > > &usb_1_qmpphy { > > status = "okay"; > > + > > + vdda-phy-supply = <&vreg_l6b_1p2>; > > + vdda-pll-supply = <&vreg_l1b_0p88>; > > };
On 20/04/2022 21:33, Bhupesh Sharma wrote: > Hi Konrad, > > Thanks for your comments. Please see my comments below: > > On Wed, 20 Apr 2022 at 22:30, Konrad Dybcio > <konrad.dybcio@somainline.org> wrote: >> >> On 19/04/2022 22:58, Bhupesh Sharma wrote: >>> As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & >>> 'vdda-phy-supply' supplies denote the power for the bus and the >>> clock of the usb qmp phy and are used by the qcom qmp phy driver. >>> >>> So, its safe to assume that the two regulators are the same as on >>> the MTP. So let's wire them up in the same way as the MTP. >>> >>> In absence of the same 'make dtbs_check' leads to following warnings: >>> >>> arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: >>> phy-wrapper@88e9000: 'vdda-phy-supply' is a required property >>> >>> arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: >>> phy-wrapper@88e9000: 'vdda-pll-supply' is a required property >>> >>> [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ >>> >>> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> >>> Cc: konrad.dybcio@somainline.org >>> Cc: Vinod Koul <vkoul@kernel.org> >>> Cc: Marijn Suijten <marijn.suijten@somainline.org> >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >>> --- >>> Changes since v1: >>> ----------------- >>> - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html >>> - Fixed the commit message to read usb qmp phy instead of ufs phy (which >>> was introduced erroraneously in the commit log). >>> >>> .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >> Hi, I actually threw together a patch with [almost] all RPMh regulators >> on Sagami quite a while ago (if thunderbird doesn't explode, you should >> find it as an attachment to this message), but I haven't sent it, as >> pm8350c_l3 and pmr735a_s3 are broken on the .c driver side (the step >> numbers and/or values are not correct), but Qualcomm in their infinite >> Qualcommery will not let us, mere mortals, access the PDFs that contain >> the correct specifications, so I have to wait for somebody with access >> to them to pick them up. That said, the values you supplied seem correct >> (say for lack of allow-set-load which will bite when (if?) UFS is ever >> functional upstream on this piece of Japanese engineering), but adding >> the configuration of all regulators at once just seems more complete to me.. > The complete set of regulator enablement was something on my mind and > which I tried before sending out the smaller (only relevant to USB) > patch. > > My main concern was leaving the upstream kernel on the board in an > unbootable form, as I don't have the sagami board to test the changes > on. Yeah, messing with power rails on somebody else's device is stressful in general, especially with downstream kernels #including 9999 other DTs, it's really easy to make a mistake.. Personally, I sometimes even dump the regulator settings from syfs on a running device just to be ultra sure. > > That said, if you intend to send the complete regulator enablement > patch for the sagami board, it would be probably better in the longer > run (as currently any change to the common binding, dts or driver > files is held up by "make dtbs_check" warnings during upstream > review). > >> On a note, USB - among other more or less necessary peripherals - along >> with its supplies, is brought up in XBL (or bootrom if you jump to EDL), >> so not setting these supplies in Linux does not bite in any way, shape >> or form other than "make dtbs_check" screaming. > I agree. If you want, you can include my simple patch as the followup > (just enabling the right regulators for the usb qmp phy), to your big > patch (which enables the complete regulator support). It should be > fine as well, as we eventually want to use the regulators defined in > the 'big' patch for some 'real' peripheral nodes like the usb qmp phy. > > Please let me know your views. > > Regards, > Bhupesh I will send this patch (of course rebased) along with the add-all-regulators one, once the .c driver is fixed then. Thanks, Konrad >> I'm not against this patch, but once again, even though this point may >> not sound very convincing to you all, I think it would be "nice" to >> configure all regulators in one go. >> >> >> Konrad >> >> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi >>> index 90b13cbe2fa6..238ac9380ca2 100644 >>> --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi >>> @@ -3,6 +3,7 @@ >>> * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> >>> */ >>> >>> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> >>> #include "sm8350.dtsi" >>> #include "pm8350.dtsi" >>> #include "pm8350b.dtsi" >>> @@ -75,6 +76,27 @@ ramoops@ffc00000 { >>> }; >>> }; >>> >>> +&apps_rsc { >>> + pm8350-rpmh-regulators { >>> + compatible = "qcom,pm8350-rpmh-regulators"; >>> + qcom,pmic-id = "b"; >>> + >>> + vreg_l1b_0p88: ldo1 { >>> + regulator-name = "vreg_l1b_0p88"; >>> + regulator-min-microvolt = <912000>; >>> + regulator-max-microvolt = <920000>; >>> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; >>> + }; >>> + >>> + vreg_l6b_1p2: ldo6 { >>> + regulator-name = "vreg_l6b_1p2"; >>> + regulator-min-microvolt = <1200000>; >>> + regulator-max-microvolt = <1208000>; >>> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; >>> + }; >>> + }; >>> +}; >>> + >>> &adsp { >>> status = "okay"; >>> firmware-name = "qcom/adsp.mbn"; >>> @@ -256,4 +278,7 @@ &usb_1_hsphy { >>> >>> &usb_1_qmpphy { >>> status = "okay"; >>> + >>> + vdda-phy-supply = <&vreg_l6b_1p2>; >>> + vdda-pll-supply = <&vreg_l1b_0p88>; >>> };
diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi index 90b13cbe2fa6..238ac9380ca2 100644 --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi @@ -3,6 +3,7 @@ * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org> */ +#include <dt-bindings/regulator/qcom,rpmh-regulator.h> #include "sm8350.dtsi" #include "pm8350.dtsi" #include "pm8350b.dtsi" @@ -75,6 +76,27 @@ ramoops@ffc00000 { }; }; +&apps_rsc { + pm8350-rpmh-regulators { + compatible = "qcom,pm8350-rpmh-regulators"; + qcom,pmic-id = "b"; + + vreg_l1b_0p88: ldo1 { + regulator-name = "vreg_l1b_0p88"; + regulator-min-microvolt = <912000>; + regulator-max-microvolt = <920000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + + vreg_l6b_1p2: ldo6 { + regulator-name = "vreg_l6b_1p2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1208000>; + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; + }; + }; +}; + &adsp { status = "okay"; firmware-name = "qcom/adsp.mbn"; @@ -256,4 +278,7 @@ &usb_1_hsphy { &usb_1_qmpphy { status = "okay"; + + vdda-phy-supply = <&vreg_l6b_1p2>; + vdda-pll-supply = <&vreg_l1b_0p88>; };
As suggested by Bjorn during review of [1], the 'vdda-pll-supply' & 'vdda-phy-supply' supplies denote the power for the bus and the clock of the usb qmp phy and are used by the qcom qmp phy driver. So, its safe to assume that the two regulators are the same as on the MTP. So let's wire them up in the same way as the MTP. In absence of the same 'make dtbs_check' leads to following warnings: arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: phy-wrapper@88e9000: 'vdda-phy-supply' is a required property arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dt.yaml: phy-wrapper@88e9000: 'vdda-pll-supply' is a required property [1]. https://lore.kernel.org/lkml/20220228123019.382037-9-bhupesh.sharma@linaro.org/ Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: konrad.dybcio@somainline.org Cc: Vinod Koul <vkoul@kernel.org> Cc: Marijn Suijten <marijn.suijten@somainline.org> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> --- Changes since v1: ----------------- - v1 can be found here: https://www.spinics.net/lists/linux-arm-msm/msg108467.html - Fixed the commit message to read usb qmp phy instead of ufs phy (which was introduced erroraneously in the commit log). .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+)