Allow use of DVS GPIOs on the RK808 PMIC variant only

This is a small series that primarily makes the rk808-regulator driver more compliant to the DT bindings, by allowing use of DVS GPIOs on the RK808 variant only. There's no point in allowing that on the other PMIC variants, because they don't support the DVS GPIOs, and it goes against the DT bindings to allow a possibly misplaced "dvs-gpios" property to actually be handled in the rk808-regulator driver.

This series also cleans up the code a bit, improves some comments, and replaces dev_err() with dev_err_probe(), where appropriate.

Dragan Simic (3):
regulator: rk808: Perform trivial code cleanups
regulator: rk808: Use dev_err_probe() in the probe path
regulator: rk808: Restrict DVS GPIOs to the RK808 variant only

 drivers/regulator/rk808-regulator.c | 41 +++++++++++++----------------
 1 file changed, 19 insertions(+), 22 deletions(-)

Perform a few trivial code cleanups, to improve the accuracy and wording of a couple of comments and the module description, and to avoid line wrapping in a few places by using the 100-column width a bit better.

No intended functional changes are introduced by these code cleanups.

---
 drivers/regulator/rk808-regulator.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

index 14b60abd6afc..e81dbb14a29e 100644
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Regulator driver for Rockchip RK805/RK808/RK818
+ * Regulator driver for Rockchip RK80x and RK81x PMIC series
  *
  * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
  * Copyright (c) 2021 Rockchip Electronics Co., Ltd.

@@ -23,7 +23,7 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/gpio/consumer.h>
 
-/* Field Definitions */
+/* Field definitions */
 #define RK808_BUCK_VSEL_MASK	0x3f
 #define RK808_BUCK4_VSEL_MASK	0xf
 #define RK808_LDO_VSEL_MASK	0x1f

@@ -1829,36 +1829,33 @@ static const struct regulator_desc rk818_reg[] = {
 		RK818_DCDC_EN_REG, BIT(7)),
 };
 
-static int rk808_regulator_dt_parse_pdata(struct device *dev,
-				   struct regmap *map,
-				   struct rk808_regulator_data *pdata)
+static int rk808_regulator_dt_parse_pdata(struct device *dev, struct regmap *map,
+					  struct rk808_regulator_data *pdata)
 {
 	struct device_node *np;
 	int tmp, ret = 0, i;
 
 	np = of_get_child_by_name(dev->of_node, "regulators");
 	if (!np)
 		return -ENXIO;
 
 	for (i = 0; i < ARRAY_SIZE(pdata->dvs_gpio); i++) {
 		pdata->dvs_gpio[i] =
-			devm_gpiod_get_index_optional(dev, "dvs", i,
-						      GPIOD_OUT_LOW);
+			devm_gpiod_get_index_optional(dev, "dvs", i, GPIOD_OUT_LOW);
 		if (IS_ERR(pdata->dvs_gpio[i])) {
 			ret = PTR_ERR(pdata->dvs_gpio[i]);
 			dev_err(dev, "failed to get dvs%d gpio (%d)\n", i, ret);
 			goto dt_parse_end;
 		}
 
 		if (!pdata->dvs_gpio[i]) {
 			dev_info(dev, "there is no dvs%d gpio\n", i);
 			continue;
 		}
 
 		tmp = i ? RK808_DVS2_POL : RK808_DVS1_POL;
 		ret = regmap_update_bits(map, RK808_IO_POL_REG, tmp,
-				gpiod_is_active_low(pdata->dvs_gpio[i]) ?
-				0 : tmp);
+					 gpiod_is_active_low(pdata->dvs_gpio[i]) ? 0 : tmp);
 	}
 
 dt_parse_end:

@@ -1954,7 +1951,7 @@ static struct platform_driver rk808_regulator_driver = {
 
 module_platform_driver(rk808_regulator_driver);
 
-MODULE_DESCRIPTION("regulator driver for the RK805/RK808/RK818 series PMICs");
+MODULE_DESCRIPTION("Rockchip RK80x/RK81x PMIC series regulator driver");
 MODULE_AUTHOR("Tony xie <[email protected]>");
 MODULE_AUTHOR("Chris Zhong <[email protected]>");
 MODULE_AUTHOR("Zhang Qing <[email protected]>");

Improve error handling in the probe path by using function dev_err_probe() instead of function dev_err(), where appropriate.

---
 drivers/regulator/rk808-regulator.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

index e81dbb14a29e..f241bb538e27 100644
@@ -1843,8 +1843,8 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev, struct regmap *map
 		pdata->dvs_gpio[i] =
 			devm_gpiod_get_index_optional(dev, "dvs", i, GPIOD_OUT_LOW);
 		if (IS_ERR(pdata->dvs_gpio[i])) {
-			ret = PTR_ERR(pdata->dvs_gpio[i]);
-			dev_err(dev, "failed to get dvs%d gpio (%d)\n", i, ret);
+			ret = dev_err_probe(dev, PTR_ERR(pdata->dvs_gpio[i]),
+					    "failed to get dvs%d gpio\n", i);
 			goto dt_parse_end;
 		}
 

@@ -1920,9 +1920,8 @@ static int rk808_regulator_probe(struct platform_device *pdev)
 		nregulators = RK818_NUM_REGULATORS;
 		break;
 	default:
-		dev_err(&pdev->dev, "unsupported RK8XX ID %lu\n",
-			rk808->variant);
-		return -EINVAL;
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "unsupported RK8xx ID %lu\n", rk808->variant);
 	}
 
 	config.dev = &pdev->dev;

The rk808-regulator driver supports multiple PMIC variants from the Rockckip RK80x and RK81x series, but the DVS GPIOs are supported on the RK808 variant only, according to the DT bindings [1][2][3][4][5][6] and the datasheets for the supported PMIC variants. [7][8][9][10][11][12]

Thus, change the probe path so the "dvs-gpios" property is checked for and its value possibly used only when the handled PMIC variant is RK808. There's no point in doing that on the other PMIC variants, because they don't support the DVS GPIOs, and it goes against the DT bindings to allow a possible out- of-place "dvs-gpios" property to actually be handled in the driver.

This eliminates the following messages, emitted when the "dvs-gpios" property isn't found in the DT, from the kernel log on boards that actually don't use the RK808 variant, which may have provided a source of confusion:

rk808-regulator rk808-regulator.2.auto: there is no dvs0 gpio rk808-regulator rk808-regulator.2.auto: there is no dvs1 gpio

Furthermore, demote these kernel messages to debug messages, because they are useful during the board bringup phase only. Emitting them afterwards, on the boards that use the RK808 variant, but actually don't use the DVS0/1 GPIOs, clutters the kernel log a bit, while they provide no value and may actually cause false impression that some PMIC-related issues are present.

[1] Documentation/devicetree/bindings/mfd/rockchip,rk805.yaml
[2] Documentation/devicetree/bindings/mfd/rockchip,rk806.yaml
[3] Documentation/devicetree/bindings/mfd/rockchip,rk808.yaml
[4] Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml
[5] Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
[6] Documentation/devicetree/bindings/mfd/rockchip,rk818.yaml
[7] https://rockchip.fr/RK805%20datasheet%20V1.2.pdf
[8] https://wmsc.lcsc.com/wmsc/upload/file/pdf/v2/lcsc/2401261533_Rockchip-RK806-1_C5156483.pdf
[9] https://rockchip.fr/RK808%20datasheet%20V1.4.pdf
[10] https://rockchip.fr/RK816%20datasheet%20V1.3.pdf
[11] https://rockchip.fr/RK817%20datasheet%20V1.01.pdf
[12] https://rockchip.fr/RK818%20datasheet%20V1.0.pdf

---
 drivers/regulator/rk808-regulator.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

index f241bb538e27..76f9a426450d 100644
@@ -1849,7 +1849,7 @@ static int rk808_regulator_dt_parse_pdata(struct device *dev, struct regmap *map
 		}
 
 		if (!pdata->dvs_gpio[i]) {
-			dev_info(dev, "there is no dvs%d gpio\n", i);
+			dev_dbg(dev, "there is no dvs%d gpio\n", i);
 			continue;
 		}
 

@@ -1884,22 +1884,21 @@ static int rk808_regulator_probe(struct platform_device *pdev)
 	if (!pdata)
 		return -ENOMEM;
 
-	ret = rk808_regulator_dt_parse_pdata(&pdev->dev, regmap, pdata);
-	if (ret < 0)
-		return ret;
-
-	platform_set_drvdata(pdev, pdata);
-
 	switch (rk808->variant) {
 	case RK805_ID:
 		regulators = rk805_reg;
 		nregulators = RK805_NUM_REGULATORS;
 		break;
 	case RK806_ID:
 		regulators = rk806_reg;
 		nregulators = ARRAY_SIZE(rk806_reg);
 		break;
 	case RK808_ID:
+		/* DVS0/1 GPIOs are supported on the RK808 only */
+		ret = rk808_regulator_dt_parse_pdata(&pdev->dev, regmap, pdata);
+		if (ret < 0)
+			return ret;
+
 		regulators = rk808_reg;
 		nregulators = RK808_NUM_REGULATORS;
 		break;

@@ -1924,6 +1923,8 @@ static int rk808_regulator_probe(struct platform_device *pdev)
 				     "unsupported RK8xx ID %lu\n", rk808->variant);
 	}
 
+	platform_set_drvdata(pdev, pdata);
+
 	config.dev = &pdev->dev;
 	config.driver_data = pdata;
 	config.regmap = regmap;

Applied to

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

Thanks!

[1/3] regulator: rk808: Perform trivial code cleanups

      commit: 1bc3f0e9868929211c6abeb7e79b566c27fe9bb4
[2/3] regulator: rk808: Use dev_err_probe() in the probe path
      commit: fde993ae0b6e5edab0563897e232b94d1d2165a4
[3/3] regulator: rk808: Restrict DVS GPIOs to the RK808 variant only
      commit: 33a846e88481cc4659c6d670759b6295903e43b9

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