Re: [PATCH 1/3] ARM: dts: imx6: phycore-som: fix arm and soc minimum voltage

Marco Felsch
2019-12-02T14:14:23Z

Hi Stefan,

On 19-12-02 14:55, Stefan Riedmüller wrote:
> Hi Marco,
>
> On 02.12.19 13:42, Marco Felsch wrote:
> > Hi Stefan,
> >
> > On 19-12-02 11:09, Stefan Riedmüller wrote:
> > > Hi Marco,
> > >
> > > your proposed setting is only valid for the LDO enabled case but not for the
> > > case where the LDO's are in bypass mode. Is that intended? In bypass mode it
> > > actually needs to be 0.925 V min for ARM and 1.15 V min for SOC.
> >
> > The case is that the driver doesn't support the bypass mode currently so
> > yes it was intended.
>
> Ok, I see.
>
> >
> > > Did you experience an issue with the current settings or is this just a
> > > cosmetic change?
> >
> > There is currently no issue because the internally LDO's don't try to
> > apply such a low voltage value. But I think it isn't a cosmetic change
> > because this value is wrong. We need to specify the valid voltage range.
>
> Please correct me if I'm wrong, but isn't the regulator-min and
> regulator-max values supposed to reflect the min and max values this
> regulator can deliver?

Nope, the constraints are hard coded within the driver e.g. da9062:

8<-----------------------------------------------------------------
/* Regulator operations */

/* Current limits array (in uA)
* - DA9061_ID_[BUCK1|BUCK3]
* - DA9062_ID_[BUCK1|BUCK2|BUCK4]
* Entry indexes corresponds to register values.
*/
static const unsigned int da9062_buck_a_limits[] = {
500000, 600000, 700000, 800000, 900000, 1000000,
1100000, 1200000,
1300000, 1400000, 1500000, 1600000, 1700000,
1800000, 1900000, 2000000
};

/* Current limits array (in uA)
* - DA9061_ID_BUCK2
* - DA9062_ID_BUCK3
* Entry indexes corresponds to register values.
*/
static const unsigned int
da9062_buck_b_limits[] = {
1500000, 1600000, 1700000, 1800000,
1900000, 2000000, 2100000, 2200000,
2300000, 2400000, 2500000,
2600000, 2700000, 2800000,
2900000, 3000000
};

8<-----------------------------------------------------------------

So you have to specify the min/max voltage for your design.

Regards,
Marco

> Maybe your change is better placed in the anatop regulators. Btw they also
> have a 0.725 V minimum voltage:
>
> From arch/arm/boot/dts/imx6qdl.dtsi:
>
> reg_arm: regulator-vddcore {
>
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddarm";
>
> regulator-min-microvolt = <725000>;
>
> regulator-max-microvolt = <1450000>;
> regulator-always-on;
>
> anatop-reg-offset = <0x140>;
>
> anatop-vol-bit-shift = <0>;
>
> anatop-vol-bit-width = <5>;
>
> anatop-delay-reg-offset = <0x170>;
>
> anatop-delay-bit-shift = <24>;
>
> anatop-delay-bit-width = <2>;
>
> anatop-min-bit-val = <1>;
>
> anatop-min-voltage = <725000>;
>
> anatop-max-voltage = <1450000>;
>
> };
>
>
> Regards,
> Stefan
>
> >
> > Regards,
> > Marco
> >
> > > Regards,
> > > Stefan
> > >
> > >
> > > On 29.11.19 17:48, Marco Felsch wrote:
> > > > The current set minimum voltage of 730000mV seems to be wrong. I don't
> > > > know the document which specifies that but the imx6qdl datasheets says
> > > > that the minimum voltage should be 1.05V for VDD_ARM (LDO enabled, lowest
> > > > opp) and 1.275V for VDD_SOC (LDO enabled, lowest opp).
> > > >
> > > > Fixes: ddec5d1c0047 ("ARM: dts: imx6: Add initial support for phyCORE-i.MX 6 SOM")
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > > arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi
> > > > index 6486df3e2942..46d4953c5588 100644
> > > > --- a/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi
> > > > +++ b/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi
> > > > @@ -107,14 +107,14 @@
> > > > regulators {
> > > > vdd_arm: buck1 {
> > > > regulator-name = "vdd_arm";
> > > > - regulator-min-microvolt = <730000>;
> > > > + regulator-min-microvolt = <1050000>;
> > > > regulator-max-microvolt = <1380000>;
> > > > regulator-always-on;
> > > > };
> > > > vdd_soc: buck2 {
> > > > regulator-name = "vdd_soc";
> > > > - regulator-min-microvolt = <730000>;
> > > > + regulator-min-microvolt = <1275000>;
> > > > regulator-max-microvolt = <1380000>;
> > > > regulator-always-on;
> > > > };
> > > >
> > >
> >
>

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |