[PATCH 0/8] Add support for Rockchip RK860X regulators

[PATCH 0/8] Add support for Rockchip RK860X regulators

[Cristian Ciocaltea]

This patch series introduces support for the Rockchip RK860X regulators, 
while also providing a few fixes and improvements to the existing fan53555 
driver.

RK8600/RK8601 are fully compatible with the FAN53555 regulators.

RK8602/RK8603 are a bit different, having a wider output voltage
selection range, from 0.5 V to 1.5 V in 6.25 mV steps. They are used 
in the Rock 5B board to power the ARM Cortex-A76 cores and the NPU.

Cristian Ciocaltea (8):
  regulator: dt-bindings: fcs,fan53555: Add support for RK860X
  regulator: fan53555: Explicitly include bits header
  regulator: fan53555: Fix wrong TCS_SLEW_MASK
  regulator: fan53555: Remove unused *_SLEW_SHIFT definitions
  regulator: fan53555: Make use of the bit macros
  regulator: fan53555: Improve vsel_mask computation
  regulator: fan53555: Use dev_err_probe
  regulator: fan53555: Add support for RK860X

 .../bindings/regulator/fcs,fan53555.yaml      |   4 +
 drivers/regulator/fan53555.c                  | 215 ++++++++++++++----
 2 files changed, 172 insertions(+), 47 deletions(-)

-- 
2.40.0

[PATCH 1/8] regulator: dt-bindings: fcs,fan53555: Add support for RK860X

[Cristian Ciocaltea]

Add compatibles to support Rockchip RK860X regulators.

RK8600/RK8601 are compatible with Fairchild FAN53555 regulators, while
RK8602/RK8603 are a bit different, having a wider voltage selection
range.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
index c0dbba843f70..687eda31ad90 100644
--- a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
+++ b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
@@ -17,6 +17,10 @@ properties:
     enum:
       - fcs,fan53555
       - fcs,fan53526
+      - rockchip,rk8600
+      - rockchip,rk8601
+      - rockchip,rk8602
+      - rockchip,rk8603
       - silergy,syr827
       - silergy,syr828
       - tcs,tcs4525
-- 
2.40.0

Re: [PATCH 1/8] regulator: dt-bindings: fcs,fan53555: Add support for RK860X

[Krzysztof Kozlowski]

On 05/04/2023 21:47, Cristian Ciocaltea wrote:
> Add compatibles to support Rockchip RK860X regulators.
> 
> RK8600/RK8601 are compatible with Fairchild FAN53555 regulators, while
> RK8602/RK8603 are a bit different, having a wider voltage selection
> range.

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

Best regards,
Krzysztof

[PATCH 2/8] regulator: fan53555: Explicitly include bits header

[Cristian Ciocaltea]

Since commit f2a9eb975ab2 ("regulator: fan53555: Add support for
FAN53526") the driver makes use of the BIT() macro, but relies on the
bits header being implicitly included.

Explicitly pull the header in to avoid potential build failures in some
configurations.

While here, reorder include directives alphabetically.

Fixes: f2a9eb975ab2 ("regulator: fan53555: Add support for FAN53526")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index d0c678767294..0754c370a21c 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -8,18 +8,19 @@
 // Copyright (c) 2012 Marvell Technology Ltd.
 // Yunfan Zhang <yfzhang@marvell.com>
 
+#include <linux/bits.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/param.h>
-#include <linux/err.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/regulator/driver.h>
+#include <linux/regulator/fan53555.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
-#include <linux/of_device.h>
-#include <linux/i2c.h>
 #include <linux/slab.h>
-#include <linux/regmap.h>
-#include <linux/regulator/fan53555.h>
 
 /* Voltage setting */
 #define FAN53555_VSEL0		0x00
-- 
2.40.0

[PATCH 3/8] regulator: fan53555: Fix wrong TCS_SLEW_MASK

[Cristian Ciocaltea]

The support for TCS4525 regulator has been introduced with a wrong
ramp-rate mask, which has been defined as a logical expression instead
of a bit shift operation.

For clarity, fix it using GENMASK() macro.

Fixes: 914df8faa7d6 ("regulator: fan53555: Add TCS4525 DCDC support")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 0754c370a21c..4d2104c3a077 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -61,7 +61,7 @@
 #define TCS_VSEL1_MODE		(1 << 6)
 
 #define TCS_SLEW_SHIFT		3
-#define TCS_SLEW_MASK		(0x3 < 3)
+#define TCS_SLEW_MASK		GENMASK(4, 3)
 
 enum fan53555_vendor {
 	FAN53526_VENDOR_FAIRCHILD = 0,
-- 
2.40.0

[PATCH 4/8] regulator: fan53555: Remove unused *_SLEW_SHIFT definitions

[Cristian Ciocaltea]

Commit b61ac767db4d ("regulator: fan53555: Convert to use
regulator_set_ramp_delay_regmap") removed the slew_shift member from
struct fan53555_device_info, hence the {CTL,TCS}_SLEW_SHIFT definitions
remained unused.  Drop them.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 4d2104c3a077..68ebcd4ccef7 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -49,7 +49,6 @@
 /* Control bit definitions */
 #define CTL_OUTPUT_DISCHG	(1 << 7)
 #define CTL_SLEW_MASK		(0x7 << 4)
-#define CTL_SLEW_SHIFT		4
 #define CTL_RESET			(1 << 2)
 #define CTL_MODE_VSEL0_MODE	BIT(0)
 #define CTL_MODE_VSEL1_MODE	BIT(1)
@@ -60,7 +59,6 @@
 #define TCS_VSEL0_MODE		(1 << 7)
 #define TCS_VSEL1_MODE		(1 << 6)
 
-#define TCS_SLEW_SHIFT		3
 #define TCS_SLEW_MASK		GENMASK(4, 3)
 
 enum fan53555_vendor {
-- 
2.40.0

[PATCH 5/8] regulator: fan53555: Make use of the bit macros

[Cristian Ciocaltea]

For consistency and improved clarity, use BIT() and GENMASK() macros for
defining the bitfields inside the registers. No functional changes
intended.

While here, also fix DIE_{ID,REV} inconsistent indentation.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 68ebcd4ccef7..181e5eb00e7a 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -41,23 +41,23 @@
 #define FAN53555_MONITOR	0x05
 
 /* VSEL bit definitions */
-#define VSEL_BUCK_EN	(1 << 7)
-#define VSEL_MODE		(1 << 6)
+#define VSEL_BUCK_EN		BIT(7)
+#define VSEL_MODE		BIT(6)
 /* Chip ID and Verison */
-#define DIE_ID		0x0F	/* ID1 */
-#define DIE_REV		0x0F	/* ID2 */
+#define DIE_ID			0x0F	/* ID1 */
+#define DIE_REV			0x0F	/* ID2 */
 /* Control bit definitions */
-#define CTL_OUTPUT_DISCHG	(1 << 7)
-#define CTL_SLEW_MASK		(0x7 << 4)
-#define CTL_RESET			(1 << 2)
+#define CTL_OUTPUT_DISCHG	BIT(7)
+#define CTL_SLEW_MASK		GENMASK(6, 4)
+#define CTL_RESET		BIT(2)
 #define CTL_MODE_VSEL0_MODE	BIT(0)
 #define CTL_MODE_VSEL1_MODE	BIT(1)
 
 #define FAN53555_NVOLTAGES	64	/* Numbers of voltages */
 #define FAN53526_NVOLTAGES	128
 
-#define TCS_VSEL0_MODE		(1 << 7)
-#define TCS_VSEL1_MODE		(1 << 6)
+#define TCS_VSEL0_MODE		BIT(7)
+#define TCS_VSEL1_MODE		BIT(6)
 
 #define TCS_SLEW_MASK		GENMASK(4, 3)
 
-- 
2.40.0

[PATCH 6/8] regulator: fan53555: Improve vsel_mask computation

[Cristian Ciocaltea]

In preparation for introducing support for additional regulators which
do not use the maximum number of voltage selectors available for a given
mask, improve the mask computation formula by using fls().

Note fls() requires the bitops header, hence include it explicitly and
drop bits.h which is already pulled by bitops.h.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 181e5eb00e7a..39c9c29f4ff6 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -8,7 +8,7 @@
 // Copyright (c) 2012 Marvell Technology Ltd.
 // Yunfan Zhang <yfzhang@marvell.com>
 
-#include <linux/bits.h>
+#include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
@@ -486,7 +486,7 @@ static int fan53555_regulator_register(struct fan53555_device_info *di,
 	rdesc->min_uV = di->vsel_min;
 	rdesc->uV_step = di->vsel_step;
 	rdesc->vsel_reg = di->vol_reg;
-	rdesc->vsel_mask = di->vsel_count - 1;
+	rdesc->vsel_mask = BIT(fls(di->vsel_count - 1)) - 1;
 	rdesc->ramp_reg = di->slew_reg;
 	rdesc->ramp_mask = di->slew_mask;
 	rdesc->ramp_delay_table = di->ramp_delay_table;
-- 
2.40.0

[PATCH 7/8] regulator: fan53555: Use dev_err_probe

[Cristian Ciocaltea]

Use dev_err_probe() instead of dev_err() in the probe function, which
ensures the error code is always printed and, additionally, simplifies
the code a bit.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 47 ++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 39c9c29f4ff6..acf14ba7aaa6 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -568,10 +568,9 @@ static int fan53555_regulator_probe(struct i2c_client *client)
 	if (!pdata)
 		pdata = fan53555_parse_dt(&client->dev, np, &di->desc);
 
-	if (!pdata || !pdata->regulator) {
-		dev_err(&client->dev, "Platform data not found!\n");
-		return -ENODEV;
-	}
+	if (!pdata || !pdata->regulator)
+		return dev_err_probe(&client->dev, -ENODEV,
+				     "Platform data not found!\n");
 
 	di->regulator = pdata->regulator;
 	if (client->dev.of_node) {
@@ -580,10 +579,9 @@ static int fan53555_regulator_probe(struct i2c_client *client)
 	} else {
 		/* if no ramp constraint set, get the pdata ramp_delay */
 		if (!di->regulator->constraints.ramp_delay) {
-			if (pdata->slew_rate >= ARRAY_SIZE(slew_rates)) {
-				dev_err(&client->dev, "Invalid slew_rate\n");
-				return -EINVAL;
-			}
+			if (pdata->slew_rate >= ARRAY_SIZE(slew_rates))
+				return dev_err_probe(&client->dev, -EINVAL,
+						     "Invalid slew_rate\n");
 
 			di->regulator->constraints.ramp_delay
 					= slew_rates[pdata->slew_rate];
@@ -593,34 +591,31 @@ static int fan53555_regulator_probe(struct i2c_client *client)
 	}
 
 	regmap = devm_regmap_init_i2c(client, &fan53555_regmap_config);
-	if (IS_ERR(regmap)) {
-		dev_err(&client->dev, "Failed to allocate regmap!\n");
-		return PTR_ERR(regmap);
-	}
+	if (IS_ERR(regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(regmap),
+				     "Failed to allocate regmap!\n");
+
 	di->dev = &client->dev;
 	i2c_set_clientdata(client, di);
 	/* Get chip ID */
 	ret = regmap_read(regmap, FAN53555_ID1, &val);
-	if (ret < 0) {
-		dev_err(&client->dev, "Failed to get chip ID!\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret, "Failed to get chip ID!\n");
+
 	di->chip_id = val & DIE_ID;
 	/* Get chip revision */
 	ret = regmap_read(regmap, FAN53555_ID2, &val);
-	if (ret < 0) {
-		dev_err(&client->dev, "Failed to get chip Rev!\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret, "Failed to get chip Rev!\n");
+
 	di->chip_rev = val & DIE_REV;
 	dev_info(&client->dev, "FAN53555 Option[%d] Rev[%d] Detected!\n",
 				di->chip_id, di->chip_rev);
 	/* Device init */
 	ret = fan53555_device_setup(di, pdata);
-	if (ret < 0) {
-		dev_err(&client->dev, "Failed to setup device!\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret, "Failed to setup device!\n");
+
 	/* Register regulator */
 	config.dev = di->dev;
 	config.init_data = di->regulator;
@@ -630,9 +625,9 @@ static int fan53555_regulator_probe(struct i2c_client *client)
 
 	ret = fan53555_regulator_register(di, &config);
 	if (ret < 0)
-		dev_err(&client->dev, "Failed to register regulator!\n");
-	return ret;
+		dev_err_probe(&client->dev, ret, "Failed to register regulator!\n");
 
+	return ret;
 }
 
 static const struct i2c_device_id fan53555_id[] = {
-- 
2.40.0

[PATCH 8/8] regulator: fan53555: Add support for RK860X

[Cristian Ciocaltea]

Extend the existing fan53555 driver to support the Rockchip RK860X
regulators.

RK8600/RK8601 are fully compatible with FAN53555 regulators.

RK8602/RK8603 are a bit different, having a wider output voltage
selection range, from 0.5 V to 1.5 V in 6.25 mV steps. They also use
additional VSEL0/VSEL1 registers for the voltage selector, but the
enable and mode bits are still located in the original FAN53555 specific
VSEL0/VSEL1 registers.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/regulator/fan53555.c | 133 ++++++++++++++++++++++++++++++++++-
 1 file changed, 130 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index acf14ba7aaa6..1c662d6ea57b 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -26,6 +26,9 @@
 #define FAN53555_VSEL0		0x00
 #define FAN53555_VSEL1		0x01
 
+#define RK8602_VSEL0		0x06
+#define RK8602_VSEL1		0x07
+
 #define TCS4525_VSEL0		0x11
 #define TCS4525_VSEL1		0x10
 #define TCS4525_TIME		0x13
@@ -55,6 +58,7 @@
 
 #define FAN53555_NVOLTAGES	64	/* Numbers of voltages */
 #define FAN53526_NVOLTAGES	128
+#define RK8602_NVOLTAGES	160
 
 #define TCS_VSEL0_MODE		BIT(7)
 #define TCS_VSEL1_MODE		BIT(6)
@@ -64,6 +68,8 @@
 enum fan53555_vendor {
 	FAN53526_VENDOR_FAIRCHILD = 0,
 	FAN53555_VENDOR_FAIRCHILD,
+	FAN53555_VENDOR_ROCKCHIP,	/* RK8600, RK8601 */
+	RK8602_VENDOR_ROCKCHIP,		/* RK8602, RK8603 */
 	FAN53555_VENDOR_SILERGY,
 	FAN53526_VENDOR_TCS,
 };
@@ -87,6 +93,14 @@ enum {
 	FAN53555_CHIP_ID_08 = 8,
 };
 
+enum {
+	RK8600_CHIP_ID_08 = 8,		/* RK8600, RK8601 */
+};
+
+enum {
+	RK8602_CHIP_ID_10 = 10,		/* RK8602, RK8603 */
+};
+
 enum {
 	TCS4525_CHIP_ID_12 = 12,
 };
@@ -117,6 +131,8 @@ struct fan53555_device_info {
 	/* Voltage setting register */
 	unsigned int vol_reg;
 	unsigned int sleep_reg;
+	unsigned int en_reg;
+	unsigned int sleep_en_reg;
 	/* Voltage range and step(linear) */
 	unsigned int vsel_min;
 	unsigned int vsel_step;
@@ -159,7 +175,7 @@ static int fan53555_set_suspend_enable(struct regulator_dev *rdev)
 {
 	struct fan53555_device_info *di = rdev_get_drvdata(rdev);
 
-	return regmap_update_bits(rdev->regmap, di->sleep_reg,
+	return regmap_update_bits(rdev->regmap, di->sleep_en_reg,
 				  VSEL_BUCK_EN, VSEL_BUCK_EN);
 }
 
@@ -167,7 +183,7 @@ static int fan53555_set_suspend_disable(struct regulator_dev *rdev)
 {
 	struct fan53555_device_info *di = rdev_get_drvdata(rdev);
 
-	return regmap_update_bits(rdev->regmap, di->sleep_reg,
+	return regmap_update_bits(rdev->regmap, di->sleep_en_reg,
 				  VSEL_BUCK_EN, 0);
 }
 
@@ -317,6 +333,50 @@ static int fan53555_voltages_setup_fairchild(struct fan53555_device_info *di)
 	return 0;
 }
 
+static int fan53555_voltages_setup_rockchip(struct fan53555_device_info *di)
+{
+	/* Init voltage range and step */
+	switch (di->chip_id) {
+	case RK8600_CHIP_ID_08:
+		di->vsel_min = 712500;
+		di->vsel_step = 12500;
+		break;
+	default:
+		dev_err(di->dev,
+			"Chip ID %d not supported!\n", di->chip_id);
+		return -EINVAL;
+	}
+	di->slew_reg = FAN53555_CONTROL;
+	di->slew_mask = CTL_SLEW_MASK;
+	di->ramp_delay_table = slew_rates;
+	di->n_ramp_values = ARRAY_SIZE(slew_rates);
+	di->vsel_count = FAN53555_NVOLTAGES;
+
+	return 0;
+}
+
+static int rk8602_voltages_setup_rockchip(struct fan53555_device_info *di)
+{
+	/* Init voltage range and step */
+	switch (di->chip_id) {
+	case RK8602_CHIP_ID_10:
+		di->vsel_min = 500000;
+		di->vsel_step = 6250;
+		break;
+	default:
+		dev_err(di->dev,
+			"Chip ID %d not supported!\n", di->chip_id);
+		return -EINVAL;
+	}
+	di->slew_reg = FAN53555_CONTROL;
+	di->slew_mask = CTL_SLEW_MASK;
+	di->ramp_delay_table = slew_rates;
+	di->n_ramp_values = ARRAY_SIZE(slew_rates);
+	di->vsel_count = RK8602_NVOLTAGES;
+
+	return 0;
+}
+
 static int fan53555_voltages_setup_silergy(struct fan53555_device_info *di)
 {
 	/* Init voltage range and step */
@@ -377,6 +437,7 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
 	switch (di->vendor) {
 	case FAN53526_VENDOR_FAIRCHILD:
 	case FAN53555_VENDOR_FAIRCHILD:
+	case FAN53555_VENDOR_ROCKCHIP:
 	case FAN53555_VENDOR_SILERGY:
 		switch (pdata->sleep_vsel_id) {
 		case FAN53555_VSEL_ID_0:
@@ -391,6 +452,27 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
 			dev_err(di->dev, "Invalid VSEL ID!\n");
 			return -EINVAL;
 		}
+		di->sleep_en_reg = di->sleep_reg;
+		di->en_reg = di->vol_reg;
+		break;
+	case RK8602_VENDOR_ROCKCHIP:
+		switch (pdata->sleep_vsel_id) {
+		case FAN53555_VSEL_ID_0:
+			di->sleep_reg = RK8602_VSEL0;
+			di->vol_reg = RK8602_VSEL1;
+			di->sleep_en_reg = FAN53555_VSEL0;
+			di->en_reg = FAN53555_VSEL1;
+			break;
+		case FAN53555_VSEL_ID_1:
+			di->sleep_reg = RK8602_VSEL1;
+			di->vol_reg = RK8602_VSEL0;
+			di->sleep_en_reg = FAN53555_VSEL1;
+			di->en_reg = FAN53555_VSEL0;
+			break;
+		default:
+			dev_err(di->dev, "Invalid VSEL ID!\n");
+			return -EINVAL;
+		}
 		break;
 	case FAN53526_VENDOR_TCS:
 		switch (pdata->sleep_vsel_id) {
@@ -406,6 +488,8 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
 			dev_err(di->dev, "Invalid VSEL ID!\n");
 			return -EINVAL;
 		}
+		di->sleep_en_reg = di->sleep_reg;
+		di->en_reg = di->vol_reg;
 		break;
 	default:
 		dev_err(di->dev, "vendor %d not supported!\n", di->vendor);
@@ -427,10 +511,23 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
 		}
 		break;
 	case FAN53555_VENDOR_FAIRCHILD:
+	case FAN53555_VENDOR_ROCKCHIP:
 	case FAN53555_VENDOR_SILERGY:
 		di->mode_reg = di->vol_reg;
 		di->mode_mask = VSEL_MODE;
 		break;
+	case RK8602_VENDOR_ROCKCHIP:
+		di->mode_mask = VSEL_MODE;
+
+		switch (pdata->sleep_vsel_id) {
+		case FAN53555_VSEL_ID_0:
+			di->mode_reg = FAN53555_VSEL1;
+			break;
+		case FAN53555_VSEL_ID_1:
+			di->mode_reg = FAN53555_VSEL0;
+			break;
+		}
+		break;
 	case FAN53526_VENDOR_TCS:
 		di->mode_reg = TCS4525_COMMAND;
 
@@ -456,6 +553,12 @@ static int fan53555_device_setup(struct fan53555_device_info *di,
 	case FAN53555_VENDOR_FAIRCHILD:
 		ret = fan53555_voltages_setup_fairchild(di);
 		break;
+	case FAN53555_VENDOR_ROCKCHIP:
+		ret = fan53555_voltages_setup_rockchip(di);
+		break;
+	case RK8602_VENDOR_ROCKCHIP:
+		ret = rk8602_voltages_setup_rockchip(di);
+		break;
 	case FAN53555_VENDOR_SILERGY:
 		ret = fan53555_voltages_setup_silergy(di);
 		break;
@@ -481,7 +584,7 @@ static int fan53555_regulator_register(struct fan53555_device_info *di,
 	rdesc->ops = &fan53555_regulator_ops;
 	rdesc->type = REGULATOR_VOLTAGE;
 	rdesc->n_voltages = di->vsel_count;
-	rdesc->enable_reg = di->vol_reg;
+	rdesc->enable_reg = di->en_reg;
 	rdesc->enable_mask = VSEL_BUCK_EN;
 	rdesc->min_uV = di->vsel_min;
 	rdesc->uV_step = di->vsel_step;
@@ -531,6 +634,18 @@ static const struct of_device_id __maybe_unused fan53555_dt_ids[] = {
 	}, {
 		.compatible = "fcs,fan53555",
 		.data = (void *)FAN53555_VENDOR_FAIRCHILD
+	}, {
+		.compatible = "rockchip,rk8600",
+		.data = (void *)FAN53555_VENDOR_ROCKCHIP
+	}, {
+		.compatible = "rockchip,rk8601",
+		.data = (void *)FAN53555_VENDOR_ROCKCHIP
+	}, {
+		.compatible = "rockchip,rk8602",
+		.data = (void *)RK8602_VENDOR_ROCKCHIP
+	}, {
+		.compatible = "rockchip,rk8603",
+		.data = (void *)RK8602_VENDOR_ROCKCHIP
 	}, {
 		.compatible = "silergy,syr827",
 		.data = (void *)FAN53555_VENDOR_SILERGY,
@@ -637,6 +752,18 @@ static const struct i2c_device_id fan53555_id[] = {
 	}, {
 		.name = "fan53555",
 		.driver_data = FAN53555_VENDOR_FAIRCHILD
+	}, {
+		.name = "rk8600",
+		.driver_data = FAN53555_VENDOR_ROCKCHIP
+	}, {
+		.name = "rk8601",
+		.driver_data = FAN53555_VENDOR_ROCKCHIP
+	}, {
+		.name = "rk8602",
+		.driver_data = RK8602_VENDOR_ROCKCHIP
+	}, {
+		.name = "rk8603",
+		.driver_data = RK8602_VENDOR_ROCKCHIP
 	}, {
 		.name = "syr827",
 		.driver_data = FAN53555_VENDOR_SILERGY
-- 
2.40.0

Re: [PATCH 8/8] regulator: fan53555: Add support for RK860X

[Krzysztof Kozlowski]

On 05/04/2023 21:47, Cristian Ciocaltea wrote:
> Extend the existing fan53555 driver to support the Rockchip RK860X
> regulators.
> 
> RK8600/RK8601 are fully compatible with FAN53555 regulators.
> 

> @@ -531,6 +634,18 @@ static const struct of_device_id __maybe_unused fan53555_dt_ids[] = {
>  	}, {
>  		.compatible = "fcs,fan53555",
>  		.data = (void *)FAN53555_VENDOR_FAIRCHILD
> +	}, {
> +		.compatible = "rockchip,rk8600",
> +		.data = (void *)FAN53555_VENDOR_ROCKCHIP
> +	}, {
> +		.compatible = "rockchip,rk8601",
> +		.data = (void *)FAN53555_VENDOR_ROCKCHIP
> +	}, {
> +		.compatible = "rockchip,rk8602",
> +		.data = (void *)RK8602_VENDOR_ROCKCHIP
> +	}, {
> +		.compatible = "rockchip,rk8603",
> +		.data = (void *)RK8602_VENDOR_ROCKCHIP
>  	}, {
>  		.compatible = "silergy,syr827",
>  		.data = (void *)FAN53555_VENDOR_SILERGY,
> @@ -637,6 +752,18 @@ static const struct i2c_device_id fan53555_id[] = {
>  	}, {
>  		.name = "fan53555",
>  		.driver_data = FAN53555_VENDOR_FAIRCHILD
> +	}, {
> +		.name = "rk8600",
> +		.driver_data = FAN53555_VENDOR_ROCKCHIP
> +	}, {
> +		.name = "rk8601",
> +		.driver_data = FAN53555_VENDOR_ROCKCHIP

Why do you need this entry match data if it is the same as rk8600?

> +	}, {
> +		.name = "rk8602",
> +		.driver_data = RK8602_VENDOR_ROCKCHIP
> +	}, {
> +		.name = "rk8603",
> +		.driver_data = RK8602_VENDOR_ROCKCHIP

Why do you need this entry match data if it is the same as rk8602?

Best regards,
Krzysztof

Re: [PATCH 8/8] regulator: fan53555: Add support for RK860X

[Cristian Ciocaltea]

On 4/6/23 11:25, Krzysztof Kozlowski wrote:
> On 05/04/2023 21:47, Cristian Ciocaltea wrote:
>> Extend the existing fan53555 driver to support the Rockchip RK860X
>> regulators.
>>
>> RK8600/RK8601 are fully compatible with FAN53555 regulators.
>>
> 
>> @@ -531,6 +634,18 @@ static const struct of_device_id __maybe_unused fan53555_dt_ids[] = {
>>  	}, {
>>  		.compatible = "fcs,fan53555",
>>  		.data = (void *)FAN53555_VENDOR_FAIRCHILD
>> +	}, {
>> +		.compatible = "rockchip,rk8600",
>> +		.data = (void *)FAN53555_VENDOR_ROCKCHIP
>> +	}, {
>> +		.compatible = "rockchip,rk8601",
>> +		.data = (void *)FAN53555_VENDOR_ROCKCHIP
>> +	}, {
>> +		.compatible = "rockchip,rk8602",
>> +		.data = (void *)RK8602_VENDOR_ROCKCHIP
>> +	}, {
>> +		.compatible = "rockchip,rk8603",
>> +		.data = (void *)RK8602_VENDOR_ROCKCHIP
>>  	}, {
>>  		.compatible = "silergy,syr827",
>>  		.data = (void *)FAN53555_VENDOR_SILERGY,
>> @@ -637,6 +752,18 @@ static const struct i2c_device_id fan53555_id[] = {
>>  	}, {
>>  		.name = "fan53555",
>>  		.driver_data = FAN53555_VENDOR_FAIRCHILD
>> +	}, {
>> +		.name = "rk8600",
>> +		.driver_data = FAN53555_VENDOR_ROCKCHIP
>> +	}, {
>> +		.name = "rk8601",
>> +		.driver_data = FAN53555_VENDOR_ROCKCHIP
> 
> Why do you need this entry match data if it is the same as rk8600?
> 
>> +	}, {
>> +		.name = "rk8602",
>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>> +	}, {
>> +		.name = "rk8603",
>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
> 
> Why do you need this entry match data if it is the same as rk8602?

This is consistent with the handling of syr827 and syr828:

		.name = "syr827",
		.driver_data = FAN53555_VENDOR_SILERGY
	}, {
		.name = "syr828",
		.driver_data = FAN53555_VENDOR_SILERGY

Thanks,
Cristian

Re: [PATCH 8/8] regulator: fan53555: Add support for RK860X

[Krzysztof Kozlowski]

On 06/04/2023 12:08, Cristian Ciocaltea wrote:
>>> +	}, {
>>> +		.name = "rk8602",
>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>> +	}, {
>>> +		.name = "rk8603",
>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>
>> Why do you need this entry match data if it is the same as rk8602?
> 
> This is consistent with the handling of syr827 and syr828:
> 
> 		.name = "syr827",
> 		.driver_data = FAN53555_VENDOR_SILERGY
> 	}, {
> 		.name = "syr828",
> 		.driver_data = FAN53555_VENDOR_SILERGY

Yeah, I understand, but it's not necessarily the pattern we want to
continue. Unless these devices are not really compatible?

Best regards,
Krzysztof

Re: [PATCH 8/8] regulator: fan53555: Add support for RK860X

[Cristian Ciocaltea]

On 4/6/23 14:03, Krzysztof Kozlowski wrote:
> On 06/04/2023 12:08, Cristian Ciocaltea wrote:
>>>> +	}, {
>>>> +		.name = "rk8602",
>>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>>> +	}, {
>>>> +		.name = "rk8603",
>>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>>
>>> Why do you need this entry match data if it is the same as rk8602?
>>
>> This is consistent with the handling of syr827 and syr828:
>>
>> 		.name = "syr827",
>> 		.driver_data = FAN53555_VENDOR_SILERGY
>> 	}, {
>> 		.name = "syr828",
>> 		.driver_data = FAN53555_VENDOR_SILERGY
> 
> Yeah, I understand, but it's not necessarily the pattern we want to
> continue. Unless these devices are not really compatible?

They are compatible, so should I simply drop the rk8601 and rk8603 entries?

Probably also renaming rk8600 and rk8602, though I'm not sure what a
proper naming scheme would be to combine the 2 variants for each.

Thanks,
Cristian

Re: [PATCH 8/8] regulator: fan53555: Add support for RK860X

[Cristian Ciocaltea]

On 4/6/23 14:15, Cristian Ciocaltea wrote:
> On 4/6/23 14:03, Krzysztof Kozlowski wrote:
>> On 06/04/2023 12:08, Cristian Ciocaltea wrote:
>>>>> +	}, {
>>>>> +		.name = "rk8602",
>>>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>>>> +	}, {
>>>>> +		.name = "rk8603",
>>>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>>>
>>>> Why do you need this entry match data if it is the same as rk8602?
>>>
>>> This is consistent with the handling of syr827 and syr828:
>>>
>>> 		.name = "syr827",
>>> 		.driver_data = FAN53555_VENDOR_SILERGY
>>> 	}, {
>>> 		.name = "syr828",
>>> 		.driver_data = FAN53555_VENDOR_SILERGY
>>
>> Yeah, I understand, but it's not necessarily the pattern we want to
>> continue. Unless these devices are not really compatible?
> 
> They are compatible, so should I simply drop the rk8601 and rk8603 entries?

I dropped the entries in [v2] and updated the binding in PATCH 1/8.
Note I didn't add the "Acked-by" for the latter, since the changes are 
significant and require a new review.

Thanks,
Cristian

v2: https://lore.kernel.org/lkml/20230406171806.948290-1-cristian.ciocaltea@collabora.com/

Re: [PATCH 8/8] regulator: fan53555: Add support for RK860X

[Krzysztof Kozlowski]

On 06/04/2023 13:15, Cristian Ciocaltea wrote:
> On 4/6/23 14:03, Krzysztof Kozlowski wrote:
>> On 06/04/2023 12:08, Cristian Ciocaltea wrote:
>>>>> +	}, {
>>>>> +		.name = "rk8602",
>>>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>>>> +	}, {
>>>>> +		.name = "rk8603",
>>>>> +		.driver_data = RK8602_VENDOR_ROCKCHIP
>>>>
>>>> Why do you need this entry match data if it is the same as rk8602?
>>>
>>> This is consistent with the handling of syr827 and syr828:
>>>
>>> 		.name = "syr827",
>>> 		.driver_data = FAN53555_VENDOR_SILERGY
>>> 	}, {
>>> 		.name = "syr828",
>>> 		.driver_data = FAN53555_VENDOR_SILERGY
>>
>> Yeah, I understand, but it's not necessarily the pattern we want to
>> continue. Unless these devices are not really compatible?
> 
> They are compatible, so should I simply drop the rk8601 and rk8603 entries?
> 
> Probably also renaming rk8600 and rk8602, though I'm not sure what a
> proper naming scheme would be to combine the 2 variants for each.

For each compatible family you should have only one entry in each ID
table. Naming of driver data does not matter really. Just use lower chip
name.

Best regards,
Krzysztof