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

Stefan Riedmüller
2019-12-02T13:55:13Z

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?

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