[PATCH v1 1/2] regulator: pca9450: add enable_val for all bucks

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 <redacted>
---
 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 <redacted>
---
 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.

Unrelated change?

Unrelated change?

Regards,
  Marco

On 2020/08/31 18:53 Marco Felsch [off-list ref] wrote:

Hi Robin,

On 20-09-01 00:48, Robin Gong wrote:

quoted

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.

quoted

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.

quoted

Signed-off-by: Robin Gong <redacted>
---
 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[] = {

quoted

 			.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[] = {

quoted

 			.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 [off-list ref] wrote:

On 2020/08/31 18:53 Marco Felsch [off-list ref] wrote:

quoted

Hi Robin,

On 20-09-01 00:48, Robin Gong wrote:

quoted

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'

quoted

as ON.

quoted

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.

quoted

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?   

quoted

quoted

Signed-off-by: Robin Gong <redacted>
---
 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[] = {

quoted

 			.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[] = {

quoted

 			.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.