diff mbox series

[v10,8/9] arm64: dts: qcom: ipq9574: Add LDO regulator node

Message ID	8894bf2c44eaf4959c7a1966b66229e6cf5cda96.1683183860.git.quic_varada@quicinc.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Varadarajan Narayanan <quic_varada@quicinc.com> To: <agross@kernel.org>, <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <vkoul@kernel.org>, <kishon@kernel.org>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <gregkh@linuxfoundation.org>, <mturquette@baylibre.com>, <sboyd@kernel.org>, <quic_wcheng@quicinc.com>, <linux-arm-msm@vger.kernel.org>, <linux-phy@lists.infradead.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-usb@vger.kernel.org>, <linux-clk@vger.kernel.org> CC: Varadarajan Narayanan <quic_varada@quicinc.com> Subject: [PATCH v10 8/9] arm64: dts: qcom: ipq9574: Add LDO regulator node Date: Fri, 5 May 2023 13:05:09 +0530 Message-ID: <8894bf2c44eaf4959c7a1966b66229e6cf5cda96.1683183860.git.quic_varada@quicinc.com> In-Reply-To: <cover.1683183860.git.quic_varada@quicinc.com> References: <cover.1683183860.git.quic_varada@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk
Series	Enable IPQ9754 USB \| [v10,0/9] Enable IPQ9754 USB [v10,1/9] dt-bindings: phy: qcom,qusb2: Document IPQ9574 compatible [v10,2/9] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY [v10,3/9] dt-bindings: usb: dwc3: Add IPQ9574 compatible [v10,4/9] clk: qcom: gcc-ipq9574: Add USB related clocks [v10,5/9] phy: qcom-qusb2: add QUSB2 support for IPQ9574 [v10,6/9] phy: qcom: qmp: Update IPQ9574 USB Phy initialization Sequence [v10,7/9] arm64: dts: qcom: ipq9574: Add USB related nodes [v10,8/9] arm64: dts: qcom: ipq9574: Add LDO regulator node [v10,9/9] arm64: dts: qcom: ipq9574: Enable USB

Commit Message

Varadarajan Narayanan May 5, 2023, 7:35 a.m. UTC

  Add LDO regulator node

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
 Changes in v10:
	- Add LDO regulator node
---
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Dmitry Baryshkov May 5, 2023, 9:29 a.m. UTC | #1

On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> Add LDO regulator node

As this LDO is provided by the PMIC, it would be nice to know why it
is modelled as an always-on regulator instead of the proper PMIC
regulator. Up to now we were doing this only for the outstanding power
rails like CX/MX or EBI.

>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
>  Changes in v10:
>         - Add LDO regulator node
> ---
>  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> index bdc1434..1f5d14f 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> @@ -60,6 +60,13 @@
>                         regulator-min-microvolt = <725000>;
>                         regulator-max-microvolt = <1075000>;
>                 };
> +
> +               mp5496_l2: l2 {
> +                       regulator-min-microvolt = <1800000>;
> +                       regulator-max-microvolt = <1800000>;
> +                       regulator-boot-on;
> +                       regulator-always-on;
> +               };
>         };
>  };
>
> --
> 2.7.4
>

Konrad Dybcio May 5, 2023, 7:26 p.m. UTC | #2

On 5.05.2023 11:29, Dmitry Baryshkov wrote:
> On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
>>
>> Add LDO regulator node
> 
> As this LDO is provided by the PMIC, it would be nice to know why it
> is modelled as an always-on regulator instead of the proper PMIC
> regulator. Up to now we were doing this only for the outstanding power
> rails like CX/MX or EBI.
(which we then stopped registering as regulators and started
to manage through rpm(h)pd drivers and the genpd framework)

Konrad
> 
>>
>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
>> ---
>>  Changes in v10:
>>         - Add LDO regulator node
>> ---
>>  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> index bdc1434..1f5d14f 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
>> @@ -60,6 +60,13 @@
>>                         regulator-min-microvolt = <725000>;
>>                         regulator-max-microvolt = <1075000>;
>>                 };
>> +
>> +               mp5496_l2: l2 {
>> +                       regulator-min-microvolt = <1800000>;
>> +                       regulator-max-microvolt = <1800000>;
>> +                       regulator-boot-on;
>> +                       regulator-always-on;
>> +               };
>>         };
>>  };
>>
>> --
>> 2.7.4
>>
> 
>

Dmitry Baryshkov May 6, 2023, 11:07 a.m. UTC | #3

On Fri, 5 May 2023 at 22:26, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 5.05.2023 11:29, Dmitry Baryshkov wrote:
> > On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> >>
> >> Add LDO regulator node
> >
> > As this LDO is provided by the PMIC, it would be nice to know why it
> > is modelled as an always-on regulator instead of the proper PMIC
> > regulator. Up to now we were doing this only for the outstanding power
> > rails like CX/MX or EBI.
> (which we then stopped registering as regulators and started
> to manage through rpm(h)pd drivers and the genpd framework)

I mean having regulator-fixed s4a on sm8250 in addition to the CX
genpd, because some of the parts really expect to have a regulator.
But anyway, we are getting away from the subj. Let's wait for the
answer from Varadarajan.

>
> Konrad
> >
> >>
> >> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> >> ---
> >>  Changes in v10:
> >>         - Add LDO regulator node
> >> ---
> >>  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> >> index bdc1434..1f5d14f 100644
> >> --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> >> +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> >> @@ -60,6 +60,13 @@
> >>                         regulator-min-microvolt = <725000>;
> >>                         regulator-max-microvolt = <1075000>;
> >>                 };
> >> +
> >> +               mp5496_l2: l2 {
> >> +                       regulator-min-microvolt = <1800000>;
> >> +                       regulator-max-microvolt = <1800000>;
> >> +                       regulator-boot-on;
> >> +                       regulator-always-on;
> >> +               };
> >>         };
> >>  };

Varadarajan Narayanan May 6, 2023, 11:09 a.m. UTC | #4

On Fri, May 05, 2023 at 12:29:54PM +0300, Dmitry Baryshkov wrote:
> On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > Add LDO regulator node
>
> As this LDO is provided by the PMIC, it would be nice to know why it
> is modelled as an always-on regulator instead of the proper PMIC
> regulator. Up to now we were doing this only for the outstanding power
> rails like CX/MX or EBI.

These are always ON because USB phy doesn't support power
collapse, and there is a chance that other IP blocks might be
sharing the rail.

Thanks
Varada

> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> >  Changes in v10:
> >         - Add LDO regulator node
> > ---
> >  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > index bdc1434..1f5d14f 100644
> > --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > @@ -60,6 +60,13 @@
> >                         regulator-min-microvolt = <725000>;
> >                         regulator-max-microvolt = <1075000>;
> >                 };
> > +
> > +               mp5496_l2: l2 {
> > +                       regulator-min-microvolt = <1800000>;
> > +                       regulator-max-microvolt = <1800000>;
> > +                       regulator-boot-on;
> > +                       regulator-always-on;
> > +               };
> >         };
> >  };
> >
> > --
> > 2.7.4
> >
>
>
> --
> With best wishes
> Dmitry

Dmitry Baryshkov May 6, 2023, 11:33 a.m. UTC | #5

On Sat, 6 May 2023 at 14:09, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Fri, May 05, 2023 at 12:29:54PM +0300, Dmitry Baryshkov wrote:
> > On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > Add LDO regulator node
> >
> > As this LDO is provided by the PMIC, it would be nice to know why it
> > is modelled as an always-on regulator instead of the proper PMIC
> > regulator. Up to now we were doing this only for the outstanding power
> > rails like CX/MX or EBI.
>
> These are always ON because USB phy doesn't support power
> collapse, and there is a chance that other IP blocks might be
> sharing the rail.

You are describing the software side here. From the hardware point of
view, it is an I2C regulator, which is probably also exported as an
SMD_RPM regulator. Unless you have a good reason not to do so, there
should be a node under rpm-requests, which describes mp5496 regulators
exported via RPM. then USB should refer to those regulators.

>
> Thanks
> Varada
>
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > >  Changes in v10:
> > >         - Add LDO regulator node
> > > ---
> > >  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > index bdc1434..1f5d14f 100644
> > > --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > @@ -60,6 +60,13 @@
> > >                         regulator-min-microvolt = <725000>;
> > >                         regulator-max-microvolt = <1075000>;
> > >                 };
> > > +
> > > +               mp5496_l2: l2 {
> > > +                       regulator-min-microvolt = <1800000>;
> > > +                       regulator-max-microvolt = <1800000>;
> > > +                       regulator-boot-on;
> > > +                       regulator-always-on;
> > > +               };
> > >         };
> > >  };
> > >
> > > --
> > > 2.7.4
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry

Varadarajan Narayanan May 8, 2023, 5:16 a.m. UTC | #6

On Sat, May 06, 2023 at 02:33:11PM +0300, Dmitry Baryshkov wrote:
> On Sat, 6 May 2023 at 14:09, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Fri, May 05, 2023 at 12:29:54PM +0300, Dmitry Baryshkov wrote:
> > > On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
> > > <quic_varada@quicinc.com> wrote:
> > > >
> > > > Add LDO regulator node
> > >
> > > As this LDO is provided by the PMIC, it would be nice to know why it
> > > is modelled as an always-on regulator instead of the proper PMIC
> > > regulator. Up to now we were doing this only for the outstanding power
> > > rails like CX/MX or EBI.
> >
> > These are always ON because USB phy doesn't support power
> > collapse, and there is a chance that other IP blocks might be
> > sharing the rail.
>
> You are describing the software side here. From the hardware point of
> view, it is an I2C regulator, which is probably also exported as an
> SMD_RPM regulator. Unless you have a good reason not to do so, there
> should be a node under rpm-requests, which describes mp5496 regulators
> exported via RPM. then USB should refer to those regulators.

Yes. It is a part of rpm-requests. That is why have mentioned V10
is dependent on
https://lore.kernel.org/lkml/20230407155727.20615-1-quic_devipriy@quicinc.com/T/.
The 4th patch of the above series
(https://lore.kernel.org/lkml/20230407155727.20615-1-quic_devipriy@quicinc.com/T/#mea3f0ea37c53cf5e39e10cd6cf3bed5243cec629)
adds the rpm_requests node and this regulator definition is added
to it. Hope that is ok.

Thanks
Varada

> > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > ---
> > > >  Changes in v10:
> > > >         - Add LDO regulator node
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > > index bdc1434..1f5d14f 100644
> > > > --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > > @@ -60,6 +60,13 @@
> > > >                         regulator-min-microvolt = <725000>;
> > > >                         regulator-max-microvolt = <1075000>;
> > > >                 };
> > > > +
> > > > +               mp5496_l2: l2 {
> > > > +                       regulator-min-microvolt = <1800000>;
> > > > +                       regulator-max-microvolt = <1800000>;
> > > > +                       regulator-boot-on;
> > > > +                       regulator-always-on;
> > > > +               };
> > > >         };
> > > >  };
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
>
>
> --
> With best wishes
> Dmitry

Dmitry Baryshkov May 8, 2023, 11:21 a.m. UTC | #7

On Mon, 8 May 2023 at 08:17, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Sat, May 06, 2023 at 02:33:11PM +0300, Dmitry Baryshkov wrote:
> > On Sat, 6 May 2023 at 14:09, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > On Fri, May 05, 2023 at 12:29:54PM +0300, Dmitry Baryshkov wrote:
> > > > On Fri, 5 May 2023 at 11:23, Varadarajan Narayanan
> > > > <quic_varada@quicinc.com> wrote:
> > > > >
> > > > > Add LDO regulator node
> > > >
> > > > As this LDO is provided by the PMIC, it would be nice to know why it
> > > > is modelled as an always-on regulator instead of the proper PMIC
> > > > regulator. Up to now we were doing this only for the outstanding power
> > > > rails like CX/MX or EBI.
> > >
> > > These are always ON because USB phy doesn't support power
> > > collapse, and there is a chance that other IP blocks might be
> > > sharing the rail.
> >
> > You are describing the software side here. From the hardware point of
> > view, it is an I2C regulator, which is probably also exported as an
> > SMD_RPM regulator. Unless you have a good reason not to do so, there
> > should be a node under rpm-requests, which describes mp5496 regulators
> > exported via RPM. then USB should refer to those regulators.
>
> Yes. It is a part of rpm-requests. That is why have mentioned V10
> is dependent on
> https://lore.kernel.org/lkml/20230407155727.20615-1-quic_devipriy@quicinc.com/T/.
> The 4th patch of the above series
> (https://lore.kernel.org/lkml/20230407155727.20615-1-quic_devipriy@quicinc.com/T/#mea3f0ea37c53cf5e39e10cd6cf3bed5243cec629)
> adds the rpm_requests node and this regulator definition is added
> to it. Hope that is ok.

It is fine. Most probably I was mistaken by the patch itself. Please
excuse me. If this this is an RPM regulator, it's is fine and correct.

>
> Thanks
> Varada
>
> > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > ---
> > > > >  Changes in v10:
> > > > >         - Add LDO regulator node
> > > > > ---
> > > > >  arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > > > index bdc1434..1f5d14f 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
> > > > > @@ -60,6 +60,13 @@
> > > > >                         regulator-min-microvolt = <725000>;
> > > > >                         regulator-max-microvolt = <1075000>;
> > > > >                 };
> > > > > +
> > > > > +               mp5496_l2: l2 {
> > > > > +                       regulator-min-microvolt = <1800000>;
> > > > > +                       regulator-max-microvolt = <1800000>;
> > > > > +                       regulator-boot-on;
> > > > > +                       regulator-always-on;
> > > > > +               };
> > > > >         };
> > > > >  };
> > > > >
> > > > > --
> > > > > 2.7.4
> > > > >
> > > >
> > > >
> > > > --
> > > > With best wishes
> > > > Dmitry
> >
> >
> >
> > --
> > With best wishes
> > Dmitry

diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
index bdc1434..1f5d14f 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
+++ b/arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts
@@ -60,6 +60,13 @@ 
 			regulator-min-microvolt = <725000>;
 			regulator-max-microvolt = <1075000>;
 		};
+
+		mp5496_l2: l2 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-boot-on;
+			regulator-always-on;
+		};
 	};
 };