Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Krzysztof Kozlowski]

On 31/05/2023 08:57, Joy Zou wrote:
> Update pca9450 bindings.
> 
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---

Subject prefix is: regulator: dt-bindings: pca9450:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v1 0/3] add pmic pca9451a support

[Joy Zou]

The patchset supports pmic pca9451a.
For the details, please check the patch commit log.

Joy Zou (3):
  dt-bindings: regulator: pca9450: add pca9451a support
  regulator: pca9450: add pca9451a support
  arm64: dts: imx93-11x11-evk: add pca9451a support

 .../regulator/nxp,pca9450-regulator.yaml      |   1 +
 .../boot/dts/freescale/imx93-11x11-evk.dts    | 111 ++++++++
 drivers/regulator/pca9450-regulator.c         | 262 +++++++++++++++++-
 include/linux/regulator/pca9450.h             |   2 +
 4 files changed, 364 insertions(+), 12 deletions(-)

-- 
2.37.1

[PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Joy Zou]

Update pca9450 bindings.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 .../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml     | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
index 3d469b8e9774..849bfa50bdba 100644
--- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
@@ -28,6 +28,7 @@ properties:
       - nxp,pca9450a
       - nxp,pca9450b
       - nxp,pca9450c
+      - nxp,pca9451a
 
   reg:
     maxItems: 1
-- 
2.37.1

[PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Joy Zou]

Adding support for pmic pca9451a.

This patch support old and new pmic pca9451a. The new pmic trimed BUCK1.
The default value of Toff_Deb is used to distinguish the old and new pmic.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 drivers/regulator/pca9450-regulator.c | 262 ++++++++++++++++++++++++--
 include/linux/regulator/pca9450.h     |   2 +
 2 files changed, 252 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c
index 91bfb7e026c9..654aa4fbe494 100644
--- a/drivers/regulator/pca9450-regulator.c
+++ b/drivers/regulator/pca9450-regulator.c
@@ -104,7 +104,15 @@ static const struct regulator_ops pca9450_ldo_regulator_ops = {
  * 0.60 to 2.1875V (12.5mV step)
  */
 static const struct linear_range pca9450_dvs_buck_volts[] = {
-	REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
+	REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500),
+};
+
+/*
+ * BUCK1/3
+ * 0.65 to 2.2375V (12.5mV step)
+ */
+static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
+	REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
 };
 
 /*
@@ -663,6 +671,207 @@ static const struct pca9450_regulator_desc pca9450bc_regulators[] = {
 	},
 };
 
+static const struct pca9450_regulator_desc pca9451a_regulators[] = {
+	{
+		.desc = {
+			.name = "buck1",
+			.of_match = of_match_ptr("BUCK1"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_BUCK1,
+			.ops = &pca9450_dvs_buck_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
+			.linear_ranges = pca9450_dvs_buck_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_dvs_buck_volts),
+			.vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
+			.vsel_mask = BUCK1OUT_DVS0_MASK,
+			.enable_reg = PCA9450_REG_BUCK1CTRL,
+			.enable_mask = BUCK1_ENMODE_MASK,
+			.enable_val = BUCK_ENMODE_ONREQ,
+			.ramp_mask = BUCK1_RAMP_MASK,
+			.ramp_delay_table = pca9450_dvs_buck_ramp_table,
+			.n_ramp_values = ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
+			.owner = THIS_MODULE,
+			.of_parse_cb = pca9450_set_dvs_levels,
+		},
+		.dvs = {
+			.run_reg = PCA9450_REG_BUCK1OUT_DVS0,
+			.run_mask = BUCK1OUT_DVS0_MASK,
+			.standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
+			.standby_mask = BUCK1OUT_DVS1_MASK,
+		},
+	},
+	{
+		.desc = {
+			.name = "buck1_trim",
+			.of_match = of_match_ptr("BUCK1"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_BUCK1,
+			.ops = &pca9450_dvs_buck_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
+			.linear_ranges = pca9450_trim_dvs_buck_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_trim_dvs_buck_volts),
+			.vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
+			.vsel_mask = BUCK1OUT_DVS0_MASK,
+			.enable_reg = PCA9450_REG_BUCK1CTRL,
+			.enable_mask = BUCK1_ENMODE_MASK,
+			.enable_val = BUCK_ENMODE_ONREQ,
+			.ramp_mask = BUCK1_RAMP_MASK,
+			.ramp_delay_table = pca9450_dvs_buck_ramp_table,
+			.n_ramp_values = ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
+			.owner = THIS_MODULE,
+			.of_parse_cb = pca9450_set_dvs_levels,
+		},
+		.dvs = {
+			.run_reg = PCA9450_REG_BUCK1OUT_DVS0,
+			.run_mask = BUCK1OUT_DVS0_MASK,
+			.standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
+			.standby_mask = BUCK1OUT_DVS1_MASK,
+		},
+	},
+	{
+		.desc = {
+			.name = "buck2",
+			.of_match = of_match_ptr("BUCK2"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_BUCK2,
+			.ops = &pca9450_dvs_buck_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_BUCK2_VOLTAGE_NUM,
+			.linear_ranges = pca9450_dvs_buck_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_dvs_buck_volts),
+			.vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
+			.vsel_mask = BUCK2OUT_DVS0_MASK,
+			.enable_reg = PCA9450_REG_BUCK2CTRL,
+			.enable_mask = BUCK2_ENMODE_MASK,
+			.enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
+			.ramp_mask = BUCK2_RAMP_MASK,
+			.ramp_delay_table = pca9450_dvs_buck_ramp_table,
+			.n_ramp_values = ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
+			.owner = THIS_MODULE,
+			.of_parse_cb = pca9450_set_dvs_levels,
+		},
+		.dvs = {
+			.run_reg = PCA9450_REG_BUCK2OUT_DVS0,
+			.run_mask = BUCK2OUT_DVS0_MASK,
+			.standby_reg = PCA9450_REG_BUCK2OUT_DVS1,
+			.standby_mask = BUCK2OUT_DVS1_MASK,
+		},
+	},
+	{
+		.desc = {
+			.name = "buck4",
+			.of_match = of_match_ptr("BUCK4"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_BUCK4,
+			.ops = &pca9450_buck_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_BUCK4_VOLTAGE_NUM,
+			.linear_ranges = pca9450_buck_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_buck_volts),
+			.vsel_reg = PCA9450_REG_BUCK4OUT,
+			.vsel_mask = BUCK4OUT_MASK,
+			.enable_reg = PCA9450_REG_BUCK4CTRL,
+			.enable_mask = BUCK4_ENMODE_MASK,
+			.enable_val = BUCK_ENMODE_ONREQ,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "buck5",
+			.of_match = of_match_ptr("BUCK5"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_BUCK5,
+			.ops = &pca9450_buck_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_BUCK5_VOLTAGE_NUM,
+			.linear_ranges = pca9450_buck_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_buck_volts),
+			.vsel_reg = PCA9450_REG_BUCK5OUT,
+			.vsel_mask = BUCK5OUT_MASK,
+			.enable_reg = PCA9450_REG_BUCK5CTRL,
+			.enable_mask = BUCK5_ENMODE_MASK,
+			.enable_val = BUCK_ENMODE_ONREQ,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "buck6",
+			.of_match = of_match_ptr("BUCK6"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_BUCK6,
+			.ops = &pca9450_buck_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_BUCK6_VOLTAGE_NUM,
+			.linear_ranges = pca9450_buck_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_buck_volts),
+			.vsel_reg = PCA9450_REG_BUCK6OUT,
+			.vsel_mask = BUCK6OUT_MASK,
+			.enable_reg = PCA9450_REG_BUCK6CTRL,
+			.enable_mask = BUCK6_ENMODE_MASK,
+			.enable_val = BUCK_ENMODE_ONREQ,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "ldo1",
+			.of_match = of_match_ptr("LDO1"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_LDO1,
+			.ops = &pca9450_ldo_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_LDO1_VOLTAGE_NUM,
+			.linear_ranges = pca9450_ldo1_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_ldo1_volts),
+			.vsel_reg = PCA9450_REG_LDO1CTRL,
+			.vsel_mask = LDO1OUT_MASK,
+			.enable_reg = PCA9450_REG_LDO1CTRL,
+			.enable_mask = LDO1_EN_MASK,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "ldo4",
+			.of_match = of_match_ptr("LDO4"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_LDO4,
+			.ops = &pca9450_ldo_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_LDO4_VOLTAGE_NUM,
+			.linear_ranges = pca9450_ldo34_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_ldo34_volts),
+			.vsel_reg = PCA9450_REG_LDO4CTRL,
+			.vsel_mask = LDO4OUT_MASK,
+			.enable_reg = PCA9450_REG_LDO4CTRL,
+			.enable_mask = LDO4_EN_MASK,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "ldo5",
+			.of_match = of_match_ptr("LDO5"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = PCA9450_LDO5,
+			.ops = &pca9450_ldo_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
+			.linear_ranges = pca9450_ldo5_volts,
+			.n_linear_ranges = ARRAY_SIZE(pca9450_ldo5_volts),
+			.vsel_reg = PCA9450_REG_LDO5CTRL_H,
+			.vsel_mask = LDO5HOUT_MASK,
+			.enable_reg = PCA9450_REG_LDO5CTRL_H,
+			.enable_mask = LDO5H_EN_MASK,
+			.owner = THIS_MODULE,
+		},
+	},
+};
+
 static irqreturn_t pca9450_irq_handler(int irq, void *data)
 {
 	struct pca9450 *pca9450 = data;
@@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 	const struct pca9450_regulator_desc	*regulator_desc;
 	struct regulator_config config = { };
 	struct pca9450 *pca9450;
-	unsigned int device_id, i;
+	unsigned int device_id, i, val;
 	unsigned int reset_ctrl;
+	bool pmic_trim = false;
 	int ret;
 
 	if (!i2c->irq) {
@@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 	if (!pca9450)
 		return -ENOMEM;
 
+	pca9450->regmap = devm_regmap_init_i2c(i2c,
+					       &pca9450_regmap_config);
+	if (IS_ERR(pca9450->regmap)) {
+		dev_err(&i2c->dev, "regmap initialization failed\n");
+		return PTR_ERR(pca9450->regmap);
+	}
+
+	ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL, &val);
+	if (ret) {
+		dev_err(&i2c->dev, "Read device id error\n");
+		return ret;
+	}
+
+	if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
+		pmic_trim = true;
+
 	switch (type) {
 	case PCA9450_TYPE_PCA9450A:
 		regulator_desc = pca9450a_regulators;
@@ -730,6 +956,10 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 		regulator_desc = pca9450bc_regulators;
 		pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
 		break;
+	case PCA9450_TYPE_PCA9451A:
+		regulator_desc = pca9451a_regulators;
+		pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
+		break;
 	default:
 		dev_err(&i2c->dev, "Unknown device type");
 		return -EINVAL;
@@ -741,13 +971,6 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 
 	dev_set_drvdata(&i2c->dev, pca9450);
 
-	pca9450->regmap = devm_regmap_init_i2c(i2c,
-					       &pca9450_regmap_config);
-	if (IS_ERR(pca9450->regmap)) {
-		dev_err(&i2c->dev, "regmap initialization failed\n");
-		return PTR_ERR(pca9450->regmap);
-	}
-
 	ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID, &device_id);
 	if (ret) {
 		dev_err(&i2c->dev, "Read device id error\n");
@@ -756,7 +979,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 
 	/* Check your board and dts for match the right pmic */
 	if (((device_id >> 4) != 0x1 && type == PCA9450_TYPE_PCA9450A) ||
-	    ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC)) {
+	    ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC) ||
+	    ((device_id >> 4) != 0x9 && type == PCA9450_TYPE_PCA9451A)) {
 		dev_err(&i2c->dev, "Device id(%x) mismatched\n",
 			device_id >> 4);
 		return -EINVAL;
@@ -767,7 +991,16 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 		struct regulator_dev *rdev;
 		const struct pca9450_regulator_desc *r;
 
-		r = &regulator_desc[i];
+		if (type == PCA9450_TYPE_PCA9451A &&
+		    !strcmp((&regulator_desc[i])->desc.name, "buck1") && pmic_trim) {
+			r = &regulator_desc[i + 1];
+			i = i + 1;
+		} else if (type == PCA9450_TYPE_PCA9451A &&
+			   !strcmp((&regulator_desc[i])->desc.name, "buck1")) {
+			r = &regulator_desc[i];
+			i = i + 1;
+		} else
+			r = &regulator_desc[i];
 		desc = &r->desc;
 
 		config.regmap = pca9450->regmap;
@@ -847,7 +1080,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
 	}
 
 	dev_info(&i2c->dev, "%s probed.\n",
-		type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
+		type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
+		(type == PCA9450_TYPE_PCA9451A ? "pca9451a" : "pca9450bc"));
 
 	return 0;
 }
@@ -865,6 +1099,10 @@ static const struct of_device_id pca9450_of_match[] = {
 		.compatible = "nxp,pca9450c",
 		.data = (void *)PCA9450_TYPE_PCA9450BC,
 	},
+	{
+		.compatible = "nxp,pca9451a",
+		.data = (void *)PCA9450_TYPE_PCA9451A,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pca9450_of_match);
diff --git a/include/linux/regulator/pca9450.h b/include/linux/regulator/pca9450.h
index 3c01c2bf84f5..5dd79f52165a 100644
--- a/include/linux/regulator/pca9450.h
+++ b/include/linux/regulator/pca9450.h
@@ -9,6 +9,7 @@
 enum pca9450_chip_type {
 	PCA9450_TYPE_PCA9450A = 0,
 	PCA9450_TYPE_PCA9450BC,
+	PCA9450_TYPE_PCA9451A,
 	PCA9450_TYPE_AMOUNT,
 };
 
@@ -93,6 +94,7 @@ enum {
 	PCA9450_MAX_REGISTER	    = 0x2F,
 };
 
+#define PCA9450_REG_PWRCTRL_TOFF_DEB    BIT(5)
 /* PCA9450 BUCK ENMODE bits */
 #define BUCK_ENMODE_OFF			0x00
 #define BUCK_ENMODE_ONREQ		0x01
-- 
2.37.1

[PATCH v1 3/3] arm64: dts: imx93-11x11-evk: add pca9451a support

[Joy Zou]

Support pca9451a on imx93-11x11-evk.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
 .../boot/dts/freescale/imx93-11x11-evk.dts    | 111 ++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
index c50f46f06f62..4de08d628b66 100644
--- a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
@@ -120,6 +120,105 @@ &wdog3 {
 	status = "okay";
 };
 
+&lpi2c2 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	clock-frequency = <400000>;
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&pinctrl_lpi2c2>;
+	pinctrl-1 = <&pinctrl_lpi2c2>;
+	status = "okay";
+
+	pmic@25 {
+		compatible = "nxp,pca9451a";
+		reg = <0x25>;
+		interrupt-parent = <&pcal6524>;
+		interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
+
+		regulators {
+			buck1: BUCK1 {
+				regulator-name = "BUCK1";
+				regulator-min-microvolt = <650000>;
+				regulator-max-microvolt = <2237500>;
+				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>;
+			};
+
+			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;
+			};
+
+			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;
+			};
+		};
+	};
+
+	pcal6524: gpio@22 {
+		compatible = "nxp,pcal6524";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pcal6524>;
+		reg = <0x22>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+};
+
 &iomuxc {
 	pinctrl_eqos: eqosgrp {
 		fsl,pins = <
@@ -166,6 +265,18 @@ MX93_PAD_UART1_TXD__LPUART1_TX			0x31e
 		>;
 	};
 
+	pinctrl_lpi2c2: lpi2c2grp {
+		fsl,pins = <
+			MX93_PAD_I2C2_SCL__LPI2C2_SCL			0x40000b9e
+			MX93_PAD_I2C2_SDA__LPI2C2_SDA			0x40000b9e
+		>;
+	};
+
+	pinctrl_pcal6524: pcal6524grp {
+		fsl,pins = <
+			MX93_PAD_CCM_CLKO2__GPIO3_IO27			0x31e
+		>;
+	};
 	pinctrl_usdhc1: usdhc1grp {
 		fsl,pins = <
 			MX93_PAD_SD1_CLK__USDHC1_CLK		0x15fe
-- 
2.37.1

RE: [EXT] Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2023年5月31日 14:56
> To: Joy Zou <joy.zou@nxp.com>; Jacky Bai <ping.bai@nxp.com>;
> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add
> pca9451a support
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On 31/05/2023 08:57, Joy Zou wrote:
> > Update pca9450 bindings.
> >
> > Signed-off-by: Joy Zou <joy.zou@nxp.com>
> > ---
> 
> Subject prefix is: regulator: dt-bindings: pca9450:
I will fix it in patch v2.
Thanks Krzysztof Kozlowski!
BR
Joy Zou
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof

Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Frieder Schrempf]

On 31.05.23 08:56, Krzysztof Kozlowski wrote:
> On 31/05/2023 08:57, Joy Zou wrote:
>> Update pca9450 bindings.
>>
>> Signed-off-by: Joy Zou <joy.zou@nxp.com>
>> ---
> 
> Subject prefix is: regulator: dt-bindings: pca9450:

Is there some way to have this consistent for all subsystems? Most
subsystems seem to use:

  dt-bindings: [subsystem]:

But some use:

  [subsystem]: dt-bindings:

Casual contributors (like me) will very often get it wrong on the first
try. Examining the history is extra effort that could be avoided and
often doesn't provide a definite hint as you find both variations in the
past.

Can we standardize this and make checkpatch validate the subject line?

Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Krzysztof Kozlowski]

On 31/05/2023 09:22, Frieder Schrempf wrote:
> On 31.05.23 08:56, Krzysztof Kozlowski wrote:
>> On 31/05/2023 08:57, Joy Zou wrote:
>>> Update pca9450 bindings.
>>>
>>> Signed-off-by: Joy Zou <joy.zou@nxp.com>
>>> ---
>>
>> Subject prefix is: regulator: dt-bindings: pca9450:
> 
> Is there some way to have this consistent for all subsystems? Most
> subsystems seem to use:
> 
>   dt-bindings: [subsystem]:
> 
> But some use:
> 
>   [subsystem]: dt-bindings:
> 
> Casual contributors (like me) will very often get it wrong on the first
> try. Examining the history is extra effort that could be avoided and
> often doesn't provide a definite hint as you find both variations in the
> past.
> 
> Can we standardize this and make checkpatch validate the subject line?

I understand your pain. :)

My expectation is just to have "dt-bindings:" prefix. It can be anywhere
- first or second, doesn't matter to me.

Then there is the generic rule that subsystem prefix should be the first
and here there is a disagreement between some folks. Most maintainers
either don't care or assume bindings are separate subsystem. Mark (spi,
ASoC, regulator) and media-folks say it is not separate subsystem (real
subsystem are spi, regulator etc), thus they want their subsystem name
as the first prefix. It sounds reasonable. Anyway it does not contradict
DT bindings maintainers expectation to have somewhere "dt-bindings:" prefix.

My comment was only to help you and there is no need to resend. I think
Mark when applying will drop "dt-bindings" prefix if is before
regulator, though. Life, no big deal.

Whether checkpatch can do this? Sure, quite likely, one just need some
Perl-foo to add such rule. :)

Best regards,
Krzysztof

Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Frieder Schrempf]

On 31.05.23 11:11, Krzysztof Kozlowski wrote:
> On 31/05/2023 09:22, Frieder Schrempf wrote:
>> On 31.05.23 08:56, Krzysztof Kozlowski wrote:
>>> On 31/05/2023 08:57, Joy Zou wrote:
>>>> Update pca9450 bindings.
>>>>
>>>> Signed-off-by: Joy Zou <joy.zou@nxp.com>
>>>> ---
>>>
>>> Subject prefix is: regulator: dt-bindings: pca9450:
>>
>> Is there some way to have this consistent for all subsystems? Most
>> subsystems seem to use:
>>
>>   dt-bindings: [subsystem]:
>>
>> But some use:
>>
>>   [subsystem]: dt-bindings:
>>
>> Casual contributors (like me) will very often get it wrong on the first
>> try. Examining the history is extra effort that could be avoided and
>> often doesn't provide a definite hint as you find both variations in the
>> past.
>>
>> Can we standardize this and make checkpatch validate the subject line?
> 
> I understand your pain. :)
> 
> My expectation is just to have "dt-bindings:" prefix. It can be anywhere
> - first or second, doesn't matter to me.
> 
> Then there is the generic rule that subsystem prefix should be the first
> and here there is a disagreement between some folks. Most maintainers
> either don't care or assume bindings are separate subsystem. Mark (spi,
> ASoC, regulator) and media-folks say it is not separate subsystem (real
> subsystem are spi, regulator etc), thus they want their subsystem name
> as the first prefix. It sounds reasonable. Anyway it does not contradict
> DT bindings maintainers expectation to have somewhere "dt-bindings:" prefix.

Ok, thanks for the explanation. Would be nice if maintainers could agree
on one version then.

> 
> My comment was only to help you and there is no need to resend. I think
> Mark when applying will drop "dt-bindings" prefix if is before
> regulator, though. Life, no big deal.

Im not the patch author, I was just jumping in as I saw your reply and
it already happened a few times to me that I needed more than one try
and used precious maintainer time just to get the subject right. So I
thought there is some potential for improvement.

> 
> Whether checkpatch can do this? Sure, quite likely, one just need some
> Perl-foo to add such rule. :)

Ok, this isn't something for me, but maybe someone around can come up
with an approach.

RE: [EXT] Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2023年5月31日 17:12
> To: Frieder Schrempf <frieder.schrempf@kontron.de>; Joy Zou
> <joy.zou@nxp.com>; Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH v1 1/3] dt-bindings: regulator: pca9450: add
> pca9451a support
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On 31/05/2023 09:22, Frieder Schrempf wrote:
> > On 31.05.23 08:56, Krzysztof Kozlowski wrote:
> >> On 31/05/2023 08:57, Joy Zou wrote:
> >>> Update pca9450 bindings.
> >>>
> >>> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> >>> ---
> >>
> >> Subject prefix is: regulator: dt-bindings: pca9450:
> >
> > Is there some way to have this consistent for all subsystems? Most
> > subsystems seem to use:
> >
> >   dt-bindings: [subsystem]:
> >
> > But some use:
> >
> >   [subsystem]: dt-bindings:
> >
> > Casual contributors (like me) will very often get it wrong on the
> > first try. Examining the history is extra effort that could be avoided
> > and often doesn't provide a definite hint as you find both variations
> > in the past.
> >
> > Can we standardize this and make checkpatch validate the subject line?
> 
> I understand your pain. :)
> 
> My expectation is just to have "dt-bindings:" prefix. It can be anywhere
> - first or second, doesn't matter to me.
> 
> Then there is the generic rule that subsystem prefix should be the first and
> here there is a disagreement between some folks. Most maintainers either
> don't care or assume bindings are separate subsystem. Mark (spi, ASoC,
> regulator) and media-folks say it is not separate subsystem (real subsystem are
> spi, regulator etc), thus they want their subsystem name as the first prefix. It
> sounds reasonable. Anyway it does not contradict DT bindings maintainers
> expectation to have somewhere "dt-bindings:" prefix.
> 
> My comment was only to help you and there is no need to resend. I think
> Mark when applying will drop "dt-bindings" prefix if is before regulator, though.
> Life, no big deal.
Ok, thank you very much for the explanation!
I better adjust the prefix.
BR
Joy Zou
> 
> Whether checkpatch can do this? Sure, quite likely, one just need some
> Perl-foo to add such rule. :)
> 
> Best regards,
> Krzysztof

Re: [PATCH v1 3/3] arm64: dts: imx93-11x11-evk: add pca9451a support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

On Wed, May 31, 2023 at 02:57:24PM +0800, Joy Zou wrote:

> +		regulators {
> +			buck1: BUCK1 {
> +				regulator-name = "BUCK1";
> +				regulator-min-microvolt = <650000>;
> +				regulator-max-microvolt = <2237500>;

These look a lot like the maximum ranges the regulator supports which
probably isn't what's safe for the board.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Alexander Stein]

Hi,

Am Mittwoch, 31. Mai 2023, 08:57:23 CEST schrieb Joy Zou:
> Adding support for pmic pca9451a.
> 
> This patch support old and new pmic pca9451a. The new pmic trimed BUCK1.
> The default value of Toff_Deb is used to distinguish the old and new pmic.
> 
> Signed-off-by: Joy Zou <joy.zou@nxp.com>
> ---
>  drivers/regulator/pca9450-regulator.c | 262 ++++++++++++++++++++++++--
>  include/linux/regulator/pca9450.h     |   2 +
>  2 files changed, 252 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/regulator/pca9450-regulator.c
> b/drivers/regulator/pca9450-regulator.c index 91bfb7e026c9..654aa4fbe494
> 100644
> --- a/drivers/regulator/pca9450-regulator.c
> +++ b/drivers/regulator/pca9450-regulator.c
> @@ -104,7 +104,15 @@ static const struct regulator_ops
> pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
>   */
>  static const struct linear_range pca9450_dvs_buck_volts[] = {
> -	REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
> +	REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500),
> +};
> +
> +/*
> + * BUCK1/3
> + * 0.65 to 2.2375V (12.5mV step)

Reading this comment, it seems the same distinction needs to be done for BUCK3 
as well, no?

> + */
> +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> +	REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
>  };
> 
>  /*
> @@ -663,6 +671,207 @@ static const struct pca9450_regulator_desc
> pca9450bc_regulators[] = { },
>  };
> 
> +static const struct pca9450_regulator_desc pca9451a_regulators[] = {
> +	{
> +		.desc = {
> +			.name = "buck1",
> +			.of_match = of_match_ptr("BUCK1"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_BUCK1,
> +			.ops = &pca9450_dvs_buck_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_dvs_buck_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_dvs_buck_volts),
> +			.vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> +			.vsel_mask = BUCK1OUT_DVS0_MASK,
> +			.enable_reg = PCA9450_REG_BUCK1CTRL,
> +			.enable_mask = BUCK1_ENMODE_MASK,
> +			.enable_val = BUCK_ENMODE_ONREQ,
> +			.ramp_mask = BUCK1_RAMP_MASK,
> +			.ramp_delay_table = pca9450_dvs_buck_ramp_table,
> +			.n_ramp_values = 
ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> +			.owner = THIS_MODULE,
> +			.of_parse_cb = pca9450_set_dvs_levels,
> +		},
> +		.dvs = {
> +			.run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> +			.run_mask = BUCK1OUT_DVS0_MASK,
> +			.standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> +			.standby_mask = BUCK1OUT_DVS1_MASK,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "buck1_trim",
> +			.of_match = of_match_ptr("BUCK1"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_BUCK1,
> +			.ops = &pca9450_dvs_buck_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_trim_dvs_buck_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_trim_dvs_buck_volts),
> +			.vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> +			.vsel_mask = BUCK1OUT_DVS0_MASK,
> +			.enable_reg = PCA9450_REG_BUCK1CTRL,
> +			.enable_mask = BUCK1_ENMODE_MASK,
> +			.enable_val = BUCK_ENMODE_ONREQ,
> +			.ramp_mask = BUCK1_RAMP_MASK,
> +			.ramp_delay_table = pca9450_dvs_buck_ramp_table,
> +			.n_ramp_values = 
ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> +			.owner = THIS_MODULE,
> +			.of_parse_cb = pca9450_set_dvs_levels,
> +		},
> +		.dvs = {
> +			.run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> +			.run_mask = BUCK1OUT_DVS0_MASK,
> +			.standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> +			.standby_mask = BUCK1OUT_DVS1_MASK,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "buck2",
> +			.of_match = of_match_ptr("BUCK2"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_BUCK2,
> +			.ops = &pca9450_dvs_buck_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_BUCK2_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_dvs_buck_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_dvs_buck_volts),
> +			.vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> +			.vsel_mask = BUCK2OUT_DVS0_MASK,
> +			.enable_reg = PCA9450_REG_BUCK2CTRL,
> +			.enable_mask = BUCK2_ENMODE_MASK,
> +			.enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> +			.ramp_mask = BUCK2_RAMP_MASK,
> +			.ramp_delay_table = pca9450_dvs_buck_ramp_table,
> +			.n_ramp_values = 
ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> +			.owner = THIS_MODULE,
> +			.of_parse_cb = pca9450_set_dvs_levels,
> +		},
> +		.dvs = {
> +			.run_reg = PCA9450_REG_BUCK2OUT_DVS0,
> +			.run_mask = BUCK2OUT_DVS0_MASK,
> +			.standby_reg = PCA9450_REG_BUCK2OUT_DVS1,
> +			.standby_mask = BUCK2OUT_DVS1_MASK,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "buck4",
> +			.of_match = of_match_ptr("BUCK4"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_BUCK4,
> +			.ops = &pca9450_buck_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_BUCK4_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_buck_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_buck_volts),
> +			.vsel_reg = PCA9450_REG_BUCK4OUT,
> +			.vsel_mask = BUCK4OUT_MASK,
> +			.enable_reg = PCA9450_REG_BUCK4CTRL,
> +			.enable_mask = BUCK4_ENMODE_MASK,
> +			.enable_val = BUCK_ENMODE_ONREQ,
> +			.owner = THIS_MODULE,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "buck5",
> +			.of_match = of_match_ptr("BUCK5"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_BUCK5,
> +			.ops = &pca9450_buck_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_BUCK5_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_buck_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_buck_volts),
> +			.vsel_reg = PCA9450_REG_BUCK5OUT,
> +			.vsel_mask = BUCK5OUT_MASK,
> +			.enable_reg = PCA9450_REG_BUCK5CTRL,
> +			.enable_mask = BUCK5_ENMODE_MASK,
> +			.enable_val = BUCK_ENMODE_ONREQ,
> +			.owner = THIS_MODULE,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "buck6",
> +			.of_match = of_match_ptr("BUCK6"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_BUCK6,
> +			.ops = &pca9450_buck_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_BUCK6_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_buck_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_buck_volts),
> +			.vsel_reg = PCA9450_REG_BUCK6OUT,
> +			.vsel_mask = BUCK6OUT_MASK,
> +			.enable_reg = PCA9450_REG_BUCK6CTRL,
> +			.enable_mask = BUCK6_ENMODE_MASK,
> +			.enable_val = BUCK_ENMODE_ONREQ,
> +			.owner = THIS_MODULE,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "ldo1",
> +			.of_match = of_match_ptr("LDO1"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_LDO1,
> +			.ops = &pca9450_ldo_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_LDO1_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_ldo1_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_ldo1_volts),
> +			.vsel_reg = PCA9450_REG_LDO1CTRL,
> +			.vsel_mask = LDO1OUT_MASK,
> +			.enable_reg = PCA9450_REG_LDO1CTRL,
> +			.enable_mask = LDO1_EN_MASK,
> +			.owner = THIS_MODULE,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "ldo4",
> +			.of_match = of_match_ptr("LDO4"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_LDO4,
> +			.ops = &pca9450_ldo_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_LDO4_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_ldo34_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_ldo34_volts),
> +			.vsel_reg = PCA9450_REG_LDO4CTRL,
> +			.vsel_mask = LDO4OUT_MASK,
> +			.enable_reg = PCA9450_REG_LDO4CTRL,
> +			.enable_mask = LDO4_EN_MASK,
> +			.owner = THIS_MODULE,
> +		},
> +	},
> +	{
> +		.desc = {
> +			.name = "ldo5",
> +			.of_match = of_match_ptr("LDO5"),
> +			.regulators_node = of_match_ptr("regulators"),
> +			.id = PCA9450_LDO5,
> +			.ops = &pca9450_ldo_regulator_ops,
> +			.type = REGULATOR_VOLTAGE,
> +			.n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
> +			.linear_ranges = pca9450_ldo5_volts,
> +			.n_linear_ranges = 
ARRAY_SIZE(pca9450_ldo5_volts),
> +			.vsel_reg = PCA9450_REG_LDO5CTRL_H,
> +			.vsel_mask = LDO5HOUT_MASK,
> +			.enable_reg = PCA9450_REG_LDO5CTRL_H,
> +			.enable_mask = LDO5H_EN_MASK,
> +			.owner = THIS_MODULE,
> +		},
> +	},
> +};
> +
>  static irqreturn_t pca9450_irq_handler(int irq, void *data)
>  {
>  	struct pca9450 *pca9450 = data;
> @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>  	const struct pca9450_regulator_desc	*regulator_desc;
>  	struct regulator_config config = { };
>  	struct pca9450 *pca9450;
> -	unsigned int device_id, i;
> +	unsigned int device_id, i, val;
>  	unsigned int reset_ctrl;
> +	bool pmic_trim = false;
>  	int ret;
> 
>  	if (!i2c->irq) {
> @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>  	if (!pca9450)
>  		return -ENOMEM;
> 
> +	pca9450->regmap = devm_regmap_init_i2c(i2c,
> +					       
&pca9450_regmap_config);
> +	if (IS_ERR(pca9450->regmap)) {
> +		dev_err(&i2c->dev, "regmap initialization failed\n");
> +		return PTR_ERR(pca9450->regmap);
> +	}
> +
> +	ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL, &val);
> +	if (ret) {
> +		dev_err(&i2c->dev, "Read device id error\n");
> +		return ret;
> +	}
> +
> +	if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> +		pmic_trim = true;

PCA9450_REG_PWRCTRL is a read/write register. How is it possible to detect a 
chip revision using a bit which can be changed by software e.g. bootloader?
Despite that this bit sets debounce time for PMIC_ON_REQ, how is this related 
to BUCK1 voltage range?

> +
>  	switch (type) {
>  	case PCA9450_TYPE_PCA9450A:
>  		regulator_desc = pca9450a_regulators;
> @@ -730,6 +956,10 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>  		regulator_desc = pca9450bc_regulators;
>  		pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
>  		break;
> +	case PCA9450_TYPE_PCA9451A:
> +		regulator_desc = pca9451a_regulators;
> +		pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> +		break;
>  	default:
>  		dev_err(&i2c->dev, "Unknown device type");
>  		return -EINVAL;
> @@ -741,13 +971,6 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> 
>  	dev_set_drvdata(&i2c->dev, pca9450);
> 
> -	pca9450->regmap = devm_regmap_init_i2c(i2c,
> -					       
&pca9450_regmap_config);
> -	if (IS_ERR(pca9450->regmap)) {
> -		dev_err(&i2c->dev, "regmap initialization failed\n");
> -		return PTR_ERR(pca9450->regmap);
> -	}
> -
>  	ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID, &device_id);
>  	if (ret) {
>  		dev_err(&i2c->dev, "Read device id error\n");
> @@ -756,7 +979,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> 
>  	/* Check your board and dts for match the right pmic */
>  	if (((device_id >> 4) != 0x1 && type == PCA9450_TYPE_PCA9450A) ||
> -	    ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC)) {
> +	    ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC) ||
> +	    ((device_id >> 4) != 0x9 && type == PCA9450_TYPE_PCA9451A)) {
>  		dev_err(&i2c->dev, "Device id(%x) mismatched\n",
>  			device_id >> 4);
>  		return -EINVAL;
> @@ -767,7 +991,16 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>  		struct regulator_dev *rdev;
>  		const struct pca9450_regulator_desc *r;
> 
> -		r = &regulator_desc[i];
> +		if (type == PCA9450_TYPE_PCA9451A &&
> +		    !strcmp((&regulator_desc[i])->desc.name, "buck1") && 
pmic_trim) {
> +			r = &regulator_desc[i + 1];
> +			i = i + 1;
> +		} else if (type == PCA9450_TYPE_PCA9451A &&
> +			   !strcmp((&regulator_desc[i])->desc.name, 
"buck1")) {
> +			r = &regulator_desc[i];
> +			i = i + 1;

I would put this in a single 'type == PCA9450_TYPE_PCA9451A' branch, to 
indicate that only PCA9451A needs some kind of special handling.

> +		} else
> +			r = &regulator_desc[i];
>  		desc = &r->desc;
> 
>  		config.regmap = pca9450->regmap;
> @@ -847,7 +1080,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
>  	}
> 
>  	dev_info(&i2c->dev, "%s probed.\n",
> -		type == PCA9450_TYPE_PCA9450A ? "pca9450a" : "pca9450bc");
> +		type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> +		(type == PCA9450_TYPE_PCA9451A ? "pca9451a" : 
"pca9450bc"));
> 
>  	return 0;
>  }
> @@ -865,6 +1099,10 @@ static const struct of_device_id pca9450_of_match[] =
> { .compatible = "nxp,pca9450c",
>  		.data = (void *)PCA9450_TYPE_PCA9450BC,
>  	},
> +	{
> +		.compatible = "nxp,pca9451a",
> +		.data = (void *)PCA9450_TYPE_PCA9451A,
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, pca9450_of_match);
> diff --git a/include/linux/regulator/pca9450.h
> b/include/linux/regulator/pca9450.h index 3c01c2bf84f5..5dd79f52165a 100644
> --- a/include/linux/regulator/pca9450.h
> +++ b/include/linux/regulator/pca9450.h
> @@ -9,6 +9,7 @@
>  enum pca9450_chip_type {
>  	PCA9450_TYPE_PCA9450A = 0,
>  	PCA9450_TYPE_PCA9450BC,
> +	PCA9450_TYPE_PCA9451A,
>  	PCA9450_TYPE_AMOUNT,
>  };
> 
> @@ -93,6 +94,7 @@ enum {
>  	PCA9450_MAX_REGISTER	    = 0x2F,
>  };
> 
> +#define PCA9450_REG_PWRCTRL_TOFF_DEB    BIT(5)

Newline here please.

Best regards,
Alexander

>  /* PCA9450 BUCK ENMODE bits */
>  #define BUCK_ENMODE_OFF			0x00
>  #define BUCK_ENMODE_ONREQ		0x01


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: 2023年5月31日 19:35
> To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> linux-arm-kernel@lists.infradead.org
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Joy Zou
> <joy.zou@nxp.com>
> Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hi,
>
> Am Mittwoch, 31. Mai 2023, 08:57:23 CEST schrieb Joy Zou:
> > Adding support for pmic pca9451a.
> >
> > This patch support old and new pmic pca9451a. The new pmic trimed
> BUCK1.
> > The default value of Toff_Deb is used to distinguish the old and new pmic.
> >
> > Signed-off-by: Joy Zou <joy.zou@nxp.com>
> > ---
> >  drivers/regulator/pca9450-regulator.c | 262
> ++++++++++++++++++++++++--
> >  include/linux/regulator/pca9450.h     |   2 +
> >  2 files changed, 252 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/regulator/pca9450-regulator.c
> > b/drivers/regulator/pca9450-regulator.c index
> > 91bfb7e026c9..654aa4fbe494
> > 100644
> > --- a/drivers/regulator/pca9450-regulator.c
> > +++ b/drivers/regulator/pca9450-regulator.c
> > @@ -104,7 +104,15 @@ static const struct regulator_ops
> > pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
> >   */
> >  static const struct linear_range pca9450_dvs_buck_volts[] = {
> > -     REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
> > +     REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500), };
> > +
> > +/*
> > + * BUCK1/3
> > + * 0.65 to 2.2375V (12.5mV step)
>
> Reading this comment, it seems the same distinction needs to be done for
> BUCK3 as well, no?
Sorry for the late reply!
The BUCK1 and BUCK3 are dual phase, so don't need to be done for BUCK3.
>
> > + */
> > +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> > +     REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
> >  };
> >
> >  /*
> > @@ -663,6 +671,207 @@ static const struct pca9450_regulator_desc
> > pca9450bc_regulators[] = { },  };
> >
> > +static const struct pca9450_regulator_desc pca9451a_regulators[] = {
> > +     {
> > +             .desc = {
> > +                     .name = "buck1",
> > +                     .of_match = of_match_ptr("BUCK1"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_BUCK1,
> > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_dvs_buck_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_dvs_buck_volts),
> > +                     .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > +                     .vsel_mask = BUCK1OUT_DVS0_MASK,
> > +                     .enable_reg = PCA9450_REG_BUCK1CTRL,
> > +                     .enable_mask = BUCK1_ENMODE_MASK,
> > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > +                     .ramp_mask = BUCK1_RAMP_MASK,
> > +                     .ramp_delay_table =
> pca9450_dvs_buck_ramp_table,
> > +                     .n_ramp_values =
> ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > +                     .owner = THIS_MODULE,
> > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > +             },
> > +             .dvs = {
> > +                     .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > +                     .run_mask = BUCK1OUT_DVS0_MASK,
> > +                     .standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> > +                     .standby_mask = BUCK1OUT_DVS1_MASK,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "buck1_trim",
> > +                     .of_match = of_match_ptr("BUCK1"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_BUCK1,
> > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_trim_dvs_buck_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_trim_dvs_buck_volts),
> > +                     .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > +                     .vsel_mask = BUCK1OUT_DVS0_MASK,
> > +                     .enable_reg = PCA9450_REG_BUCK1CTRL,
> > +                     .enable_mask = BUCK1_ENMODE_MASK,
> > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > +                     .ramp_mask = BUCK1_RAMP_MASK,
> > +                     .ramp_delay_table =
> pca9450_dvs_buck_ramp_table,
> > +                     .n_ramp_values =
> ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > +                     .owner = THIS_MODULE,
> > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > +             },
> > +             .dvs = {
> > +                     .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > +                     .run_mask = BUCK1OUT_DVS0_MASK,
> > +                     .standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> > +                     .standby_mask = BUCK1OUT_DVS1_MASK,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "buck2",
> > +                     .of_match = of_match_ptr("BUCK2"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_BUCK2,
> > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_BUCK2_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_dvs_buck_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_dvs_buck_volts),
> > +                     .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > +                     .vsel_mask = BUCK2OUT_DVS0_MASK,
> > +                     .enable_reg = PCA9450_REG_BUCK2CTRL,
> > +                     .enable_mask = BUCK2_ENMODE_MASK,
> > +                     .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> > +                     .ramp_mask = BUCK2_RAMP_MASK,
> > +                     .ramp_delay_table =
> pca9450_dvs_buck_ramp_table,
> > +                     .n_ramp_values =
> ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > +                     .owner = THIS_MODULE,
> > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > +             },
> > +             .dvs = {
> > +                     .run_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > +                     .run_mask = BUCK2OUT_DVS0_MASK,
> > +                     .standby_reg = PCA9450_REG_BUCK2OUT_DVS1,
> > +                     .standby_mask = BUCK2OUT_DVS1_MASK,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "buck4",
> > +                     .of_match = of_match_ptr("BUCK4"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_BUCK4,
> > +                     .ops = &pca9450_buck_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_BUCK4_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_buck_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_buck_volts),
> > +                     .vsel_reg = PCA9450_REG_BUCK4OUT,
> > +                     .vsel_mask = BUCK4OUT_MASK,
> > +                     .enable_reg = PCA9450_REG_BUCK4CTRL,
> > +                     .enable_mask = BUCK4_ENMODE_MASK,
> > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > +                     .owner = THIS_MODULE,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "buck5",
> > +                     .of_match = of_match_ptr("BUCK5"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_BUCK5,
> > +                     .ops = &pca9450_buck_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_BUCK5_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_buck_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_buck_volts),
> > +                     .vsel_reg = PCA9450_REG_BUCK5OUT,
> > +                     .vsel_mask = BUCK5OUT_MASK,
> > +                     .enable_reg = PCA9450_REG_BUCK5CTRL,
> > +                     .enable_mask = BUCK5_ENMODE_MASK,
> > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > +                     .owner = THIS_MODULE,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "buck6",
> > +                     .of_match = of_match_ptr("BUCK6"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_BUCK6,
> > +                     .ops = &pca9450_buck_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_BUCK6_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_buck_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_buck_volts),
> > +                     .vsel_reg = PCA9450_REG_BUCK6OUT,
> > +                     .vsel_mask = BUCK6OUT_MASK,
> > +                     .enable_reg = PCA9450_REG_BUCK6CTRL,
> > +                     .enable_mask = BUCK6_ENMODE_MASK,
> > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > +                     .owner = THIS_MODULE,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "ldo1",
> > +                     .of_match = of_match_ptr("LDO1"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_LDO1,
> > +                     .ops = &pca9450_ldo_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_LDO1_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_ldo1_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_ldo1_volts),
> > +                     .vsel_reg = PCA9450_REG_LDO1CTRL,
> > +                     .vsel_mask = LDO1OUT_MASK,
> > +                     .enable_reg = PCA9450_REG_LDO1CTRL,
> > +                     .enable_mask = LDO1_EN_MASK,
> > +                     .owner = THIS_MODULE,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "ldo4",
> > +                     .of_match = of_match_ptr("LDO4"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_LDO4,
> > +                     .ops = &pca9450_ldo_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_LDO4_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_ldo34_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_ldo34_volts),
> > +                     .vsel_reg = PCA9450_REG_LDO4CTRL,
> > +                     .vsel_mask = LDO4OUT_MASK,
> > +                     .enable_reg = PCA9450_REG_LDO4CTRL,
> > +                     .enable_mask = LDO4_EN_MASK,
> > +                     .owner = THIS_MODULE,
> > +             },
> > +     },
> > +     {
> > +             .desc = {
> > +                     .name = "ldo5",
> > +                     .of_match = of_match_ptr("LDO5"),
> > +                     .regulators_node = of_match_ptr("regulators"),
> > +                     .id = PCA9450_LDO5,
> > +                     .ops = &pca9450_ldo_regulator_ops,
> > +                     .type = REGULATOR_VOLTAGE,
> > +                     .n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
> > +                     .linear_ranges = pca9450_ldo5_volts,
> > +                     .n_linear_ranges =
> ARRAY_SIZE(pca9450_ldo5_volts),
> > +                     .vsel_reg = PCA9450_REG_LDO5CTRL_H,
> > +                     .vsel_mask = LDO5HOUT_MASK,
> > +                     .enable_reg = PCA9450_REG_LDO5CTRL_H,
> > +                     .enable_mask = LDO5H_EN_MASK,
> > +                     .owner = THIS_MODULE,
> > +             },
> > +     },
> > +};
> > +
> >  static irqreturn_t pca9450_irq_handler(int irq, void *data)  {
> >       struct pca9450 *pca9450 = data;
> > @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> >       const struct pca9450_regulator_desc     *regulator_desc;
> >       struct regulator_config config = { };
> >       struct pca9450 *pca9450;
> > -     unsigned int device_id, i;
> > +     unsigned int device_id, i, val;
> >       unsigned int reset_ctrl;
> > +     bool pmic_trim = false;
> >       int ret;
> >
> >       if (!i2c->irq) {
> > @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> >       if (!pca9450)
> >               return -ENOMEM;
> >
> > +     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > +
> &pca9450_regmap_config);
> > +     if (IS_ERR(pca9450->regmap)) {
> > +             dev_err(&i2c->dev, "regmap initialization failed\n");
> > +             return PTR_ERR(pca9450->regmap);
> > +     }
> > +
> > +     ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL,
> &val);
> > +     if (ret) {
> > +             dev_err(&i2c->dev, "Read device id error\n");
> > +             return ret;
> > +     }
> > +
> > +     if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> > +             pmic_trim = true;
>
> PCA9450_REG_PWRCTRL is a read/write register. How is it possible to detect a
> chip revision using a bit which can be changed by software e.g. bootloader?
> Despite that this bit sets debounce time for PMIC_ON_REQ, how is this related
> to BUCK1 voltage range?
There are old and new two kind PMIC pca9451a. This bit sets debounce time in PCA9450_REG_PWRCTRL was
set different value by hardware in order to only distinguish the old and new PMIC. This bit isn't related to the
BUCK1 voltage range. If the pmic_trim is true that means it's new pca9451a.
>
> > +
> >       switch (type) {
> >       case PCA9450_TYPE_PCA9450A:
> >               regulator_desc = pca9450a_regulators; @@ -730,6 +956,10
> > @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> >               regulator_desc = pca9450bc_regulators;
> >               pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
> >               break;
> > +     case PCA9450_TYPE_PCA9451A:
> > +             regulator_desc = pca9451a_regulators;
> > +             pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> > +             break;
> >       default:
> >               dev_err(&i2c->dev, "Unknown device type");
> >               return -EINVAL;
> > @@ -741,13 +971,6 @@ static int pca9450_i2c_probe(struct i2c_client
> > *i2c)
> >
> >       dev_set_drvdata(&i2c->dev, pca9450);
> >
> > -     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > -
> &pca9450_regmap_config);
> > -     if (IS_ERR(pca9450->regmap)) {
> > -             dev_err(&i2c->dev, "regmap initialization failed\n");
> > -             return PTR_ERR(pca9450->regmap);
> > -     }
> > -
> >       ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID,
> &device_id);
> >       if (ret) {
> >               dev_err(&i2c->dev, "Read device id error\n"); @@ -756,7
> > +979,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> >
> >       /* Check your board and dts for match the right pmic */
> >       if (((device_id >> 4) != 0x1 && type == PCA9450_TYPE_PCA9450A) ||
> > -         ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC))
> {
> > +         ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC)
> ||
> > +         ((device_id >> 4) != 0x9 && type == PCA9450_TYPE_PCA9451A))
> > + {
> >               dev_err(&i2c->dev, "Device id(%x) mismatched\n",
> >                       device_id >> 4);
> >               return -EINVAL;
> > @@ -767,7 +991,16 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> >               struct regulator_dev *rdev;
> >               const struct pca9450_regulator_desc *r;
> >
> > -             r = &regulator_desc[i];
> > +             if (type == PCA9450_TYPE_PCA9451A &&
> > +                 !strcmp((&regulator_desc[i])->desc.name, "buck1") &&
> pmic_trim) {
> > +                     r = &regulator_desc[i + 1];
> > +                     i = i + 1;
> > +             } else if (type == PCA9450_TYPE_PCA9451A &&
> > +                        !strcmp((&regulator_desc[i])->desc.name,
> "buck1")) {
> > +                     r = &regulator_desc[i];
> > +                     i = i + 1;
>
> I would put this in a single 'type == PCA9450_TYPE_PCA9451A' branch, to
> indicate that only PCA9451A needs some kind of special handling.
Yes, this makes the logic clearer. I will change in next version.
>
> > +             } else
> > +                     r = &regulator_desc[i];
> >               desc = &r->desc;
> >
> >               config.regmap = pca9450->regmap; @@ -847,7 +1080,8
> @@
> > static int pca9450_i2c_probe(struct i2c_client *i2c)
> >       }
> >
> >       dev_info(&i2c->dev, "%s probed.\n",
> > -             type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> "pca9450bc");
> > +             type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> > +             (type == PCA9450_TYPE_PCA9451A ? "pca9451a" :
> "pca9450bc"));
> >
> >       return 0;
> >  }
> > @@ -865,6 +1099,10 @@ static const struct of_device_id
> > pca9450_of_match[] = { .compatible = "nxp,pca9450c",
> >               .data = (void *)PCA9450_TYPE_PCA9450BC,
> >       },
> > +     {
> > +             .compatible = "nxp,pca9451a",
> > +             .data = (void *)PCA9450_TYPE_PCA9451A,
> > +     },
> >       { }
> >  };
> >  MODULE_DEVICE_TABLE(of, pca9450_of_match); diff --git
> > a/include/linux/regulator/pca9450.h
> > b/include/linux/regulator/pca9450.h index 3c01c2bf84f5..5dd79f52165a
> > 100644
> > --- a/include/linux/regulator/pca9450.h
> > +++ b/include/linux/regulator/pca9450.h
> > @@ -9,6 +9,7 @@
> >  enum pca9450_chip_type {
> >       PCA9450_TYPE_PCA9450A = 0,
> >       PCA9450_TYPE_PCA9450BC,
> > +     PCA9450_TYPE_PCA9451A,
> >       PCA9450_TYPE_AMOUNT,
> >  };
> >
> > @@ -93,6 +94,7 @@ enum {
> >       PCA9450_MAX_REGISTER        = 0x2F,
> >  };
> >
> > +#define PCA9450_REG_PWRCTRL_TOFF_DEB    BIT(5)
>
> Newline here please.
Ok, will modify it.
Thank you very much for your comments and efforts.
BR
Joy Zou
>
> Best regards,
> Alexander
>
> >  /* PCA9450 BUCK ENMODE bits */
> >  #define BUCK_ENMODE_OFF                      0x00
> >  #define BUCK_ENMODE_ONREQ            0x01
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-/
> group.com%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C1ecbe75c432d464
> cb45f08db61cb08bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C638211296883052738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> 7C&sdata=mz7vtBiDvAxY9FO7tTZO3oPBA8zvYS6f8hLgAwrU8uk%3D&reserved
> =0
>

Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Alexander Stein]

Hello,

Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> 
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: 2023年5月31日 19:35
> > To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> > broonie@kernel.org; robh+dt@kernel.org;
> > krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > linux-arm-kernel@lists.infradead.org
> > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Joy
> > Zou
 <joy.zou@nxp.com>
> > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > support
> >
> >
> >
> > Caution: This is an external email. Please take care when clicking links
> > or
 opening attachments. When in doubt, report the message using the
> > 'Report this email' button
> >
> >
> >
> >
> > Hi,
> >
> >
> >
> > Am Mittwoch, 31. Mai 2023, 08:57:23 CEST schrieb Joy Zou:
> > 
> > > Adding support for pmic pca9451a.
> > >
> > >
> > >
> > > This patch support old and new pmic pca9451a. The new pmic trimed
> > 
> > BUCK1.
> > 
> > > The default value of Toff_Deb is used to distinguish the old and new
> > > pmic.
> > >
> > >
> > >
> > > Signed-off-by: Joy Zou <joy.zou@nxp.com>
> > > ---
> > > 
> > >  drivers/regulator/pca9450-regulator.c | 262
> > 
> > ++++++++++++++++++++++++--
> > 
> > >  include/linux/regulator/pca9450.h     |   2 +
> > >  2 files changed, 252 insertions(+), 12 deletions(-)
> > >
> > >
> > >
> > > diff --git a/drivers/regulator/pca9450-regulator.c
> > > b/drivers/regulator/pca9450-regulator.c index
> > > 91bfb7e026c9..654aa4fbe494
> > > 100644
> > > --- a/drivers/regulator/pca9450-regulator.c
> > > +++ b/drivers/regulator/pca9450-regulator.c
> > > @@ -104,7 +104,15 @@ static const struct regulator_ops
> > > pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
> > > 
> > >   */
> > >  
> > >  static const struct linear_range pca9450_dvs_buck_volts[] = {
> > > 
> > > -     REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
> > > +     REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500), };
> > > +
> > > +/*
> > > + * BUCK1/3
> > > + * 0.65 to 2.2375V (12.5mV step)
> >
> >
> >
> > Reading this comment, it seems the same distinction needs to be done for
> > BUCK3 as well, no?
> 
> Sorry for the late reply!
> The BUCK1 and BUCK3 are dual phase, so don't need to be done for BUCK3.
> 
> >
> >
> > > + */
> > > +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> > > +     REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
> > > 
> > >  };
> > >
> > >
> > >
> > >  /*
> > > 
> > > @@ -663,6 +671,207 @@ static const struct pca9450_regulator_desc
> > > pca9450bc_regulators[] = { },  };
> > >
> > >
> > >
> > > +static const struct pca9450_regulator_desc pca9451a_regulators[] = {
> > > +     {
> > > +             .desc = {
> > > +                     .name = "buck1",
> > > +                     .of_match = of_match_ptr("BUCK1"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_BUCK1,
> > > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_dvs_buck_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_dvs_buck_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > +                     .vsel_mask = BUCK1OUT_DVS0_MASK,
> > > +                     .enable_reg = PCA9450_REG_BUCK1CTRL,
> > > +                     .enable_mask = BUCK1_ENMODE_MASK,
> > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > +                     .ramp_mask = BUCK1_RAMP_MASK,
> > > +                     .ramp_delay_table =
> > 
> > pca9450_dvs_buck_ramp_table,
> > 
> > > +                     .n_ramp_values =
> > 
> > ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > 
> > > +                     .owner = THIS_MODULE,
> > > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > > +             },
> > > +             .dvs = {
> > > +                     .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > +                     .run_mask = BUCK1OUT_DVS0_MASK,
> > > +                     .standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> > > +                     .standby_mask = BUCK1OUT_DVS1_MASK,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "buck1_trim",
> > > +                     .of_match = of_match_ptr("BUCK1"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_BUCK1,
> > > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_trim_dvs_buck_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_trim_dvs_buck_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > +                     .vsel_mask = BUCK1OUT_DVS0_MASK,
> > > +                     .enable_reg = PCA9450_REG_BUCK1CTRL,
> > > +                     .enable_mask = BUCK1_ENMODE_MASK,
> > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > +                     .ramp_mask = BUCK1_RAMP_MASK,
> > > +                     .ramp_delay_table =
> > 
> > pca9450_dvs_buck_ramp_table,
> > 
> > > +                     .n_ramp_values =
> > 
> > ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > 
> > > +                     .owner = THIS_MODULE,
> > > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > > +             },
> > > +             .dvs = {
> > > +                     .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > +                     .run_mask = BUCK1OUT_DVS0_MASK,
> > > +                     .standby_reg = PCA9450_REG_BUCK1OUT_DVS1,
> > > +                     .standby_mask = BUCK1OUT_DVS1_MASK,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "buck2",
> > > +                     .of_match = of_match_ptr("BUCK2"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_BUCK2,
> > > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_BUCK2_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_dvs_buck_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_dvs_buck_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > > +                     .vsel_mask = BUCK2OUT_DVS0_MASK,
> > > +                     .enable_reg = PCA9450_REG_BUCK2CTRL,
> > > +                     .enable_mask = BUCK2_ENMODE_MASK,
> > > +                     .enable_val = BUCK_ENMODE_ONREQ_STBYREQ,
> > > +                     .ramp_mask = BUCK2_RAMP_MASK,
> > > +                     .ramp_delay_table =
> > 
> > pca9450_dvs_buck_ramp_table,
> > 
> > > +                     .n_ramp_values =
> > 
> > ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > 
> > > +                     .owner = THIS_MODULE,
> > > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > > +             },
> > > +             .dvs = {
> > > +                     .run_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > > +                     .run_mask = BUCK2OUT_DVS0_MASK,
> > > +                     .standby_reg = PCA9450_REG_BUCK2OUT_DVS1,
> > > +                     .standby_mask = BUCK2OUT_DVS1_MASK,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "buck4",
> > > +                     .of_match = of_match_ptr("BUCK4"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_BUCK4,
> > > +                     .ops = &pca9450_buck_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_BUCK4_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_buck_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_buck_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_BUCK4OUT,
> > > +                     .vsel_mask = BUCK4OUT_MASK,
> > > +                     .enable_reg = PCA9450_REG_BUCK4CTRL,
> > > +                     .enable_mask = BUCK4_ENMODE_MASK,
> > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > +                     .owner = THIS_MODULE,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "buck5",
> > > +                     .of_match = of_match_ptr("BUCK5"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_BUCK5,
> > > +                     .ops = &pca9450_buck_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_BUCK5_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_buck_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_buck_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_BUCK5OUT,
> > > +                     .vsel_mask = BUCK5OUT_MASK,
> > > +                     .enable_reg = PCA9450_REG_BUCK5CTRL,
> > > +                     .enable_mask = BUCK5_ENMODE_MASK,
> > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > +                     .owner = THIS_MODULE,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "buck6",
> > > +                     .of_match = of_match_ptr("BUCK6"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_BUCK6,
> > > +                     .ops = &pca9450_buck_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_BUCK6_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_buck_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_buck_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_BUCK6OUT,
> > > +                     .vsel_mask = BUCK6OUT_MASK,
> > > +                     .enable_reg = PCA9450_REG_BUCK6CTRL,
> > > +                     .enable_mask = BUCK6_ENMODE_MASK,
> > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > +                     .owner = THIS_MODULE,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "ldo1",
> > > +                     .of_match = of_match_ptr("LDO1"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_LDO1,
> > > +                     .ops = &pca9450_ldo_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_LDO1_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_ldo1_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_ldo1_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_LDO1CTRL,
> > > +                     .vsel_mask = LDO1OUT_MASK,
> > > +                     .enable_reg = PCA9450_REG_LDO1CTRL,
> > > +                     .enable_mask = LDO1_EN_MASK,
> > > +                     .owner = THIS_MODULE,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "ldo4",
> > > +                     .of_match = of_match_ptr("LDO4"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_LDO4,
> > > +                     .ops = &pca9450_ldo_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_LDO4_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_ldo34_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_ldo34_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_LDO4CTRL,
> > > +                     .vsel_mask = LDO4OUT_MASK,
> > > +                     .enable_reg = PCA9450_REG_LDO4CTRL,
> > > +                     .enable_mask = LDO4_EN_MASK,
> > > +                     .owner = THIS_MODULE,
> > > +             },
> > > +     },
> > > +     {
> > > +             .desc = {
> > > +                     .name = "ldo5",
> > > +                     .of_match = of_match_ptr("LDO5"),
> > > +                     .regulators_node = of_match_ptr("regulators"),
> > > +                     .id = PCA9450_LDO5,
> > > +                     .ops = &pca9450_ldo_regulator_ops,
> > > +                     .type = REGULATOR_VOLTAGE,
> > > +                     .n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
> > > +                     .linear_ranges = pca9450_ldo5_volts,
> > > +                     .n_linear_ranges =
> > 
> > ARRAY_SIZE(pca9450_ldo5_volts),
> > 
> > > +                     .vsel_reg = PCA9450_REG_LDO5CTRL_H,
> > > +                     .vsel_mask = LDO5HOUT_MASK,
> > > +                     .enable_reg = PCA9450_REG_LDO5CTRL_H,
> > > +                     .enable_mask = LDO5H_EN_MASK,
> > > +                     .owner = THIS_MODULE,
> > > +             },
> > > +     },
> > > +};
> > > +
> > > 
> > >  static irqreturn_t pca9450_irq_handler(int irq, void *data)  {
> > >  
> > >       struct pca9450 *pca9450 = data;
> > > 
> > > @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client
> > > *i2c)
> > > 
> > >       const struct pca9450_regulator_desc     *regulator_desc;
> > >       struct regulator_config config = { };
> > >       struct pca9450 *pca9450;
> > > 
> > > -     unsigned int device_id, i;
> > > +     unsigned int device_id, i, val;
> > > 
> > >       unsigned int reset_ctrl;
> > > 
> > > +     bool pmic_trim = false;
> > > 
> > >       int ret;
> > >
> > >
> > >
> > >       if (!i2c->irq) {
> > > 
> > > @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct i2c_client
> > > *i2c)
> > > 
> > >       if (!pca9450)
> > >       
> > >               return -ENOMEM;
> > >
> > >
> > >
> > > +     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > > +
> > 
> > &pca9450_regmap_config);
> > 
> > > +     if (IS_ERR(pca9450->regmap)) {
> > > +             dev_err(&i2c->dev, "regmap initialization failed\n");
> > > +             return PTR_ERR(pca9450->regmap);
> > > +     }
> > > +
> > > +     ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL,
> > 
> > &val);
> > 
> > > +     if (ret) {
> > > +             dev_err(&i2c->dev, "Read device id error\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> > > +             pmic_trim = true;
> >
> >
> >
> > PCA9450_REG_PWRCTRL is a read/write register. How is it possible to detect
> > a chip revision using a bit which can be changed by software e.g.
> > bootloader? Despite that this bit sets debounce time for PMIC_ON_REQ, how
> > is this related to BUCK1 voltage range?
> 
> There are old and new two kind PMIC pca9451a. 

There is only one part mentioned in the ordering options. How can I 
distinguish them? Any chip ID, date codes, markings?

> This bit sets debounce time in
> PCA9450_REG_PWRCTRL was set different value by hardware in order to only
> distinguish the old and new PMIC. This bit isn't related to the BUCK1
> voltage range. If the pmic_trim is true that means it's new pca9451a.

But this bit is writable. How do you know it has not been modified since 
reset?

Best regards,
Alexander

> > > +
> > > 
> > >       switch (type) {
> > >       case PCA9450_TYPE_PCA9450A:
> > >       
> > >               regulator_desc = pca9450a_regulators; @@ -730,6 +956,10
> > > 
> > > @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> > > 
> > >               regulator_desc = pca9450bc_regulators;
> > >               pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
> > >               break;
> > > 
> > > +     case PCA9450_TYPE_PCA9451A:
> > > +             regulator_desc = pca9451a_regulators;
> > > +             pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> > > +             break;
> > > 
> > >       default:
> > >       
> > >               dev_err(&i2c->dev, "Unknown device type");
> > >               return -EINVAL;
> > > 
> > > @@ -741,13 +971,6 @@ static int pca9450_i2c_probe(struct i2c_client
> > > *i2c)
> > >
> > >
> > >
> > >       dev_set_drvdata(&i2c->dev, pca9450);
> > >
> > >
> > >
> > > -     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > > -
> > 
> > &pca9450_regmap_config);
> > 
> > > -     if (IS_ERR(pca9450->regmap)) {
> > > -             dev_err(&i2c->dev, "regmap initialization failed\n");
> > > -             return PTR_ERR(pca9450->regmap);
> > > -     }
> > > -
> > > 
> > >       ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID,
> > 
> > &device_id);
> > 
> > >       if (ret) {
> > >       
> > >               dev_err(&i2c->dev, "Read device id error\n"); @@ -756,7
> > > 
> > > +979,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> > >
> > >
> > >
> > >       /* Check your board and dts for match the right pmic */
> > >       if (((device_id >> 4) != 0x1 && type == PCA9450_TYPE_PCA9450A) ||
> > > 
> > > -         ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC))
> > 
> > {
> > 
> > > +         ((device_id >> 4) != 0x3 && type == PCA9450_TYPE_PCA9450BC)
> > > 
> > ||
> > ||
> > > +         ((device_id >> 4) != 0x9 && type == PCA9450_TYPE_PCA9451A))
> > > + {
> > > 
> > >               dev_err(&i2c->dev, "Device id(%x) mismatched\n",
> > >               
> > >                       device_id >> 4);
> > >               
> > >               return -EINVAL;
> > > 
> > > @@ -767,7 +991,16 @@ static int pca9450_i2c_probe(struct i2c_client
> > > *i2c)
> > > 
> > >               struct regulator_dev *rdev;
> > >               const struct pca9450_regulator_desc *r;
> > >
> > >
> > >
> > > -             r = &regulator_desc[i];
> > > +             if (type == PCA9450_TYPE_PCA9451A &&
> > > +                 !strcmp((&regulator_desc[i])->desc.name, "buck1") &&
> > 
> > pmic_trim) {
> > 
> > > +                     r = &regulator_desc[i + 1];
> > > +                     i = i + 1;
> > > +             } else if (type == PCA9450_TYPE_PCA9451A &&
> > > +                        !strcmp((&regulator_desc[i])->desc.name,
> > 
> > "buck1")) {
> > 
> > > +                     r = &regulator_desc[i];
> > > +                     i = i + 1;
> >
> >
> >
> > I would put this in a single 'type == PCA9450_TYPE_PCA9451A' branch, to
> > indicate that only PCA9451A needs some kind of special handling.
> 
> Yes, this makes the logic clearer. I will change in next version.
> 
> >
> >
> > > +             } else
> > > +                     r = &regulator_desc[i];
> > > 
> > >               desc = &r->desc;
> > >
> > >
> > >
> > >               config.regmap = pca9450->regmap; @@ -847,7 +1080,8
> > 
> > @@
> > 
> > > static int pca9450_i2c_probe(struct i2c_client *i2c)
> > > 
> > >       }
> > >
> > >
> > >
> > >       dev_info(&i2c->dev, "%s probed.\n",
> > > 
> > > -             type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> > 
> > "pca9450bc");
> > 
> > > +             type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> > > +             (type == PCA9450_TYPE_PCA9451A ? "pca9451a" :
> > 
> > "pca9450bc"));
> > 
> > >
> > >
> > >       return 0;
> > >  
> > >  }
> > > 
> > > @@ -865,6 +1099,10 @@ static const struct of_device_id
> > > pca9450_of_match[] = { .compatible = "nxp,pca9450c",
> > > 
> > >               .data = (void *)PCA9450_TYPE_PCA9450BC,
> > >       
> > >       },
> > > 
> > > +     {
> > > +             .compatible = "nxp,pca9451a",
> > > +             .data = (void *)PCA9450_TYPE_PCA9451A,
> > > +     },
> > > 
> > >       { }
> > >  
> > >  };
> > >  MODULE_DEVICE_TABLE(of, pca9450_of_match); diff --git
> > > 
> > > a/include/linux/regulator/pca9450.h
> > > b/include/linux/regulator/pca9450.h index 3c01c2bf84f5..5dd79f52165a
> > > 100644
> > > --- a/include/linux/regulator/pca9450.h
> > > +++ b/include/linux/regulator/pca9450.h
> > > @@ -9,6 +9,7 @@
> > > 
> > >  enum pca9450_chip_type {
> > >  
> > >       PCA9450_TYPE_PCA9450A = 0,
> > >       PCA9450_TYPE_PCA9450BC,
> > > 
> > > +     PCA9450_TYPE_PCA9451A,
> > > 
> > >       PCA9450_TYPE_AMOUNT,
> > >  
> > >  };
> > >
> > >
> > >
> > > @@ -93,6 +94,7 @@ enum {
> > > 
> > >       PCA9450_MAX_REGISTER        = 0x2F,
> > >  
> > >  };
> > >
> > >
> > >
> > > +#define PCA9450_REG_PWRCTRL_TOFF_DEB    BIT(5)
> >
> >
> >
> > Newline here please.
> 
> Ok, will modify it.
> Thank you very much for your comments and efforts.
> BR
> Joy Zou
> 
> >
> >
> > Best regards,
> > Alexander
> >
> >
> >
> > >  /* PCA9450 BUCK ENMODE bits */
> > >  #define BUCK_ENMODE_OFF                      0x00
> > >  #define BUCK_ENMODE_ONREQ            0x01
> >
> >
> >
> >
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > http://www.tq-/
> > group.com%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C1ecbe75c432d464
> > cb45f08db61cb08bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C638211296883052738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> > 7C&sdata=mz7vtBiDvAxY9FO7tTZO3oPBA8zvYS6f8hLgAwrU8uk%3D&reserved
> > =0
> >
> >
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: 2023年7月5日 21:13
> To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> linux-arm-kernel@lists.infradead.org; Joy Zou <joy.zou@nxp.com>
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hello,
>
> Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> >
> > > -----Original Message-----
> > > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > Sent: 2023年5月31日 19:35
> > > To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> > > broonie@kernel.org; robh+dt@kernel.org;
> > > krzysztof.kozlowski+dt@linaro.org;
> > > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > linux-arm-kernel@lists.infradead.org
> > > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > > <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > Joy Zou
>  <joy.zou@nxp.com>
> > > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > > support
> > >
> > >
> > >
> > > Caution: This is an external email. Please take care when clicking
> > > links or
>  opening attachments. When in doubt, report the message using the
> > > 'Report this email' button
> > >
> > >
> > >
> > >
> > > Hi,
> > >
> > >
> > >
> > > Am Mittwoch, 31. Mai 2023, 08:57:23 CEST schrieb Joy Zou:
> > >
> > > > Adding support for pmic pca9451a.
> > > >
> > > >
> > > >
> > > > This patch support old and new pmic pca9451a. The new pmic trimed
> > >
> > > BUCK1.
> > >
> > > > The default value of Toff_Deb is used to distinguish the old and
> > > > new pmic.
> > > >
> > > >
> > > >
> > > > Signed-off-by: Joy Zou <joy.zou@nxp.com>
> > > > ---
> > > >
> > > >  drivers/regulator/pca9450-regulator.c | 262
> > >
> > > ++++++++++++++++++++++++--
> > >
> > > >  include/linux/regulator/pca9450.h     |   2 +
> > > >  2 files changed, 252 insertions(+), 12 deletions(-)
> > > >
> > > >
> > > >
> > > > diff --git a/drivers/regulator/pca9450-regulator.c
> > > > b/drivers/regulator/pca9450-regulator.c index
> > > > 91bfb7e026c9..654aa4fbe494
> > > > 100644
> > > > --- a/drivers/regulator/pca9450-regulator.c
> > > > +++ b/drivers/regulator/pca9450-regulator.c
> > > > @@ -104,7 +104,15 @@ static const struct regulator_ops
> > > > pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
> > > >
> > > >   */
> > > >
> > > >  static const struct linear_range pca9450_dvs_buck_volts[] = {
> > > >
> > > > -     REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
> > > > +     REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500), };
> > > > +
> > > > +/*
> > > > + * BUCK1/3
> > > > + * 0.65 to 2.2375V (12.5mV step)
> > >
> > >
> > >
> > > Reading this comment, it seems the same distinction needs to be done
> > > for
> > > BUCK3 as well, no?
> >
> > Sorry for the late reply!
> > The BUCK1 and BUCK3 are dual phase, so don't need to be done for BUCK3.
> >
> > >
> > >
> > > > + */
> > > > +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> > > > +     REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
> > > >
> > > >  };
> > > >
> > > >
> > > >
> > > >  /*
> > > >
> > > > @@ -663,6 +671,207 @@ static const struct pca9450_regulator_desc
> > > > pca9450bc_regulators[] = { },  };
> > > >
> > > >
> > > >
> > > > +static const struct pca9450_regulator_desc pca9451a_regulators[] = {
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "buck1",
> > > > +                     .of_match = of_match_ptr("BUCK1"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_BUCK1,
> > > > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_dvs_buck_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_dvs_buck_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > > +                     .vsel_mask = BUCK1OUT_DVS0_MASK,
> > > > +                     .enable_reg = PCA9450_REG_BUCK1CTRL,
> > > > +                     .enable_mask = BUCK1_ENMODE_MASK,
> > > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > > +                     .ramp_mask = BUCK1_RAMP_MASK,
> > > > +                     .ramp_delay_table =
> > >
> > > pca9450_dvs_buck_ramp_table,
> > >
> > > > +                     .n_ramp_values =
> > >
> > > ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > >
> > > > +                     .owner = THIS_MODULE,
> > > > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > > > +             },
> > > > +             .dvs = {
> > > > +                     .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > > +                     .run_mask = BUCK1OUT_DVS0_MASK,
> > > > +                     .standby_reg =
> PCA9450_REG_BUCK1OUT_DVS1,
> > > > +                     .standby_mask = BUCK1OUT_DVS1_MASK,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "buck1_trim",
> > > > +                     .of_match = of_match_ptr("BUCK1"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_BUCK1,
> > > > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_BUCK1_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_trim_dvs_buck_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_trim_dvs_buck_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > > +                     .vsel_mask = BUCK1OUT_DVS0_MASK,
> > > > +                     .enable_reg = PCA9450_REG_BUCK1CTRL,
> > > > +                     .enable_mask = BUCK1_ENMODE_MASK,
> > > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > > +                     .ramp_mask = BUCK1_RAMP_MASK,
> > > > +                     .ramp_delay_table =
> > >
> > > pca9450_dvs_buck_ramp_table,
> > >
> > > > +                     .n_ramp_values =
> > >
> > > ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > >
> > > > +                     .owner = THIS_MODULE,
> > > > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > > > +             },
> > > > +             .dvs = {
> > > > +                     .run_reg = PCA9450_REG_BUCK1OUT_DVS0,
> > > > +                     .run_mask = BUCK1OUT_DVS0_MASK,
> > > > +                     .standby_reg =
> PCA9450_REG_BUCK1OUT_DVS1,
> > > > +                     .standby_mask = BUCK1OUT_DVS1_MASK,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "buck2",
> > > > +                     .of_match = of_match_ptr("BUCK2"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_BUCK2,
> > > > +                     .ops = &pca9450_dvs_buck_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_BUCK2_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_dvs_buck_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_dvs_buck_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > > > +                     .vsel_mask = BUCK2OUT_DVS0_MASK,
> > > > +                     .enable_reg = PCA9450_REG_BUCK2CTRL,
> > > > +                     .enable_mask = BUCK2_ENMODE_MASK,
> > > > +                     .enable_val =
> BUCK_ENMODE_ONREQ_STBYREQ,
> > > > +                     .ramp_mask = BUCK2_RAMP_MASK,
> > > > +                     .ramp_delay_table =
> > >
> > > pca9450_dvs_buck_ramp_table,
> > >
> > > > +                     .n_ramp_values =
> > >
> > > ARRAY_SIZE(pca9450_dvs_buck_ramp_table),
> > >
> > > > +                     .owner = THIS_MODULE,
> > > > +                     .of_parse_cb = pca9450_set_dvs_levels,
> > > > +             },
> > > > +             .dvs = {
> > > > +                     .run_reg = PCA9450_REG_BUCK2OUT_DVS0,
> > > > +                     .run_mask = BUCK2OUT_DVS0_MASK,
> > > > +                     .standby_reg =
> PCA9450_REG_BUCK2OUT_DVS1,
> > > > +                     .standby_mask = BUCK2OUT_DVS1_MASK,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "buck4",
> > > > +                     .of_match = of_match_ptr("BUCK4"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_BUCK4,
> > > > +                     .ops = &pca9450_buck_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_BUCK4_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_buck_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_buck_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_BUCK4OUT,
> > > > +                     .vsel_mask = BUCK4OUT_MASK,
> > > > +                     .enable_reg = PCA9450_REG_BUCK4CTRL,
> > > > +                     .enable_mask = BUCK4_ENMODE_MASK,
> > > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > > +                     .owner = THIS_MODULE,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "buck5",
> > > > +                     .of_match = of_match_ptr("BUCK5"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_BUCK5,
> > > > +                     .ops = &pca9450_buck_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_BUCK5_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_buck_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_buck_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_BUCK5OUT,
> > > > +                     .vsel_mask = BUCK5OUT_MASK,
> > > > +                     .enable_reg = PCA9450_REG_BUCK5CTRL,
> > > > +                     .enable_mask = BUCK5_ENMODE_MASK,
> > > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > > +                     .owner = THIS_MODULE,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "buck6",
> > > > +                     .of_match = of_match_ptr("BUCK6"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_BUCK6,
> > > > +                     .ops = &pca9450_buck_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_BUCK6_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_buck_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_buck_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_BUCK6OUT,
> > > > +                     .vsel_mask = BUCK6OUT_MASK,
> > > > +                     .enable_reg = PCA9450_REG_BUCK6CTRL,
> > > > +                     .enable_mask = BUCK6_ENMODE_MASK,
> > > > +                     .enable_val = BUCK_ENMODE_ONREQ,
> > > > +                     .owner = THIS_MODULE,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "ldo1",
> > > > +                     .of_match = of_match_ptr("LDO1"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_LDO1,
> > > > +                     .ops = &pca9450_ldo_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_LDO1_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_ldo1_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_ldo1_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_LDO1CTRL,
> > > > +                     .vsel_mask = LDO1OUT_MASK,
> > > > +                     .enable_reg = PCA9450_REG_LDO1CTRL,
> > > > +                     .enable_mask = LDO1_EN_MASK,
> > > > +                     .owner = THIS_MODULE,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "ldo4",
> > > > +                     .of_match = of_match_ptr("LDO4"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_LDO4,
> > > > +                     .ops = &pca9450_ldo_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_LDO4_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_ldo34_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_ldo34_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_LDO4CTRL,
> > > > +                     .vsel_mask = LDO4OUT_MASK,
> > > > +                     .enable_reg = PCA9450_REG_LDO4CTRL,
> > > > +                     .enable_mask = LDO4_EN_MASK,
> > > > +                     .owner = THIS_MODULE,
> > > > +             },
> > > > +     },
> > > > +     {
> > > > +             .desc = {
> > > > +                     .name = "ldo5",
> > > > +                     .of_match = of_match_ptr("LDO5"),
> > > > +                     .regulators_node = of_match_ptr("regulators"),
> > > > +                     .id = PCA9450_LDO5,
> > > > +                     .ops = &pca9450_ldo_regulator_ops,
> > > > +                     .type = REGULATOR_VOLTAGE,
> > > > +                     .n_voltages = PCA9450_LDO5_VOLTAGE_NUM,
> > > > +                     .linear_ranges = pca9450_ldo5_volts,
> > > > +                     .n_linear_ranges =
> > >
> > > ARRAY_SIZE(pca9450_ldo5_volts),
> > >
> > > > +                     .vsel_reg = PCA9450_REG_LDO5CTRL_H,
> > > > +                     .vsel_mask = LDO5HOUT_MASK,
> > > > +                     .enable_reg = PCA9450_REG_LDO5CTRL_H,
> > > > +                     .enable_mask = LDO5H_EN_MASK,
> > > > +                     .owner = THIS_MODULE,
> > > > +             },
> > > > +     },
> > > > +};
> > > > +
> > > >
> > > >  static irqreturn_t pca9450_irq_handler(int irq, void *data)  {
> > > >
> > > >       struct pca9450 *pca9450 = data;
> > > >
> > > > @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client
> > > > *i2c)
> > > >
> > > >       const struct pca9450_regulator_desc     *regulator_desc;
> > > >       struct regulator_config config = { };
> > > >       struct pca9450 *pca9450;
> > > >
> > > > -     unsigned int device_id, i;
> > > > +     unsigned int device_id, i, val;
> > > >
> > > >       unsigned int reset_ctrl;
> > > >
> > > > +     bool pmic_trim = false;
> > > >
> > > >       int ret;
> > > >
> > > >
> > > >
> > > >       if (!i2c->irq) {
> > > >
> > > > @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct
> > > > i2c_client
> > > > *i2c)
> > > >
> > > >       if (!pca9450)
> > > >
> > > >               return -ENOMEM;
> > > >
> > > >
> > > >
> > > > +     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > > > +
> > >
> > > &pca9450_regmap_config);
> > >
> > > > +     if (IS_ERR(pca9450->regmap)) {
> > > > +             dev_err(&i2c->dev, "regmap initialization failed\n");
> > > > +             return PTR_ERR(pca9450->regmap);
> > > > +     }
> > > > +
> > > > +     ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL,
> > >
> > > &val);
> > >
> > > > +     if (ret) {
> > > > +             dev_err(&i2c->dev, "Read device id error\n");
> > > > +             return ret;
> > > > +     }
> > > > +
> > > > +     if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> > > > +             pmic_trim = true;
> > >
> > >
> > >
> > > PCA9450_REG_PWRCTRL is a read/write register. How is it possible to
> > > detect a chip revision using a bit which can be changed by software e.g.
> > > bootloader? Despite that this bit sets debounce time for
> > > PMIC_ON_REQ, how is this related to BUCK1 voltage range?
> >
> > There are old and new two kind PMIC pca9451a.
>
> There is only one part mentioned in the ordering options. How can I
> distinguish them? Any chip ID, date codes, markings?
Yes, there is only one part. We distinguish the new and old part by this bit Toff_Deb of
PCA9450_REG_PWRCTRL reset value. The reset value 0 means it's old part, and the reset
value 1 means it's new part.
>
> > This bit sets debounce time in
> > PCA9450_REG_PWRCTRL was set different value by hardware in order to
> > only distinguish the old and new PMIC. This bit isn't related to the
> > BUCK1 voltage range. If the pmic_trim is true that means it's new pca9451a.
>
> But this bit is writable. How do you know it has not been modified since reset?
Yes, we don't consider modify the debounce bit case. Modify the Toff_deb value will influence the old and new part judgement.

For example, this default value of Toff_deb is 1 in the new part, if the customers change the Toff_deb value from 1 to 0, and then make the board warm reset, the Toff_deb value still keep 0, if the Toff_deb value is 0, the PMIC driver will think this part is old part. But this part is new part in fact.

Have discussed this issue with our internal team member, we will add a note to PCA9451 datasheet – “Please contract NXP If you want to change Toff_deb.” But till now, we am not aware any customers case which need to adjust Toff_deb.

Make it more clear: If customers do need to manually adjust Toff_deb, It need PMIC driver update to bypass this bit check and directly apply corresponding voltage config table old or new.
Thank you very much!
BR
Joy Zou

>
> Best regards,
> Alexander
>
> > > > +
> > > >
> > > >       switch (type) {
> > > >       case PCA9450_TYPE_PCA9450A:
> > > >
> > > >               regulator_desc = pca9450a_regulators; @@ -730,6
> > > > +956,10
> > > >
> > > > @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> > > >
> > > >               regulator_desc = pca9450bc_regulators;
> > > >               pca9450->rcnt = ARRAY_SIZE(pca9450bc_regulators);
> > > >               break;
> > > >
> > > > +     case PCA9450_TYPE_PCA9451A:
> > > > +             regulator_desc = pca9451a_regulators;
> > > > +             pca9450->rcnt = ARRAY_SIZE(pca9451a_regulators);
> > > > +             break;
> > > >
> > > >       default:
> > > >
> > > >               dev_err(&i2c->dev, "Unknown device type");
> > > >               return -EINVAL;
> > > >
> > > > @@ -741,13 +971,6 @@ static int pca9450_i2c_probe(struct
> > > > i2c_client
> > > > *i2c)
> > > >
> > > >
> > > >
> > > >       dev_set_drvdata(&i2c->dev, pca9450);
> > > >
> > > >
> > > >
> > > > -     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > > > -
> > >
> > > &pca9450_regmap_config);
> > >
> > > > -     if (IS_ERR(pca9450->regmap)) {
> > > > -             dev_err(&i2c->dev, "regmap initialization failed\n");
> > > > -             return PTR_ERR(pca9450->regmap);
> > > > -     }
> > > > -
> > > >
> > > >       ret = regmap_read(pca9450->regmap, PCA9450_REG_DEV_ID,
> > >
> > > &device_id);
> > >
> > > >       if (ret) {
> > > >
> > > >               dev_err(&i2c->dev, "Read device id error\n"); @@
> > > > -756,7
> > > >
> > > > +979,8 @@ static int pca9450_i2c_probe(struct i2c_client *i2c)
> > > >
> > > >
> > > >
> > > >       /* Check your board and dts for match the right pmic */
> > > >       if (((device_id >> 4) != 0x1 && type ==
> > > > PCA9450_TYPE_PCA9450A) ||
> > > >
> > > > -         ((device_id >> 4) != 0x3 && type ==
> PCA9450_TYPE_PCA9450BC))
> > >
> > > {
> > >
> > > > +         ((device_id >> 4) != 0x3 && type ==
> > > > + PCA9450_TYPE_PCA9450BC)
> > > >
> > > ||
> > > ||
> > > > +         ((device_id >> 4) != 0x9 && type ==
> > > > + PCA9450_TYPE_PCA9451A)) {
> > > >
> > > >               dev_err(&i2c->dev, "Device id(%x) mismatched\n",
> > > >
> > > >                       device_id >> 4);
> > > >
> > > >               return -EINVAL;
> > > >
> > > > @@ -767,7 +991,16 @@ static int pca9450_i2c_probe(struct
> > > > i2c_client
> > > > *i2c)
> > > >
> > > >               struct regulator_dev *rdev;
> > > >               const struct pca9450_regulator_desc *r;
> > > >
> > > >
> > > >
> > > > -             r = &regulator_desc[i];
> > > > +             if (type == PCA9450_TYPE_PCA9451A &&
> > > > +                 !strcmp((&regulator_desc[i])->desc.name,
> > > > + "buck1") &&
> > >
> > > pmic_trim) {
> > >
> > > > +                     r = &regulator_desc[i + 1];
> > > > +                     i = i + 1;
> > > > +             } else if (type == PCA9450_TYPE_PCA9451A &&
> > > > +                        !strcmp((&regulator_desc[i])->desc.name,
> > >
> > > "buck1")) {
> > >
> > > > +                     r = &regulator_desc[i];
> > > > +                     i = i + 1;
> > >
> > >
> > >
> > > I would put this in a single 'type == PCA9450_TYPE_PCA9451A' branch,
> > > to indicate that only PCA9451A needs some kind of special handling.
> >
> > Yes, this makes the logic clearer. I will change in next version.
> >
> > >
> > >
> > > > +             } else
> > > > +                     r = &regulator_desc[i];
> > > >
> > > >               desc = &r->desc;
> > > >
> > > >
> > > >
> > > >               config.regmap = pca9450->regmap; @@ -847,7 +1080,8
> > >
> > > @@
> > >
> > > > static int pca9450_i2c_probe(struct i2c_client *i2c)
> > > >
> > > >       }
> > > >
> > > >
> > > >
> > > >       dev_info(&i2c->dev, "%s probed.\n",
> > > >
> > > > -             type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> > >
> > > "pca9450bc");
> > >
> > > > +             type == PCA9450_TYPE_PCA9450A ? "pca9450a" :
> > > > +             (type == PCA9450_TYPE_PCA9451A ? "pca9451a" :
> > >
> > > "pca9450bc"));
> > >
> > > >
> > > >
> > > >       return 0;
> > > >
> > > >  }
> > > >
> > > > @@ -865,6 +1099,10 @@ static const struct of_device_id
> > > > pca9450_of_match[] = { .compatible = "nxp,pca9450c",
> > > >
> > > >               .data = (void *)PCA9450_TYPE_PCA9450BC,
> > > >
> > > >       },
> > > >
> > > > +     {
> > > > +             .compatible = "nxp,pca9451a",
> > > > +             .data = (void *)PCA9450_TYPE_PCA9451A,
> > > > +     },
> > > >
> > > >       { }
> > > >
> > > >  };
> > > >  MODULE_DEVICE_TABLE(of, pca9450_of_match); diff --git
> > > >
> > > > a/include/linux/regulator/pca9450.h
> > > > b/include/linux/regulator/pca9450.h index
> > > > 3c01c2bf84f5..5dd79f52165a
> > > > 100644
> > > > --- a/include/linux/regulator/pca9450.h
> > > > +++ b/include/linux/regulator/pca9450.h
> > > > @@ -9,6 +9,7 @@
> > > >
> > > >  enum pca9450_chip_type {
> > > >
> > > >       PCA9450_TYPE_PCA9450A = 0,
> > > >       PCA9450_TYPE_PCA9450BC,
> > > >
> > > > +     PCA9450_TYPE_PCA9451A,
> > > >
> > > >       PCA9450_TYPE_AMOUNT,
> > > >
> > > >  };
> > > >
> > > >
> > > >
> > > > @@ -93,6 +94,7 @@ enum {
> > > >
> > > >       PCA9450_MAX_REGISTER        = 0x2F,
> > > >
> > > >  };
> > > >
> > > >
> > > >
> > > > +#define PCA9450_REG_PWRCTRL_TOFF_DEB    BIT(5)
> > >
> > >
> > >
> > > Newline here please.
> >
> > Ok, will modify it.
> > Thank you very much for your comments and efforts.
> > BR
> > Joy Zou
> >
> > >
> > >
> > > Best regards,
> > > Alexander
> > >
> > >
> > >
> > > >  /* PCA9450 BUCK ENMODE bits */
> > > >  #define BUCK_ENMODE_OFF                      0x00
> > > >  #define BUCK_ENMODE_ONREQ            0x01
> > >
> > >
> > >
> > >
> > > --
> > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > > Amtsgericht München, HRB 105018
> > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > > http://www/
> > > .tq-%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C19989354bc054cffbfc
> b08db7
> > >
> d59888c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63824159
> 5727354
> > >
> 598%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
> IiLCJB
> > >
> TiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HBR3huCMS9N
> UdrmUBfs
> > > rbzr0Rlfy43g8i8ZH2v6blYw%3D&reserved=0
> > >
> group.com%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C1ecbe75c432d464
> > >
> cb45f08db61cb08bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > >
> C638211296883052738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> > >
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> > >
> 7C&sdata=mz7vtBiDvAxY9FO7tTZO3oPBA8zvYS6f8hLgAwrU8uk%3D&reserved
> > > =0
> > >
> > >
> >
> >
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-/
> group.com%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C19989354bc054cf
> fbfcb08db7d59888c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C638241595727354598%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> 7C&sdata=D71%2FiRInAUfBu2F32j4579TV0hwqMAWjM5gf%2Fx3xxdM%3D&r
> eserved=0
>

Re: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On Mon, Jul 17, 2023 at 09:53:15AM +0000, Joy Zou wrote:
> 
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: 2023年7月5日 21:13
> > To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> > broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > linux-arm-kernel@lists.infradead.org; Joy Zou <joy.zou@nxp.com>
> > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 2023年7月17日 21:33
> To: Joy Zou <joy.zou@nxp.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; Jacky Bai
> <ping.bai@nxp.com>; lgirdwood@gmail.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de;
> linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> support
> 
> On Mon, Jul 17, 2023 at 09:53:15AM +0000, Joy Zou wrote:
> >
> > > -----Original Message-----
> > > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > Sent: 2023年7月5日 21:13
> > > To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> > > broonie@kernel.org; robh+dt@kernel.org;
> > > krzysztof.kozlowski+dt@linaro.org;
> > > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > linux-arm-kernel@lists.infradead.org; Joy Zou <joy.zou@nxp.com>
> > > Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > > <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > > support
> 
> Please delete unneeded context from mails when replying.  Doing this makes
> it much easier to find your reply in the message, helping ensure it won't be
> missed by people scrolling through the irrelevant quoted material.
Thanks your kind reminder.
I have deleted unneeded context and reply again. It can be more clearer. 
BR
Joy Zou

RE: [EXT] Re: [PATCH v1 3/3] arm64: dts: imx93-11x11-evk: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: 2023年5月31日 19:18
> To: Joy Zou <joy.zou@nxp.com>
> Cc: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v1 3/3] arm64: dts: imx93-11x11-evk: add pca9451a
> support
> 
> On Wed, May 31, 2023 at 02:57:24PM +0800, Joy Zou wrote:
> 
> > +		regulators {
> > +			buck1: BUCK1 {
> > +				regulator-name = "BUCK1";
> > +				regulator-min-microvolt = <650000>;
> > +				regulator-max-microvolt = <2237500>;
> 
> These look a lot like the maximum ranges the regulator supports which
> probably isn't what's safe for the board.
Sorry for late reply. Because my outlook filter rule has issue.
The maximum ranges is the PMIC support according to datasheet, but we don't use actually the maximum value, the actual use maximum value is 0.95V. So it's safe for the board.
Do you hope us to list only the voltage ranges actually used?
Thank you very much for your comments and efforts.
BR
Joy Zou

Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Alexander Stein]

Hi,

Am Dienstag, 1. August 2023, 12:17:20 CEST schrieb Joy Zou:
> > -----Original Message-----
> > From: Alexander Stein
> > <alexander.stein@ew.tq-group.com<mailto:alexander.stein@ew.tq-group.com>>
> > 
 Sent: 2023年7月5日 21:13
> > To: Jacky Bai <ping.bai@nxp.com<mailto:ping.bai@nxp.com>>;
> > lgirdwood@gmail.com<mailto:lgirdwood@gmail.com>;
> > broonie@kernel.org<mailto:broonie@kernel.org>;
> > robh+dt@kernel.org<mailto:robh+dt@kernel.org>;
> > krzysztof.kozlowski+dt@linaro.org<mailto:krzysztof.kozlowski+dt@linaro.or
> > g>; conor+dt@kernel.org<mailto:conor+dt@kernel.org>;
> > shawnguo@kernel.org<mailto:shawnguo@kernel.org>;
> > s.hauer@pengutronix.de<mailto:s.hauer@pengutronix.de>;
> > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.infrad
> > ead.org>; Joy Zou <joy.zou@nxp.com<mailto:joy.zou@nxp.com>> Cc:
> > kernel@pengutronix.de<mailto:kernel@pengutronix.de>;
> > festevam@gmail.com<mailto:festevam@gmail.com>; dl-linux-imx
> > <linux-imx@nxp.com<mailto:linux-imx@nxp.com>>;
> > devicetree@vger.kernel.org<mailto:devicetree@vger.kernel.org>;
> > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.infrad
> > ead.org>;
> > linux-kernel@vger.kernel.org<mailto:linux-kernel@vger.kernel.org>
> > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > support>
> >
> >
> > Caution: This is an external email. Please take care when clicking links
> > or
 opening attachments. When in doubt, report the message using the
> > 'Report this email' button
> >
> >
> >
> >
> > Hello,
> >
> >
> >
> > Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> > 
> > >
> > >
> > > > -----Original Message-----
> > > > From: Alexander Stein
> > > > <alexander.stein@ew.tq-group.com<mailto:alexander.stein@ew.tq-group.c
> > > > om>>
 Sent: 2023年5月31日 19:35
> > > > To: Jacky Bai <ping.bai@nxp.com<mailto:ping.bai@nxp.com>>;
> > > > lgirdwood@gmail.com<mailto:lgirdwood@gmail.com>;
> > > > broonie@kernel.org<mailto:broonie@kernel.org>;
> > > > robh+dt@kernel.org<mailto:robh+dt@kernel.org>;
> > > > krzysztof.kozlowski+dt@linaro.org<mailto:krzysztof.kozlowski+dt@linar
> > > > o.org>; conor+dt@kernel.org<mailto:conor+dt@kernel.org>;
> > > > shawnguo@kernel.org<mailto:shawnguo@kernel.org>;
> > > > s.hauer@pengutronix.de<mailto:s.hauer@pengutronix.de>;
> > > > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.in
> > > > fradead.org> Cc: kernel@pengutronix.de<mailto:kernel@pengutronix.de>;
> > > > festevam@gmail.com<mailto:festevam@gmail.com>; dl-linux-imx
> > > > <linux-imx@nxp.com<mailto:linux-imx@nxp.com>>;
> > > > devicetree@vger.kernel.org<mailto:devicetree@vger.kernel.org>;
> > > > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.in
> > > > fradead.org>;
> > > > linux-kernel@vger.kernel.org<mailto:linux-kernel@vger.kernel.org>;
> > > > Joy Zou
> >  
> >  <joy.zou@nxp.com<mailto:joy.zou@nxp.com>>
> >  
> > > > Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > > > support
> > > >
> > > >
> > > >
> > > > Hi,
> > > > 
> > > > > @@ -104,7 +104,15 @@ static const struct regulator_ops
> > > > > pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
> > > > >
> > > > >
> > > > >
> > > > >   */
> > > > >
> > > > >
> > > > >
> > > > >  static const struct linear_range pca9450_dvs_buck_volts[] = {
> > > > >
> > > > >
> > > > >
> > > > > -     REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
> > > > > +     REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500), };
> > > > > +
> > > > > +/*
> > > > > + * BUCK1/3
> > > > > + * 0.65 to 2.2375V (12.5mV step)
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > Reading this comment, it seems the same distinction needs to be done
> > > > for
> > > > BUCK3 as well, no?
> > >
> > >
> > >
> > > Sorry for the late reply!
> > > The BUCK1 and BUCK3 are dual phase, so don't need to be done for BUCK3.
> > >
> > >
> > >
> > > >
> > > >
> > > >
> > > > > + */
> > > > > +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> > > > > +     REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
> > > > >
> > > > >
> > > > >
> > > > >  };
> 
> 
> 
> > > > > @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct i2c_client
> > > > > *i2c)
> > > > >
> > > > >
> > > > >
> > > > >       const struct pca9450_regulator_desc     *regulator_desc;
> > > > >       struct regulator_config config = { };
> > > > >       struct pca9450 *pca9450;
> > > > >
> > > > >
> > > > >
> > > > > -     unsigned int device_id, i;
> > > > > +     unsigned int device_id, i, val;
> > > > >
> > > > >
> > > > >
> > > > >       unsigned int reset_ctrl;
> > > > >
> > > > >
> > > > >
> > > > > +     bool pmic_trim = false;
> > > > >
> > > > >
> > > > >
> > > > >       int ret;
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >       if (!i2c->irq) {
> > > > >
> > > > >
> > > > >
> > > > > @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct
> > > > > i2c_client
> > > > > *i2c)
> > > > >
> > > > >
> > > > >
> > > > >       if (!pca9450)
> > > > >
> > > > >
> > > > >
> > > > >               return -ENOMEM;
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > +     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > > > > +
> > > >
> > > >
> > > >
> > > > &pca9450_regmap_config);
> > > >
> > > >
> > > >
> > > > > +     if (IS_ERR(pca9450->regmap)) {
> > > > > +             dev_err(&i2c->dev, "regmap initialization failed\n");
> > > > > +             return PTR_ERR(pca9450->regmap);
> > > > > +     }
> > > > > +
> > > > > +     ret = regmap_read(pca9450->regmap, PCA9450_REG_PWRCTRL,
> > > >
> > > >
> > > >
> > > > &val);
> > > >
> > > >
> > > >
> > > > > +     if (ret) {
> > > > > +             dev_err(&i2c->dev, "Read device id error\n");
> > > > > +             return ret;
> > > > > +     }
> > > > > +
> > > > > +     if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> > > > > +             pmic_trim = true;
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > PCA9450_REG_PWRCTRL is a read/write register. How is it possible to
> > > > detect a chip revision using a bit which can be changed by software
> > > > e.g.
> > > > bootloader? Despite that this bit sets debounce time for
> > > > PMIC_ON_REQ, how is this related to BUCK1 voltage range?
> > >
> > >
> > >
> > > There are old and new two kind PMIC pca9451a.
> >
> >
> >
> > There is only one part mentioned in the ordering options. How can I
> > distinguish them? Any chip ID, date codes, markings?
> 
> Yes, there is only one part. We distinguish the new and old part by this
> bit
 Toff_Deb of PCA9450_REG_PWRCTRL reset value. The reset value 0 means
> it's old part, and the reset value 1 means it's new part.

Is the "old" part by coincidence an unofficial prerelease/sample chip?

> >
> >
> > > This bit sets debounce time in
> > > PCA9450_REG_PWRCTRL was set different value by hardware in order to
> > > only distinguish the old and new PMIC. This bit isn't related to the
> > > BUCK1 voltage range. If the pmic_trim is true that means it's new
> > > pca9451a.
>
> >
> >
> > But this bit is writable. How do you know it has not been modified since
> > reset?
> Yes, we don't consider modify the debounce bit case. Modify the Toff_deb
> value
 will influence the old and new part judgement.

This judgement seems broken to me. How can I know offline whether I have old 
or new parts? I would like to know if there is a difference on some my boards.

> For example, this default value of Toff_deb is 1 in the new part, if the
> customers
 change the Toff_deb value from 1 to 0, and then make the board
> warm reset, the Toff_deb value still keep 0, if the Toff_deb value is 0,
> the PMIC driver will think this part is old part. But this part is new part
> in fact.

This should show you it's a bad idea to decide the chip revision depending on 
Toff_deb.

> Have discussed this issue with our internal team member, we will add a note
> to PCA9451
 datasheet – “Please contract NXP If you want to change
> Toff_deb.” But till now, we am not aware any customers case which need to
> adjust Toff_deb.
> 
> Make it more clear: If customers do need to manually adjust Toff_deb, It
> need PMIC driver
 update to bypass this bit check and directly apply
> corresponding voltage config table old or new. Thank you very much for your
> comments and efforts.

In this case I need to know if I use old, new or both revision of these parts.

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support

[Joy Zou]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: 2023年10月17日 22:02
> To: Jacky Bai <ping.bai@nxp.com>; lgirdwood@gmail.com;
> broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> linux-arm-kernel@lists.infradead.org; Joy Zou <joy.zou@nxp.com>
> Cc: kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a support
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hi,
>
> Am Dienstag, 1. August 2023, 12:17:20 CEST schrieb Joy Zou:
> > > -----Original Message-----
> > > From: Alexander Stein
> > > <alexander.stein@ew.tq-group.com<mailto:alexander.stein@ew.tq-group.
> > > com>>
> > >
>  Sent: 2023年7月5日 21:13
> > > To: Jacky Bai <ping.bai@nxp.com<mailto:ping.bai@nxp.com>>;
> > > lgirdwood@gmail.com<mailto:lgirdwood@gmail.com>;
> > > broonie@kernel.org<mailto:broonie@kernel.org>;
> > > robh+dt@kernel.org<mailto:robh+dt@kernel.org>;
> > > krzysztof.kozlowski+dt@linaro.org<mailto:krzysztof.kozlowski+dt@lina
> > > ro.or
> > > g>; conor+dt@kernel.org<mailto:conor+dt@kernel.org>;
> > > shawnguo@kernel.org<mailto:shawnguo@kernel.org>;
> > > s.hauer@pengutronix.de<mailto:s.hauer@pengutronix.de>;
> > > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.i
> > > nfrad ead.org>; Joy Zou <joy.zou@nxp.com<mailto:joy.zou@nxp.com>>
> > > Cc:
> > > kernel@pengutronix.de<mailto:kernel@pengutronix.de>;
> > > festevam@gmail.com<mailto:festevam@gmail.com>; dl-linux-imx
> > > <linux-imx@nxp.com<mailto:linux-imx@nxp.com>>;
> > > devicetree@vger.kernel.org<mailto:devicetree@vger.kernel.org>;
> > > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lists.i
> > > nfrad
> > > ead.org>;
> > > linux-kernel@vger.kernel.org<mailto:linux-kernel@vger.kernel.org>
> > > Subject: [EXT] Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > > support>
> > >
> > >
> > > Caution: This is an external email. Please take care when clicking
> > > links or
>  opening attachments. When in doubt, report the message using the
> > > 'Report this email' button
> > >
> > >
> > >
> > >
> > > Hello,
> > >
> > >
> > >
> > > Am Mittwoch, 5. Juli 2023, 08:50:24 CEST schrieb Joy Zou:
> > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Alexander Stein
> > > > > <alexander.stein@ew.tq-group.com<mailto:alexander.stein@ew.tq-gr
> > > > > oup.c
> > > > > om>>
>  Sent: 2023年5月31日 19:35
> > > > > To: Jacky Bai <ping.bai@nxp.com<mailto:ping.bai@nxp.com>>;
> > > > > lgirdwood@gmail.com<mailto:lgirdwood@gmail.com>;
> > > > > broonie@kernel.org<mailto:broonie@kernel.org>;
> > > > > robh+dt@kernel.org<mailto:robh+dt@kernel.org>;
> > > > > krzysztof.kozlowski+dt@linaro.org<mailto:krzysztof.kozlowski+dt@
> > > > > linar o.org>; conor+dt@kernel.org<mailto:conor+dt@kernel.org>;
> > > > > shawnguo@kernel.org<mailto:shawnguo@kernel.org>;
> > > > > s.hauer@pengutronix.de<mailto:s.hauer@pengutronix.de>;
> > > > > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lis
> > > > > ts.in fradead.org> Cc:
> > > > > kernel@pengutronix.de<mailto:kernel@pengutronix.de>;
> > > > > festevam@gmail.com<mailto:festevam@gmail.com>; dl-linux-imx
> > > > > <linux-imx@nxp.com<mailto:linux-imx@nxp.com>>;
> > > > > devicetree@vger.kernel.org<mailto:devicetree@vger.kernel.org>;
> > > > > linux-arm-kernel@lists.infradead.org<mailto:linux-arm-kernel@lis
> > > > > ts.in
> > > > > fradead.org>;
> > > > > linux-kernel@vger.kernel.org<mailto:linux-kernel@vger.kernel.org
> > > > > >;
> > > > > Joy Zou
> > >
> > >  <joy.zou@nxp.com<mailto:joy.zou@nxp.com>>
> > >
> > > > > Subject: Re: [PATCH v1 2/3] regulator: pca9450: add pca9451a
> > > > > support
> > > > >
> > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > > @@ -104,7 +104,15 @@ static const struct regulator_ops
> > > > > > pca9450_ldo_regulator_ops = { * 0.60 to 2.1875V (12.5mV step)
> > > > > >
> > > > > >
> > > > > >
> > > > > >   */
> > > > > >
> > > > > >
> > > > > >
> > > > > >  static const struct linear_range pca9450_dvs_buck_volts[] = {
> > > > > >
> > > > > >
> > > > > >
> > > > > > -     REGULATOR_LINEAR_RANGE(600000,  0x00, 0x7F, 12500),
> > > > > > +     REGULATOR_LINEAR_RANGE(600000, 0x00, 0x7F, 12500), };
> > > > > > +
> > > > > > +/*
> > > > > > + * BUCK1/3
> > > > > > + * 0.65 to 2.2375V (12.5mV step)
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Reading this comment, it seems the same distinction needs to be
> > > > > done for
> > > > > BUCK3 as well, no?
> > > >
> > > >
> > > >
> > > > Sorry for the late reply!
> > > > The BUCK1 and BUCK3 are dual phase, so don't need to be done for
> BUCK3.
> > > >
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > > + */
> > > > > > +static const struct linear_range pca9450_trim_dvs_buck_volts[] = {
> > > > > > +     REGULATOR_LINEAR_RANGE(650000, 0x00, 0x7F, 12500),
> > > > > >
> > > > > >
> > > > > >
> > > > > >  };
> >
> >
> >
> > > > > > @@ -708,8 +917,9 @@ static int pca9450_i2c_probe(struct
> > > > > > i2c_client
> > > > > > *i2c)
> > > > > >
> > > > > >
> > > > > >
> > > > > >       const struct pca9450_regulator_desc     *regulator_desc;
> > > > > >       struct regulator_config config = { };
> > > > > >       struct pca9450 *pca9450;
> > > > > >
> > > > > >
> > > > > >
> > > > > > -     unsigned int device_id, i;
> > > > > > +     unsigned int device_id, i, val;
> > > > > >
> > > > > >
> > > > > >
> > > > > >       unsigned int reset_ctrl;
> > > > > >
> > > > > >
> > > > > >
> > > > > > +     bool pmic_trim = false;
> > > > > >
> > > > > >
> > > > > >
> > > > > >       int ret;
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >       if (!i2c->irq) {
> > > > > >
> > > > > >
> > > > > >
> > > > > > @@ -721,6 +931,22 @@ static int pca9450_i2c_probe(struct
> > > > > > i2c_client
> > > > > > *i2c)
> > > > > >
> > > > > >
> > > > > >
> > > > > >       if (!pca9450)
> > > > > >
> > > > > >
> > > > > >
> > > > > >               return -ENOMEM;
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > +     pca9450->regmap = devm_regmap_init_i2c(i2c,
> > > > > > +
> > > > >
> > > > >
> > > > >
> > > > > &pca9450_regmap_config);
> > > > >
> > > > >
> > > > >
> > > > > > +     if (IS_ERR(pca9450->regmap)) {
> > > > > > +             dev_err(&i2c->dev, "regmap initialization failed\n");
> > > > > > +             return PTR_ERR(pca9450->regmap);
> > > > > > +     }
> > > > > > +
> > > > > > +     ret = regmap_read(pca9450->regmap,
> PCA9450_REG_PWRCTRL,
> > > > >
> > > > >
> > > > >
> > > > > &val);
> > > > >
> > > > >
> > > > >
> > > > > > +     if (ret) {
> > > > > > +             dev_err(&i2c->dev, "Read device id error\n");
> > > > > > +             return ret;
> > > > > > +     }
> > > > > > +
> > > > > > +     if (val & PCA9450_REG_PWRCTRL_TOFF_DEB)
> > > > > > +             pmic_trim = true;
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > PCA9450_REG_PWRCTRL is a read/write register. How is it possible
> > > > > to detect a chip revision using a bit which can be changed by
> > > > > software e.g.
> > > > > bootloader? Despite that this bit sets debounce time for
> > > > > PMIC_ON_REQ, how is this related to BUCK1 voltage range?
> > > >
> > > >
> > > >
> > > > There are old and new two kind PMIC pca9451a.
> > >
> > >
> > >
> > > There is only one part mentioned in the ordering options. How can I
> > > distinguish them? Any chip ID, date codes, markings?
> >
> > Yes, there is only one part. We distinguish the new and old part by
> > this bit
>  Toff_Deb of PCA9450_REG_PWRCTRL reset value. The reset value 0 means
> > it's old part, and the reset value 1 means it's new part.
>
> Is the "old" part by coincidence an unofficial prerelease/sample chip?
Yes, the "old" part is sample chip for the customer.
>
> > >
> > >
> > > > This bit sets debounce time in
> > > > PCA9450_REG_PWRCTRL was set different value by hardware in order
> > > > to only distinguish the old and new PMIC. This bit isn't related
> > > > to the
> > > > BUCK1 voltage range. If the pmic_trim is true that means it's new
> > > > pca9451a.
> >
> > >
> > >
> > > But this bit is writable. How do you know it has not been modified
> > > since reset?
> > Yes, we don't consider modify the debounce bit case. Modify the
> > Toff_deb value
>  will influence the old and new part judgement.
>
> This judgement seems broken to me. How can I know offline whether I have
> old or new parts? I would like to know if there is a difference on some my
> boards.
Yes, We can't differentiate the old and new parts offline. there is no
difference on some my boards. So we select the register bit default value as the
check value. Unfortunately, the register is readable and writable.
>
> > For example, this default value of Toff_deb is 1 in the new part, if
> > the customers
>  change the Toff_deb value from 1 to 0, and then make the board
> > warm reset, the Toff_deb value still keep 0, if the Toff_deb value is
> > 0, the PMIC driver will think this part is old part. But this part is
> > new part in fact.
>
> This should show you it's a bad idea to decide the chip revision depending on
> Toff_deb.
Yes, I think so, so we have discussed that we firstly only new pmic. We will bypass
the register bit check. And send new patch again.
>
> > Have discussed this issue with our internal team member, we will add a
> > note to PCA9451
>  datasheet – “Please contract NXP If you want to change
> > Toff_deb.” But till now, we am not aware any customers case which need
> > to adjust Toff_deb.
> >
> > Make it more clear: If customers do need to manually adjust Toff_deb,
> > It need PMIC driver
>  update to bypass this bit check and directly apply
> > corresponding voltage config table old or new. Thank you very much for
> > your comments and efforts.
>
> In this case I need to know if I use old, new or both revision of these parts.
For the user, you don't need to know the revision. There is no function difference
about different version.
Thank you very much for your comments and efforts.
BR
Joy Zou
>
> Best regards,
> Alexander
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-/
> group.com%2F&data=05%7C01%7Cjoy.zou%40nxp.com%7C7195b18abca549c
> 7afa208dbcf19a8c9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C638331481351694496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> 7C&sdata=9SxW%2FUPpKWt0HIUsPoQ54ykworKw7gOZIhKsHfEPZS0%3D&rese
> rved=0
>

Re: (subset) [PATCH v1 0/3] add pmic pca9451a support

[Mark Brown]

On Wed, 31 May 2023 14:57:21 +0800, Joy Zou wrote:
> The patchset supports pmic pca9451a.
> For the details, please check the patch commit log.
> 
> Joy Zou (3):
>   dt-bindings: regulator: pca9450: add pca9451a support
>   regulator: pca9450: add pca9451a support
>   arm64: dts: imx93-11x11-evk: add pca9451a support
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] dt-bindings: regulator: pca9450: add pca9451a support
      (no commit info)
[2/3] regulator: pca9450: add pca9451a support
      commit: 5edeb7d312628961046eec9b26a7e72f44baf846

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark