BuckX enable mode
00b = OFF
01b = ON by PMIC_ON_REQ = H
10b = ON by PMIC_ON_REQ = H && PMIC_STBY_REQ = L
11b = Always ON
For such enable mode, enable_value should be clearly set in requlator desc,
00/11 is not enough, correct it now for different bucks. For example, buck2
is designed for vddarm which could be off in 'PMIC_STBY_REQ = H' after kernel
enter suspend, so should be set '10b' as ON, while others is '01b' as ON.
All are the same as the default setting which means bucks no need to be
enabled again during kernel boot even if they have been enabled already after
pmic on.
Signed-off-by: Robin Gong <[email protected]>
---
drivers/regulator/pca9450-regulator.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index eb5822b..79f2a5a 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -249,6 +249,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.vsel_mask = BUCK1OUT_DVS0_MASK,
.enable_reg = PCA9450_REG_BUCK1CTRL,
.enable_mask = BUCK1_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
.of_parse_cb = pca9450_set_dvs_levels,
},
@@ -273,7 +274,8 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
.vsel_mask = BUCK2OUT_DVS0_MASK,
.enable_reg = PCA9450_REG_BUCK2CTRL,
- .enable_mask = BUCK1_ENMODE_MASK,
+ .enable_mask = BUCK2_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
.owner = THIS_MODULE,
.of_parse_cb = pca9450_set_dvs_levels,
},
@@ -299,6 +301,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.vsel_mask = BUCK3OUT_DVS0_MASK,
.enable_reg = PCA9450_REG_BUCK3CTRL,
.enable_mask = BUCK3_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
.of_parse_cb = pca9450_set_dvs_levels,
},
@@ -324,6 +327,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.vsel_mask = BUCK4OUT_MASK,
.enable_reg = PCA9450_REG_BUCK4CTRL,
.enable_mask = BUCK4_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
},
},
@@ -342,6 +346,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.vsel_mask = BUCK5OUT_MASK,
.enable_reg = PCA9450_REG_BUCK5CTRL,
.enable_mask = BUCK5_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
},
},
@@ -360,6 +365,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
.vsel_mask = BUCK6OUT_MASK,
.enable_reg = PCA9450_REG_BUCK6CTRL,
.enable_mask = BUCK6_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
},
},
@@ -475,6 +481,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.vsel_mask = BUCK1OUT_DVS0_MASK,
.enable_reg = PCA9450_REG_BUCK1CTRL,
.enable_mask = BUCK1_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
.of_parse_cb = pca9450_set_dvs_levels,
},
@@ -499,7 +506,8 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
.vsel_mask = BUCK2OUT_DVS0_MASK,
.enable_reg = PCA9450_REG_BUCK2CTRL,
- .enable_mask = BUCK1_ENMODE_MASK,
+ .enable_mask = BUCK2_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
.owner = THIS_MODULE,
.of_parse_cb = pca9450_set_dvs_levels,
},
@@ -525,6 +533,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.vsel_mask = BUCK4OUT_MASK,
.enable_reg = PCA9450_REG_BUCK4CTRL,
.enable_mask = BUCK4_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
},
},
@@ -543,6 +552,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.vsel_mask = BUCK5OUT_MASK,
.enable_reg = PCA9450_REG_BUCK5CTRL,
.enable_mask = BUCK5_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
},
},
@@ -561,6 +571,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
.vsel_mask = BUCK6OUT_MASK,
.enable_reg = PCA9450_REG_BUCK6CTRL,
.enable_mask = BUCK6_ENMODE_MASK,
+ .enable_val = BUCK_ENMODE_ONREQ,
.owner = THIS_MODULE,
},
},
--
2.7.4
Add pca9450c pmic for imx8mp-evk board.
Signed-off-by: Robin Gong <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 115 +++++++++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index 3da1fff..8e000c1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -66,6 +66,108 @@
};
};
+&i2c1 {
+ clock-frequency = <400000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c1>;
+ status = "okay";
+
+ pmic: pca9450@25 {
+ reg = <0x25>;
+ compatible = "nxp,pca9450c";
+ /* PMIC PCA9450 PMIC_nINT GPIO1_IO3 */
+ pinctrl-0 = <&pinctrl_pmic>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <3 GPIO_ACTIVE_LOW>;
+
+ regulators {
+ buck1: BUCK1 {
+ regulator-name = "BUCK1";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <2187500>;
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-ramp-delay = <3125>;
+ };
+
+ buck2: BUCK2 {
+ regulator-name = "BUCK2";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <2187500>;
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-ramp-delay = <3125>;
+ nxp,dvs-run-voltage = <950000>;
+ nxp,dvs-standby-voltage = <850000>;
+ };
+
+ buck4: BUCK4{
+ regulator-name = "BUCK4";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ buck5: BUCK5{
+ regulator-name = "BUCK5";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ buck6: BUCK6 {
+ regulator-name = "BUCK6";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ ldo1: LDO1 {
+ regulator-name = "LDO1";
+ regulator-min-microvolt = <1600000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ ldo2: LDO2 {
+ regulator-name = "LDO2";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <1150000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ ldo3: LDO3 {
+ regulator-name = "LDO3";
+ 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;
+ };
+
+ ldo5: LDO5 {
+ regulator-name = "LDO5";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+ };
+ };
+};
+
&i2c3 {
clock-frequency = <400000>;
pinctrl-names = "default";
@@ -152,6 +254,13 @@
>;
};
+ pinctrl_i2c1: i2c1grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL 0x400001c3
+ MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA 0x400001c3
+ >;
+ };
+
pinctrl_i2c3: i2c3grp {
fsl,pins = <
MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL 0x400001c3
@@ -159,6 +268,12 @@
>;
};
+ pinctrl_pmic: pmicirq {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03 0x41
+ >;
+ };
+
pinctrl_reg_usdhc2_vmmc: regusdhc2vmmc {
fsl,pins = <
MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19 0x41
--
2.7.4
Hi Robin,
On 20-09-01 00:48, Robin Gong wrote:
> BuckX enable mode
> 00b = OFF
> 01b = ON by PMIC_ON_REQ = H
> 10b = ON by PMIC_ON_REQ = H && PMIC_STBY_REQ = L
> 11b = Always ON
>
> For such enable mode, enable_value should be clearly set in requlator desc,
> 00/11 is not enough, correct it now for different bucks. For example, buck2
> is designed for vddarm which could be off in 'PMIC_STBY_REQ = H' after kernel
> enter suspend, so should be set '10b' as ON, while others is '01b' as ON.
> All are the same as the default setting which means bucks no need to be
> enabled again during kernel boot even if they have been enabled already after
> pmic on.
I wouldn't hard-code the regulator behaviour because the behaviour comes
from the system design which in most cases are comming from our hw-guys.
Till now I saw a few intelligent designs don't following the pmic user
recommendations to save money. I would love to specify the regulator
behaviour/mode within the dt or acpi.
> Signed-off-by: Robin Gong <[email protected]>
> ---
> drivers/regulator/pca9450-regulator.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
> index eb5822b..79f2a5a 100644
> --- a/drivers/regulator/pca9450-regulator.c
> +++ b/drivers/regulator/pca9450-regulator.c
> @@ -249,6 +249,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK1OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK1CTRL,
> .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -273,7 +274,8 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> .vsel_mask = BUCK2OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK2CTRL,
> - .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_mask = BUCK2_ENMODE_MASK,
Unrelated change?
> + .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -299,6 +301,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK3OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK3CTRL,
> .enable_mask = BUCK3_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -324,6 +327,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK4OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK4CTRL,
> .enable_mask = BUCK4_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -342,6 +346,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK5OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK5CTRL,
> .enable_mask = BUCK5_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -360,6 +365,7 @@ static const struct pca9450_regulator_desc pca9450a_regulators[] = {
> .vsel_mask = BUCK6OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK6CTRL,
> .enable_mask = BUCK6_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -475,6 +481,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK1OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK1CTRL,
> .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -499,7 +506,8 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> .vsel_mask = BUCK2OUT_DVS0_MASK,
> .enable_reg = PCA9450_REG_BUCK2CTRL,
> - .enable_mask = BUCK1_ENMODE_MASK,
> + .enable_mask = BUCK2_ENMODE_MASK,
Unrelated change?
Regards,
Marco
> + .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> .owner = THIS_MODULE,
> .of_parse_cb = pca9450_set_dvs_levels,
> },
> @@ -525,6 +533,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK4OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK4CTRL,
> .enable_mask = BUCK4_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -543,6 +552,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK5OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK5CTRL,
> .enable_mask = BUCK5_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> @@ -561,6 +571,7 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
> .vsel_mask = BUCK6OUT_MASK,
> .enable_reg = PCA9450_REG_BUCK6CTRL,
> .enable_mask = BUCK6_ENMODE_MASK,
> + .enable_val = BUCK_ENMODE_ONREQ,
> .owner = THIS_MODULE,
> },
> },
> --
> 2.7.4
>
On 2020/08/31 18:53 Marco Felsch <[email protected]> wrote:
> Hi Robin,
>
> On 20-09-01 00:48, Robin Gong wrote:
> > BuckX enable mode
> > 00b = OFF
> > 01b = ON by PMIC_ON_REQ = H
> > 10b = ON by PMIC_ON_REQ = H && PMIC_STBY_REQ = L 11b = Always ON
> >
> > For such enable mode, enable_value should be clearly set in requlator
> > desc,
> > 00/11 is not enough, correct it now for different bucks. For example,
> > buck2 is designed for vddarm which could be off in 'PMIC_STBY_REQ = H'
> > after kernel enter suspend, so should be set '10b' as ON, while others is '01b'
> as ON.
> > All are the same as the default setting which means bucks no need to
> > be enabled again during kernel boot even if they have been enabled
> > already after pmic on.
>
> I wouldn't hard-code the regulator behaviour because the behaviour comes
> from the system design which in most cases are comming from our hw-guys.
> Till now I saw a few intelligent designs don't following the pmic user
> recommendations to save money. I would love to specify the regulator
> behaviour/mode within the dt or acpi.
Well, if so the better way is moving into dts. Will implement it in v2.
>
> > Signed-off-by: Robin Gong <[email protected]>
> > ---
> > drivers/regulator/pca9450-regulator.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/regulator/pca9450-regulator.c
> > b/drivers/regulator/pca9450-regulator.c
> > index eb5822b..79f2a5a 100644
> > --- a/drivers/regulator/pca9450-regulator.c
> > +++ b/drivers/regulator/pca9450-regulator.c
> > @@ -249,6 +249,7 @@ static const struct pca9450_regulator_desc
> pca9450a_regulators[] = {
> > .vsel_mask = BUCK1OUT_DVS0_MASK,
> > .enable_reg = PCA9450_REG_BUCK1CTRL,
> > .enable_mask = BUCK1_ENMODE_MASK,
> > + .enable_val = BUCK_ENMODE_ONREQ,
> > .owner = THIS_MODULE,
> > .of_parse_cb = pca9450_set_dvs_levels,
> > },
> > @@ -273,7 +274,8 @@ static const struct pca9450_regulator_desc
> pca9450a_regulators[] = {
> > .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > .vsel_mask = BUCK2OUT_DVS0_MASK,
> > .enable_reg = PCA9450_REG_BUCK2CTRL,
> > - .enable_mask = BUCK1_ENMODE_MASK,
> > + .enable_mask = BUCK2_ENMODE_MASK,
>
> Unrelated change?
Yes, that's just correct it minor literal since that's BUCK2 although
they're the same, will split it anyway.
On 2020/09/01 9:58 Robin Gong <[email protected]> wrote:
> On 2020/08/31 18:53 Marco Felsch <[email protected]> wrote:
> > Hi Robin,
> >
> > On 20-09-01 00:48, Robin Gong wrote:
> > > BuckX enable mode
> > > 00b = OFF
> > > 01b = ON by PMIC_ON_REQ = H
> > > 10b = ON by PMIC_ON_REQ = H && PMIC_STBY_REQ = L 11b = Always ON
> > >
> > > For such enable mode, enable_value should be clearly set in
> > > requlator desc,
> > > 00/11 is not enough, correct it now for different bucks. For
> > > example,
> > > buck2 is designed for vddarm which could be off in 'PMIC_STBY_REQ = H'
> > > after kernel enter suspend, so should be set '10b' as ON, while others is
> '01b'
> > as ON.
> > > All are the same as the default setting which means bucks no need to
> > > be enabled again during kernel boot even if they have been enabled
> > > already after pmic on.
> >
> > I wouldn't hard-code the regulator behaviour because the behaviour
> > comes from the system design which in most cases are comming from our
> hw-guys.
> > Till now I saw a few intelligent designs don't following the pmic user
> > recommendations to save money. I would love to specify the regulator
> > behaviour/mode within the dt or acpi.
> Well, if so the better way is moving into dts. Will implement it in v2.
Hi Marco,
Sorry, I'm afraid that big changes have to be made for moving it to dts, because
current pca9450 driver is using common regulator_enable_regmap() which means
rdev->desc is const and 'enable_value' can't be changed runtime with the value in
dts. Regards to pca9450 is the specific pmic designed for i.mx8m family rather than
generic pmic, the power rails defining should be the same as NXP MEK reference
board, especially for the BUCKs which match well with SOC's power. So may I leave it
at this time, and pick it up if the 'unbelievable trouble' really come out in the future?
>
> >
> > > Signed-off-by: Robin Gong <[email protected]>
> > > ---
> > > drivers/regulator/pca9450-regulator.c | 15 +++++++++++++--
> > > 1 file changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/regulator/pca9450-regulator.c
> > > b/drivers/regulator/pca9450-regulator.c
> > > index eb5822b..79f2a5a 100644
> > > --- a/drivers/regulator/pca9450-regulator.c
> > > +++ b/drivers/regulator/pca9450-regulator.c
> > > @@ -249,6 +249,7 @@ static const struct pca9450_regulator_desc
> > pca9450a_regulators[] = {
> > > .vsel_mask = BUCK1OUT_DVS0_MASK,
> > > .enable_reg = PCA9450_REG_BUCK1CTRL,
> > > .enable_mask = BUCK1_ENMODE_MASK,
> > > + .enable_val = BUCK_ENMODE_ONREQ,
> > > .owner = THIS_MODULE,
> > > .of_parse_cb = pca9450_set_dvs_levels,
> > > },
> > > @@ -273,7 +274,8 @@ static const struct pca9450_regulator_desc
> > pca9450a_regulators[] = {
> > > .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > > .vsel_mask = BUCK2OUT_DVS0_MASK,
> > > .enable_reg = PCA9450_REG_BUCK2CTRL,
> > > - .enable_mask = BUCK1_ENMODE_MASK,
> > > + .enable_mask = BUCK2_ENMODE_MASK,
> >
> > Unrelated change?
> Yes, that's just correct it minor literal since that's BUCK2 although they're the
> same, will split it anyway.