[PATCH V1 0/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[PATCH V1 0/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[like]

From: Alec Li <like@awinic.com>

Add regulator driver for the device AWINIC AW37503 which is single
inductor - dual output power supply device. AW37503 device is
designed to support general positive/negative driven applications
like TFT display panels.

Alec Li (2):
  regulator: aw37503: add regulator driver for AWINIC AW37503
  regulator: aw37503: add device-tree binding

 .../bindings/regulator/awinic,aw37503.yaml    |  73 ++++++
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/aw37503-regulator.c         | 246 ++++++++++++++++++
 4 files changed, 328 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
 create mode 100644 drivers/regulator/aw37503-regulator.c


base-commit: 18b44bc5a67275641fb26f2c54ba7eef80ac5950
-- 
2.41.0

[PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[like]

From: Alec Li <like@awinic.com>

Add regulator driver for the device AWINIC AW37503 which is single
inductor - dual output power supply device. AW37503 device is
designed to support general positive/negative driven applications
like TFT display panels.

AW37503 regulator driver supports to enable/disable and set voltage
on its output.

Signed-off-by: Alec Li <like@awinic.com>
---
 drivers/regulator/Kconfig             |   8 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/aw37503-regulator.c | 246 ++++++++++++++++++++++++++
 3 files changed, 255 insertions(+)
 create mode 100644 drivers/regulator/aw37503-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 823f8e6e4801..4c2a8a54f1d8 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1618,4 +1618,12 @@ config REGULATOR_QCOM_LABIBB
 	  boost regulator and IBB can be used as a negative boost regulator
 	  for LCD display panel.
 
+config REGULATOR_AW37503
+	tristate "AWINIC AW37503 Dual Output Power regulators"
+	depends on I2C && GPIOLIB
+	select REGMAP_I2C
+	help
+	  This driver supports AW37503 single inductor - dual output
+	  power supply specifically designed for display panels.
+
 endif
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 15e0d614ff66..96acea0d0ae1 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -191,5 +191,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
+obj-$(CONFIG_REGULATOR_AW37503) += aw37503-regulator.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/aw37503-regulator.c b/drivers/regulator/aw37503-regulator.c
new file mode 100644
index 000000000000..5c4acbda3a82
--- /dev/null
+++ b/drivers/regulator/aw37503-regulator.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// AWINIC AW37503 Regulator Driver
+//
+// Copyright (C) 2023 awinic. All Rights Reserved
+//
+// Author: <like@awinic.com>
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+#define AW37503_REG_VPOS		0x00
+#define AW37503_REG_VNEG		0x01
+#define AW37503_REG_APPS		0x03
+#define AW37503_REG_CONTROL		0x04
+#define AW37503_REG_WPRTEN		0x21
+
+#define AW37503_VOUT_MASK		0x1F
+#define AW37503_VOUT_N_VOLTAGE		0x15
+#define AW37503_VOUT_VMIN		4000000
+#define AW37503_VOUT_VMAX		6000000
+#define AW37503_VOUT_STEP		100000
+
+#define AW37503_REG_APPS_DIS_VPOS	BIT(1)
+#define AW37503_REG_APPS_DIS_VNEG	BIT(0)
+
+#define AW37503_REGULATOR_ID_VPOS	0
+#define AW37503_REGULATOR_ID_VNEG	1
+#define AW37503_MAX_REGULATORS		2
+
+struct aw37503_reg_pdata {
+	struct gpio_desc *en_gpiod;
+	int ena_gpio_state;
+};
+
+struct aw37503_regulator {
+	struct device *dev;
+	struct aw37503_reg_pdata reg_pdata[AW37503_MAX_REGULATORS];
+};
+
+static int aw37503_regulator_enable(struct regulator_dev *rdev)
+{
+	struct aw37503_regulator *chip = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct aw37503_reg_pdata *rpdata = &chip->reg_pdata[id];
+	int ret;
+
+	if (!IS_ERR(rpdata->en_gpiod)) {
+		gpiod_set_value_cansleep(rpdata->en_gpiod, 1);
+		rpdata->ena_gpio_state = 1;
+	}
+
+	/* Hardware automatically enable discharge bit in enable */
+	if (rdev->constraints->active_discharge ==
+			REGULATOR_ACTIVE_DISCHARGE_DISABLE) {
+		ret = regulator_set_active_discharge_regmap(rdev, false);
+		if (ret < 0) {
+			dev_err(chip->dev, "Failed to disable active discharge: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int aw37503_regulator_disable(struct regulator_dev *rdev)
+{
+	struct aw37503_regulator *chip = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct aw37503_reg_pdata *rpdata = &chip->reg_pdata[id];
+
+	if (!IS_ERR(rpdata->en_gpiod)) {
+		gpiod_set_value_cansleep(rpdata->en_gpiod, 0);
+		rpdata->ena_gpio_state = 0;
+	}
+
+	return 0;
+}
+
+static int aw37503_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct aw37503_regulator *chip = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct aw37503_reg_pdata *rpdata = &chip->reg_pdata[id];
+
+	if (!IS_ERR(rpdata->en_gpiod))
+		return rpdata->ena_gpio_state;
+
+	return 1;
+}
+
+static const struct regulator_ops aw37503_regulator_ops = {
+	.enable = aw37503_regulator_enable,
+	.disable = aw37503_regulator_disable,
+	.is_enabled = aw37503_regulator_is_enabled,
+	.list_voltage = regulator_list_voltage_linear,
+	.map_voltage = regulator_map_voltage_linear,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+};
+
+static int aw37503_of_parse_cb(struct device_node *np,
+				const struct regulator_desc *desc,
+				struct regulator_config *config)
+{
+	struct aw37503_regulator *chip = config->driver_data;
+	struct aw37503_reg_pdata *rpdata = &chip->reg_pdata[desc->id];
+	int ret;
+
+	rpdata->en_gpiod = devm_fwnode_gpiod_get(chip->dev, of_fwnode_handle(np),
+						 "enable", GPIOD_OUT_LOW,
+						 "enable");
+
+	if (IS_ERR(rpdata->en_gpiod)) {
+		ret = PTR_ERR(rpdata->en_gpiod);
+
+		/* Ignore the error other than probe defer */
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		return 0;
+	}
+
+	return 0;
+}
+
+#define AW37503_REGULATOR_DESC(_id, _name)		\
+	[AW37503_REGULATOR_ID_##_id] = {		\
+		.name = "aw37503-"#_name,		\
+		.supply_name = "vin",			\
+		.id = AW37503_REGULATOR_ID_##_id,	\
+		.of_match = of_match_ptr(#_name),	\
+		.of_parse_cb = aw37503_of_parse_cb,	\
+		.ops = &aw37503_regulator_ops,		\
+		.n_voltages = AW37503_VOUT_N_VOLTAGE,	\
+		.min_uV = AW37503_VOUT_VMIN,		\
+		.uV_step = AW37503_VOUT_STEP,		\
+		.enable_time = 500,			\
+		.vsel_mask = AW37503_VOUT_MASK,	\
+		.vsel_reg = AW37503_REG_##_id,		\
+		.active_discharge_off = 0,			\
+		.active_discharge_on = AW37503_REG_APPS_DIS_##_id, \
+		.active_discharge_mask = AW37503_REG_APPS_DIS_##_id, \
+		.active_discharge_reg = AW37503_REG_APPS, \
+		.type = REGULATOR_VOLTAGE,		\
+		.owner = THIS_MODULE,			\
+	}
+
+static const struct regulator_desc aw_regs_desc[AW37503_MAX_REGULATORS] = {
+	AW37503_REGULATOR_DESC(VPOS, outp),
+	AW37503_REGULATOR_DESC(VNEG, outn),
+};
+
+static const struct regmap_range aw37503_no_reg_ranges[] = {
+	regmap_reg_range(AW37503_REG_CONTROL + 1,
+			 AW37503_REG_WPRTEN - 1),
+};
+
+static const struct regmap_access_table aw37503_no_reg_table = {
+	.no_ranges = aw37503_no_reg_ranges,
+	.n_no_ranges = ARRAY_SIZE(aw37503_no_reg_ranges),
+};
+
+static const struct regmap_config aw37503_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.max_register	= AW37503_REG_WPRTEN,
+	.cache_type	= REGCACHE_NONE,
+	.rd_table	= &aw37503_no_reg_table,
+	.wr_table	= &aw37503_no_reg_table,
+};
+
+static int aw37503_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct aw37503_regulator *chip;
+	struct regulator_dev *rdev;
+	struct regmap *regmap;
+	struct regulator_config config = { };
+	int id;
+	int ret;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_i2c(client, &aw37503_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(dev, "regmap init failed: %d\n", ret);
+		return ret;
+	}
+
+	i2c_set_clientdata(client, chip);
+	chip->dev = dev;
+
+	for (id = 0; id < AW37503_MAX_REGULATORS; ++id) {
+		config.regmap = regmap;
+		config.dev = dev;
+		config.driver_data = chip;
+
+		rdev = devm_regulator_register(dev, &aw_regs_desc[id],
+					       &config);
+		if (IS_ERR(rdev)) {
+			ret = PTR_ERR(rdev);
+			dev_err(dev, "regulator %s register failed: %d\n",
+				aw_regs_desc[id].name, ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+static const struct i2c_device_id aw37503_id[] = {
+	{.name = "aw37503",},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, aw37503_id);
+
+static const struct of_device_id aw37503_dt_ids[] = {
+	{.compatible = "awinic,aw37503",},
+	{ /* Sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, aw37503_dt_ids);
+
+static struct i2c_driver aw37503_i2c_driver = {
+	.driver = {
+		.name = "aw37503",
+		.of_match_table = of_match_ptr(aw37503_dt_ids),
+	},
+	.probe_new = aw37503_probe,
+	.id_table = aw37503_id,
+};
+
+module_i2c_driver(aw37503_i2c_driver);
+
+MODULE_DESCRIPTION("aw37503 regulator driver");
+MODULE_AUTHOR("Alec Li <like@awinic.com>");
+MODULE_LICENSE("GPL");
-- 
2.41.0

Re: [PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[Krzysztof Kozlowski]

On 26/07/2023 10:16, like@awinic.com wrote:
> From: Alec Li <like@awinic.com>
> 
> Add regulator driver for the device AWINIC AW37503 which is single
> inductor - dual output power supply device. AW37503 device is
> designed to support general positive/negative driven applications
> like TFT display panels.
> 

Thank you for your patch. There is something to discuss/improve.


> +
> +static int aw37503_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct aw37503_regulator *chip;
> +	struct regulator_dev *rdev;
> +	struct regmap *regmap;
> +	struct regulator_config config = { };
> +	int id;
> +	int ret;
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &aw37503_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(dev, "regmap init failed: %d\n", ret);
> +		return ret;

return dev_err_probe

> +	}
> +
> +	i2c_set_clientdata(client, chip);
> +	chip->dev = dev;
> +
> +	for (id = 0; id < AW37503_MAX_REGULATORS; ++id) {
> +		config.regmap = regmap;
> +		config.dev = dev;
> +		config.driver_data = chip;
> +
> +		rdev = devm_regulator_register(dev, &aw_regs_desc[id],
> +					       &config);
> +		if (IS_ERR(rdev)) {
> +			ret = PTR_ERR(rdev);
> +			dev_err(dev, "regulator %s register failed: %d\n",
> +				aw_regs_desc[id].name, ret);
> +			return ret;

return dev_err_probe will be easier

> +		}
> +	}
> +	return 0;
> +}
> +
> +static const struct i2c_device_id aw37503_id[] = {
> +	{.name = "aw37503",},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, aw37503_id);
> +
> +static const struct of_device_id aw37503_dt_ids[] = {
> +	{.compatible = "awinic,aw37503",},
> +	{ /* Sentinel */ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, aw37503_dt_ids);
> +
> +static struct i2c_driver aw37503_i2c_driver = {
> +	.driver = {
> +		.name = "aw37503",
> +		.of_match_table = of_match_ptr(aw37503_dt_ids),

Drop of_match_ptr()


Best regards,
Krzysztof

Re: [PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[like]

Hello Krzysztof Kozlowski,
Thank you very much for your advice.

> On 26/07/2023 10:16, like@awinic.com wrote:
> > From: Alec Li <like@awinic.com>
> > 
> > Add regulator driver for the device AWINIC AW37503 which is single
> > inductor - dual output power supply device. AW37503 device is
> > designed to support general positive/negative driven applications
> > like TFT display panels.
> > 
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> 
> > +
> > +static int aw37503_probe(struct i2c_client *client)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct aw37503_regulator *chip;
> > +	struct regulator_dev *rdev;
> > +	struct regmap *regmap;
> > +	struct regulator_config config = { };
> > +	int id;
> > +	int ret;
> > +
> > +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> > +	if (!chip)
> > +		return -ENOMEM;
> > +
> > +	regmap = devm_regmap_init_i2c(client, &aw37503_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		ret = PTR_ERR(regmap);
> > +		dev_err(dev, "regmap init failed: %d\n", ret);
> > +		return ret;
> 
> return dev_err_probe

I will modify it as suggested.

> > +	}
> > +
> > +	i2c_set_clientdata(client, chip);
> > +	chip->dev = dev;
> > +
> > +	for (id = 0; id < AW37503_MAX_REGULATORS; ++id) {
> > +		config.regmap = regmap;
> > +		config.dev = dev;
> > +		config.driver_data = chip;
> > +
> > +		rdev = devm_regulator_register(dev, &aw_regs_desc[id],
> > +					       &config);
> > +		if (IS_ERR(rdev)) {
> > +			ret = PTR_ERR(rdev);
> > +			dev_err(dev, "regulator %s register failed: %d\n",
> > +				aw_regs_desc[id].name, ret);
> > +			return ret;
> 
> return dev_err_probe will be easier

I will modify it as suggested.

> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +static const struct i2c_device_id aw37503_id[] = {
> > +	{.name = "aw37503",},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(i2c, aw37503_id);
> > +
> > +static const struct of_device_id aw37503_dt_ids[] = {
> > +	{.compatible = "awinic,aw37503",},
> > +	{ /* Sentinel */ },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, aw37503_dt_ids);
> > +
> > +static struct i2c_driver aw37503_i2c_driver = {
> > +	.driver = {
> > +		.name = "aw37503",
> > +		.of_match_table = of_match_ptr(aw37503_dt_ids),
> 
> Drop of_match_ptr()

I will modify it as suggested.


Best regards,
Alec Li

Re: [PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[Mark Brown]

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

On Wed, Jul 26, 2023 at 08:16:11AM +0000, like@awinic.com wrote:

> @@ -191,5 +191,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
>  obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>  obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
> +obj-$(CONFIG_REGULATOR_AW37503) += aw37503-regulator.o
>  

Please keep the Kconfig and Makefile sorted.

> +static const struct regmap_config aw37503_regmap_config = {
> +	.reg_bits	= 8,
> +	.val_bits	= 8,
> +	.max_register	= AW37503_REG_WPRTEN,
> +	.cache_type	= REGCACHE_NONE,

No need to specify no cache, it's the default.

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

Re: [PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[like]

Hello Mark Brown,
Thank you very much for your advice.

> On Wed, Jul 26, 2023 at 08:16:11AM +0000, like@awinic.com wrote:
> 
> > @@ -191,5 +191,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
> >  obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
> >  obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
> >  obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
> > +obj-$(CONFIG_REGULATOR_AW37503) += aw37503-regulator.o
> >  
> 
> Please keep the Kconfig and Makefile sorted.

I will modify the sorting of AW37503 as suggested.

> > +static const struct regmap_config aw37503_regmap_config = {
> > +	.reg_bits	= 8,
> > +	.val_bits	= 8,
> > +	.max_register	= AW37503_REG_WPRTEN,
> > +	.cache_type	= REGCACHE_NONE,
> 
> No need to specify no cache, it's the default.

I will modify it as suggested.


Best regards,
Alec Li

Re: [PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[Christophe JAILLET]

Le 26/07/2023 à 10:16, like-tUEr1MkLeujQT0dZR+AlfA@public.gmane.org a 
écrit :
> From: Alec Li <like-tUEr1MkLeujQT0dZR+AlfA@public.gmane.org>
> 
> Add regulator driver for the device AWINIC AW37503 which is single
> inductor - dual output power supply device. AW37503 device is
> designed to support general positive/negative driven applications
> like TFT display panels.
> 
> AW37503 regulator driver supports to enable/disable and set voltage
> on its output.
> 
> Signed-off-by: Alec Li <like-tUEr1MkLeujQT0dZR+AlfA@public.gmane.org>

[...]

> +static int aw37503_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct aw37503_regulator *chip;
> +	struct regulator_dev *rdev;
> +	struct regmap *regmap;
> +	struct regulator_config config = { };
> +	int id;
> +	int ret;
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &aw37503_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(dev, "regmap init failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	i2c_set_clientdata(client, chip);
> +	chip->dev = dev;
> +
> +	for (id = 0; id < AW37503_MAX_REGULATORS; ++id) {
> +		config.regmap = regmap;
> +		config.dev = dev;
> +		config.driver_data = chip;

I think that these 3 lines could be outside of the loop.

> +
> +		rdev = devm_regulator_register(dev, &aw_regs_desc[id],
> +					       &config);
> +		if (IS_ERR(rdev)) {
> +			ret = PTR_ERR(rdev);
> +			dev_err(dev, "regulator %s register failed: %d\n",
> +				aw_regs_desc[id].name, ret);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}

[...]

Re: [PATCH V1 1/2] regulator: aw37503: add regulator driver for AWINIC AW37503

[like]

Hello Christophe JAILLET,
Thank you very much for your advice.

> > From: Alec Li <like-tUEr1MkLeujQT0dZR+AlfA@public.gmane.org>
> > 
> > Add regulator driver for the device AWINIC AW37503 which is single
> > inductor - dual output power supply device. AW37503 device is
> > designed to support general positive/negative driven applications
> > like TFT display panels.
> > 
> > AW37503 regulator driver supports to enable/disable and set voltage
> > on its output.
> > 
> > Signed-off-by: Alec Li <like-tUEr1MkLeujQT0dZR+AlfA@public.gmane.org>
> 
> [...]
> 
> > +static int aw37503_probe(struct i2c_client *client)
> > +{
> > +	struct device *dev = &client->dev;
> > +	struct aw37503_regulator *chip;
> > +	struct regulator_dev *rdev;
> > +	struct regmap *regmap;
> > +	struct regulator_config config = { };
> > +	int id;
> > +	int ret;
> > +
> > +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> > +	if (!chip)
> > +		return -ENOMEM;
> > +
> > +	regmap = devm_regmap_init_i2c(client, &aw37503_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		ret = PTR_ERR(regmap);
> > +		dev_err(dev, "regmap init failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	i2c_set_clientdata(client, chip);
> > +	chip->dev = dev;
> > +
> > +	for (id = 0; id < AW37503_MAX_REGULATORS; ++id) {
> > +		config.regmap = regmap;
> > +		config.dev = dev;
> > +		config.driver_data = chip;
> 
> I think that these 3 lines could be outside of the loop.

I will modify it as suggested.

> > +
> > +		rdev = devm_regulator_register(dev, &aw_regs_desc[id],
> > +					       &config);
> > +		if (IS_ERR(rdev)) {
> > +			ret = PTR_ERR(rdev);
> > +			dev_err(dev, "regulator %s register failed: %d\n",
> > +				aw_regs_desc[id].name, ret);
> > +			return ret;
> > +		}
> > +	}
> > +	return 0;
> > +}


Best regards,
Alec Li

[PATCH V1 2/2] regulator: aw37503: add device-tree binding

[like]

From: Alec Li <like@awinic.com>

Add aw37503 regulator device-tree binding documentation

Signed-off-by: Alec Li <like@awinic.com>
---
 .../bindings/regulator/awinic,aw37503.yaml    | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml

diff --git a/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml b/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
new file mode 100644
index 000000000000..0cd6fb001e20
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/awinic,aw37503.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Awinic AW37503 Voltage Regulator
+
+maintainers:
+  - Alec Li <like@awinic.com>
+
+description: |
+  The AW37503 is dual voltage regulator, designed to support positive/negative
+  supply for driving TFT-LCD panels. It support software-configurable output
+  switching and monitoring. The output voltages can be programmed via an I2C
+  compatible interface.
+
+properties:
+  compatible:
+    const:
+      - awinic,aw37503
+
+  reg:
+    maxItems: 1
+
+  patternProperties:
+    "^out[pn]$":
+      type: object
+      $ref: regulator.yaml#
+      unvaluatedproperties: false
+
+      required:
+        - regulator-name
+        - enable-gpios
+
+      additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - outp
+  - outn
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        aw37503@3e {
+            compatible = "awinic,aw37503";
+            reg = <0x3e>;
+
+            outp {
+                regulator-name = "outp";
+                regulator-boot-on;
+                regulator-always-on;
+                enable-gpios = <&gpio 17 0>;
+            };
+
+            outn {
+                regulator-name = "outn";
+                regulator-boot-on;
+                regulator-always-on;
+                enable-gpios = <&gpio 27 0>;
+            };
+        };
+    };
+...
-- 
2.41.0

Re: [PATCH V1 2/2] regulator: aw37503: add device-tree binding

[Rob Herring]

On Wed, 26 Jul 2023 08:16:12 +0000, like@awinic.com wrote:
> From: Alec Li <like@awinic.com>
> 
> Add aw37503 regulator device-tree binding documentation
> 
> Signed-off-by: Alec Li <like@awinic.com>
> ---
>  .../bindings/regulator/awinic,aw37503.yaml    | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties: 'patternProperties' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
	hint: A json-schema keyword was found instead of a DT property name.
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties:compatible:const: ['awinic,aw37503'] is not of type 'integer', 'string'
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties:patternProperties: 'anyOf' conditional failed, one must be fixed:
	'^out[pn]$' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
	'type' was expected
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties:compatible:const: ['awinic,aw37503'] is not of type 'string'
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230726081612.586295-3-like@awinic.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH V1 2/2] regulator: aw37503: add device-tree binding

[like]

Hello Rob Herring,
Thank you very much for your advice. 

On Wed, 26 Jul 2023 08:16:12 +0000, like@awinic.com wrote:
> > From: Alec Li <like@awinic.com>
> > 
> > Add aw37503 regulator device-tree binding documentation
> > 
> > Signed-off-by: Alec Li <like@awinic.com>
> > ---
> >  .../bindings/regulator/awinic,aw37503.yaml    | 73 +++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties: 'patternProperties' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
> 	hint: A json-schema keyword was found instead of a DT property name.
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties:compatible:const: ['awinic,aw37503'] is not of type 'integer', 'string'
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties:patternProperties: 'anyOf' conditional failed, one must be fixed:
> 	'^out[pn]$' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
> 	'type' was expected
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml: properties:compatible:const: ['awinic,aw37503'] is not of type 'string'
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230726081612.586295-3-like@awinic.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.

It was my mistake. I will run the above command and correct this error on patch v2.


Best regards,
Alec Li

Re: [PATCH V1 2/2] regulator: aw37503: add device-tree binding

[Krzysztof Kozlowski]

On 26/07/2023 10:16, like@awinic.com wrote:
> From: Alec Li <like@awinic.com>
> 
> Add aw37503 regulator device-tree binding documentation

subject rather like:
regulator: dt-bindings: Add Awinic aw37503

> 
> Signed-off-by: Alec Li <like@awinic.com>
> ---
>  .../bindings/regulator/awinic,aw37503.yaml    | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml b/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> new file mode 100644
> index 000000000000..0cd6fb001e20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/awinic,aw37503.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Awinic AW37503 Voltage Regulator
> +
> +maintainers:
> +  - Alec Li <like@awinic.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The AW37503 is dual voltage regulator, designed to support positive/negative
> +  supply for driving TFT-LCD panels. It support software-configurable output
> +  switching and monitoring. The output voltages can be programmed via an I2C
> +  compatible interface.
> +
> +properties:
> +  compatible:
> +    const:
> +      - awinic,aw37503

This has to be in one line. As pointed out by Rob's bot: please test it
before sending.

> +
> +  reg:
> +    maxItems: 1
> +
> +  patternProperties:

<sigh>

> +    "^out[pn]$":
> +      type: object
> +      $ref: regulator.yaml#
> +      unvaluatedproperties: false
> +
> +      required:
> +        - regulator-name
> +        - enable-gpios

regulator.yaml does not define enable-gpios, so you must define it in
properties.

> +
> +      additionalProperties: false

Drop, you already have unevaluatedProperties.

> +
> +required:
> +  - compatible
> +  - reg
> +  - outp
> +  - outn
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        aw37503@3e {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


Best regards,
Krzysztof

Re: [PATCH V1 2/2] regulator: aw37503: add device-tree binding

[like]

Hello Krzysztof Kozlowski,
Thank you very much for your advice.

> On 26/07/2023 10:16, like@awinic.com wrote:
> > From: Alec Li <like@awinic.com>
> > 
> > Add aw37503 regulator device-tree binding documentation
> 
> subject rather like:
> regulator: dt-bindings: Add Awinic aw37503

I will modify it as suggested.

> > 
> > Signed-off-by: Alec Li <like@awinic.com>
> > ---
> >  .../bindings/regulator/awinic,aw37503.yaml    | 73 +++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml b/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> > new file mode 100644
> > index 000000000000..0cd6fb001e20
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/awinic,aw37503.yaml
> > @@ -0,0 +1,73 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/awinic,aw37503.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Awinic AW37503 Voltage Regulator
> > +
> > +maintainers:
> > +  - Alec Li <like@awinic.com>
> > +
> > +description: |
> 
> Do not need '|' unless you need to preserve formatting.

I will modify it as suggested.

> > +  The AW37503 is dual voltage regulator, designed to support positive/negative
> > +  supply for driving TFT-LCD panels. It support software-configurable output
> > +  switching and monitoring. The output voltages can be programmed via an I2C
> > +  compatible interface.
> > +
> > +properties:
> > +  compatible:
> > +    const:
> > +      - awinic,aw37503
> 
> This has to be in one line. As pointed out by Rob's bot: please test it
> before sending.

I will test it before sending and modify it as suggested.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  patternProperties:
> 
> <sigh>
> 
> > +    "^out[pn]$":
> > +      type: object
> > +      $ref: regulator.yaml#
> > +      unvaluatedproperties: false
> > +
> > +      required:
> > +        - regulator-name
> > +        - enable-gpios
> 
> regulator.yaml does not define enable-gpios, so you must define it in
> properties.

I will modify it as suggested.

> > +
> > +      additionalProperties: false
> 
> Drop, you already have unevaluatedProperties.

I will modify it as suggested.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - outp
> > +  - outn
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        aw37503@3e {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

I will modify it as suggested.

 
Best regards,
Alec Li