| Message ID | 8894bf2c44eaf4959c7a1966b66229e6cf5cda96.1683183860.git.quic_varada@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable IPQ9754 USB | expand |
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 >
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 >> > >
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; > >> + }; > >> }; > >> };
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
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
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
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 --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; + }; }; };
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(+)