[PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Bhupesh Sharma]

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(+)

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

Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Bjorn Andersson]

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
> 

Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Bhupesh Sharma]

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
> >

Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Marijn Suijten]

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
> > >

Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Konrad Dybcio]

[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]


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>;
>   };

[-- Attachment #2: sagami_regulators.patch --]
[-- Type: text/plain, Size: 10687 bytes --]

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";

Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Bhupesh Sharma]

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>;
> >   };

Re: [PATCH v2 1/1] arm64: dts: qcom: sm8350-sagami: usb qmp phy node - add 'vdda-pll-supply' & 'vdda-phy-supply'

[Konrad Dybcio]

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>;
>>>    };