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

Lucas Stach
2019-12-03T11:44:33Z

On Di, 2019-12-03 at 09:33 +0100, Marco Felsch wrote:
[...]
> > > Please check the following structs:
> > >
> > > - static const struct da9062_regulator_info local_da9061_regulator_info[]
> > > - static const struct da9062_regulator_info local_da9062_regulator_info[]
> > >
> > > There you have the min_uV, uV_step, n_voltages so the core can validate
> > > if the dt-value is within the range.
> >
> > Thanks, that makes more sense.
> >
> > > > The regulator bindings state:
> > > > - regulator-min-microvolt: smallest voltage consumers may set
> > > >
> > > > - regulator-max-microvolt: largest voltage consumers may set
> > >
> > > Yes and according the datasheet I mentionied the current values aren't
> > > correct.
> > >
> > > > For me that is device depended and not design depended.
> > > >
> > > > What is the scenario you're thinking about which would cause the SOC, as a
> > > > consumer, to request a lower voltage as it needs?
> > >
> > > The thing is that the DT abstracts the HW and these values are not
> > > correct. As mentioned in my commit message the values should meet
> > > the datasheet restrictions and this isn't the case yet.
> >
> > I don't agree. The datasheet you mention is the i.MX 6 datasheet and thus
> > the limitation should reside in the i.MX 6 regulators and not in the PMIC's.
> > This limitation is not just valid in combination with that PMIC but for all
> > i.MX 6.
>
> The datasheet tells you which voltage should be applied to the imx6 and
> so you have to set this here. What happens if the internally ldo
> request a voltage value below 0.9V? Then the value will be applied
> because we specified 0.73V and the system don't work anymore or did you
> verified that case?

The DT constraints are supposed to reflect absolute maximum ratings of
the board design. The regulator driver already knows the limits of the
PMIC chip, so there is no point in duplicating this information in the
DT.

The DT constraints are there to make sure the regulator core can
constrain the voltage setting to something safe for the specific
design. A consumer driver bug must never be able to set a voltage that
is outside of the absolute maximum ratings of _all_ consumers of this
specific power rail. I know that a lot of DTs get this detail wrong,
but we shouldn't block patches to fix this. :)

> > If I have this wrong and the maintainers agree with you could you please
> > make sure to account for the bypass mode as well since these values from the
> > datasheet are valid too?
>
> As I said, the bypass mode isn't supported by the driver and all imx6
> based devicetrees follow that case. So we don't have to take that into
> account. Also we can't meet both contraints with one dt and futhermore
> the bypass mode decrease your imx6 lifetime due the the increased ripple
> on the arm-core supply. So I think no one wants this setup in the near
> future.

As a violation of the minimum voltage setting is very unlikely to cause
any permanent damage to the design (expect if you got reverse voltage
flows somewhere) I think it is safe to include the LDO bypass supply
limits as the lower bound in the DT constraints, even if this mode
isn't currently used anywhere.

Regards,
Lucas