[PATCH v2 0/2] Match dtat improvements for pv880x0 regulator driver

[PATCH v2 0/2] Match dtat improvements for pv880x0 regulator driver

[Biju Das]

This patch series aims to add match data improvements for pv880x0
regulator driver.

This patch is only compile tested.

v1->v2:
 * Moved OF/ID tables close to the user
 * Removed trailing comma in the terminator entry for OF/ID tables.
 * Created patc#2 for dropping CONFIG_OF ifdeffery.

Biju Das (2):
  regulator: pv880x0: Simplify probe()
  regulator: pv880x0: Drop ifdeffery

 drivers/regulator/pv88080-regulator.c | 58 ++++++++-------------------
 1 file changed, 17 insertions(+), 41 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] regulator: pv880x0: Simplify probe()

[Biju Das]

Replace pv88080_types->pv88080_compatible_regmap in OF/ID tables and
simplify the probe() by replacing of_match_node() and ID lookup for
retrieving match data by i2c_get_match_data(). After this there is
no user of enum pv88080_types. So drop it.

While at it, remove trailing comma in the terminator entry for OF/ID
tables.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Moved OF/ID tables close to the user
 * Removed trailing comma in the terminator entry for OF/ID tables.
---
 drivers/regulator/pv88080-regulator.c | 57 ++++++++-------------------
 1 file changed, 17 insertions(+), 40 deletions(-)

diff --git a/drivers/regulator/pv88080-regulator.c b/drivers/regulator/pv88080-regulator.c
index 7ab3e4a9bd28..4009239b6784 100644
--- a/drivers/regulator/pv88080-regulator.c
+++ b/drivers/regulator/pv88080-regulator.c
@@ -28,11 +28,6 @@ enum {
 	PV88080_ID_HVBUCK,
 };
 
-enum pv88080_types {
-	TYPE_PV88080_AA,
-	TYPE_PV88080_BA,
-};
-
 struct pv88080_regulator {
 	struct regulator_desc desc;
 	unsigned int mode_reg;
@@ -196,16 +191,6 @@ static const struct pv88080_compatible_regmap pv88080_ba_regs = {
 	.hvbuck_vsel_mask		  = PV88080_VHVBUCK_MASK,
 };
 
-#ifdef CONFIG_OF
-static const struct of_device_id pv88080_dt_ids[] = {
-	{ .compatible = "pvs,pv88080",    .data = (void *)TYPE_PV88080_AA },
-	{ .compatible = "pvs,pv88080-aa", .data = (void *)TYPE_PV88080_AA },
-	{ .compatible = "pvs,pv88080-ba", .data = (void *)TYPE_PV88080_BA },
-	{},
-};
-MODULE_DEVICE_TABLE(of, pv88080_dt_ids);
-#endif
-
 static unsigned int pv88080_buck_get_mode(struct regulator_dev *rdev)
 {
 	struct pv88080_regulator *info = rdev_get_drvdata(rdev);
@@ -376,11 +361,9 @@ static irqreturn_t pv88080_irq_handler(int irq, void *data)
  */
 static int pv88080_i2c_probe(struct i2c_client *i2c)
 {
-	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
 	struct regulator_init_data *init_data = dev_get_platdata(&i2c->dev);
 	struct pv88080 *chip;
 	const struct pv88080_compatible_regmap *regmap_config;
-	const struct of_device_id *match;
 	struct regulator_config config = { };
 	int i, error, ret;
 	unsigned int conf2, conf5;
@@ -398,16 +381,9 @@ static int pv88080_i2c_probe(struct i2c_client *i2c)
 		return error;
 	}
 
-	if (i2c->dev.of_node) {
-		match = of_match_node(pv88080_dt_ids, i2c->dev.of_node);
-		if (!match) {
-			dev_err(chip->dev, "Failed to get of_match_node\n");
-			return -EINVAL;
-		}
-		chip->type = (unsigned long)match->data;
-	} else {
-		chip->type = id->driver_data;
-	}
+	chip->regmap_config = i2c_get_match_data(i2c);
+	if (!chip->regmap_config)
+		return -ENODEV;
 
 	i2c_set_clientdata(i2c, chip);
 
@@ -452,15 +428,6 @@ static int pv88080_i2c_probe(struct i2c_client *i2c)
 		dev_warn(chip->dev, "No IRQ configured\n");
 	}
 
-	switch (chip->type) {
-	case TYPE_PV88080_AA:
-		chip->regmap_config = &pv88080_aa_regs;
-		break;
-	case TYPE_PV88080_BA:
-		chip->regmap_config = &pv88080_ba_regs;
-		break;
-	}
-
 	regmap_config = chip->regmap_config;
 	config.dev = chip->dev;
 	config.regmap = chip->regmap;
@@ -546,11 +513,21 @@ static int pv88080_i2c_probe(struct i2c_client *i2c)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id pv88080_dt_ids[] = {
+	{ .compatible = "pvs,pv88080",    .data = &pv88080_aa_regs },
+	{ .compatible = "pvs,pv88080-aa", .data = &pv88080_aa_regs  },
+	{ .compatible = "pvs,pv88080-ba", .data = &pv88080_ba_regs },
+	{}
+};
+MODULE_DEVICE_TABLE(of, pv88080_dt_ids);
+#endif
+
 static const struct i2c_device_id pv88080_i2c_id[] = {
-	{ "pv88080",    TYPE_PV88080_AA },
-	{ "pv88080-aa", TYPE_PV88080_AA },
-	{ "pv88080-ba", TYPE_PV88080_BA },
-	{},
+	{ "pv88080",    (kernel_ulong_t)&pv88080_aa_regs },
+	{ "pv88080-aa", (kernel_ulong_t)&pv88080_aa_regs },
+	{ "pv88080-ba", (kernel_ulong_t)&pv88080_ba_regs },
+	{}
 };
 MODULE_DEVICE_TABLE(i2c, pv88080_i2c_id);
 
-- 
2.25.1

Re: [PATCH v2 1/2] regulator: pv880x0: Simplify probe()

[Andy Shevchenko]

On Mon, Aug 28, 2023 at 06:24:16PM +0100, Biju Das wrote:
> Replace pv88080_types->pv88080_compatible_regmap in OF/ID tables and
> simplify the probe() by replacing of_match_node() and ID lookup for
> retrieving match data by i2c_get_match_data(). After this there is
> no user of enum pv88080_types. So drop it.
> 
> While at it, remove trailing comma in the terminator entry for OF/ID
> tables.

...

> +#ifdef CONFIG_OF

Do you still need this? (Perhaps in a separate patch to remove?)

...

> +static const struct of_device_id pv88080_dt_ids[] = {
> +	{ .compatible = "pvs,pv88080",    .data = &pv88080_aa_regs },
> +	{ .compatible = "pvs,pv88080-aa", .data = &pv88080_aa_regs  },

One space too many.

> +	{ .compatible = "pvs,pv88080-ba", .data = &pv88080_ba_regs },
> +	{}
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] regulator: pv880x0: Simplify probe()

[Andy Shevchenko]

On Tue, Aug 29, 2023 at 06:11:36PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 28, 2023 at 06:24:16PM +0100, Biju Das wrote:

...

> > +#ifdef CONFIG_OF
> 
> Do you still need this? (Perhaps in a separate patch to remove?)

Ah, it's a second in the series!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/2] regulator: pv880x0: Drop CONFIG_OF ifdeffery

[Biju Das]

Get rid of ugly CONFIG_OF ifdeffery by adding mod_devicetable.h include.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2:
 * New patch.
---
 drivers/regulator/pv88080-regulator.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/pv88080-regulator.c b/drivers/regulator/pv88080-regulator.c
index 4009239b6784..2404df78d639 100644
--- a/drivers/regulator/pv88080-regulator.c
+++ b/drivers/regulator/pv88080-regulator.c
@@ -5,6 +5,7 @@
 
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/init.h>
@@ -513,7 +514,6 @@ static int pv88080_i2c_probe(struct i2c_client *i2c)
 	return 0;
 }
 
-#ifdef CONFIG_OF
 static const struct of_device_id pv88080_dt_ids[] = {
 	{ .compatible = "pvs,pv88080",    .data = &pv88080_aa_regs },
 	{ .compatible = "pvs,pv88080-aa", .data = &pv88080_aa_regs  },
@@ -521,7 +521,6 @@ static const struct of_device_id pv88080_dt_ids[] = {
 	{}
 };
 MODULE_DEVICE_TABLE(of, pv88080_dt_ids);
-#endif
 
 static const struct i2c_device_id pv88080_i2c_id[] = {
 	{ "pv88080",    (kernel_ulong_t)&pv88080_aa_regs },
@@ -535,7 +534,7 @@ static struct i2c_driver pv88080_regulator_driver = {
 	.driver = {
 		.name = "pv88080",
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
-		.of_match_table = of_match_ptr(pv88080_dt_ids),
+		.of_match_table = pv88080_dt_ids,
 	},
 	.probe = pv88080_i2c_probe,
 	.id_table = pv88080_i2c_id,
-- 
2.25.1

Re: [PATCH v2 2/2] regulator: pv880x0: Drop CONFIG_OF ifdeffery

[Andy Shevchenko]

On Mon, Aug 28, 2023 at 06:24:17PM +0100, Biju Das wrote:
> Get rid of ugly CONFIG_OF ifdeffery by adding mod_devicetable.h include.

Commit message needs to be improved.
Inclusion of that header doesn't allow to do what you have done.
Dropping of_match_ptr() does. And you need to explain that leaving
OF table for non-OF system is fine and doesn't waste too much memory
anyway. Look for the existing changes like this in the kernel history
(`git log --no-merges --grep ...` at your service).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] Match dtat improvements for pv880x0 regulator driver

[Mark Brown]

On Mon, 28 Aug 2023 18:24:15 +0100, Biju Das wrote:
> This patch series aims to add match data improvements for pv880x0
> regulator driver.
> 
> This patch is only compile tested.
> 
> v1->v2:
>  * Moved OF/ID tables close to the user
>  * Removed trailing comma in the terminator entry for OF/ID tables.
>  * Created patc#2 for dropping CONFIG_OF ifdeffery.
> 
> [...]

Applied to

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

Thanks!

[1/2] regulator: pv880x0: Simplify probe()
      commit: 1b4daf643a3a21cff4bcad5b799960d5a0673d11
[2/2] regulator: pv880x0: Drop CONFIG_OF ifdeffery
      (no commit info)

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