[RESEND PATCH v3 0/5] Add support for TI TPS65224 PMIC

[RESEND PATCH v3 0/5] Add support for TI TPS65224 PMIC

[Bhargav Raviprakash]

TPS65224 is a Power Management IC which provides regulators and others
features like GPIOs, RTC, watchdog, ADC, ESMs (Error Signal Monitor),
and PFSM (Pre-configurable Finite State Machine). The SoC and the PMIC
can communicate through the I2C.

Data Sheet for TPS65224: https://www.ti.com/product/TPS65224-Q1

Reusing the TPS65941 PMIC driver to add support for TPS65224 PMIC 
in U-boot. This includes driver for PMIC and regulator.

The driver was tested on Ti's custom AM62P EVM using U-boot's
pmic list, regulator list, regulator status and regulator value commands.
Since, support for Ti's AM62P is absent in u-boot next, the patches
were applied on ti-u-boot ti-u-boot-2023.04-next and tested.

Changelog v2 -> v3:
- use pr_err instead of printf
- comments added in ldo conversion functions

Bhargav Raviprakash (5):
  power: tps65941: Add macros of TPS65224 PMIC
  power: pmic: tps65941: Add TI TPS65224 PMIC
  power: regulator: tps65941: Added macros for BUCK ID
  power: regulator: tps65941: use function callbacks for conversion ops
  power: regulator: tps65941: Add TPS65224 PMIC regulator support

 drivers/power/pmic/tps65941.c                |   1 +
 drivers/power/regulator/tps65941_regulator.c | 384 +++++++++++++++++--
 include/power/tps65941.h                     |  30 ++
 3 files changed, 384 insertions(+), 31 deletions(-)


base-commit: 4b151562bb8e54160adedbc6a1c0c749c00a2f84
-- 
2.25.1

[RESEND PATCH v3 1/5] power: tps65941: Add macros of TPS65224 PMIC

[Bhargav Raviprakash]

Re-use the TPS65941 PMIC driver for TPS65224 PMIC.
Add additional macros of TPS65224 to aid in the driver
re-use.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
---
 include/power/tps65941.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/power/tps65941.h b/include/power/tps65941.h
index a2bc6814ba..cec85333f0 100644
--- a/include/power/tps65941.h
+++ b/include/power/tps65941.h
@@ -3,11 +3,14 @@
 #define TPS659413		0x2
 #define TPS659414		0x3
 #define  LP876441		0x4
+#define  TPS65224		0x5
 
 /* I2C device address for pmic tps65941 */
 #define TPS65941_I2C_ADDR	(0x12 >> 1)
 #define TPS65941_LDO_NUM		4
 #define TPS65941_BUCK_NUM		5
+#define TPS65224_LDO_NUM		3
+#define TPS65224_BUCK_NUM		4
 
 /* Drivers name */
 #define TPS65941_LDO_DRIVER		"tps65941_ldo"
@@ -25,3 +28,30 @@
 #define TPS65941_LDO_MODE_MASK		0x1
 #define TPS65941_LDO_BYPASS_EN		0x80
 #define TP65941_BUCK_CONF_SLEW_MASK	0x7
+
+#define TPS65224_BUCK_VOLT_MAX		3300000
+#define TPS65224_BUCK1_VOLT_MAX_HEX      0xFD
+#define TPS65224_BUCK234_VOLT_MAX_HEX    0x45
+
+#define TPS65224_BUCK_CONF_SLEW_MASK     0x3
+#define TPS65224_LDO_VOLT_MASK    (0x3F << 1)
+
+#define TPS65224_LDO1_VOLT_MIN_HEX       0x0C
+#define TPS65224_LDO23_VOLT_MIN_HEX      0x00
+#define TPS65224_LDO1_VOLT_MAX_HEX       0x36
+#define TPS65224_LDO23_VOLT_MAX_HEX      0x38
+
+#define TPS65224_LDO1_VOLT_MAX        3300000
+#define TPS65224_LDO23_VOLT_MAX       3400000
+#define TPS65224_LDO1_VOLT_MIN        1200000
+#define TPS65224_LDO23_VOLT_MIN        600000
+
+#define TPS65224_LDO_STEP               50000
+
+#define TPS65224_LDO_BYP_CONFIG             7
+
+#define TPS65224_LDO1_VOLT_BYP_MIN    2200000
+#define TPS65224_LDO1_VOLT_BYP_MAX    3600000
+
+#define TPS65224_LDO23_VOLT_BYP_MIN   1500000
+#define TPS65224_LDO23_VOLT_BYP_MAX   5500000
-- 
2.25.1

[RESEND PATCH v3 2/5] power: pmic: tps65941: Add TI TPS65224 PMIC

[Bhargav Raviprakash]

Adds compatible and data field values of TPS65224 driver in
TPS65941 PMIC driver.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/power/pmic/tps65941.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/power/pmic/tps65941.c b/drivers/power/pmic/tps65941.c
index 727b42747a..ef63eb733a 100644
--- a/drivers/power/pmic/tps65941.c
+++ b/drivers/power/pmic/tps65941.c
@@ -75,6 +75,7 @@ static const struct udevice_id tps65941_ids[] = {
 	{ .compatible = "ti,tps659412", .data = TPS659411 },
 	{ .compatible = "ti,tps659413", .data = TPS659413 },
 	{ .compatible = "ti,lp876441",  .data =  LP876441 },
+	{ .compatible = "ti,tps65224",  .data =  TPS65224 },
 	{ }
 };
 
-- 
2.25.1

[RESEND PATCH v3 3/5] power: regulator: tps65941: Added macros for BUCK ID

[Bhargav Raviprakash]

Adds macros for buck and ldo ids and switched to using switch
case instead of if else in probe functions. Helps in adding
support for TPS65224 PMIC.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/power/regulator/tps65941_regulator.c | 54 +++++++++++++++-----
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
index b041126775..cf54e30df5 100644
--- a/drivers/power/regulator/tps65941_regulator.c
+++ b/drivers/power/regulator/tps65941_regulator.c
@@ -16,6 +16,25 @@
 #include <power/regulator.h>
 #include <power/tps65941.h>
 
+/* Single Phase Buck IDs */
+#define TPS65941_BUCK_ID_1        1
+#define TPS65941_BUCK_ID_2        2
+#define TPS65941_BUCK_ID_3        3
+#define TPS65941_BUCK_ID_4        4
+#define TPS65941_BUCK_ID_5        5
+
+/* Multi Phase Buck IDs */
+#define TPS65941_BUCK_ID_12      12
+#define TPS65941_BUCK_ID_34      34
+#define TPS65941_BUCK_ID_123    123
+#define TPS65941_BUCK_ID_1234  1234
+
+/* LDO IDs */
+#define TPS65941_LDO_ID_1         1
+#define TPS65941_LDO_ID_2         2
+#define TPS65941_LDO_ID_3         3
+#define TPS65941_LDO_ID_4         4
+
 static const char tps65941_buck_ctrl[TPS65941_BUCK_NUM] = {0x4, 0x6, 0x8, 0xA,
 								0xC};
 static const char tps65941_buck_vout[TPS65941_BUCK_NUM] = {0xE, 0x10, 0x12,
@@ -270,10 +289,15 @@ static int tps65941_ldo_probe(struct udevice *dev)
 	uc_pdata->type = REGULATOR_TYPE_LDO;
 
 	idx = dev->driver_data;
-	if (idx == 1 || idx == 2 || idx == 3 || idx == 4) {
+	switch (idx) {
+	case TPS65941_LDO_ID_1:
+	case TPS65941_LDO_ID_2:
+	case TPS65941_LDO_ID_3:
+	case TPS65941_LDO_ID_4:
 		debug("Single phase regulator\n");
-	} else {
-		printf("Wrong ID for regulator\n");
+		break;
+	default:
+		pr_err("Wrong ID for regulator\n");
 		return -EINVAL;
 	}
 
@@ -292,18 +316,24 @@ static int tps65941_buck_probe(struct udevice *dev)
 	uc_pdata->type = REGULATOR_TYPE_BUCK;
 
 	idx = dev->driver_data;
-	if (idx == 1 || idx == 2 || idx == 3 || idx == 4 || idx == 5) {
+	switch (idx) {
+	case TPS65941_BUCK_ID_1:
+	case TPS65941_BUCK_ID_2:
+	case TPS65941_BUCK_ID_3:
+	case TPS65941_BUCK_ID_4:
+	case TPS65941_BUCK_ID_5:
 		debug("Single phase regulator\n");
-	} else if (idx == 12) {
+		break;
+	case TPS65941_BUCK_ID_12:
+	case TPS65941_BUCK_ID_123:
+	case TPS65941_BUCK_ID_1234:
 		idx = 1;
-	} else if (idx == 34) {
+		break;
+	case TPS65941_BUCK_ID_34:
 		idx = 3;
-	} else if (idx == 123) {
-		idx = 1;
-	} else if (idx == 1234) {
-		idx = 1;
-	} else {
-		printf("Wrong ID for regulator\n");
+		break;
+	default:
+		pr_err("Wrong ID for regulator\n");
 		return -EINVAL;
 	}
 
-- 
2.25.1

[RESEND PATCH v3 4/5] power: regulator: tps65941: use function callbacks for conversion ops

[Bhargav Raviprakash]

Use function callbacks for volt2val, val2volt and slewrate lookups.
This makes it easier to add support for TPS65224 PMIC regulators.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
---
 drivers/power/regulator/tps65941_regulator.c | 61 +++++++++++++++-----
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
index cf54e30df5..d879c2301b 100644
--- a/drivers/power/regulator/tps65941_regulator.c
+++ b/drivers/power/regulator/tps65941_regulator.c
@@ -35,6 +35,17 @@
 #define TPS65941_LDO_ID_3         3
 #define TPS65941_LDO_ID_4         4
 
+#define TPS65941_BUCK_CONV_OPS_IDX  0
+#define TPS65941_LDO_CONV_OPS_IDX   0
+
+struct tps65941_reg_conv_ops {
+	int volt_mask;
+	int (*volt2val)(int idx, int uV);
+	int (*val2volt)(int idx, int volt);
+	int slew_mask;
+	int (*lookup_slew)(int id);
+};
+
 static const char tps65941_buck_ctrl[TPS65941_BUCK_NUM] = {0x4, 0x6, 0x8, 0xA,
 								0xC};
 static const char tps65941_buck_vout[TPS65941_BUCK_NUM] = {0xE, 0x10, 0x12,
@@ -79,7 +90,7 @@ static int tps65941_buck_enable(struct udevice *dev, int op, bool *enable)
 	return 0;
 }
 
-static int tps65941_buck_volt2val(int uV)
+static int tps65941_buck_volt2val(__maybe_unused int idx, int uV)
 {
 	if (uV > TPS65941_BUCK_VOLT_MAX)
 		return -EINVAL;
@@ -95,7 +106,7 @@ static int tps65941_buck_volt2val(int uV)
 		return -EINVAL;
 }
 
-static int tps65941_buck_val2volt(int val)
+static int tps65941_buck_val2volt(__maybe_unused int idx, int val)
 {
 	if (val > TPS65941_BUCK_VOLT_MAX_HEX)
 		return -EINVAL;
@@ -135,12 +146,25 @@ int tps65941_lookup_slew(int id)
 	}
 }
 
+static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
+	[TPS65941_BUCK_CONV_OPS_IDX] = {
+		.volt_mask = TPS65941_BUCK_VOLT_MASK,
+		.volt2val = tps65941_buck_volt2val,
+		.val2volt = tps65941_buck_val2volt,
+		.slew_mask = TP65941_BUCK_CONF_SLEW_MASK,
+		.lookup_slew = tps65941_lookup_slew,
+	},
+};
+
 static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
 {
 	unsigned int hex, adr;
-	int ret, delta, uwait, slew;
+	int ret, delta, uwait, slew, idx;
 	struct dm_regulator_uclass_plat *uc_pdata;
+	const struct tps65941_reg_conv_ops *conv_ops;
 
+	idx = dev->driver_data;
+	conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
 	uc_pdata = dev_get_uclass_plat(dev);
 
 	if (op == PMIC_OP_GET)
@@ -152,8 +176,8 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
 	if (ret < 0)
 		return ret;
 
-	ret &= TPS65941_BUCK_VOLT_MASK;
-	ret = tps65941_buck_val2volt(ret);
+	ret &= conv_ops->volt_mask;
+	ret = conv_ops->val2volt(idx, ret);
 	if (ret < 0)
 		return ret;
 
@@ -175,14 +199,14 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
 	if (slew < 0)
 		return ret;
 
-	slew &= TP65941_BUCK_CONF_SLEW_MASK;
-	slew = tps65941_lookup_slew(slew);
+	slew &= conv_ops->slew_mask;
+	slew = conv_ops->lookup_slew(slew);
 	if (slew <= 0)
 		return ret;
 
 	uwait = delta / slew;
 
-	hex = tps65941_buck_volt2val(*uV);
+	hex = conv_ops->volt2val(idx, *uV);
 	if (hex < 0)
 		return hex;
 
@@ -231,7 +255,7 @@ static int tps65941_ldo_enable(struct udevice *dev, int op, bool *enable)
 	return 0;
 }
 
-static int tps65941_ldo_val2volt(int val)
+static int tps65941_ldo_val2volt(__maybe_unused int idx, int val)
 {
 	if (val > TPS65941_LDO_VOLT_MAX_HEX || val < TPS65941_LDO_VOLT_MIN_HEX)
 		return -EINVAL;
@@ -241,12 +265,23 @@ static int tps65941_ldo_val2volt(int val)
 		return -EINVAL;
 }
 
+static const struct tps65941_reg_conv_ops ldo_conv_ops[] = {
+	[TPS65941_LDO_CONV_OPS_IDX] = {
+		.volt_mask = TPS65941_LDO_VOLT_MASK,
+		.volt2val = tps65941_buck_volt2val,
+		.val2volt = tps65941_ldo_val2volt,
+	},
+};
+
 static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
 {
 	unsigned int hex, adr;
-	int ret;
+	int ret, idx;
 	struct dm_regulator_uclass_plat *uc_pdata;
+	const struct tps65941_reg_conv_ops *conv_ops;
 
+	idx = dev->driver_data;
+	conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
 	uc_pdata = dev_get_uclass_plat(dev);
 
 	if (op == PMIC_OP_GET)
@@ -258,8 +293,8 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
 	if (ret < 0)
 		return ret;
 
-	ret &= TPS65941_LDO_VOLT_MASK;
-	ret = tps65941_ldo_val2volt(ret);
+	ret &= conv_ops->volt_mask;
+	ret = conv_ops->val2volt(idx, ret);
 	if (ret < 0)
 		return ret;
 
@@ -268,7 +303,7 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
 		return 0;
 	}
 
-	hex = tps65941_buck_volt2val(*uV);
+	hex = conv_ops->volt2val(idx, *uV);
 	if (hex < 0)
 		return hex;
 
-- 
2.25.1

[RESEND PATCH v3 5/5] power: regulator: tps65941: Add TPS65224 PMIC regulator support

[Bhargav Raviprakash]

Reuse TPS65941 regulator driver to adds support for
TPS65224 PMIC's regulators. 4 BUCKs and 3 LDOs, where
BUCK1 and BUCK2 can be configured in dual phase mode.

Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
---
 drivers/power/regulator/tps65941_regulator.c | 283 ++++++++++++++++++-
 1 file changed, 270 insertions(+), 13 deletions(-)

diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
index d879c2301b..b1b9462fd3 100644
--- a/drivers/power/regulator/tps65941_regulator.c
+++ b/drivers/power/regulator/tps65941_regulator.c
@@ -37,6 +37,8 @@
 
 #define TPS65941_BUCK_CONV_OPS_IDX  0
 #define TPS65941_LDO_CONV_OPS_IDX   0
+#define TPS65224_LDO_CONV_OPS_IDX   1
+#define TPS65224_BUCK_CONV_OPS_IDX  1
 
 struct tps65941_reg_conv_ops {
 	int volt_mask;
@@ -55,6 +57,11 @@ static const char tps65941_ldo_ctrl[TPS65941_BUCK_NUM] = {0x1D, 0x1E, 0x1F,
 static const char tps65941_ldo_vout[TPS65941_BUCK_NUM] = {0x23, 0x24, 0x25,
 								0x26};
 
+static inline int tps65941_get_chip_id(struct udevice *dev)
+{
+	return dev->parent->driver_data;
+}
+
 static int tps65941_buck_enable(struct udevice *dev, int op, bool *enable)
 {
 	int ret;
@@ -146,6 +153,112 @@ int tps65941_lookup_slew(int id)
 	}
 }
 
+static int tps65224_buck_volt2val(int idx, int uV)
+{
+	/* This functions maps a value which is in micro Volts to the VSET value.
+	 * The mapping is as per the datasheet of TPS65224.
+	 */
+
+	if (uV > TPS65224_BUCK_VOLT_MAX)
+		return -EINVAL;
+
+	if (idx > 0) {
+		/* Buck2, Buck3 and Buck4 of TPS65224 has a different schema in
+		 * converting b/w micro_volt and VSET hex values
+		 *
+		 * VSET value starts from 0x00 for 0.5V, and for every increment
+		 * in VSET value the output voltage increases by 25mV. This is upto
+		 * 1.15V where VSET is 0x1A.
+		 *
+		 * For 0x1B the output voltage is 1.2V, and for every increment of
+		 * VSET the output voltage increases by 50mV upto the max voltage of
+		 * 3.3V
+		 *
+		 * | Voltage Ranges  | VSET Ranges  | Voltage Step |
+		 * +-----------------+--------------+--------------+
+		 * | 0.5V to 1.50V   | 0x00 to 0x1A |  25mV        |
+		 * | 1.2V to 3.3V    | 0x1B to 0x45 |  50mV        |
+		 */
+		if (uV >= 1200000)
+			return (uV - 1200000) / 50000 + 0x1B;
+		else if (uV >= 500000)
+			return (uV - 500000) / 25000;
+		else
+			return -EINVAL;
+	}
+
+	/* Buck1 and Buck12(dual phase) has a different mapping b/w output
+	 * voltage and VSET value.
+	 *
+	 * | Voltage Ranges  | VSET Ranges  | Voltage Step |
+	 * +-----------------+--------------+--------------+
+	 * | 0.5V to 0.58V   | 0xA to 0xE   |  20mV        |
+	 * | 0.6V to 1.095V  | 0xF to 0x72  |  5mV         |
+	 * | 1.1V to 1.65V   | 0x73 to 0xAA |  10mV        |
+	 * | 1.6V to 3.3V    | 0xAB to 0xFD |  20mV        |
+	 *
+	 */
+	if (uV >= 1660000)
+		return (uV - 1660000) / 20000 + 0xAB;
+	else if (uV >= 1100000)
+		return (uV - 1100000) / 10000 + 0x73;
+	else if (uV >= 600000)
+		return (uV - 600000) / 5000 + 0x0F;
+	else if (uV >= 500000)
+		return (uV - 500000) / 20000 + 0x0A;
+	else
+		return -EINVAL;
+}
+
+static int tps65224_buck_val2volt(int idx, int val)
+{
+	/* This function does the opposite to the tps65224_buck_volt2val function
+	 * described above.
+	 * This maps the VSET value to micro volts. Please refer to the ranges
+	 * mentioned the comments of tps65224_buck_volt2val.
+	 */
+
+	if (idx > 0) {
+		if (val > TPS65224_BUCK234_VOLT_MAX_HEX)
+			return -EINVAL;
+		else if (val >= 0x1B)
+			return 1200000 + (val - 0x1B) * 50000;
+		else if (val >= 0x00)
+			return 500000 + (val - 0x00) * 25000;
+		else
+			return -EINVAL;
+	}
+
+	if (val > TPS65224_BUCK1_VOLT_MAX_HEX)
+		return -EINVAL;
+	else if (val >= 0xAB)
+		return 1660000 + (val - 0xAB) * 20000;
+	else if (val >= 0x73)
+		return 1100000 + (val - 0x73) * 10000;
+	else if (val >= 0xF)
+		return 600000 + (val - 0xF) * 5000;
+	else if (val >= 0xA)
+		return 500000 + (val - 0xA) * 20000;
+	else
+		return -EINVAL;
+}
+
+int tps65224_lookup_slew(int id)
+{
+	switch (id) {
+	case 0:
+		return 10000;
+	case 1:
+		return 5000;
+	case 2:
+		return 2500;
+	case 3:
+		return 1250;
+	default:
+		return -1;
+	}
+}
+
 static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
 	[TPS65941_BUCK_CONV_OPS_IDX] = {
 		.volt_mask = TPS65941_BUCK_VOLT_MASK,
@@ -154,6 +267,13 @@ static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
 		.slew_mask = TP65941_BUCK_CONF_SLEW_MASK,
 		.lookup_slew = tps65941_lookup_slew,
 	},
+	[TPS65224_BUCK_CONV_OPS_IDX] = {
+		.volt_mask = TPS65941_BUCK_VOLT_MASK,
+		.volt2val = tps65224_buck_volt2val,
+		.val2volt = tps65224_buck_val2volt,
+		.slew_mask = TPS65224_BUCK_CONF_SLEW_MASK,
+		.lookup_slew = tps65224_lookup_slew,
+	},
 };
 
 static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
@@ -162,9 +282,23 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
 	int ret, delta, uwait, slew, idx;
 	struct dm_regulator_uclass_plat *uc_pdata;
 	const struct tps65941_reg_conv_ops *conv_ops;
+	ulong chip_id;
 
 	idx = dev->driver_data;
-	conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
+	chip_id = tps65941_get_chip_id(dev);
+	if (chip_id == TPS65224) {
+		/* idx is the buck id number as per devicetree node which will be same
+		 * as the regulator name in the datasheet.
+		 * The idx for buck1. buck2, buck3, buck4, buck12 will be 1, 2, 3, 4
+		 * and 12 respectively.
+		 * In the driver the numbering is from 0. Hence the -1.
+		 */
+		idx = (idx == TPS65941_BUCK_ID_12) ? 0 : (idx - 1);
+		conv_ops = &buck_conv_ops[TPS65224_BUCK_CONV_OPS_IDX];
+	} else {
+		conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
+	}
+
 	uc_pdata = dev_get_uclass_plat(dev);
 
 	if (op == PMIC_OP_GET)
@@ -265,23 +399,99 @@ static int tps65941_ldo_val2volt(__maybe_unused int idx, int val)
 		return -EINVAL;
 }
 
+static int tps65224_ldo_volt2val(int idx, int uV)
+{
+	int base = TPS65224_LDO1_VOLT_MIN;
+	int max = TPS65224_LDO1_VOLT_MAX;
+	int offset = TPS65224_LDO1_VOLT_MIN_HEX;
+	int step = TPS65224_LDO_STEP;
+
+	if (idx > 0) {
+		base = TPS65224_LDO23_VOLT_MIN;
+		max = TPS65224_LDO23_VOLT_MAX;
+		offset = TPS65224_LDO23_VOLT_MIN_HEX;
+	}
+
+	if (uV > max)
+		return -EINVAL;
+	else if (uV >= base)
+		return (uV - base) / step + offset;
+	else
+		return -EINVAL;
+}
+
+static int tps65224_ldo_val2volt(int idx, int val)
+{
+	int reg_base = TPS65224_LDO1_VOLT_MIN_HEX;
+	int reg_max = TPS65224_LDO1_VOLT_MAX_HEX;
+	int base = TPS65224_LDO1_VOLT_MIN;
+	int max = TPS65224_LDO1_VOLT_MAX;
+	int step = TPS65224_LDO_STEP;
+	/* In LDOx_VOUT reg the BIT0 is reserved and the
+	 * vout value is stored from BIT1 to BIT7.
+	 * Hence the below bit shit is done.
+	 */
+	int mask = TPS65224_LDO_VOLT_MASK >> 1;
+
+	if (idx > 0) {
+		base = TPS65224_LDO23_VOLT_MIN;
+		max = TPS65224_LDO23_VOLT_MAX;
+		reg_base = TPS65224_LDO23_VOLT_MIN_HEX;
+		reg_max = TPS65224_LDO23_VOLT_MAX_HEX;
+	}
+
+	/* The VSET register of LDO has its 0th bit as reserved
+	 * hence shifting the value to right by 1 bit.
+	 */
+	val = val >> 1;
+
+	if (val < 0 || val > mask)
+		return -EINVAL;
+
+	if (val <= reg_base)
+		return base;
+
+	if (val >= reg_max)
+		return max;
+
+	return base + (step * (val - reg_base));
+}
+
 static const struct tps65941_reg_conv_ops ldo_conv_ops[] = {
 	[TPS65941_LDO_CONV_OPS_IDX] = {
 		.volt_mask = TPS65941_LDO_VOLT_MASK,
 		.volt2val = tps65941_buck_volt2val,
 		.val2volt = tps65941_ldo_val2volt,
 	},
+	[TPS65224_LDO_CONV_OPS_IDX] = {
+		.volt_mask = TPS65224_LDO_VOLT_MASK,
+		.volt2val = tps65224_ldo_volt2val,
+		.val2volt = tps65224_ldo_val2volt,
+	},
 };
 
 static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
 {
 	unsigned int hex, adr;
-	int ret, idx;
+	int ret, ret_volt, idx;
 	struct dm_regulator_uclass_plat *uc_pdata;
 	const struct tps65941_reg_conv_ops *conv_ops;
+	ulong chip_id;
 
+	chip_id = tps65941_get_chip_id(dev);
 	idx = dev->driver_data;
-	conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
+	if (chip_id == TPS65224) {
+		/* idx is the ldo id number as per devicetree node which will be same
+		 * as the regulator name in the datasheet.
+		 * The idx for ldo1, ldo2, ldo3 will be 1, 2 & 3 respectively.
+		 * In the driver the numbering is from 0. Hence the -1.
+		 */
+		idx = idx - 1;
+		conv_ops = &ldo_conv_ops[TPS65224_LDO_CONV_OPS_IDX];
+	} else {
+		conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
+	}
+
 	uc_pdata = dev_get_uclass_plat(dev);
 
 	if (op == PMIC_OP_GET)
@@ -294,21 +504,36 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
 		return ret;
 
 	ret &= conv_ops->volt_mask;
-	ret = conv_ops->val2volt(idx, ret);
-	if (ret < 0)
-		return ret;
+	ret_volt = conv_ops->val2volt(idx, ret);
+	if (ret_volt < 0)
+		return ret_volt;
 
 	if (op == PMIC_OP_GET) {
-		*uV = ret;
+		*uV = ret_volt;
 		return 0;
 	}
 
+	/* TPS65224 LDO1 in BYPASS mode only supports 2.2V min to 3.6V max */
+	if (chip_id == TPS65224 && idx == 0 && (ret & BIT(TPS65224_LDO_BYP_CONFIG)) &&
+	    *uV < TPS65224_LDO1_VOLT_BYP_MIN)
+		return -EINVAL;
+
+	/* TPS65224 LDO2 & LDO3 in BYPASS mode supports 1.5V min to 5.5V max */
+	if (chip_id == TPS65224 && idx > 0 && (ret & BIT(TPS65224_LDO_BYP_CONFIG)) &&
+	    *uV < TPS65224_LDO23_VOLT_BYP_MIN)
+		return -EINVAL;
+
 	hex = conv_ops->volt2val(idx, *uV);
 	if (hex < 0)
 		return hex;
 
-	ret &= 0x0;
-	ret = hex;
+	if (chip_id == TPS65224) {
+		hex = hex << TPS65941_LDO_MODE_MASK;
+		ret &= ~TPS65224_LDO_VOLT_MASK;
+		ret |= hex;
+	} else {
+		ret = hex;
+	}
 
 	ret = pmic_reg_write(dev->parent, adr, ret);
 
@@ -319,6 +544,9 @@ static int tps65941_ldo_probe(struct udevice *dev)
 {
 	struct dm_regulator_uclass_plat *uc_pdata;
 	int idx;
+	ulong chip_id;
+
+	chip_id = tps65941_get_chip_id(dev);
 
 	uc_pdata = dev_get_uclass_plat(dev);
 	uc_pdata->type = REGULATOR_TYPE_LDO;
@@ -328,9 +556,16 @@ static int tps65941_ldo_probe(struct udevice *dev)
 	case TPS65941_LDO_ID_1:
 	case TPS65941_LDO_ID_2:
 	case TPS65941_LDO_ID_3:
-	case TPS65941_LDO_ID_4:
 		debug("Single phase regulator\n");
 		break;
+	case TPS65941_LDO_ID_4:
+		if (chip_id != TPS65224) {
+			debug("Single phase regulator\n");
+		} else {
+			pr_err("Wrong ID for regulator\n");
+			return -EINVAL;
+		}
+		break;
 	default:
 		pr_err("Wrong ID for regulator\n");
 		return -EINVAL;
@@ -346,6 +581,9 @@ static int tps65941_buck_probe(struct udevice *dev)
 {
 	struct dm_regulator_uclass_plat *uc_pdata;
 	int idx;
+	ulong chip_id;
+
+	chip_id = tps65941_get_chip_id(dev);
 
 	uc_pdata = dev_get_uclass_plat(dev);
 	uc_pdata->type = REGULATOR_TYPE_BUCK;
@@ -356,16 +594,35 @@ static int tps65941_buck_probe(struct udevice *dev)
 	case TPS65941_BUCK_ID_2:
 	case TPS65941_BUCK_ID_3:
 	case TPS65941_BUCK_ID_4:
-	case TPS65941_BUCK_ID_5:
 		debug("Single phase regulator\n");
 		break;
+	case TPS65941_BUCK_ID_5:
+		if (chip_id != TPS65224) {
+			debug("Single phase regulator\n");
+		} else {
+			pr_err("Wrong ID for regulator\n");
+			return -EINVAL;
+		}
+		break;
 	case TPS65941_BUCK_ID_12:
+		idx = 1;
+		break;
 	case TPS65941_BUCK_ID_123:
 	case TPS65941_BUCK_ID_1234:
-		idx = 1;
+		if (chip_id != TPS65224) {
+			idx = 1;
+		} else {
+			pr_err("Wrong ID for regulator\n");
+			return -EINVAL;
+		}
 		break;
 	case TPS65941_BUCK_ID_34:
-		idx = 3;
+		if (chip_id != TPS65224) {
+			idx = 3;
+		} else {
+			pr_err("Wrong ID for regulator\n");
+			return -EINVAL;
+		}
 		break;
 	default:
 		pr_err("Wrong ID for regulator\n");
-- 
2.25.1

Re: [RESEND PATCH v3 1/5] power: tps65941: Add macros of TPS65224 PMIC

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Re-use the TPS65941 PMIC driver for TPS65224 PMIC.
> Add additional macros of TPS65224 to aid in the driver
> re-use.
> 
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> Reviewed-by: Dhruva Gole <d-gole@ti.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  include/power/tps65941.h | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/include/power/tps65941.h b/include/power/tps65941.h
> index a2bc6814ba..cec85333f0 100644
> --- a/include/power/tps65941.h
> +++ b/include/power/tps65941.h
> @@ -3,11 +3,14 @@
>  #define TPS659413		0x2
>  #define TPS659414		0x3
>  #define  LP876441		0x4
> +#define  TPS65224		0x5
>  
>  /* I2C device address for pmic tps65941 */
>  #define TPS65941_I2C_ADDR	(0x12 >> 1)
>  #define TPS65941_LDO_NUM		4
>  #define TPS65941_BUCK_NUM		5
> +#define TPS65224_LDO_NUM		3
> +#define TPS65224_BUCK_NUM		4
>  
>  /* Drivers name */
>  #define TPS65941_LDO_DRIVER		"tps65941_ldo"
> @@ -25,3 +28,30 @@
>  #define TPS65941_LDO_MODE_MASK		0x1
>  #define TPS65941_LDO_BYPASS_EN		0x80
>  #define TP65941_BUCK_CONF_SLEW_MASK	0x7
> +
> +#define TPS65224_BUCK_VOLT_MAX		3300000
> +#define TPS65224_BUCK1_VOLT_MAX_HEX      0xFD
> +#define TPS65224_BUCK234_VOLT_MAX_HEX    0x45
> +
> +#define TPS65224_BUCK_CONF_SLEW_MASK     0x3
> +#define TPS65224_LDO_VOLT_MASK    (0x3F << 1)
> +
> +#define TPS65224_LDO1_VOLT_MIN_HEX       0x0C
> +#define TPS65224_LDO23_VOLT_MIN_HEX      0x00
> +#define TPS65224_LDO1_VOLT_MAX_HEX       0x36
> +#define TPS65224_LDO23_VOLT_MAX_HEX      0x38
> +
> +#define TPS65224_LDO1_VOLT_MAX        3300000
> +#define TPS65224_LDO23_VOLT_MAX       3400000
> +#define TPS65224_LDO1_VOLT_MIN        1200000
> +#define TPS65224_LDO23_VOLT_MIN        600000
> +
> +#define TPS65224_LDO_STEP               50000
> +
> +#define TPS65224_LDO_BYP_CONFIG             7
> +
> +#define TPS65224_LDO1_VOLT_BYP_MIN    2200000
> +#define TPS65224_LDO1_VOLT_BYP_MAX    3600000
> +
> +#define TPS65224_LDO23_VOLT_BYP_MIN   1500000
> +#define TPS65224_LDO23_VOLT_BYP_MAX   5500000

Re: [RESEND PATCH v3 2/5] power: pmic: tps65941: Add TI TPS65224 PMIC

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Adds compatible and data field values of TPS65224 driver in
> TPS65941 PMIC driver.
> 
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> Reviewed-by: Dhruva Gole <d-gole@ti.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/pmic/tps65941.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/power/pmic/tps65941.c b/drivers/power/pmic/tps65941.c
> index 727b42747a..ef63eb733a 100644
> --- a/drivers/power/pmic/tps65941.c
> +++ b/drivers/power/pmic/tps65941.c
> @@ -75,6 +75,7 @@ static const struct udevice_id tps65941_ids[] = {
>  	{ .compatible = "ti,tps659412", .data = TPS659411 },
>  	{ .compatible = "ti,tps659413", .data = TPS659413 },
>  	{ .compatible = "ti,lp876441",  .data =  LP876441 },
> +	{ .compatible = "ti,tps65224",  .data =  TPS65224 },
>  	{ }
>  };
>  

Re: [RESEND PATCH v3 4/5] power: regulator: tps65941: use function callbacks for conversion ops

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Use function callbacks for volt2val, val2volt and slewrate lookups.
> This makes it easier to add support for TPS65224 PMIC regulators.
> 
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/regulator/tps65941_regulator.c | 61 +++++++++++++++-----
>  1 file changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
> index cf54e30df5..d879c2301b 100644
> --- a/drivers/power/regulator/tps65941_regulator.c
> +++ b/drivers/power/regulator/tps65941_regulator.c
> @@ -35,6 +35,17 @@
>  #define TPS65941_LDO_ID_3         3
>  #define TPS65941_LDO_ID_4         4
>  
> +#define TPS65941_BUCK_CONV_OPS_IDX  0
> +#define TPS65941_LDO_CONV_OPS_IDX   0
> +
> +struct tps65941_reg_conv_ops {
> +	int volt_mask;
> +	int (*volt2val)(int idx, int uV);
> +	int (*val2volt)(int idx, int volt);
> +	int slew_mask;
> +	int (*lookup_slew)(int id);
> +};
> +
>  static const char tps65941_buck_ctrl[TPS65941_BUCK_NUM] = {0x4, 0x6, 0x8, 0xA,
>  								0xC};
>  static const char tps65941_buck_vout[TPS65941_BUCK_NUM] = {0xE, 0x10, 0x12,
> @@ -79,7 +90,7 @@ static int tps65941_buck_enable(struct udevice *dev, int op, bool *enable)
>  	return 0;
>  }
>  
> -static int tps65941_buck_volt2val(int uV)
> +static int tps65941_buck_volt2val(__maybe_unused int idx, int uV)
>  {
>  	if (uV > TPS65941_BUCK_VOLT_MAX)
>  		return -EINVAL;
> @@ -95,7 +106,7 @@ static int tps65941_buck_volt2val(int uV)
>  		return -EINVAL;
>  }
>  
> -static int tps65941_buck_val2volt(int val)
> +static int tps65941_buck_val2volt(__maybe_unused int idx, int val)
>  {
>  	if (val > TPS65941_BUCK_VOLT_MAX_HEX)
>  		return -EINVAL;
> @@ -135,12 +146,25 @@ int tps65941_lookup_slew(int id)
>  	}
>  }
>  
> +static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
> +	[TPS65941_BUCK_CONV_OPS_IDX] = {
> +		.volt_mask = TPS65941_BUCK_VOLT_MASK,
> +		.volt2val = tps65941_buck_volt2val,
> +		.val2volt = tps65941_buck_val2volt,
> +		.slew_mask = TP65941_BUCK_CONF_SLEW_MASK,
> +		.lookup_slew = tps65941_lookup_slew,
> +	},
> +};
> +
>  static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
>  {
>  	unsigned int hex, adr;
> -	int ret, delta, uwait, slew;
> +	int ret, delta, uwait, slew, idx;
>  	struct dm_regulator_uclass_plat *uc_pdata;
> +	const struct tps65941_reg_conv_ops *conv_ops;
>  
> +	idx = dev->driver_data;
> +	conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
>  	uc_pdata = dev_get_uclass_plat(dev);
>  
>  	if (op == PMIC_OP_GET)
> @@ -152,8 +176,8 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret &= TPS65941_BUCK_VOLT_MASK;
> -	ret = tps65941_buck_val2volt(ret);
> +	ret &= conv_ops->volt_mask;
> +	ret = conv_ops->val2volt(idx, ret);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -175,14 +199,14 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
>  	if (slew < 0)
>  		return ret;
>  
> -	slew &= TP65941_BUCK_CONF_SLEW_MASK;
> -	slew = tps65941_lookup_slew(slew);
> +	slew &= conv_ops->slew_mask;
> +	slew = conv_ops->lookup_slew(slew);
>  	if (slew <= 0)
>  		return ret;
>  
>  	uwait = delta / slew;
>  
> -	hex = tps65941_buck_volt2val(*uV);
> +	hex = conv_ops->volt2val(idx, *uV);
>  	if (hex < 0)
>  		return hex;
>  
> @@ -231,7 +255,7 @@ static int tps65941_ldo_enable(struct udevice *dev, int op, bool *enable)
>  	return 0;
>  }
>  
> -static int tps65941_ldo_val2volt(int val)
> +static int tps65941_ldo_val2volt(__maybe_unused int idx, int val)
>  {
>  	if (val > TPS65941_LDO_VOLT_MAX_HEX || val < TPS65941_LDO_VOLT_MIN_HEX)
>  		return -EINVAL;
> @@ -241,12 +265,23 @@ static int tps65941_ldo_val2volt(int val)
>  		return -EINVAL;
>  }
>  
> +static const struct tps65941_reg_conv_ops ldo_conv_ops[] = {
> +	[TPS65941_LDO_CONV_OPS_IDX] = {
> +		.volt_mask = TPS65941_LDO_VOLT_MASK,
> +		.volt2val = tps65941_buck_volt2val,
> +		.val2volt = tps65941_ldo_val2volt,
> +	},
> +};
> +
>  static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
>  {
>  	unsigned int hex, adr;
> -	int ret;
> +	int ret, idx;
>  	struct dm_regulator_uclass_plat *uc_pdata;
> +	const struct tps65941_reg_conv_ops *conv_ops;
>  
> +	idx = dev->driver_data;
> +	conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
>  	uc_pdata = dev_get_uclass_plat(dev);
>  
>  	if (op == PMIC_OP_GET)
> @@ -258,8 +293,8 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret &= TPS65941_LDO_VOLT_MASK;
> -	ret = tps65941_ldo_val2volt(ret);
> +	ret &= conv_ops->volt_mask;
> +	ret = conv_ops->val2volt(idx, ret);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -268,7 +303,7 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
>  		return 0;
>  	}
>  
> -	hex = tps65941_buck_volt2val(*uV);
> +	hex = conv_ops->volt2val(idx, *uV);
>  	if (hex < 0)
>  		return hex;
>  

Re: [RESEND PATCH v3 3/5] power: regulator: tps65941: Added macros for BUCK ID

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Adds macros for buck and ldo ids and switched to using switch
> case instead of if else in probe functions. Helps in adding
> support for TPS65224 PMIC.
> 
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> Reviewed-by: Dhruva Gole <d-gole@ti.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/regulator/tps65941_regulator.c | 54 +++++++++++++++-----
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
> index b041126775..cf54e30df5 100644
> --- a/drivers/power/regulator/tps65941_regulator.c
> +++ b/drivers/power/regulator/tps65941_regulator.c
> @@ -16,6 +16,25 @@
>  #include <power/regulator.h>
>  #include <power/tps65941.h>
>  
> +/* Single Phase Buck IDs */
> +#define TPS65941_BUCK_ID_1        1
> +#define TPS65941_BUCK_ID_2        2
> +#define TPS65941_BUCK_ID_3        3
> +#define TPS65941_BUCK_ID_4        4
> +#define TPS65941_BUCK_ID_5        5
> +
> +/* Multi Phase Buck IDs */
> +#define TPS65941_BUCK_ID_12      12
> +#define TPS65941_BUCK_ID_34      34
> +#define TPS65941_BUCK_ID_123    123
> +#define TPS65941_BUCK_ID_1234  1234
> +
> +/* LDO IDs */
> +#define TPS65941_LDO_ID_1         1
> +#define TPS65941_LDO_ID_2         2
> +#define TPS65941_LDO_ID_3         3
> +#define TPS65941_LDO_ID_4         4
> +
>  static const char tps65941_buck_ctrl[TPS65941_BUCK_NUM] = {0x4, 0x6, 0x8, 0xA,
>  								0xC};
>  static const char tps65941_buck_vout[TPS65941_BUCK_NUM] = {0xE, 0x10, 0x12,
> @@ -270,10 +289,15 @@ static int tps65941_ldo_probe(struct udevice *dev)
>  	uc_pdata->type = REGULATOR_TYPE_LDO;
>  
>  	idx = dev->driver_data;
> -	if (idx == 1 || idx == 2 || idx == 3 || idx == 4) {
> +	switch (idx) {
> +	case TPS65941_LDO_ID_1:
> +	case TPS65941_LDO_ID_2:
> +	case TPS65941_LDO_ID_3:
> +	case TPS65941_LDO_ID_4:
>  		debug("Single phase regulator\n");
> -	} else {
> -		printf("Wrong ID for regulator\n");
> +		break;
> +	default:
> +		pr_err("Wrong ID for regulator\n");
>  		return -EINVAL;
>  	}
>  
> @@ -292,18 +316,24 @@ static int tps65941_buck_probe(struct udevice *dev)
>  	uc_pdata->type = REGULATOR_TYPE_BUCK;
>  
>  	idx = dev->driver_data;
> -	if (idx == 1 || idx == 2 || idx == 3 || idx == 4 || idx == 5) {
> +	switch (idx) {
> +	case TPS65941_BUCK_ID_1:
> +	case TPS65941_BUCK_ID_2:
> +	case TPS65941_BUCK_ID_3:
> +	case TPS65941_BUCK_ID_4:
> +	case TPS65941_BUCK_ID_5:
>  		debug("Single phase regulator\n");
> -	} else if (idx == 12) {
> +		break;
> +	case TPS65941_BUCK_ID_12:
> +	case TPS65941_BUCK_ID_123:
> +	case TPS65941_BUCK_ID_1234:
>  		idx = 1;
> -	} else if (idx == 34) {
> +		break;
> +	case TPS65941_BUCK_ID_34:
>  		idx = 3;
> -	} else if (idx == 123) {
> -		idx = 1;
> -	} else if (idx == 1234) {
> -		idx = 1;
> -	} else {
> -		printf("Wrong ID for regulator\n");
> +		break;
> +	default:
> +		pr_err("Wrong ID for regulator\n");
>  		return -EINVAL;
>  	}
>  

Re: [RESEND PATCH v3 5/5] power: regulator: tps65941: Add TPS65224 PMIC regulator support

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Reuse TPS65941 regulator driver to adds support for
> TPS65224 PMIC's regulators. 4 BUCKs and 3 LDOs, where
> BUCK1 and BUCK2 can be configured in dual phase mode.
> 
> Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> ---
>  drivers/power/regulator/tps65941_regulator.c | 283 ++++++++++++++++++-
>  1 file changed, 270 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
> index d879c2301b..b1b9462fd3 100644
> --- a/drivers/power/regulator/tps65941_regulator.c
> +++ b/drivers/power/regulator/tps65941_regulator.c
> @@ -37,6 +37,8 @@
>  
>  #define TPS65941_BUCK_CONV_OPS_IDX  0
>  #define TPS65941_LDO_CONV_OPS_IDX   0
> +#define TPS65224_LDO_CONV_OPS_IDX   1
> +#define TPS65224_BUCK_CONV_OPS_IDX  1
>  
>  struct tps65941_reg_conv_ops {
>  	int volt_mask;
> @@ -55,6 +57,11 @@ static const char tps65941_ldo_ctrl[TPS65941_BUCK_NUM] = {0x1D, 0x1E, 0x1F,
>  static const char tps65941_ldo_vout[TPS65941_BUCK_NUM] = {0x23, 0x24, 0x25,
>  								0x26};
>  
> +static inline int tps65941_get_chip_id(struct udevice *dev)
> +{
> +	return dev->parent->driver_data;
> +}
> +
>  static int tps65941_buck_enable(struct udevice *dev, int op, bool *enable)
>  {
>  	int ret;
> @@ -146,6 +153,112 @@ int tps65941_lookup_slew(int id)
>  	}
>  }
>  
> +static int tps65224_buck_volt2val(int idx, int uV)
> +{
> +	/* This functions maps a value which is in micro Volts to the VSET value.
> +	 * The mapping is as per the datasheet of TPS65224.
> +	 */
> +
> +	if (uV > TPS65224_BUCK_VOLT_MAX)
> +		return -EINVAL;
> +
> +	if (idx > 0) {
> +		/* Buck2, Buck3 and Buck4 of TPS65224 has a different schema in
> +		 * converting b/w micro_volt and VSET hex values
> +		 *
> +		 * VSET value starts from 0x00 for 0.5V, and for every increment
> +		 * in VSET value the output voltage increases by 25mV. This is upto
> +		 * 1.15V where VSET is 0x1A.
> +		 *
> +		 * For 0x1B the output voltage is 1.2V, and for every increment of
> +		 * VSET the output voltage increases by 50mV upto the max voltage of
> +		 * 3.3V
> +		 *
> +		 * | Voltage Ranges  | VSET Ranges  | Voltage Step |
> +		 * +-----------------+--------------+--------------+
> +		 * | 0.5V to 1.50V   | 0x00 to 0x1A |  25mV        |
> +		 * | 1.2V to 3.3V    | 0x1B to 0x45 |  50mV        |
> +		 */
> +		if (uV >= 1200000)
> +			return (uV - 1200000) / 50000 + 0x1B;
> +		else if (uV >= 500000)
> +			return (uV - 500000) / 25000;
> +		else
> +			return -EINVAL;
> +	}
> +
> +	/* Buck1 and Buck12(dual phase) has a different mapping b/w output
> +	 * voltage and VSET value.
> +	 *
> +	 * | Voltage Ranges  | VSET Ranges  | Voltage Step |
> +	 * +-----------------+--------------+--------------+
> +	 * | 0.5V to 0.58V   | 0xA to 0xE   |  20mV        |
> +	 * | 0.6V to 1.095V  | 0xF to 0x72  |  5mV         |
> +	 * | 1.1V to 1.65V   | 0x73 to 0xAA |  10mV        |
> +	 * | 1.6V to 3.3V    | 0xAB to 0xFD |  20mV        |
> +	 *
> +	 */
> +	if (uV >= 1660000)
> +		return (uV - 1660000) / 20000 + 0xAB;
> +	else if (uV >= 1100000)
> +		return (uV - 1100000) / 10000 + 0x73;
> +	else if (uV >= 600000)
> +		return (uV - 600000) / 5000 + 0x0F;
> +	else if (uV >= 500000)
> +		return (uV - 500000) / 20000 + 0x0A;
> +	else
> +		return -EINVAL;
> +}
> +
> +static int tps65224_buck_val2volt(int idx, int val)
> +{
> +	/* This function does the opposite to the tps65224_buck_volt2val function
> +	 * described above.
> +	 * This maps the VSET value to micro volts. Please refer to the ranges
> +	 * mentioned the comments of tps65224_buck_volt2val.
> +	 */
> +
> +	if (idx > 0) {
> +		if (val > TPS65224_BUCK234_VOLT_MAX_HEX)
> +			return -EINVAL;
> +		else if (val >= 0x1B)
> +			return 1200000 + (val - 0x1B) * 50000;
> +		else if (val >= 0x00)
> +			return 500000 + (val - 0x00) * 25000;
> +		else
> +			return -EINVAL;
> +	}
> +
> +	if (val > TPS65224_BUCK1_VOLT_MAX_HEX)
> +		return -EINVAL;
> +	else if (val >= 0xAB)
> +		return 1660000 + (val - 0xAB) * 20000;
> +	else if (val >= 0x73)
> +		return 1100000 + (val - 0x73) * 10000;
> +	else if (val >= 0xF)
> +		return 600000 + (val - 0xF) * 5000;
> +	else if (val >= 0xA)
> +		return 500000 + (val - 0xA) * 20000;
> +	else
> +		return -EINVAL;
> +}
> +
> +int tps65224_lookup_slew(int id)
> +{
> +	switch (id) {
> +	case 0:
> +		return 10000;
> +	case 1:
> +		return 5000;
> +	case 2:
> +		return 2500;
> +	case 3:
> +		return 1250;
> +	default:
> +		return -1;
> +	}
> +}
> +
>  static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
>  	[TPS65941_BUCK_CONV_OPS_IDX] = {
>  		.volt_mask = TPS65941_BUCK_VOLT_MASK,
> @@ -154,6 +267,13 @@ static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
>  		.slew_mask = TP65941_BUCK_CONF_SLEW_MASK,
>  		.lookup_slew = tps65941_lookup_slew,
>  	},
> +	[TPS65224_BUCK_CONV_OPS_IDX] = {
> +		.volt_mask = TPS65941_BUCK_VOLT_MASK,
> +		.volt2val = tps65224_buck_volt2val,
> +		.val2volt = tps65224_buck_val2volt,
> +		.slew_mask = TPS65224_BUCK_CONF_SLEW_MASK,
> +		.lookup_slew = tps65224_lookup_slew,
> +	},
>  };
>  
>  static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
> @@ -162,9 +282,23 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
>  	int ret, delta, uwait, slew, idx;
>  	struct dm_regulator_uclass_plat *uc_pdata;
>  	const struct tps65941_reg_conv_ops *conv_ops;
> +	ulong chip_id;
>  
>  	idx = dev->driver_data;
> -	conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
> +	chip_id = tps65941_get_chip_id(dev);
> +	if (chip_id == TPS65224) {
> +		/* idx is the buck id number as per devicetree node which will be same
> +		 * as the regulator name in the datasheet.
> +		 * The idx for buck1. buck2, buck3, buck4, buck12 will be 1, 2, 3, 4
> +		 * and 12 respectively.
> +		 * In the driver the numbering is from 0. Hence the -1.
> +		 */
> +		idx = (idx == TPS65941_BUCK_ID_12) ? 0 : (idx - 1);
> +		conv_ops = &buck_conv_ops[TPS65224_BUCK_CONV_OPS_IDX];
> +	} else {
> +		conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
> +	}
> +
>  	uc_pdata = dev_get_uclass_plat(dev);
>  
>  	if (op == PMIC_OP_GET)
> @@ -265,23 +399,99 @@ static int tps65941_ldo_val2volt(__maybe_unused int idx, int val)
>  		return -EINVAL;
>  }
>  
> +static int tps65224_ldo_volt2val(int idx, int uV)
> +{
> +	int base = TPS65224_LDO1_VOLT_MIN;
> +	int max = TPS65224_LDO1_VOLT_MAX;
> +	int offset = TPS65224_LDO1_VOLT_MIN_HEX;
> +	int step = TPS65224_LDO_STEP;
> +
> +	if (idx > 0) {
> +		base = TPS65224_LDO23_VOLT_MIN;
> +		max = TPS65224_LDO23_VOLT_MAX;
> +		offset = TPS65224_LDO23_VOLT_MIN_HEX;
> +	}
> +
> +	if (uV > max)
> +		return -EINVAL;
> +	else if (uV >= base)
> +		return (uV - base) / step + offset;
> +	else
> +		return -EINVAL;
> +}
> +
> +static int tps65224_ldo_val2volt(int idx, int val)
> +{
> +	int reg_base = TPS65224_LDO1_VOLT_MIN_HEX;
> +	int reg_max = TPS65224_LDO1_VOLT_MAX_HEX;
> +	int base = TPS65224_LDO1_VOLT_MIN;
> +	int max = TPS65224_LDO1_VOLT_MAX;
> +	int step = TPS65224_LDO_STEP;
> +	/* In LDOx_VOUT reg the BIT0 is reserved and the
> +	 * vout value is stored from BIT1 to BIT7.
> +	 * Hence the below bit shit is done.
> +	 */
> +	int mask = TPS65224_LDO_VOLT_MASK >> 1;
> +
> +	if (idx > 0) {
> +		base = TPS65224_LDO23_VOLT_MIN;
> +		max = TPS65224_LDO23_VOLT_MAX;
> +		reg_base = TPS65224_LDO23_VOLT_MIN_HEX;
> +		reg_max = TPS65224_LDO23_VOLT_MAX_HEX;
> +	}
> +
> +	/* The VSET register of LDO has its 0th bit as reserved
> +	 * hence shifting the value to right by 1 bit.
> +	 */
> +	val = val >> 1;
> +
> +	if (val < 0 || val > mask)
> +		return -EINVAL;
> +
> +	if (val <= reg_base)
> +		return base;
> +
> +	if (val >= reg_max)
> +		return max;
> +
> +	return base + (step * (val - reg_base));
> +}
> +
>  static const struct tps65941_reg_conv_ops ldo_conv_ops[] = {
>  	[TPS65941_LDO_CONV_OPS_IDX] = {
>  		.volt_mask = TPS65941_LDO_VOLT_MASK,
>  		.volt2val = tps65941_buck_volt2val,
>  		.val2volt = tps65941_ldo_val2volt,
>  	},
> +	[TPS65224_LDO_CONV_OPS_IDX] = {
> +		.volt_mask = TPS65224_LDO_VOLT_MASK,
> +		.volt2val = tps65224_ldo_volt2val,
> +		.val2volt = tps65224_ldo_val2volt,
> +	},
>  };
>  
>  static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
>  {
>  	unsigned int hex, adr;
> -	int ret, idx;
> +	int ret, ret_volt, idx;
>  	struct dm_regulator_uclass_plat *uc_pdata;
>  	const struct tps65941_reg_conv_ops *conv_ops;
> +	ulong chip_id;
>  
> +	chip_id = tps65941_get_chip_id(dev);
>  	idx = dev->driver_data;
> -	conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
> +	if (chip_id == TPS65224) {
> +		/* idx is the ldo id number as per devicetree node which will be same
> +		 * as the regulator name in the datasheet.
> +		 * The idx for ldo1, ldo2, ldo3 will be 1, 2 & 3 respectively.
> +		 * In the driver the numbering is from 0. Hence the -1.
> +		 */
> +		idx = idx - 1;
> +		conv_ops = &ldo_conv_ops[TPS65224_LDO_CONV_OPS_IDX];
> +	} else {
> +		conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
> +	}
> +
>  	uc_pdata = dev_get_uclass_plat(dev);
>  
>  	if (op == PMIC_OP_GET)
> @@ -294,21 +504,36 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
>  		return ret;
>  
>  	ret &= conv_ops->volt_mask;
> -	ret = conv_ops->val2volt(idx, ret);
> -	if (ret < 0)
> -		return ret;
> +	ret_volt = conv_ops->val2volt(idx, ret);
> +	if (ret_volt < 0)
> +		return ret_volt;
>  
>  	if (op == PMIC_OP_GET) {
> -		*uV = ret;
> +		*uV = ret_volt;
>  		return 0;
>  	}
>  
> +	/* TPS65224 LDO1 in BYPASS mode only supports 2.2V min to 3.6V max */
> +	if (chip_id == TPS65224 && idx == 0 && (ret & BIT(TPS65224_LDO_BYP_CONFIG)) &&
> +	    *uV < TPS65224_LDO1_VOLT_BYP_MIN)
> +		return -EINVAL;
> +
> +	/* TPS65224 LDO2 & LDO3 in BYPASS mode supports 1.5V min to 5.5V max */
> +	if (chip_id == TPS65224 && idx > 0 && (ret & BIT(TPS65224_LDO_BYP_CONFIG)) &&
> +	    *uV < TPS65224_LDO23_VOLT_BYP_MIN)
> +		return -EINVAL;
> +
>  	hex = conv_ops->volt2val(idx, *uV);
>  	if (hex < 0)
>  		return hex;
>  
> -	ret &= 0x0;
> -	ret = hex;
> +	if (chip_id == TPS65224) {
> +		hex = hex << TPS65941_LDO_MODE_MASK;
> +		ret &= ~TPS65224_LDO_VOLT_MASK;
> +		ret |= hex;
> +	} else {
> +		ret = hex;
> +	}
>  
>  	ret = pmic_reg_write(dev->parent, adr, ret);
>  
> @@ -319,6 +544,9 @@ static int tps65941_ldo_probe(struct udevice *dev)
>  {
>  	struct dm_regulator_uclass_plat *uc_pdata;
>  	int idx;
> +	ulong chip_id;
> +
> +	chip_id = tps65941_get_chip_id(dev);
>  
>  	uc_pdata = dev_get_uclass_plat(dev);
>  	uc_pdata->type = REGULATOR_TYPE_LDO;
> @@ -328,9 +556,16 @@ static int tps65941_ldo_probe(struct udevice *dev)
>  	case TPS65941_LDO_ID_1:
>  	case TPS65941_LDO_ID_2:
>  	case TPS65941_LDO_ID_3:
> -	case TPS65941_LDO_ID_4:
>  		debug("Single phase regulator\n");
>  		break;
> +	case TPS65941_LDO_ID_4:
> +		if (chip_id != TPS65224) {
> +			debug("Single phase regulator\n");
> +		} else {
> +			pr_err("Wrong ID for regulator\n");
> +			return -EINVAL;
> +		}
> +		break;

Is it possible to use the below?

if (chip_id != TPS65224) {
	debug("Single phase regulator\n");
	break;
}

/* fallthrough */

Best Regards,
Jaehoon Chung

>  	default:
>  		pr_err("Wrong ID for regulator\n");
>  		return -EINVAL;
> @@ -346,6 +581,9 @@ static int tps65941_buck_probe(struct udevice *dev)
>  {
>  	struct dm_regulator_uclass_plat *uc_pdata;
>  	int idx;
> +	ulong chip_id;
> +
> +	chip_id = tps65941_get_chip_id(dev);
>  
>  	uc_pdata = dev_get_uclass_plat(dev);
>  	uc_pdata->type = REGULATOR_TYPE_BUCK;
> @@ -356,16 +594,35 @@ static int tps65941_buck_probe(struct udevice *dev)
>  	case TPS65941_BUCK_ID_2:
>  	case TPS65941_BUCK_ID_3:
>  	case TPS65941_BUCK_ID_4:
> -	case TPS65941_BUCK_ID_5:
>  		debug("Single phase regulator\n");
>  		break;
> +	case TPS65941_BUCK_ID_5:
> +		if (chip_id != TPS65224) {
> +			debug("Single phase regulator\n");
> +		} else {
> +			pr_err("Wrong ID for regulator\n");
> +			return -EINVAL;
> +		}
> +		break;
>  	case TPS65941_BUCK_ID_12:
> +		idx = 1;
> +		break;
>  	case TPS65941_BUCK_ID_123:
>  	case TPS65941_BUCK_ID_1234:
> -		idx = 1;
> +		if (chip_id != TPS65224) {
> +			idx = 1;
> +		} else {
> +			pr_err("Wrong ID for regulator\n");
> +			return -EINVAL;
> +		}
>  		break;
>  	case TPS65941_BUCK_ID_34:
> -		idx = 3;
> +		if (chip_id != TPS65224) {
> +			idx = 3;
> +		} else {
> +			pr_err("Wrong ID for regulator\n");
> +			return -EINVAL;
> +		}
>  		break;
>  	default:
>  		pr_err("Wrong ID for regulator\n");

Re: [RESEND PATCH v3 5/5] power: regulator: tps65941: Add TPS65224 PMIC regulator support

[Bhargav Raviprakash]

On Thu, 18 Apr 2024 09:33:30 +0900, Jaehoon Chung wrote:
> On 3/18/24 18:49, Bhargav Raviprakash wrote:
> > Reuse TPS65941 regulator driver to adds support for
> > TPS65224 PMIC's regulators. 4 BUCKs and 3 LDOs, where
> > BUCK1 and BUCK2 can be configured in dual phase mode.
> > 
> > Signed-off-by: Bhargav Raviprakash <bhargav.r@ltts.com>
> > ---
> >  drivers/power/regulator/tps65941_regulator.c | 283 ++++++++++++++++++-
> >  1 file changed, 270 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/power/regulator/tps65941_regulator.c
> > index d879c2301b..b1b9462fd3 100644
> > --- a/drivers/power/regulator/tps65941_regulator.c
> > +++ b/drivers/power/regulator/tps65941_regulator.c
> > @@ -37,6 +37,8 @@
> >  
> >  #define TPS65941_BUCK_CONV_OPS_IDX  0
> >  #define TPS65941_LDO_CONV_OPS_IDX   0
> > +#define TPS65224_LDO_CONV_OPS_IDX   1
> > +#define TPS65224_BUCK_CONV_OPS_IDX  1
> >  
> >  struct tps65941_reg_conv_ops {
> >  	int volt_mask;
> > @@ -55,6 +57,11 @@ static const char tps65941_ldo_ctrl[TPS65941_BUCK_NUM] = {0x1D, 0x1E, 0x1F,
> >  static const char tps65941_ldo_vout[TPS65941_BUCK_NUM] = {0x23, 0x24, 0x25,
> >  								0x26};
> >  
> > +static inline int tps65941_get_chip_id(struct udevice *dev)
> > +{
> > +	return dev->parent->driver_data;
> > +}
> > +
> >  static int tps65941_buck_enable(struct udevice *dev, int op, bool *enable)
> >  {
> >  	int ret;
> > @@ -146,6 +153,112 @@ int tps65941_lookup_slew(int id)
> >  	}
> >  }
> >  
> > +static int tps65224_buck_volt2val(int idx, int uV)
> > +{
> > +	/* This functions maps a value which is in micro Volts to the VSET value.
> > +	 * The mapping is as per the datasheet of TPS65224.
> > +	 */
> > +
> > +	if (uV > TPS65224_BUCK_VOLT_MAX)
> > +		return -EINVAL;
> > +
> > +	if (idx > 0) {
> > +		/* Buck2, Buck3 and Buck4 of TPS65224 has a different schema in
> > +		 * converting b/w micro_volt and VSET hex values
> > +		 *
> > +		 * VSET value starts from 0x00 for 0.5V, and for every increment
> > +		 * in VSET value the output voltage increases by 25mV. This is upto
> > +		 * 1.15V where VSET is 0x1A.
> > +		 *
> > +		 * For 0x1B the output voltage is 1.2V, and for every increment of
> > +		 * VSET the output voltage increases by 50mV upto the max voltage of
> > +		 * 3.3V
> > +		 *
> > +		 * | Voltage Ranges  | VSET Ranges  | Voltage Step |
> > +		 * +-----------------+--------------+--------------+
> > +		 * | 0.5V to 1.50V   | 0x00 to 0x1A |  25mV        |
> > +		 * | 1.2V to 3.3V    | 0x1B to 0x45 |  50mV        |
> > +		 */
> > +		if (uV >= 1200000)
> > +			return (uV - 1200000) / 50000 + 0x1B;
> > +		else if (uV >= 500000)
> > +			return (uV - 500000) / 25000;
> > +		else
> > +			return -EINVAL;
> > +	}
> > +
> > +	/* Buck1 and Buck12(dual phase) has a different mapping b/w output
> > +	 * voltage and VSET value.
> > +	 *
> > +	 * | Voltage Ranges  | VSET Ranges  | Voltage Step |
> > +	 * +-----------------+--------------+--------------+
> > +	 * | 0.5V to 0.58V   | 0xA to 0xE   |  20mV        |
> > +	 * | 0.6V to 1.095V  | 0xF to 0x72  |  5mV         |
> > +	 * | 1.1V to 1.65V   | 0x73 to 0xAA |  10mV        |
> > +	 * | 1.6V to 3.3V    | 0xAB to 0xFD |  20mV        |
> > +	 *
> > +	 */
> > +	if (uV >= 1660000)
> > +		return (uV - 1660000) / 20000 + 0xAB;
> > +	else if (uV >= 1100000)
> > +		return (uV - 1100000) / 10000 + 0x73;
> > +	else if (uV >= 600000)
> > +		return (uV - 600000) / 5000 + 0x0F;
> > +	else if (uV >= 500000)
> > +		return (uV - 500000) / 20000 + 0x0A;
> > +	else
> > +		return -EINVAL;
> > +}
> > +
> > +static int tps65224_buck_val2volt(int idx, int val)
> > +{
> > +	/* This function does the opposite to the tps65224_buck_volt2val function
> > +	 * described above.
> > +	 * This maps the VSET value to micro volts. Please refer to the ranges
> > +	 * mentioned the comments of tps65224_buck_volt2val.
> > +	 */
> > +
> > +	if (idx > 0) {
> > +		if (val > TPS65224_BUCK234_VOLT_MAX_HEX)
> > +			return -EINVAL;
> > +		else if (val >= 0x1B)
> > +			return 1200000 + (val - 0x1B) * 50000;
> > +		else if (val >= 0x00)
> > +			return 500000 + (val - 0x00) * 25000;
> > +		else
> > +			return -EINVAL;
> > +	}
> > +
> > +	if (val > TPS65224_BUCK1_VOLT_MAX_HEX)
> > +		return -EINVAL;
> > +	else if (val >= 0xAB)
> > +		return 1660000 + (val - 0xAB) * 20000;
> > +	else if (val >= 0x73)
> > +		return 1100000 + (val - 0x73) * 10000;
> > +	else if (val >= 0xF)
> > +		return 600000 + (val - 0xF) * 5000;
> > +	else if (val >= 0xA)
> > +		return 500000 + (val - 0xA) * 20000;
> > +	else
> > +		return -EINVAL;
> > +}
> > +
> > +int tps65224_lookup_slew(int id)
> > +{
> > +	switch (id) {
> > +	case 0:
> > +		return 10000;
> > +	case 1:
> > +		return 5000;
> > +	case 2:
> > +		return 2500;
> > +	case 3:
> > +		return 1250;
> > +	default:
> > +		return -1;
> > +	}
> > +}
> > +
> >  static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
> >  	[TPS65941_BUCK_CONV_OPS_IDX] = {
> >  		.volt_mask = TPS65941_BUCK_VOLT_MASK,
> > @@ -154,6 +267,13 @@ static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
> >  		.slew_mask = TP65941_BUCK_CONF_SLEW_MASK,
> >  		.lookup_slew = tps65941_lookup_slew,
> >  	},
> > +	[TPS65224_BUCK_CONV_OPS_IDX] = {
> > +		.volt_mask = TPS65941_BUCK_VOLT_MASK,
> > +		.volt2val = tps65224_buck_volt2val,
> > +		.val2volt = tps65224_buck_val2volt,
> > +		.slew_mask = TPS65224_BUCK_CONF_SLEW_MASK,
> > +		.lookup_slew = tps65224_lookup_slew,
> > +	},
> >  };
> >  
> >  static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
> > @@ -162,9 +282,23 @@ static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
> >  	int ret, delta, uwait, slew, idx;
> >  	struct dm_regulator_uclass_plat *uc_pdata;
> >  	const struct tps65941_reg_conv_ops *conv_ops;
> > +	ulong chip_id;
> >  
> >  	idx = dev->driver_data;
> > -	conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
> > +	chip_id = tps65941_get_chip_id(dev);
> > +	if (chip_id == TPS65224) {
> > +		/* idx is the buck id number as per devicetree node which will be same
> > +		 * as the regulator name in the datasheet.
> > +		 * The idx for buck1. buck2, buck3, buck4, buck12 will be 1, 2, 3, 4
> > +		 * and 12 respectively.
> > +		 * In the driver the numbering is from 0. Hence the -1.
> > +		 */
> > +		idx = (idx == TPS65941_BUCK_ID_12) ? 0 : (idx - 1);
> > +		conv_ops = &buck_conv_ops[TPS65224_BUCK_CONV_OPS_IDX];
> > +	} else {
> > +		conv_ops = &buck_conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
> > +	}
> > +
> >  	uc_pdata = dev_get_uclass_plat(dev);
> >  
> >  	if (op == PMIC_OP_GET)
> > @@ -265,23 +399,99 @@ static int tps65941_ldo_val2volt(__maybe_unused int idx, int val)
> >  		return -EINVAL;
> >  }
> >  
> > +static int tps65224_ldo_volt2val(int idx, int uV)
> > +{
> > +	int base = TPS65224_LDO1_VOLT_MIN;
> > +	int max = TPS65224_LDO1_VOLT_MAX;
> > +	int offset = TPS65224_LDO1_VOLT_MIN_HEX;
> > +	int step = TPS65224_LDO_STEP;
> > +
> > +	if (idx > 0) {
> > +		base = TPS65224_LDO23_VOLT_MIN;
> > +		max = TPS65224_LDO23_VOLT_MAX;
> > +		offset = TPS65224_LDO23_VOLT_MIN_HEX;
> > +	}
> > +
> > +	if (uV > max)
> > +		return -EINVAL;
> > +	else if (uV >= base)
> > +		return (uV - base) / step + offset;
> > +	else
> > +		return -EINVAL;
> > +}
> > +
> > +static int tps65224_ldo_val2volt(int idx, int val)
> > +{
> > +	int reg_base = TPS65224_LDO1_VOLT_MIN_HEX;
> > +	int reg_max = TPS65224_LDO1_VOLT_MAX_HEX;
> > +	int base = TPS65224_LDO1_VOLT_MIN;
> > +	int max = TPS65224_LDO1_VOLT_MAX;
> > +	int step = TPS65224_LDO_STEP;
> > +	/* In LDOx_VOUT reg the BIT0 is reserved and the
> > +	 * vout value is stored from BIT1 to BIT7.
> > +	 * Hence the below bit shit is done.
> > +	 */
> > +	int mask = TPS65224_LDO_VOLT_MASK >> 1;
> > +
> > +	if (idx > 0) {
> > +		base = TPS65224_LDO23_VOLT_MIN;
> > +		max = TPS65224_LDO23_VOLT_MAX;
> > +		reg_base = TPS65224_LDO23_VOLT_MIN_HEX;
> > +		reg_max = TPS65224_LDO23_VOLT_MAX_HEX;
> > +	}
> > +
> > +	/* The VSET register of LDO has its 0th bit as reserved
> > +	 * hence shifting the value to right by 1 bit.
> > +	 */
> > +	val = val >> 1;
> > +
> > +	if (val < 0 || val > mask)
> > +		return -EINVAL;
> > +
> > +	if (val <= reg_base)
> > +		return base;
> > +
> > +	if (val >= reg_max)
> > +		return max;
> > +
> > +	return base + (step * (val - reg_base));
> > +}
> > +
> >  static const struct tps65941_reg_conv_ops ldo_conv_ops[] = {
> >  	[TPS65941_LDO_CONV_OPS_IDX] = {
> >  		.volt_mask = TPS65941_LDO_VOLT_MASK,
> >  		.volt2val = tps65941_buck_volt2val,
> >  		.val2volt = tps65941_ldo_val2volt,
> >  	},
> > +	[TPS65224_LDO_CONV_OPS_IDX] = {
> > +		.volt_mask = TPS65224_LDO_VOLT_MASK,
> > +		.volt2val = tps65224_ldo_volt2val,
> > +		.val2volt = tps65224_ldo_val2volt,
> > +	},
> >  };
> >  
> >  static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
> >  {
> >  	unsigned int hex, adr;
> > -	int ret, idx;
> > +	int ret, ret_volt, idx;
> >  	struct dm_regulator_uclass_plat *uc_pdata;
> >  	const struct tps65941_reg_conv_ops *conv_ops;
> > +	ulong chip_id;
> >  
> > +	chip_id = tps65941_get_chip_id(dev);
> >  	idx = dev->driver_data;
> > -	conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
> > +	if (chip_id == TPS65224) {
> > +		/* idx is the ldo id number as per devicetree node which will be same
> > +		 * as the regulator name in the datasheet.
> > +		 * The idx for ldo1, ldo2, ldo3 will be 1, 2 & 3 respectively.
> > +		 * In the driver the numbering is from 0. Hence the -1.
> > +		 */
> > +		idx = idx - 1;
> > +		conv_ops = &ldo_conv_ops[TPS65224_LDO_CONV_OPS_IDX];
> > +	} else {
> > +		conv_ops = &ldo_conv_ops[TPS65941_LDO_CONV_OPS_IDX];
> > +	}
> > +
> >  	uc_pdata = dev_get_uclass_plat(dev);
> >  
> >  	if (op == PMIC_OP_GET)
> > @@ -294,21 +504,36 @@ static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
> >  		return ret;
> >  
> >  	ret &= conv_ops->volt_mask;
> > -	ret = conv_ops->val2volt(idx, ret);
> > -	if (ret < 0)
> > -		return ret;
> > +	ret_volt = conv_ops->val2volt(idx, ret);
> > +	if (ret_volt < 0)
> > +		return ret_volt;
> >  
> >  	if (op == PMIC_OP_GET) {
> > -		*uV = ret;
> > +		*uV = ret_volt;
> >  		return 0;
> >  	}
> >  
> > +	/* TPS65224 LDO1 in BYPASS mode only supports 2.2V min to 3.6V max */
> > +	if (chip_id == TPS65224 && idx == 0 && (ret & BIT(TPS65224_LDO_BYP_CONFIG)) &&
> > +	    *uV < TPS65224_LDO1_VOLT_BYP_MIN)
> > +		return -EINVAL;
> > +
> > +	/* TPS65224 LDO2 & LDO3 in BYPASS mode supports 1.5V min to 5.5V max */
> > +	if (chip_id == TPS65224 && idx > 0 && (ret & BIT(TPS65224_LDO_BYP_CONFIG)) &&
> > +	    *uV < TPS65224_LDO23_VOLT_BYP_MIN)
> > +		return -EINVAL;
> > +
> >  	hex = conv_ops->volt2val(idx, *uV);
> >  	if (hex < 0)
> >  		return hex;
> >  
> > -	ret &= 0x0;
> > -	ret = hex;
> > +	if (chip_id == TPS65224) {
> > +		hex = hex << TPS65941_LDO_MODE_MASK;
> > +		ret &= ~TPS65224_LDO_VOLT_MASK;
> > +		ret |= hex;
> > +	} else {
> > +		ret = hex;
> > +	}
> >  
> >  	ret = pmic_reg_write(dev->parent, adr, ret);
> >  
> > @@ -319,6 +544,9 @@ static int tps65941_ldo_probe(struct udevice *dev)
> >  {
> >  	struct dm_regulator_uclass_plat *uc_pdata;
> >  	int idx;
> > +	ulong chip_id;
> > +
> > +	chip_id = tps65941_get_chip_id(dev);
> >  
> >  	uc_pdata = dev_get_uclass_plat(dev);
> >  	uc_pdata->type = REGULATOR_TYPE_LDO;
> > @@ -328,9 +556,16 @@ static int tps65941_ldo_probe(struct udevice *dev)
> >  	case TPS65941_LDO_ID_1:
> >  	case TPS65941_LDO_ID_2:
> >  	case TPS65941_LDO_ID_3:
> > -	case TPS65941_LDO_ID_4:
> >  		debug("Single phase regulator\n");
> >  		break;
> > +	case TPS65941_LDO_ID_4:
> > +		if (chip_id != TPS65224) {
> > +			debug("Single phase regulator\n");
> > +		} else {
> > +			pr_err("Wrong ID for regulator\n");
> > +			return -EINVAL;
> > +		}
> > +		break;
> 
> Is it possible to use the below?
> 
> if (chip_id != TPS65224) {
> 	debug("Single phase regulator\n");
> 	break;
> }
> 
> /* fallthrough */
> 
> Best Regards,
> Jaehoon Chung
> 

Got it, thanks for suggesting it!
Will do the change and send it in next version.

> >  	default:
> >  		pr_err("Wrong ID for regulator\n");
> >  		return -EINVAL;
> > @@ -346,6 +581,9 @@ static int tps65941_buck_probe(struct udevice *dev)
> >  {
> >  	struct dm_regulator_uclass_plat *uc_pdata;
> >  	int idx;
> > +	ulong chip_id;
> > +
> > +	chip_id = tps65941_get_chip_id(dev);
> >  
> >  	uc_pdata = dev_get_uclass_plat(dev);
> >  	uc_pdata->type = REGULATOR_TYPE_BUCK;
> > @@ -356,16 +594,35 @@ static int tps65941_buck_probe(struct udevice *dev)
> >  	case TPS65941_BUCK_ID_2:
> >  	case TPS65941_BUCK_ID_3:
> >  	case TPS65941_BUCK_ID_4:
> > -	case TPS65941_BUCK_ID_5:
> >  		debug("Single phase regulator\n");
> >  		break;
> > +	case TPS65941_BUCK_ID_5:
> > +		if (chip_id != TPS65224) {
> > +			debug("Single phase regulator\n");
> > +		} else {
> > +			pr_err("Wrong ID for regulator\n");
> > +			return -EINVAL;
> > +		}
> > +		break;
> >  	case TPS65941_BUCK_ID_12:
> > +		idx = 1;
> > +		break;
> >  	case TPS65941_BUCK_ID_123:
> >  	case TPS65941_BUCK_ID_1234:
> > -		idx = 1;
> > +		if (chip_id != TPS65224) {
> > +			idx = 1;
> > +		} else {
> > +			pr_err("Wrong ID for regulator\n");
> > +			return -EINVAL;
> > +		}
> >  		break;
> >  	case TPS65941_BUCK_ID_34:
> > -		idx = 3;
> > +		if (chip_id != TPS65224) {
> > +			idx = 3;
> > +		} else {
> > +			pr_err("Wrong ID for regulator\n");
> > +			return -EINVAL;
> > +		}
> >  		break;
> >  	default:
> >  		pr_err("Wrong ID for regulator\n");

Regards,
Bhargav