RE: [PATCH 03/15] arm64: dts: imx8mp-evk: fix BUCK/LDO voltage

[Jacky Bai]

> Subject: Re: [PATCH 03/15] arm64: dts: imx8mp-evk: fix BUCK/LDO voltage
> 
> Hi Marco
> 
> On 10/20/2022 7:02 PM, Marco Felsch wrote:
> > Hi Peng,
> >
> > On 22-10-20, Peng Fan (OSS) wrote:
> >> From: Peng Fan <peng.fan@xxxxxxx>
> >>
> >> Per PCA9450C datasheet, the voltage range as below:
> >> BUCK1 0.6 - 2.1875
> >> BUCK2 0.6 - 2.1875
> >> BUCK4 0.6 - 3.4
> >> BUCK5 0.6 - 3.4
> >> BUCK6 0.6 - 3.4
> >>
> >> LDO1 1.6-1.9, 3.0-3.3
> >> LDO2 0.8 – 1.15
> >> LDO3 0.8 - 3.3
> >> LDO4 0.8 - 3.3
> >> LDO5 1.8 - 3.3
> >>
> >> So correct them, and also add LDO[2,4]
> >
> > In the DTS you specify voltage constraints for a specific hardware and
> > not the one supported by the PMIC. What the PMIC supports (min/max) is
> > specified within the driver.
> 

IMO, the dts properties regulator-min/max-microvolt in the regulator node should be properties
to list the ability of each regulator of PMIC. If these properties is not for PMIC itself,
how to define the board range using these properties? ^_^

And in the PMIC driver, no each PMIC's min/max range define.

BR
> Technically we could set board voltage to PMIC voltage range, it not damage
> the board from hardware level.
> but I am not sure whether Linux could work proper.
> Saying set VDD_SOC/DDR to pmic min voltage, system will crash, because it
> could not support higher freq.
> 
> I am not sure how to proceed on this, just update commit log to say that the
> board voltage range could be set to pmic voltage range?
> 
> Thanks,
> Peng.
> 
> >
> > Regards,
> >    Marco
> >
> >>
> >> Fixes: 5497bc2a2bff ("arm64: dts: imx8mp-evk: Add PMIC device")
> >> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> >> ---
> >>   arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 44
> +++++++++++++-------
> >>   1 file changed, 30 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> >> b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> >> index b4c1ef2559f2..a4cddc5a8620 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> >> @@ -248,8 +248,8 @@ pmic@25 {
> >>   		regulators {
> >>   			BUCK1 {
> >>   				regulator-name = "BUCK1";
> >> -				regulator-min-microvolt = <720000>;
> >> -				regulator-max-microvolt = <1000000>;
> >> +				regulator-min-microvolt = <600000>;
> >> +				regulator-max-microvolt = <2187500>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   				regulator-ramp-delay = <3125>;
> >> @@ -257,8 +257,8 @@ BUCK1 {
> >>
> >>   			reg_arm: BUCK2 {
> >>   				regulator-name = "BUCK2";
> >> -				regulator-min-microvolt = <720000>;
> >> -				regulator-max-microvolt = <1025000>;
> >> +				regulator-min-microvolt = <600000>;
> >> +				regulator-max-microvolt = <2187500>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   				regulator-ramp-delay = <3125>;
> >> @@ -268,40 +268,56 @@ reg_arm: BUCK2 {
> >>
> >>   			BUCK4 {
> >>   				regulator-name = "BUCK4";
> >> -				regulator-min-microvolt = <3000000>;
> >> -				regulator-max-microvolt = <3600000>;
> >> +				regulator-min-microvolt = <600000>;
> >> +				regulator-max-microvolt = <3400000>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   			};
> >>
> >>   			BUCK5 {
> >>   				regulator-name = "BUCK5";
> >> -				regulator-min-microvolt = <1650000>;
> >> -				regulator-max-microvolt = <1950000>;
> >> +				regulator-min-microvolt = <600000>;
> >> +				regulator-max-microvolt = <3400000>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   			};
> >>
> >>   			BUCK6 {
> >>   				regulator-name = "BUCK6";
> >> -				regulator-min-microvolt = <1045000>;
> >> -				regulator-max-microvolt = <1155000>;
> >> +				regulator-min-microvolt = <600000>;
> >> +				regulator-max-microvolt = <3400000>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   			};
> >>
> >>   			LDO1 {
> >>   				regulator-name = "LDO1";
> >> -				regulator-min-microvolt = <1650000>;
> >> -				regulator-max-microvolt = <1950000>;
> >> +				regulator-min-microvolt = <1600000>;
> >> +				regulator-max-microvolt = <3300000>;
> >> +				regulator-boot-on;
> >> +				regulator-always-on;
> >> +			};
> >> +
> >> +			LDO2 {
> >> +				regulator-name = "LDO2";
> >> +				regulator-min-microvolt = <800000>;
> >> +				regulator-max-microvolt = <1150000>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   			};
> >>
> >>   			LDO3 {
> >>   				regulator-name = "LDO3";
> >> -				regulator-min-microvolt = <1710000>;
> >> -				regulator-max-microvolt = <1890000>;
> >> +				regulator-min-microvolt = <800000>;
> >> +				regulator-max-microvolt = <3300000>;
> >> +				regulator-boot-on;
> >> +				regulator-always-on;
> >> +			};
> >> +
> >> +			ldo4: LDO4 {
> >> +				regulator-name = "LDO4";
> >> +				regulator-min-microvolt = <800000>;
> >> +				regulator-max-microvolt = <3300000>;
> >>   				regulator-boot-on;
> >>   				regulator-always-on;
> >>   			};
> >> --
> >> 2.37.1
> >>
> >>
> >>

Attachment: smime.p7s
Description: S/MIME cryptographic signature