[PATCH v3 01/10] mfd: da9063: Fix failpath in core

[PATCH v3 01/10] mfd: da9063: Fix failpath in core

[Marek Vasut]

In case mfd_add_devices() fails, da9063_irq_exit() is not called to
undo the IRQchip setup done by da9063_irq_init(). Fix this by adding
the missing fail path.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
---
V3: New patch
---
 drivers/mfd/da9063-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
index 6c2870d4e754..8226ebd8b96d 100644
--- a/drivers/mfd/da9063-core.c
+++ b/drivers/mfd/da9063-core.c
@@ -229,10 +229,16 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
 	ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
 			      ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
 			      NULL);
-	if (ret)
+	if (ret) {
 		dev_err(da9063->dev, "Cannot add MFD cells\n");
+		goto err_irq_exit;
+	}
 
 	return ret;
+
+err_irq_exit:
+	da9063_irq_exit(da9063);
+	return ret;
 }
 
 void da9063_device_exit(struct da9063 *da9063)
-- 
2.16.2

[PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Marek Vasut]

Convert the regmap_irq table to use REGMAP_IRQ_REG().

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
---
V3: New patch
Note: A sed oneliner was used:
      sed -i "/\[DA9063_IRQ_/ {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* = \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
---
 drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
 1 file changed, 29 insertions(+), 116 deletions(-)

diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
index 207bbfe55449..5b406ecfc14a 100644
--- a/drivers/mfd/da9063-irq.c
+++ b/drivers/mfd/da9063-irq.c
@@ -28,125 +28,38 @@
 
 static const struct regmap_irq da9063_irqs[] = {
 	/* DA9063 event A register */
-	[DA9063_IRQ_ONKEY] = {
-		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
-		.mask = DA9063_M_ONKEY,
-	},
-	[DA9063_IRQ_ALARM] = {
-		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
-		.mask = DA9063_M_ALARM,
-	},
-	[DA9063_IRQ_TICK] = {
-		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
-		.mask = DA9063_M_TICK,
-	},
-	[DA9063_IRQ_ADC_RDY] = {
-		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
-		.mask = DA9063_M_ADC_RDY,
-	},
-	[DA9063_IRQ_SEQ_RDY] = {
-		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
-		.mask = DA9063_M_SEQ_RDY,
-	},
+	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
+	REGMAP_IRQ_REG(DA9063_IRQ_ALARM, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
+	REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET, DA9063_M_TICK),
+	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
+	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
 	/* DA9063 event B register */
-	[DA9063_IRQ_WAKE] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_WAKE,
-	},
-	[DA9063_IRQ_TEMP] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_TEMP,
-	},
-	[DA9063_IRQ_COMP_1V2] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_COMP_1V2,
-	},
-	[DA9063_IRQ_LDO_LIM] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_LDO_LIM,
-	},
-	[DA9063_IRQ_REG_UVOV] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_UVOV,
-	},
-	[DA9063_IRQ_DVC_RDY] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_DVC_RDY,
-	},
-	[DA9063_IRQ_VDD_MON] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_VDD_MON,
-	},
-	[DA9063_IRQ_WARN] = {
-		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
-		.mask = DA9063_M_VDD_WARN,
-	},
+	REGMAP_IRQ_REG(DA9063_IRQ_WAKE, DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
+	REGMAP_IRQ_REG(DA9063_IRQ_TEMP, DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
+	REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2, DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
+	REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM, DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
+	REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV, DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
+	REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY, DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
+	REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
+	REGMAP_IRQ_REG(DA9063_IRQ_WARN, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
 	/* DA9063 event C register */
-	[DA9063_IRQ_GPI0] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI0,
-	},
-	[DA9063_IRQ_GPI1] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI1,
-	},
-	[DA9063_IRQ_GPI2] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI2,
-	},
-	[DA9063_IRQ_GPI3] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI3,
-	},
-	[DA9063_IRQ_GPI4] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI4,
-	},
-	[DA9063_IRQ_GPI5] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI5,
-	},
-	[DA9063_IRQ_GPI6] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI6,
-	},
-	[DA9063_IRQ_GPI7] = {
-		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
-		.mask = DA9063_M_GPI7,
-	},
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI0),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI1),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI2),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI3),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI4),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI5),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI6),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI7),
 	/* DA9063 event D register */
-	[DA9063_IRQ_GPI8] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI8,
-	},
-	[DA9063_IRQ_GPI9] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI9,
-	},
-	[DA9063_IRQ_GPI10] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI10,
-	},
-	[DA9063_IRQ_GPI11] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI11,
-	},
-	[DA9063_IRQ_GPI12] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI12,
-	},
-	[DA9063_IRQ_GPI13] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI13,
-	},
-	[DA9063_IRQ_GPI14] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI14,
-	},
-	[DA9063_IRQ_GPI15] = {
-		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
-		.mask = DA9063_M_GPI15,
-	},
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI8),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI9),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI10, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI11, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI12, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI13, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI14, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI15, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
 };
 
 static const struct regmap_irq_chip da9063_irq_chip = {
-- 
2.16.2

[PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Marek Vasut]

The PMIC_DA9063 is a complete misnomer, it denotes the value of the
DA9063 chip ID register, so rename it as such. It is also the value
of chip ID register of DA9063L though, so drop the enum as all the
DA9063 "models" share the same chip ID and thus the distinction will
have to be made using DT or otherwise.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V2: No change
V3: No change
---
 drivers/mfd/da9063-core.c            | 2 +-
 drivers/mfd/da9063-i2c.c             | 2 +-
 drivers/regulator/da9063-regulator.c | 2 +-
 include/linux/mfd/da9063/core.h      | 4 +---
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
index 8226ebd8b96d..fb122316c421 100644
--- a/drivers/mfd/da9063-core.c
+++ b/drivers/mfd/da9063-core.c
@@ -192,7 +192,7 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
 		dev_err(da9063->dev, "Cannot read chip model id.\n");
 		return -EIO;
 	}
-	if (model != PMIC_DA9063) {
+	if (model != PMIC_CHIP_ID_DA9063) {
 		dev_err(da9063->dev, "Invalid chip model id: 0x%02x\n", model);
 		return -ENODEV;
 	}
diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index 981805a2c521..7f84030c8d53 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -280,7 +280,7 @@ static int da9063_i2c_remove(struct i2c_client *i2c)
 }
 
 static const struct i2c_device_id da9063_i2c_id[] = {
-	{"da9063", PMIC_DA9063},
+	{ "da9063", PMIC_CHIP_ID_DA9063 },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, da9063_i2c_id);
diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index 6a8f9cd69f52..87c884ae0064 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -585,7 +585,7 @@ static struct da9063_dev_model regulators_models[] = {
 	{
 		.regulator_info = da9063_regulator_info,
 		.n_regulators = ARRAY_SIZE(da9063_regulator_info),
-		.dev_model = PMIC_DA9063,
+		.dev_model = PMIC_CHIP_ID_DA9063,
 	},
 	{ }
 };
diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h
index f3ae65db4c86..664f650d0086 100644
--- a/include/linux/mfd/da9063/core.h
+++ b/include/linux/mfd/da9063/core.h
@@ -29,9 +29,7 @@
 #define DA9063_DRVNAME_RTC		"da9063-rtc"
 #define DA9063_DRVNAME_VIBRATION	"da9063-vibration"
 
-enum da9063_models {
-	PMIC_DA9063 = 0x61,
-};
+#define PMIC_CHIP_ID_DA9063		0x61
 
 enum da9063_variant_codes {
 	PMIC_DA9063_AD = 0x3,
-- 
2.16.2

[PATCH v3 04/10] mfd: da9063: Replace model with type

[Marek Vasut]

The model number stored in the struct da9063 is the same for all
variants of the da9063 since it is the chip ID, which is always
the same. Replace that with a separate identifier instead, which
allows us to discern the DA9063 variants by setting the type
based on either DT match or otherwise.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V2: Drop useless cast of id->driver_data
V3: Fix kernel 0day error , s/dev_model/type/
---
 drivers/mfd/da9063-core.c            | 1 -
 drivers/mfd/da9063-i2c.c             | 5 +++--
 drivers/regulator/da9063-regulator.c | 6 +++---
 include/linux/mfd/da9063/core.h      | 6 +++++-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
index fb122316c421..eebca3442cf3 100644
--- a/drivers/mfd/da9063-core.c
+++ b/drivers/mfd/da9063-core.c
@@ -215,7 +215,6 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
 		return -ENODEV;
 	}
 
-	da9063->model = model;
 	da9063->variant_code = variant_code;
 
 	ret = da9063_irq_init(da9063);
diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index 7f84030c8d53..048ce55ebc5b 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -236,7 +236,7 @@ static const struct of_device_id da9063_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, da9063_dt_ids);
 static int da9063_i2c_probe(struct i2c_client *i2c,
-	const struct i2c_device_id *id)
+			    const struct i2c_device_id *id)
 {
 	struct da9063 *da9063;
 	int ret;
@@ -248,6 +248,7 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
 	i2c_set_clientdata(i2c, da9063);
 	da9063->dev = &i2c->dev;
 	da9063->chip_irq = i2c->irq;
+	da9063->type = id->driver_data;
 
 	if (da9063->variant_code == PMIC_DA9063_AD) {
 		da9063_regmap_config.rd_table = &da9063_ad_readable_table;
@@ -280,7 +281,7 @@ static int da9063_i2c_remove(struct i2c_client *i2c)
 }
 
 static const struct i2c_device_id da9063_i2c_id[] = {
-	{ "da9063", PMIC_CHIP_ID_DA9063 },
+	{ "da9063", PMIC_TYPE_DA9063 },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, da9063_i2c_id);
diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index 87c884ae0064..eeb0d431dda1 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -98,7 +98,7 @@ struct da9063_regulator_info {
 struct da9063_dev_model {
 	const struct da9063_regulator_info	*regulator_info;
 	unsigned				n_regulators;
-	unsigned				dev_model;
+	enum da9063_type			type;
 };
 
 /* Single regulator settings */
@@ -585,7 +585,7 @@ static struct da9063_dev_model regulators_models[] = {
 	{
 		.regulator_info = da9063_regulator_info,
 		.n_regulators = ARRAY_SIZE(da9063_regulator_info),
-		.dev_model = PMIC_CHIP_ID_DA9063,
+		.type = PMIC_TYPE_DA9063,
 	},
 	{ }
 };
@@ -741,7 +741,7 @@ static int da9063_regulator_probe(struct platform_device *pdev)
 
 	/* Find regulators set for particular device model */
 	for (model = regulators_models; model->regulator_info; model++) {
-		if (model->dev_model == da9063->model)
+		if (model->type == da9063->type)
 			break;
 	}
 	if (!model->regulator_info) {
diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h
index 664f650d0086..eb234582dcb2 100644
--- a/include/linux/mfd/da9063/core.h
+++ b/include/linux/mfd/da9063/core.h
@@ -31,6 +31,10 @@
 
 #define PMIC_CHIP_ID_DA9063		0x61
 
+enum da9063_type {
+	PMIC_TYPE_DA9063 = 0,
+};
+
 enum da9063_variant_codes {
 	PMIC_DA9063_AD = 0x3,
 	PMIC_DA9063_BB = 0x5,
@@ -76,7 +80,7 @@ enum da9063_irqs {
 struct da9063 {
 	/* Device */
 	struct device	*dev;
-	unsigned short	model;
+	enum da9063_type type;
 	unsigned char	variant_code;
 	unsigned int	flags;
 
-- 
2.16.2

[PATCH v3 05/10] mfd: da9063: Add DA9063L type

[Marek Vasut]

Add type for DA9063L, which is a reduced variant of the DA9063
without RTC block and with less regulators.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V2: No change
V3: No change
---
 include/linux/mfd/da9063/core.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h
index eb234582dcb2..c3e917dd1860 100644
--- a/include/linux/mfd/da9063/core.h
+++ b/include/linux/mfd/da9063/core.h
@@ -33,6 +33,7 @@
 
 enum da9063_type {
 	PMIC_TYPE_DA9063 = 0,
+	PMIC_TYPE_DA9063L,
 };
 
 enum da9063_variant_codes {
-- 
2.16.2

[PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Marek Vasut]

While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
prevent access into that register block.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
---
V3: New patch
---
 drivers/mfd/da9063-i2c.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index 048ce55ebc5b..f0d92a37df6b 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -208,6 +208,93 @@ static const struct regmap_access_table da9063_bb_volatile_table = {
 	.n_yes_ranges = ARRAY_SIZE(da9063_bb_volatile_ranges),
 };
 
+static const struct regmap_range da9063l_bb_readable_ranges[] = {
+	{
+		.range_min = DA9063_REG_PAGE_CON,
+		.range_max = DA9063_REG_MON_A10_RES,
+	}, {
+		.range_min = DA9063_REG_SEQ,
+		.range_max = DA9063_REG_ID_32_31,
+	}, {
+		.range_min = DA9063_REG_SEQ_A,
+		.range_max = DA9063_REG_AUTO3_LOW,
+	}, {
+		.range_min = DA9063_REG_T_OFFSET,
+		.range_max = DA9063_BB_REG_GP_ID_19,
+	}, {
+		.range_min = DA9063_REG_CHIP_ID,
+		.range_max = DA9063_REG_CHIP_VARIANT,
+	},
+};
+
+static const struct regmap_range da9063l_bb_writeable_ranges[] = {
+	{
+		.range_min = DA9063_REG_PAGE_CON,
+		.range_max = DA9063_REG_PAGE_CON,
+	}, {
+		.range_min = DA9063_REG_FAULT_LOG,
+		.range_max = DA9063_REG_VSYS_MON,
+	}, {
+		.range_min = DA9063_REG_SEQ,
+		.range_max = DA9063_REG_ID_32_31,
+	}, {
+		.range_min = DA9063_REG_SEQ_A,
+		.range_max = DA9063_REG_AUTO3_LOW,
+	}, {
+		.range_min = DA9063_REG_CONFIG_I,
+		.range_max = DA9063_BB_REG_MON_REG_4,
+	}, {
+		.range_min = DA9063_BB_REG_GP_ID_0,
+		.range_max = DA9063_BB_REG_GP_ID_19,
+	},
+};
+
+static const struct regmap_range da9063l_bb_volatile_ranges[] = {
+	{
+		.range_min = DA9063_REG_PAGE_CON,
+		.range_max = DA9063_REG_EVENT_D,
+	}, {
+		.range_min = DA9063_REG_CONTROL_A,
+		.range_max = DA9063_REG_CONTROL_B,
+	}, {
+		.range_min = DA9063_REG_CONTROL_E,
+		.range_max = DA9063_REG_CONTROL_F,
+	}, {
+		.range_min = DA9063_REG_BCORE2_CONT,
+		.range_max = DA9063_REG_LDO11_CONT,
+	}, {
+		.range_min = DA9063_REG_DVC_1,
+		.range_max = DA9063_REG_ADC_MAN,
+	}, {
+		.range_min = DA9063_REG_ADC_RES_L,
+		.range_max = DA9063_REG_MON_A10_RES,
+	}, {
+		.range_min = DA9063_REG_SEQ,
+		.range_max = DA9063_REG_SEQ,
+	}, {
+		.range_min = DA9063_REG_EN_32K,
+		.range_max = DA9063_REG_EN_32K,
+	}, {
+		.range_min = DA9063_BB_REG_MON_REG_5,
+		.range_max = DA9063_BB_REG_MON_REG_6,
+	},
+};
+
+static const struct regmap_access_table da9063l_bb_readable_table = {
+	.yes_ranges = da9063l_bb_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9063l_bb_readable_ranges),
+};
+
+static const struct regmap_access_table da9063l_bb_writeable_table = {
+	.yes_ranges = da9063l_bb_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9063l_bb_writeable_ranges),
+};
+
+static const struct regmap_access_table da9063l_bb_volatile_table = {
+	.yes_ranges = da9063l_bb_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9063l_bb_volatile_ranges),
+};
+
 static const struct regmap_range_cfg da9063_range_cfg[] = {
 	{
 		.range_min = DA9063_REG_PAGE_CON,
@@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
 		da9063_regmap_config.rd_table = &da9063_ad_readable_table;
 		da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
 		da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
+	} else if (da9063->type == PMIC_TYPE_DA9063L) {
+		da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
+		da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
+		da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
 	} else {
 		da9063_regmap_config.rd_table = &da9063_bb_readable_table;
 		da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
-- 
2.16.2

[PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L

[Marek Vasut]

While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
block, the DA9063L does not have an RTC. Add custom IRQ map for DA9063L to
ignore the Alarm and Tick IRQs from the PMIC.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
---
V3: New patch
---
 drivers/mfd/da9063-irq.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
index 5b406ecfc14a..b6a88861cc2e 100644
--- a/drivers/mfd/da9063-irq.c
+++ b/drivers/mfd/da9063-irq.c
@@ -74,8 +74,55 @@ static const struct regmap_irq_chip da9063_irq_chip = {
 	.init_ack_masked = true,
 };
 
+static const struct regmap_irq da9063l_irqs[] = {
+	/* DA9063 event A register */
+	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
+	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
+	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
+	/* DA9063 event B register */
+	REGMAP_IRQ_REG(DA9063_IRQ_WAKE, DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
+	REGMAP_IRQ_REG(DA9063_IRQ_TEMP, DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
+	REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2, DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
+	REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM, DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
+	REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV, DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
+	REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY, DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
+	REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
+	REGMAP_IRQ_REG(DA9063_IRQ_WARN, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
+	/* DA9063 event C register */
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI0),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI1),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI2),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI3),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI4),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI5),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI6),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI7),
+	/* DA9063 event D register */
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI8),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI9),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI10, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI11, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI12, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI13, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI14, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
+	REGMAP_IRQ_REG(DA9063_IRQ_GPI15, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
+};
+
+static const struct regmap_irq_chip da9063l_irq_chip = {
+	.name = "da9063l-irq",
+	.irqs = da9063l_irqs,
+	.num_irqs = DA9063_NUM_IRQ,
+
+	.num_regs = 4,
+	.status_base = DA9063_REG_EVENT_A,
+	.mask_base = DA9063_REG_IRQ_MASK_A,
+	.ack_base = DA9063_REG_EVENT_A,
+	.init_ack_masked = true,
+};
+
 int da9063_irq_init(struct da9063 *da9063)
 {
+	struct regmap_irq_chip *irq_chip;
 	int ret;
 
 	if (!da9063->chip_irq) {
@@ -83,10 +130,14 @@ int da9063_irq_init(struct da9063 *da9063)
 		return -EINVAL;
 	}
 
+	if (da9063->type == PMIC_TYPE_DA9063)
+		irq_chip = &da9063_irq_chip;
+	else
+		irq_chip = &da9063l_irq_chip;
+
 	ret = regmap_add_irq_chip(da9063->regmap, da9063->chip_irq,
 			IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED,
-			da9063->irq_base, &da9063_irq_chip,
-			&da9063->regmap_irq);
+			da9063->irq_base, irq_chip, &da9063->regmap_irq);
 	if (ret) {
 		dev_err(da9063->dev, "Failed to reguest IRQ %d: %d\n",
 				da9063->chip_irq, ret);
-- 
2.16.2

[PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Marek Vasut]

The DA9063L does not contain RTC block, unlike the full DA9063.
Split the RTC block into separate mfd cell and register it only
on DA9063.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
---
V2: No change
V3: Rework of mfd: da9063: Disallow RTC on DA9063L
---
 drivers/mfd/da9063-core.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
index eebca3442cf3..b05910c797af 100644
--- a/drivers/mfd/da9063-core.c
+++ b/drivers/mfd/da9063-core.c
@@ -76,7 +76,7 @@ static struct resource da9063_hwmon_resources[] = {
 };
 
 
-static const struct mfd_cell da9063_devs[] = {
+static const struct mfd_cell da9063_common_devs[] = {
 	{
 		.name		= DA9063_DRVNAME_REGULATORS,
 		.num_resources	= ARRAY_SIZE(da9063_regulators_resources),
@@ -100,15 +100,19 @@ static const struct mfd_cell da9063_devs[] = {
 		.resources	= da9063_onkey_resources,
 		.of_compatible = "dlg,da9063-onkey",
 	},
+	{
+		.name		= DA9063_DRVNAME_VIBRATION,
+	},
+};
+
+/* Only present on DA9063 , not on DA9063L */
+static const struct mfd_cell da9063_devs[] = {
 	{
 		.name		= DA9063_DRVNAME_RTC,
 		.num_resources	= ARRAY_SIZE(da9063_rtc_resources),
 		.resources	= da9063_rtc_resources,
 		.of_compatible	= "dlg,da9063-rtc",
 	},
-	{
-		.name		= DA9063_DRVNAME_VIBRATION,
-	},
 };
 
 static int da9063_clear_fault_log(struct da9063 *da9063)
@@ -225,16 +229,28 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
 
 	da9063->irq_base = regmap_irq_chip_get_base(da9063->regmap_irq);
 
-	ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
-			      ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
-			      NULL);
+	ret = mfd_add_devices(da9063->dev, -1, da9063_common_devs,
+			      ARRAY_SIZE(da9063_common_devs),
+			      NULL, da9063->irq_base, NULL);
 	if (ret) {
 		dev_err(da9063->dev, "Cannot add MFD cells\n");
 		goto err_irq_exit;
 	}
 
+	if (da9063->type == PMIC_TYPE_DA9063) {
+		ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
+				      ARRAY_SIZE(da9063_devs),
+				      NULL, da9063->irq_base, NULL);
+		if (ret) {
+			dev_err(da9063->dev, "Cannot add MFD cells\n");
+			goto err_mfd_cleanup;
+		}
+	}
+
 	return ret;
 
+err_mfd_cleanup:
+	mfd_remove_devices(da9063->dev);
 err_irq_exit:
 	da9063_irq_exit(da9063);
 	return ret;
-- 
2.16.2

[PATCH v3 09/10] mfd: da9063: Handle less LDOs on DA9063L

[Marek Vasut]

Move the LDOs present only on DA9063 at the end of the list, so that
the DA9063L can simply indicate less LDOs and still share the list of
regulators with DA9063.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V2: No change
V3: No change
---
 drivers/regulator/da9063-regulator.c | 76 +++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index eeb0d431dda1..0c09e73e0b23 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -529,6 +529,32 @@ static const struct da9063_regulator_info da9063_regulator_info[] = {
 		.ilimit = BFIELD(DA9063_REG_BUCK_ILIM_A,
 				 DA9063_BMEM_ILIM_MASK),
 	},
+	{
+		DA9063_LDO(DA9063, LDO3, 900, 20, 3440),
+		.suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO3_SEL),
+		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO3_LIM),
+	},
+	{
+		DA9063_LDO(DA9063, LDO7, 900, 50, 3600),
+		.suspend = BFIELD(DA9063_REG_LDO7_CONT, DA9063_VLDO7_SEL),
+		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO7_LIM),
+	},
+	{
+		DA9063_LDO(DA9063, LDO8, 900, 50, 3600),
+		.suspend = BFIELD(DA9063_REG_LDO8_CONT, DA9063_VLDO8_SEL),
+		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO8_LIM),
+	},
+	{
+		DA9063_LDO(DA9063, LDO9, 950, 50, 3600),
+		.suspend = BFIELD(DA9063_REG_LDO9_CONT, DA9063_VLDO9_SEL),
+	},
+	{
+		DA9063_LDO(DA9063, LDO11, 900, 50, 3600),
+		.suspend = BFIELD(DA9063_REG_LDO11_CONT, DA9063_VLDO11_SEL),
+		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO11_LIM),
+	},
+
+	/* The following LDOs are present only on DA9063, not on DA9063L */
 	{
 		DA9063_LDO(DA9063, LDO1, 600, 20, 1860),
 		.suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO1_SEL),
@@ -537,11 +563,6 @@ static const struct da9063_regulator_info da9063_regulator_info[] = {
 		DA9063_LDO(DA9063, LDO2, 600, 20, 1860),
 		.suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO2_SEL),
 	},
-	{
-		DA9063_LDO(DA9063, LDO3, 900, 20, 3440),
-		.suspend = BFIELD(DA9063_REG_DVC_1, DA9063_VLDO3_SEL),
-		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO3_LIM),
-	},
 	{
 		DA9063_LDO(DA9063, LDO4, 900, 20, 3440),
 		.suspend = BFIELD(DA9063_REG_DVC_2, DA9063_VLDO4_SEL),
@@ -555,29 +576,11 @@ static const struct da9063_regulator_info da9063_regulator_info[] = {
 		DA9063_LDO(DA9063, LDO6, 900, 50, 3600),
 		.suspend = BFIELD(DA9063_REG_LDO6_CONT, DA9063_VLDO6_SEL),
 	},
-	{
-		DA9063_LDO(DA9063, LDO7, 900, 50, 3600),
-		.suspend = BFIELD(DA9063_REG_LDO7_CONT, DA9063_VLDO7_SEL),
-		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO7_LIM),
-	},
-	{
-		DA9063_LDO(DA9063, LDO8, 900, 50, 3600),
-		.suspend = BFIELD(DA9063_REG_LDO8_CONT, DA9063_VLDO8_SEL),
-		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO8_LIM),
-	},
-	{
-		DA9063_LDO(DA9063, LDO9, 950, 50, 3600),
-		.suspend = BFIELD(DA9063_REG_LDO9_CONT, DA9063_VLDO9_SEL),
-	},
+
 	{
 		DA9063_LDO(DA9063, LDO10, 900, 50, 3600),
 		.suspend = BFIELD(DA9063_REG_LDO10_CONT, DA9063_VLDO10_SEL),
 	},
-	{
-		DA9063_LDO(DA9063, LDO11, 900, 50, 3600),
-		.suspend = BFIELD(DA9063_REG_LDO11_CONT, DA9063_VLDO11_SEL),
-		.oc_event = BFIELD(DA9063_REG_STATUS_D, DA9063_LDO11_LIM),
-	},
 };
 
 /* Link chip model with regulators info table */
@@ -587,6 +590,11 @@ static struct da9063_dev_model regulators_models[] = {
 		.n_regulators = ARRAY_SIZE(da9063_regulator_info),
 		.type = PMIC_TYPE_DA9063,
 	},
+	{
+		.regulator_info = da9063_regulator_info,
+		.n_regulators = ARRAY_SIZE(da9063_regulator_info) - 6,
+		.type = PMIC_TYPE_DA9063L,
+	},
 	{ }
 };
 
@@ -641,28 +649,34 @@ static struct of_regulator_match da9063_matches[] = {
 	[DA9063_ID_BPERI]            = { .name = "bperi",           },
 	[DA9063_ID_BCORES_MERGED]    = { .name = "bcores-merged"    },
 	[DA9063_ID_BMEM_BIO_MERGED]  = { .name = "bmem-bio-merged", },
+	[DA9063_ID_LDO3]             = { .name = "ldo3",            },
+	[DA9063_ID_LDO7]             = { .name = "ldo7",            },
+	[DA9063_ID_LDO8]             = { .name = "ldo8",            },
+	[DA9063_ID_LDO9]             = { .name = "ldo9",            },
+	[DA9063_ID_LDO11]            = { .name = "ldo11",           },
+	/* The following LDOs are present only on DA9063, not on DA9063L */
 	[DA9063_ID_LDO1]             = { .name = "ldo1",            },
 	[DA9063_ID_LDO2]             = { .name = "ldo2",            },
-	[DA9063_ID_LDO3]             = { .name = "ldo3",            },
 	[DA9063_ID_LDO4]             = { .name = "ldo4",            },
 	[DA9063_ID_LDO5]             = { .name = "ldo5",            },
 	[DA9063_ID_LDO6]             = { .name = "ldo6",            },
-	[DA9063_ID_LDO7]             = { .name = "ldo7",            },
-	[DA9063_ID_LDO8]             = { .name = "ldo8",            },
-	[DA9063_ID_LDO9]             = { .name = "ldo9",            },
 	[DA9063_ID_LDO10]            = { .name = "ldo10",           },
-	[DA9063_ID_LDO11]            = { .name = "ldo11",           },
 };
 
 static struct da9063_regulators_pdata *da9063_parse_regulators_dt(
 		struct platform_device *pdev,
 		struct of_regulator_match **da9063_reg_matches)
 {
+	struct da9063 *da9063 = dev_get_drvdata(pdev->dev.parent);
 	struct da9063_regulators_pdata *pdata;
 	struct da9063_regulator_data *rdata;
 	struct device_node *node;
+	int da9063_matches_len = ARRAY_SIZE(da9063_matches);
 	int i, n, num;
 
+	if (da9063->type == PMIC_TYPE_DA9063L)
+		da9063_matches_len -= 6;
+
 	node = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
 	if (!node) {
 		dev_err(&pdev->dev, "Regulators device node not found\n");
@@ -670,7 +684,7 @@ static struct da9063_regulators_pdata *da9063_parse_regulators_dt(
 	}
 
 	num = of_regulator_match(&pdev->dev, node, da9063_matches,
-				 ARRAY_SIZE(da9063_matches));
+				 da9063_matches_len);
 	of_node_put(node);
 	if (num < 0) {
 		dev_err(&pdev->dev, "Failed to match regulators\n");
@@ -689,7 +703,7 @@ static struct da9063_regulators_pdata *da9063_parse_regulators_dt(
 	pdata->n_regulators = num;
 
 	n = 0;
-	for (i = 0; i < ARRAY_SIZE(da9063_matches); i++) {
+	for (i = 0; i < da9063_matches_len; i++) {
 		if (!da9063_matches[i].init_data)
 			continue;
 
-- 
2.16.2

[PATCH v3 10/10] mfd: da9063: Add DA9063L support

[Marek Vasut]

Add support for DA9063L, which is a reduced variant of the DA9063
with less regulators and without RTC.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
V2: No change
V3: No change
---
 drivers/mfd/da9063-i2c.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index f0d92a37df6b..74a3b10a7208 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -319,6 +319,7 @@ static struct regmap_config da9063_regmap_config = {
 
 static const struct of_device_id da9063_dt_ids[] = {
 	{ .compatible = "dlg,da9063", },
+	{ .compatible = "dlg,da9063l", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da9063_dt_ids);
@@ -373,6 +374,7 @@ static int da9063_i2c_remove(struct i2c_client *i2c)
 
 static const struct i2c_device_id da9063_i2c_id[] = {
 	{ "da9063", PMIC_TYPE_DA9063 },
+	{ "da9063l", PMIC_TYPE_DA9063L },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, da9063_i2c_id);
-- 
2.16.2

Re: [PATCH v3 01/10] mfd: da9063: Fix failpath in core

[Geert Uytterhoeven]

On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> In case mfd_add_devices() fails, da9063_irq_exit() is not called to
> undo the IRQchip setup done by da9063_irq_init(). Fix this by adding
> the missing fail path.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Geert Uytterhoeven]

On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Convert the regmap_irq table to use REGMAP_IRQ_REG().
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Geert Uytterhoeven]

Hi Marek, Steve,

On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
> prevent access into that register block.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Thanks for your patch!

> --- a/drivers/mfd/da9063-i2c.c
> +++ b/drivers/mfd/da9063-i2c.c
> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,

Note that the line above doesn't check da9063->type, but da9063->variant_code...

>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {

... so this may be slightly confusing.

> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>         } else {
>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;

However, da9063->variant_code doesn't seem to have been filled in at this
point yet (the call to da9063_device_init() doing so is below, at the end
of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
support for AD silicon variant") never actually handled the AD silicon variant
correctly? Or am I missing something?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/10] mfd: da9063: Fix failpath in core

[Vaishali Thakkar]

On Sat, Jun 2, 2018 at 3:41 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> In case mfd_add_devices() fails, da9063_irq_exit() is not called to
> undo the IRQchip setup done by da9063_irq_init(). Fix this by adding
> the missing fail path.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed by: Vaishali Thakkar <vthakkar@vaishalithakkar.in>

> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V3: New patch
> ---
>  drivers/mfd/da9063-core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
> index 6c2870d4e754..8226ebd8b96d 100644
> --- a/drivers/mfd/da9063-core.c
> +++ b/drivers/mfd/da9063-core.c
> @@ -229,10 +229,16 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>         ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
>                               ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
>                               NULL);
> -       if (ret)
> +       if (ret) {
>                 dev_err(da9063->dev, "Cannot add MFD cells\n");
> +               goto err_irq_exit;
> +       }
>
>         return ret;
> +
> +err_irq_exit:
> +       da9063_irq_exit(da9063);
> +       return ret;
>  }
>
>  void da9063_device_exit(struct da9063 *da9063)
> --
> 2.16.2
>

Re: [PATCH v3 01/10] mfd: da9063: Fix failpath in core

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> In case mfd_add_devices() fails, da9063_irq_exit() is not called to
> undo the IRQchip setup done by da9063_irq_init(). Fix this by adding
> the missing fail path.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V3: New patch
> ---
>  drivers/mfd/da9063-core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
> index 6c2870d4e754..8226ebd8b96d 100644
> --- a/drivers/mfd/da9063-core.c
> +++ b/drivers/mfd/da9063-core.c
> @@ -229,10 +229,16 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>  	ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
>  			      ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
>  			      NULL);
> -	if (ret)
> +	if (ret) {
>  		dev_err(da9063->dev, "Cannot add MFD cells\n");
> +		goto err_irq_exit;
> +	}
>  
>  	return ret;
> +
> +err_irq_exit:
> +	da9063_irq_exit(da9063);
> +	return ret;
>  }
>  
>  void da9063_device_exit(struct da9063 *da9063)

I haven't seen the later patches yet, so maybe the goto label expanded
on, but if it's not, then this would be better:

 -	if (ret)
 +	if (ret) {
  		dev_err(da9063->dev, "Cannot add MFD cells\n");
 +		da9063_irq_exit(da9063);
 +	}
  
  	return ret;
  }

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> Convert the regmap_irq table to use REGMAP_IRQ_REG().
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V3: New patch
> Note: A sed oneliner was used:
>       sed -i "/\[DA9063_IRQ_/ {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* = \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
> ---
>  drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
>  1 file changed, 29 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
> index 207bbfe55449..5b406ecfc14a 100644
> --- a/drivers/mfd/da9063-irq.c
> +++ b/drivers/mfd/da9063-irq.c
> @@ -28,125 +28,38 @@
>  
>  static const struct regmap_irq da9063_irqs[] = {
>  	/* DA9063 event A register */
> -	[DA9063_IRQ_ONKEY] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_ONKEY,
> -	},
> -	[DA9063_IRQ_ALARM] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_ALARM,
> -	},
> -	[DA9063_IRQ_TICK] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_TICK,
> -	},
> -	[DA9063_IRQ_ADC_RDY] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_ADC_RDY,
> -	},
> -	[DA9063_IRQ_SEQ_RDY] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_SEQ_RDY,
> -	},
> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_ALARM, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
> +	REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET, DA9063_M_TICK),
> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),

Nice, but I think checkpatch.pl would complain.

Better to break after the first argument I think.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> The PMIC_DA9063 is a complete misnomer, it denotes the value of the
> DA9063 chip ID register, so rename it as such. It is also the value
> of chip ID register of DA9063L though, so drop the enum as all the
> DA9063 "models" share the same chip ID and thus the distinction will
> have to be made using DT or otherwise.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Acked-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  drivers/mfd/da9063-core.c            | 2 +-
>  drivers/mfd/da9063-i2c.c             | 2 +-
>  drivers/regulator/da9063-regulator.c | 2 +-
>  include/linux/mfd/da9063/core.h      | 4 +---
>  4 files changed, 4 insertions(+), 6 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 01/10] mfd: da9063: Fix failpath in core

[Marek Vasut]

On 06/04/2018 02:24 PM, Lee Jones wrote:
> On Sat, 02 Jun 2018, Marek Vasut wrote:
> 
>> In case mfd_add_devices() fails, da9063_irq_exit() is not called to
>> undo the IRQchip setup done by da9063_irq_init(). Fix this by adding
>> the missing fail path.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>> V3: New patch
>> ---
>>  drivers/mfd/da9063-core.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
>> index 6c2870d4e754..8226ebd8b96d 100644
>> --- a/drivers/mfd/da9063-core.c
>> +++ b/drivers/mfd/da9063-core.c
>> @@ -229,10 +229,16 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>>  	ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
>>  			      ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
>>  			      NULL);
>> -	if (ret)
>> +	if (ret) {
>>  		dev_err(da9063->dev, "Cannot add MFD cells\n");
>> +		goto err_irq_exit;
>> +	}
>>  
>>  	return ret;
>> +
>> +err_irq_exit:
>> +	da9063_irq_exit(da9063);
>> +	return ret;
>>  }
>>  
>>  void da9063_device_exit(struct da9063 *da9063)
> 
> I haven't seen the later patches yet, so maybe the goto label expanded
> on, but if it's not, then this would be better:
> 
>  -	if (ret)
>  +	if (ret) {
>   		dev_err(da9063->dev, "Cannot add MFD cells\n");
>  +		da9063_irq_exit(da9063);
>  +	}
>   
>   	return ret;
>   }

It did expand in the later patches, yes.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Marek Vasut]

On 06/04/2018 02:26 PM, Lee Jones wrote:
> On Sat, 02 Jun 2018, Marek Vasut wrote:
> 
>> Convert the regmap_irq table to use REGMAP_IRQ_REG().
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>> V3: New patch
>> Note: A sed oneliner was used:
>>       sed -i "/\[DA9063_IRQ_/ {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* = \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
>> ---
>>  drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
>>  1 file changed, 29 insertions(+), 116 deletions(-)
>>
>> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
>> index 207bbfe55449..5b406ecfc14a 100644
>> --- a/drivers/mfd/da9063-irq.c
>> +++ b/drivers/mfd/da9063-irq.c
>> @@ -28,125 +28,38 @@
>>  
>>  static const struct regmap_irq da9063_irqs[] = {
>>  	/* DA9063 event A register */
>> -	[DA9063_IRQ_ONKEY] = {
>> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> -		.mask = DA9063_M_ONKEY,
>> -	},
>> -	[DA9063_IRQ_ALARM] = {
>> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> -		.mask = DA9063_M_ALARM,
>> -	},
>> -	[DA9063_IRQ_TICK] = {
>> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> -		.mask = DA9063_M_TICK,
>> -	},
>> -	[DA9063_IRQ_ADC_RDY] = {
>> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> -		.mask = DA9063_M_ADC_RDY,
>> -	},
>> -	[DA9063_IRQ_SEQ_RDY] = {
>> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> -		.mask = DA9063_M_SEQ_RDY,
>> -	},
>> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_ALARM, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET, DA9063_M_TICK),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
> 
> Nice, but I think checkpatch.pl would complain.
> 
> Better to break after the first argument I think.

Doesn't really help the readability, but done.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Marek Vasut]

On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
> Hi Marek, Steve,
> 
> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
>> prevent access into that register block.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/mfd/da9063-i2c.c
>> +++ b/drivers/mfd/da9063-i2c.c
>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
> 
> Note that the line above doesn't check da9063->type, but da9063->variant_code...
> 
>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
> 
> ... so this may be slightly confusing.

I know.

>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>>         } else {
>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
> 
> However, da9063->variant_code doesn't seem to have been filled in at this
> point yet (the call to da9063_device_init() doing so is below, at the end
> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
> support for AD silicon variant") never actually handled the AD silicon variant
> correctly? Or am I missing something?

Ha, that is a good point.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Marek Vasut]

On 06/04/2018 02:26 PM, Lee Jones wrote:
> On Sat, 02 Jun 2018, Marek Vasut wrote:
> 
>> The PMIC_DA9063 is a complete misnomer, it denotes the value of the
>> DA9063 chip ID register, so rename it as such. It is also the value
>> of chip ID register of DA9063L though, so drop the enum as all the
>> DA9063 "models" share the same chip ID and thus the distinction will
>> have to be made using DT or otherwise.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> Acked-by: Mark Brown <broonie@kernel.org>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> V2: No change
>> V3: No change
>> ---
>>  drivers/mfd/da9063-core.c            | 2 +-
>>  drivers/mfd/da9063-i2c.c             | 2 +-
>>  drivers/regulator/da9063-regulator.c | 2 +-
>>  include/linux/mfd/da9063/core.h      | 4 +---
>>  4 files changed, 4 insertions(+), 6 deletions(-)
> 
> For my own reference:
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Thanks.

Any other comments before I submit the next round in a few days?

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Lee Jones]

On Mon, 04 Jun 2018, Marek Vasut wrote:

> On 06/04/2018 02:26 PM, Lee Jones wrote:
> > On Sat, 02 Jun 2018, Marek Vasut wrote:
> > 
> >> The PMIC_DA9063 is a complete misnomer, it denotes the value of the
> >> DA9063 chip ID register, so rename it as such. It is also the value
> >> of chip ID register of DA9063L though, so drop the enum as all the
> >> DA9063 "models" share the same chip ID and thus the distinction will
> >> have to be made using DT or otherwise.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> Cc: Mark Brown <broonie@kernel.org>
> >> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> Acked-by: Mark Brown <broonie@kernel.org>
> >> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> V2: No change
> >> V3: No change
> >> ---
> >>  drivers/mfd/da9063-core.c            | 2 +-
> >>  drivers/mfd/da9063-i2c.c             | 2 +-
> >>  drivers/regulator/da9063-regulator.c | 2 +-
> >>  include/linux/mfd/da9063/core.h      | 4 +---
> >>  4 files changed, 4 insertions(+), 6 deletions(-)
> > 
> > For my own reference:
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks.
> 
> Any other comments before I submit the next round in a few days?

Yes, please hold off until I can complete the set.

Should be done by EOD today.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Lee Jones]

On Mon, 04 Jun 2018, Marek Vasut wrote:

> On 06/04/2018 02:26 PM, Lee Jones wrote:
> > On Sat, 02 Jun 2018, Marek Vasut wrote:
> > 
> >> Convert the regmap_irq table to use REGMAP_IRQ_REG().
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> Cc: Mark Brown <broonie@kernel.org>
> >> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> ---
> >> V3: New patch
> >> Note: A sed oneliner was used:
> >>       sed -i "/\[DA9063_IRQ_/ {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* = \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
> >> ---
> >>  drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
> >>  1 file changed, 29 insertions(+), 116 deletions(-)
> >>
> >> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
> >> index 207bbfe55449..5b406ecfc14a 100644
> >> --- a/drivers/mfd/da9063-irq.c
> >> +++ b/drivers/mfd/da9063-irq.c
> >> @@ -28,125 +28,38 @@
> >>  
> >>  static const struct regmap_irq da9063_irqs[] = {
> >>  	/* DA9063 event A register */
> >> -	[DA9063_IRQ_ONKEY] = {
> >> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> -		.mask = DA9063_M_ONKEY,
> >> -	},
> >> -	[DA9063_IRQ_ALARM] = {
> >> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> -		.mask = DA9063_M_ALARM,
> >> -	},
> >> -	[DA9063_IRQ_TICK] = {
> >> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> -		.mask = DA9063_M_TICK,
> >> -	},
> >> -	[DA9063_IRQ_ADC_RDY] = {
> >> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> -		.mask = DA9063_M_ADC_RDY,
> >> -	},
> >> -	[DA9063_IRQ_SEQ_RDY] = {
> >> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> -		.mask = DA9063_M_SEQ_RDY,
> >> -	},
> >> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> >> +	REGMAP_IRQ_REG(DA9063_IRQ_ALARM, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
> >> +	REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET, DA9063_M_TICK),
> >> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> >> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
> > 
> > Nice, but I think checkpatch.pl would complain.
> > 
> > Better to break after the first argument I think.
> 
> Doesn't really help the readability, but done.

I don't make the rules. :)

Personally /me is hoping that the 80 char limit is lifted to ~120 soon.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 04/10] mfd: da9063: Replace model with type

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> The model number stored in the struct da9063 is the same for all
> variants of the da9063 since it is the chip ID, which is always
> the same. Replace that with a separate identifier instead, which
> allows us to discern the DA9063 variants by setting the type
> based on either DT match or otherwise.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Acked-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: Drop useless cast of id->driver_data
> V3: Fix kernel 0day error , s/dev_model/type/
> ---
>  drivers/mfd/da9063-core.c            | 1 -
>  drivers/mfd/da9063-i2c.c             | 5 +++--
>  drivers/regulator/da9063-regulator.c | 6 +++---
>  include/linux/mfd/da9063/core.h      | 6 +++++-
>  4 files changed, 11 insertions(+), 7 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 05/10] mfd: da9063: Add DA9063L type

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> Add type for DA9063L, which is a reduced variant of the DA9063
> without RTC block and with less regulators.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  include/linux/mfd/da9063/core.h | 1 +
>  1 file changed, 1 insertion(+)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Geert Uytterhoeven]

Hi Lee,

On Tue, Jun 5, 2018 at 9:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 04 Jun 2018, Marek Vasut wrote:
>> On 06/04/2018 02:26 PM, Lee Jones wrote:
>> > On Sat, 02 Jun 2018, Marek Vasut wrote:
>> >> Convert the regmap_irq table to use REGMAP_IRQ_REG().
>> >>
>> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> >> Cc: Lee Jones <lee.jones@linaro.org>
>> >> Cc: Mark Brown <broonie@kernel.org>
>> >> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> >> Cc: linux-renesas-soc@vger.kernel.org
>> >> ---
>> >> V3: New patch
>> >> Note: A sed oneliner was used:
>> >>       sed -i "/\[DA9063_IRQ_/ {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* = \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
>> >> ---
>> >>  drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
>> >>  1 file changed, 29 insertions(+), 116 deletions(-)
>> >>
>> >> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
>> >> index 207bbfe55449..5b406ecfc14a 100644
>> >> --- a/drivers/mfd/da9063-irq.c
>> >> +++ b/drivers/mfd/da9063-irq.c
>> >> @@ -28,125 +28,38 @@
>> >>
>> >>  static const struct regmap_irq da9063_irqs[] = {
>> >>    /* DA9063 event A register */
>> >> -  [DA9063_IRQ_ONKEY] = {
>> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> >> -          .mask = DA9063_M_ONKEY,
>> >> -  },
>> >> -  [DA9063_IRQ_ALARM] = {
>> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> >> -          .mask = DA9063_M_ALARM,
>> >> -  },
>> >> -  [DA9063_IRQ_TICK] = {
>> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> >> -          .mask = DA9063_M_TICK,
>> >> -  },
>> >> -  [DA9063_IRQ_ADC_RDY] = {
>> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> >> -          .mask = DA9063_M_ADC_RDY,
>> >> -  },
>> >> -  [DA9063_IRQ_SEQ_RDY] = {
>> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
>> >> -          .mask = DA9063_M_SEQ_RDY,
>> >> -  },
>> >> +  REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
>> >> +  REGMAP_IRQ_REG(DA9063_IRQ_ALARM, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
>> >> +  REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET, DA9063_M_TICK),
>> >> +  REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
>> >> +  REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
>> >
>> > Nice, but I think checkpatch.pl would complain.
>> >
>> > Better to break after the first argument I think.
>>
>> Doesn't really help the readability, but done.
>
> I don't make the rules. :)

Documentation/process/coding-style.rst:

"Statements longer than 80 columns will be broken into sensible chunks, unless
 exceeding 80 columns significantly increases readability and does not hide
 information."

> Personally /me is hoping that the 80 char limit is lifted to ~120 soon.

Please no ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> block, the DA9063L does not have an RTC. Add custom IRQ map for DA9063L to
> ignore the Alarm and Tick IRQs from the PMIC.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V3: New patch
> ---
>  drivers/mfd/da9063-irq.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
> index 5b406ecfc14a..b6a88861cc2e 100644
> --- a/drivers/mfd/da9063-irq.c
> +++ b/drivers/mfd/da9063-irq.c
> @@ -74,8 +74,55 @@ static const struct regmap_irq_chip da9063_irq_chip = {
>  	.init_ack_masked = true,
>  };
>  
> +static const struct regmap_irq da9063l_irqs[] = {
> +	/* DA9063 event A register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
> +	/* DA9063 event B register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_WAKE, DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
> +	REGMAP_IRQ_REG(DA9063_IRQ_TEMP, DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
> +	REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2, DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
> +	REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM, DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
> +	REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV, DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
> +	REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY, DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
> +	REGMAP_IRQ_REG(DA9063_IRQ_WARN, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
> +	/* DA9063 event C register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI0),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI1),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI2),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI3),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI4),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI5),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI6),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI7),
> +	/* DA9063 event D register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI8),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI9),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI10, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI11, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI12, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI13, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI14, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI15, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
> +};

Same here.  Please make checkpatch.pl happen, even if it makes the
code slightly less readable.

> +static const struct regmap_irq_chip da9063l_irq_chip = {
> +	.name = "da9063l-irq",
> +	.irqs = da9063l_irqs,
> +	.num_irqs = DA9063_NUM_IRQ,
> +

Nit: This '\n' is superfluous.

> +	.num_regs = 4,
> +	.status_base = DA9063_REG_EVENT_A,
> +	.mask_base = DA9063_REG_IRQ_MASK_A,
> +	.ack_base = DA9063_REG_EVENT_A,
> +	.init_ack_masked = true,
> +};
> +
>  int da9063_irq_init(struct da9063 *da9063)
>  {
> +	struct regmap_irq_chip *irq_chip;
>  	int ret;
>  
>  	if (!da9063->chip_irq) {
> @@ -83,10 +130,14 @@ int da9063_irq_init(struct da9063 *da9063)
>  		return -EINVAL;
>  	}
>  
> +	if (da9063->type == PMIC_TYPE_DA9063)
> +		irq_chip = &da9063_irq_chip;
> +	else
> +		irq_chip = &da9063l_irq_chip;
> +
>  	ret = regmap_add_irq_chip(da9063->regmap, da9063->chip_irq,
>  			IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED,
> -			da9063->irq_base, &da9063_irq_chip,
> -			&da9063->regmap_irq);
> +			da9063->irq_base, irq_chip, &da9063->regmap_irq);
>  	if (ret) {
>  		dev_err(da9063->dev, "Failed to reguest IRQ %d: %d\n",
>  				da9063->chip_irq, ret);

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> The DA9063L does not contain RTC block, unlike the full DA9063.
> Split the RTC block into separate mfd cell and register it only
> on DA9063.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: No change
> V3: Rework of mfd: da9063: Disallow RTC on DA9063L
> ---
>  drivers/mfd/da9063-core.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
> index eebca3442cf3..b05910c797af 100644
> --- a/drivers/mfd/da9063-core.c
> +++ b/drivers/mfd/da9063-core.c
> @@ -76,7 +76,7 @@ static struct resource da9063_hwmon_resources[] = {
>  };
>  
>  
> -static const struct mfd_cell da9063_devs[] = {
> +static const struct mfd_cell da9063_common_devs[] = {
>  	{
>  		.name		= DA9063_DRVNAME_REGULATORS,

Appreciate that these are historical, but these device name defines
make me shudder.  They only serve to act as an obfuscation layer when
debugging at platform level.  Please consider getting rid of them.

>  		.num_resources	= ARRAY_SIZE(da9063_regulators_resources),
> @@ -100,15 +100,19 @@ static const struct mfd_cell da9063_devs[] = {
>  		.resources	= da9063_onkey_resources,
>  		.of_compatible = "dlg,da9063-onkey",
>  	},
> +	{
> +		.name		= DA9063_DRVNAME_VIBRATION,
> +	},

Place this on a single line please.

        { .name	= DA9063_DRVNAME_VIBRATION },

> +};
> +
> +/* Only present on DA9063 , not on DA9063L */
> +static const struct mfd_cell da9063_devs[] = {
>  	{
>  		.name		= DA9063_DRVNAME_RTC,
>  		.num_resources	= ARRAY_SIZE(da9063_rtc_resources),
>  		.resources	= da9063_rtc_resources,
>  		.of_compatible	= "dlg,da9063-rtc",
>  	},
> -	{
> -		.name		= DA9063_DRVNAME_VIBRATION,
> -	},
>  };
>  
>  static int da9063_clear_fault_log(struct da9063 *da9063)
> @@ -225,16 +229,28 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>  
>  	da9063->irq_base = regmap_irq_chip_get_base(da9063->regmap_irq);
>  
> -	ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
> -			      ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
> -			      NULL);
> +	ret = mfd_add_devices(da9063->dev, -1, da9063_common_devs,

Please consider updating the -1's in this file with the appropriate
define in a separate patch.

> +			      ARRAY_SIZE(da9063_common_devs),
> +			      NULL, da9063->irq_base, NULL);
>  	if (ret) {
>  		dev_err(da9063->dev, "Cannot add MFD cells\n");
>  		goto err_irq_exit;
>  	}
>  
> +	if (da9063->type == PMIC_TYPE_DA9063) {
> +		ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
> +				      ARRAY_SIZE(da9063_devs),
> +				      NULL, da9063->irq_base, NULL);
> +		if (ret) {
> +			dev_err(da9063->dev, "Cannot add MFD cells\n");

Better to be more general here.

"Failed to add child devices" or such.

Users don't tend to care about MFD cells.

> +			goto err_mfd_cleanup;
> +		}
> +	}
> +
>  	return ret;
>  
> +err_mfd_cleanup:
> +	mfd_remove_devices(da9063->dev);

Any reason why you can't use devm_*?

>  err_irq_exit:
>  	da9063_irq_exit(da9063);
>  	return ret;

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L

[Geert Uytterhoeven]

Hi Lee,

On Tue, Jun 5, 2018 at 9:47 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Sat, 02 Jun 2018, Marek Vasut wrote:
>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>> block, the DA9063L does not have an RTC. Add custom IRQ map for DA9063L to
>> ignore the Alarm and Tick IRQs from the PMIC.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

>> --- a/drivers/mfd/da9063-irq.c
>> +++ b/drivers/mfd/da9063-irq.c
>> @@ -74,8 +74,55 @@ static const struct regmap_irq_chip da9063_irq_chip = {
>>       .init_ack_masked = true,
>>  };
>>
>> +static const struct regmap_irq da9063l_irqs[] = {
>> +     /* DA9063 event A register */
>> +     REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
>> +     /* DA9063 event B register */
>> +     REGMAP_IRQ_REG(DA9063_IRQ_WAKE, DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_TEMP, DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2, DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM, DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV, DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY, DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_WARN, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
>> +     /* DA9063 event C register */
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI0),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI1),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI2),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI3),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI4),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI5),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI6),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI7),
>> +     /* DA9063 event D register */
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI8),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI9),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI10, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI11, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI12, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI13, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI14, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
>> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI15, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
>> +};
>
> Same here.  Please make checkpatch.pl happen, even if it makes the
> code slightly less readable.

I beg to disagree: source code should be optimized for reading.
Checkpatch is a hinting tool, not an absolute check.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Lee Jones]

On Tue, 05 Jun 2018, Geert Uytterhoeven wrote:

> Hi Lee,
> 
> On Tue, Jun 5, 2018 at 9:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 04 Jun 2018, Marek Vasut wrote:
> >> On 06/04/2018 02:26 PM, Lee Jones wrote:
> >> > On Sat, 02 Jun 2018, Marek Vasut wrote:
> >> >> Convert the regmap_irq table to use REGMAP_IRQ_REG().
> >> >>
> >> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> >> Cc: Mark Brown <broonie@kernel.org>
> >> >> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> >> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> >> Cc: linux-renesas-soc@vger.kernel.org
> >> >> ---
> >> >> V3: New patch
> >> >> Note: A sed oneliner was used:
> >> >>       sed -i "/\[DA9063_IRQ_/ {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* = \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
> >> >> ---
> >> >>  drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
> >> >>  1 file changed, 29 insertions(+), 116 deletions(-)
> >> >>
> >> >> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
> >> >> index 207bbfe55449..5b406ecfc14a 100644
> >> >> --- a/drivers/mfd/da9063-irq.c
> >> >> +++ b/drivers/mfd/da9063-irq.c
> >> >> @@ -28,125 +28,38 @@
> >> >>
> >> >>  static const struct regmap_irq da9063_irqs[] = {
> >> >>    /* DA9063 event A register */
> >> >> -  [DA9063_IRQ_ONKEY] = {
> >> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> >> -          .mask = DA9063_M_ONKEY,
> >> >> -  },
> >> >> -  [DA9063_IRQ_ALARM] = {
> >> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> >> -          .mask = DA9063_M_ALARM,
> >> >> -  },
> >> >> -  [DA9063_IRQ_TICK] = {
> >> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> >> -          .mask = DA9063_M_TICK,
> >> >> -  },
> >> >> -  [DA9063_IRQ_ADC_RDY] = {
> >> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> >> -          .mask = DA9063_M_ADC_RDY,
> >> >> -  },
> >> >> -  [DA9063_IRQ_SEQ_RDY] = {
> >> >> -          .reg_offset = DA9063_REG_EVENT_A_OFFSET,
> >> >> -          .mask = DA9063_M_SEQ_RDY,
> >> >> -  },
> >> >> +  REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> >> >> +  REGMAP_IRQ_REG(DA9063_IRQ_ALARM, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
> >> >> +  REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET, DA9063_M_TICK),
> >> >> +  REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> >> >> +  REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
> >> >
> >> > Nice, but I think checkpatch.pl would complain.
> >> >
> >> > Better to break after the first argument I think.
> >>
> >> Doesn't really help the readability, but done.
> >
> > I don't make the rules. :)
> 
> Documentation/process/coding-style.rst:
> 
> "Statements longer than 80 columns will be broken into sensible chunks, unless
>  exceeding 80 columns significantly increases readability and does not hide
>  information."

Operative word here is "significantly".

> > Personally /me is hoping that the 80 char limit is lifted to ~120 soon.
> 
> Please no ;-)

Maybe 120 is pushing it a little, but 80 is so 19xx? ... well, 80!

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 09/10] mfd: da9063: Handle less LDOs on DA9063L

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> Move the LDOs present only on DA9063 at the end of the list, so that
> the DA9063L can simply indicate less LDOs and still share the list of
> regulators with DA9063.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Acked-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  drivers/regulator/da9063-regulator.c | 76 +++++++++++++++++++++---------------
>  1 file changed, 45 insertions(+), 31 deletions(-)

Nit: Please change the $SUBJECT line.

s/mfd/regulator/

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 10/10] mfd: da9063: Add DA9063L support

[Lee Jones]

On Sat, 02 Jun 2018, Marek Vasut wrote:

> Add support for DA9063L, which is a reduced variant of the DA9063
> with less regulators and without RTC.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Acked-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  drivers/mfd/da9063-i2c.c | 2 ++
>  1 file changed, 2 insertions(+)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L

[Lee Jones]

On Tue, 05 Jun 2018, Geert Uytterhoeven wrote:

> Hi Lee,
> 
> On Tue, Jun 5, 2018 at 9:47 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Sat, 02 Jun 2018, Marek Vasut wrote:
> >> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> >> block, the DA9063L does not have an RTC. Add custom IRQ map for DA9063L to
> >> ignore the Alarm and Tick IRQs from the PMIC.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> >> --- a/drivers/mfd/da9063-irq.c
> >> +++ b/drivers/mfd/da9063-irq.c
> >> @@ -74,8 +74,55 @@ static const struct regmap_irq_chip da9063_irq_chip = {
> >>       .init_ack_masked = true,
> >>  };
> >>
> >> +static const struct regmap_irq da9063l_irqs[] = {
> >> +     /* DA9063 event A register */
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_ONKEY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY, DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
> >> +     /* DA9063 event B register */
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_WAKE, DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_TEMP, DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2, DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM, DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV, DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY, DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_WARN, DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
> >> +     /* DA9063 event C register */
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI0),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI1),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI2),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI3),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI4),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI5),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI6),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET, DA9063_M_GPI7),
> >> +     /* DA9063 event D register */
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI8),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI9),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI10, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI11, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI12, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI13, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI14, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
> >> +     REGMAP_IRQ_REG(DA9063_IRQ_GPI15, DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
> >> +};
> >
> > Same here.  Please make checkpatch.pl happen, even if it makes the
> > code slightly less readable.
> 
> I beg to disagree: source code should be optimized for reading.
> Checkpatch is a hinting tool, not an absolute check.

I agree with you to some degree, but as per the Coding Standards we
are to use max 80-chars unless it *significantly* reduces
readability.

Breaking lines into two on a natural break only *slightly* reduces
readability, if at all to be frank.

We have MACROS which encompass many arguments and we have to draw the
line somewhere.  I choose 80-chars.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Marek Vasut]

On 06/05/2018 09:53 AM, Lee Jones wrote:
> On Sat, 02 Jun 2018, Marek Vasut wrote:
> 
>> The DA9063L does not contain RTC block, unlike the full DA9063.
>> Split the RTC block into separate mfd cell and register it only
>> on DA9063.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>> V2: No change
>> V3: Rework of mfd: da9063: Disallow RTC on DA9063L
>> ---
>>  drivers/mfd/da9063-core.c | 30 +++++++++++++++++++++++-------
>>  1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
>> index eebca3442cf3..b05910c797af 100644
>> --- a/drivers/mfd/da9063-core.c
>> +++ b/drivers/mfd/da9063-core.c
>> @@ -76,7 +76,7 @@ static struct resource da9063_hwmon_resources[] = {
>>  };
>>  
>>  
>> -static const struct mfd_cell da9063_devs[] = {
>> +static const struct mfd_cell da9063_common_devs[] = {
>>  	{
>>  		.name		= DA9063_DRVNAME_REGULATORS,
> 
> Appreciate that these are historical, but these device name defines
> make me shudder.  They only serve to act as an obfuscation layer when
> debugging at platform level.  Please consider getting rid of them.

The macro can be shared between the core and the drivers, so the names
never run out of sync.

>>  		.num_resources	= ARRAY_SIZE(da9063_regulators_resources),
>> @@ -100,15 +100,19 @@ static const struct mfd_cell da9063_devs[] = {
>>  		.resources	= da9063_onkey_resources,
>>  		.of_compatible = "dlg,da9063-onkey",
>>  	},
>> +	{
>> +		.name		= DA9063_DRVNAME_VIBRATION,
>> +	},
> 
> Place this on a single line please.

This would only make the style inconsistent with the ie. LEDs entry.

>         { .name	= DA9063_DRVNAME_VIBRATION },
> 
>> +};
>> +
>> +/* Only present on DA9063 , not on DA9063L */
>> +static const struct mfd_cell da9063_devs[] = {
>>  	{
>>  		.name		= DA9063_DRVNAME_RTC,
>>  		.num_resources	= ARRAY_SIZE(da9063_rtc_resources),
>>  		.resources	= da9063_rtc_resources,
>>  		.of_compatible	= "dlg,da9063-rtc",
>>  	},
>> -	{
>> -		.name		= DA9063_DRVNAME_VIBRATION,
>> -	},
>>  };
>>  
>>  static int da9063_clear_fault_log(struct da9063 *da9063)
>> @@ -225,16 +229,28 @@ int da9063_device_init(struct da9063 *da9063, unsigned int irq)
>>  
>>  	da9063->irq_base = regmap_irq_chip_get_base(da9063->regmap_irq);
>>  
>> -	ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
>> -			      ARRAY_SIZE(da9063_devs), NULL, da9063->irq_base,
>> -			      NULL);
>> +	ret = mfd_add_devices(da9063->dev, -1, da9063_common_devs,
> 
> Please consider updating the -1's in this file with the appropriate
> define in a separate patch.

Done

>> +			      ARRAY_SIZE(da9063_common_devs),
>> +			      NULL, da9063->irq_base, NULL);
>>  	if (ret) {
>>  		dev_err(da9063->dev, "Cannot add MFD cells\n");
>>  		goto err_irq_exit;
>>  	}
>>  
>> +	if (da9063->type == PMIC_TYPE_DA9063) {
>> +		ret = mfd_add_devices(da9063->dev, -1, da9063_devs,
>> +				      ARRAY_SIZE(da9063_devs),
>> +				      NULL, da9063->irq_base, NULL);
>> +		if (ret) {
>> +			dev_err(da9063->dev, "Cannot add MFD cells\n");
> 
> Better to be more general here.
> 
> "Failed to add child devices" or such.
> 
> Users don't tend to care about MFD cells.

Hum, done.

>> +			goto err_mfd_cleanup;
>> +		}
>> +	}
>> +
>>  	return ret;
>>  
>> +err_mfd_cleanup:
>> +	mfd_remove_devices(da9063->dev);
> 
> Any reason why you can't use devm_*?

Because we need to undo the MFD setup before the IRQ setup.

>>  err_irq_exit:
>>  	da9063_irq_exit(da9063);
>>  	return ret;
> 


-- 
Best regards,
Marek Vasut

RE: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Steve Twiss]

On 04 June 2018 19:32, Marek Vasut wrote,

> Subject: Re: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to
> PMIC_CHIP_ID_DA9063
> 
> On 06/04/2018 02:26 PM, Lee Jones wrote:
> > On Sat, 02 Jun 2018, Marek Vasut wrote:
> >
> >> The PMIC_DA9063 is a complete misnomer, it denotes the value of the
> >> DA9063 chip ID register, so rename it as such. It is also the value
> >> of chip ID register of DA9063L though, so drop the enum as all the
> >> DA9063 "models" share the same chip ID and thus the distinction will
> >> have to be made using DT or otherwise.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> Cc: Mark Brown <broonie@kernel.org>
> >> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> Acked-by: Mark Brown <broonie@kernel.org>
> >> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> V2: No change
> >> V3: No change
> >> ---
> >>  drivers/mfd/da9063-core.c            | 2 +-
> >>  drivers/mfd/da9063-i2c.c             | 2 +-
> >>  drivers/regulator/da9063-regulator.c | 2 +-
> >>  include/linux/mfd/da9063/core.h      | 4 +---
> >>  4 files changed, 4 insertions(+), 6 deletions(-)
> >
> > For my own reference:
> >   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks.
> 
> Any other comments before I submit the next round in a few days?
> 

Hi Marek,

Thanks.
I'm reviewing it now.
Could you hold off until tomorrow please?

Regards,
Steve

Re: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Marek Vasut]

On 06/05/2018 07:16 PM, Steve Twiss wrote:
> 
> 
> On 04 June 2018 19:32, Marek Vasut wrote,
> 
>> Subject: Re: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to
>> PMIC_CHIP_ID_DA9063
>>
>> On 06/04/2018 02:26 PM, Lee Jones wrote:
>>> On Sat, 02 Jun 2018, Marek Vasut wrote:
>>>
>>>> The PMIC_DA9063 is a complete misnomer, it denotes the value of the
>>>> DA9063 chip ID register, so rename it as such. It is also the value
>>>> of chip ID register of DA9063L though, so drop the enum as all the
>>>> DA9063 "models" share the same chip ID and thus the distinction will
>>>> have to be made using DT or otherwise.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> Cc: Lee Jones <lee.jones@linaro.org>
>>>> Cc: Mark Brown <broonie@kernel.org>
>>>> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> Acked-by: Mark Brown <broonie@kernel.org>
>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> ---
>>>> V2: No change
>>>> V3: No change
>>>> ---
>>>>  drivers/mfd/da9063-core.c            | 2 +-
>>>>  drivers/mfd/da9063-i2c.c             | 2 +-
>>>>  drivers/regulator/da9063-regulator.c | 2 +-
>>>>  include/linux/mfd/da9063/core.h      | 4 +---
>>>>  4 files changed, 4 insertions(+), 6 deletions(-)
>>>
>>> For my own reference:
>>>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
>>
>> Thanks.
>>
>> Any other comments before I submit the next round in a few days?
>>
> 
> Hi Marek,
> 
> Thanks.
> I'm reviewing it now.
> Could you hold off until tomorrow please?

Yeah, why ?

-- 
Best regards,
Marek Vasut

RE: [PATCH v3 10/10] mfd: da9063: Add DA9063L support

[Steve Twiss]

On 02 June 2018 11:12, Marek Vasut wrote,

> Subject: [PATCH v3 10/10] mfd: da9063: Add DA9063L support
> 
> Add support for DA9063L, which is a reduced variant of the DA9063
> with less regulators and without RTC.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Acked-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  drivers/mfd/da9063-i2c.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
> index f0d92a37df6b..74a3b10a7208 100644
> --- a/drivers/mfd/da9063-i2c.c
> +++ b/drivers/mfd/da9063-i2c.c
> @@ -319,6 +319,7 @@ static struct regmap_config da9063_regmap_config = {
> 
>  static const struct of_device_id da9063_dt_ids[] = {
>  	{ .compatible = "dlg,da9063", },
> +	{ .compatible = "dlg,da9063l", },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, da9063_dt_ids);
> @@ -373,6 +374,7 @@ static int da9063_i2c_remove(struct i2c_client *i2c)
> 
>  static const struct i2c_device_id da9063_i2c_id[] = {
>  	{ "da9063", PMIC_TYPE_DA9063 },
> +	{ "da9063l", PMIC_TYPE_DA9063L },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(i2c, da9063_i2c_id);
> --
> 2.16.2

Hi Marek,

Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>

Regards,
Steve 

RE: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG

[Steve Twiss]

On 02 June 2018 11:12, Marek Vasut wrote,

> Subject: [PATCH v3 02/10] mfd: da9063: Use REGMAP_IRQ_REG
> 
> Convert the regmap_irq table to use REGMAP_IRQ_REG().
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V3: New patch
> Note: A sed oneliner was used:
>       sed -i "/\[DA9063_IRQ_/
> {N;N;N;s/\n//g;s/.*\[\(DA9063_IRQ_[^]]\+\)].*reg_offset = \([^,]\+\),.* =
> \([^,]\+\),.*/\tREGMAP_IRQ_REG(\1, \2, \3),/}" drivers/mfd/da9063-irq.c
> ---
>  drivers/mfd/da9063-irq.c | 145 ++++++++++-------------------------------------
>  1 file changed, 29 insertions(+), 116 deletions(-)
> 
> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
> index 207bbfe55449..5b406ecfc14a 100644
> --- a/drivers/mfd/da9063-irq.c
> +++ b/drivers/mfd/da9063-irq.c
> @@ -28,125 +28,38 @@
> 
>  static const struct regmap_irq da9063_irqs[] = {
>  	/* DA9063 event A register */
> -	[DA9063_IRQ_ONKEY] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_ONKEY,
> -	},
> -	[DA9063_IRQ_ALARM] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_ALARM,
> -	},
> -	[DA9063_IRQ_TICK] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_TICK,
> -	},
> -	[DA9063_IRQ_ADC_RDY] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_ADC_RDY,
> -	},
> -	[DA9063_IRQ_SEQ_RDY] = {
> -		.reg_offset = DA9063_REG_EVENT_A_OFFSET,
> -		.mask = DA9063_M_SEQ_RDY,
> -	},
> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_ALARM,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ALARM),
> +	REGMAP_IRQ_REG(DA9063_IRQ_TICK, DA9063_REG_EVENT_A_OFFSET,
> DA9063_M_TICK),
> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
>  	/* DA9063 event B register */
> -	[DA9063_IRQ_WAKE] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_WAKE,
> -	},
> -	[DA9063_IRQ_TEMP] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_TEMP,
> -	},
> -	[DA9063_IRQ_COMP_1V2] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_COMP_1V2,
> -	},
> -	[DA9063_IRQ_LDO_LIM] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_LDO_LIM,
> -	},
> -	[DA9063_IRQ_REG_UVOV] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_UVOV,
> -	},
> -	[DA9063_IRQ_DVC_RDY] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_DVC_RDY,
> -	},
> -	[DA9063_IRQ_VDD_MON] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_VDD_MON,
> -	},
> -	[DA9063_IRQ_WARN] = {
> -		.reg_offset = DA9063_REG_EVENT_B_OFFSET,
> -		.mask = DA9063_M_VDD_WARN,
> -	},
> +	REGMAP_IRQ_REG(DA9063_IRQ_WAKE,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
> +	REGMAP_IRQ_REG(DA9063_IRQ_TEMP,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
> +	REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
> +	REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
> +	REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
> +	REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
> +	REGMAP_IRQ_REG(DA9063_IRQ_WARN,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
>  	/* DA9063 event C register */
> -	[DA9063_IRQ_GPI0] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI0,
> -	},
> -	[DA9063_IRQ_GPI1] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI1,
> -	},
> -	[DA9063_IRQ_GPI2] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI2,
> -	},
> -	[DA9063_IRQ_GPI3] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI3,
> -	},
> -	[DA9063_IRQ_GPI4] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI4,
> -	},
> -	[DA9063_IRQ_GPI5] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI5,
> -	},
> -	[DA9063_IRQ_GPI6] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI6,
> -	},
> -	[DA9063_IRQ_GPI7] = {
> -		.reg_offset = DA9063_REG_EVENT_C_OFFSET,
> -		.mask = DA9063_M_GPI7,
> -	},
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI0),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI1),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI2),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI3),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI4),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI5),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI6),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI7),
>  	/* DA9063 event D register */
> -	[DA9063_IRQ_GPI8] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI8,
> -	},
> -	[DA9063_IRQ_GPI9] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI9,
> -	},
> -	[DA9063_IRQ_GPI10] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI10,
> -	},
> -	[DA9063_IRQ_GPI11] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI11,
> -	},
> -	[DA9063_IRQ_GPI12] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI12,
> -	},
> -	[DA9063_IRQ_GPI13] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI13,
> -	},
> -	[DA9063_IRQ_GPI14] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI14,
> -	},
> -	[DA9063_IRQ_GPI15] = {
> -		.reg_offset = DA9063_REG_EVENT_D_OFFSET,
> -		.mask = DA9063_M_GPI15,
> -	},
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET,
> DA9063_M_GPI8),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET,
> DA9063_M_GPI9),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI10,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI11,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI12,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI13,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI14,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI15,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
>  };
> 
>  static const struct regmap_irq_chip da9063_irq_chip = {
> --
> 2.16.2

Hi Marek,

Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>

Regards,
Steve

RE: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to PMIC_CHIP_ID_DA9063

[Steve Twiss]

On 02 June 2018 11:12, Marek Vasut wrote,

> Subject: [PATCH v3 03/10] mfd: da9063: Rename PMIC_DA9063 to
> PMIC_CHIP_ID_DA9063
> 
> The PMIC_DA9063 is a complete misnomer, it denotes the value of the
> DA9063 chip ID register, so rename it as such. It is also the value
> of chip ID register of DA9063L though, so drop the enum as all the
> DA9063 "models" share the same chip ID and thus the distinction will
> have to be made using DT or otherwise.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Acked-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  drivers/mfd/da9063-core.c            | 2 +-
>  drivers/mfd/da9063-i2c.c             | 2 +-
>  drivers/regulator/da9063-regulator.c | 2 +-
>  include/linux/mfd/da9063/core.h      | 4 +---
>  4 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
> index 8226ebd8b96d..fb122316c421 100644
> --- a/drivers/mfd/da9063-core.c
> +++ b/drivers/mfd/da9063-core.c
> @@ -192,7 +192,7 @@ int da9063_device_init(struct da9063 *da9063, unsigned
> int irq)
>  		dev_err(da9063->dev, "Cannot read chip model id.\n");
>  		return -EIO;
>  	}
> -	if (model != PMIC_DA9063) {
> +	if (model != PMIC_CHIP_ID_DA9063) {
>  		dev_err(da9063->dev, "Invalid chip model id: 0x%02x\n", model);
>  		return -ENODEV;
>  	}
> diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
> index 981805a2c521..7f84030c8d53 100644
> --- a/drivers/mfd/da9063-i2c.c
> +++ b/drivers/mfd/da9063-i2c.c
> @@ -280,7 +280,7 @@ static int da9063_i2c_remove(struct i2c_client *i2c)
>  }
> 
>  static const struct i2c_device_id da9063_i2c_id[] = {
> -	{"da9063", PMIC_DA9063},
> +	{ "da9063", PMIC_CHIP_ID_DA9063 },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(i2c, da9063_i2c_id);
> diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-
> regulator.c
> index 6a8f9cd69f52..87c884ae0064 100644
> --- a/drivers/regulator/da9063-regulator.c
> +++ b/drivers/regulator/da9063-regulator.c
> @@ -585,7 +585,7 @@ static struct da9063_dev_model regulators_models[] = {
>  	{
>  		.regulator_info = da9063_regulator_info,
>  		.n_regulators = ARRAY_SIZE(da9063_regulator_info),
> -		.dev_model = PMIC_DA9063,
> +		.dev_model = PMIC_CHIP_ID_DA9063,
>  	},
>  	{ }
>  };
> diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h
> index f3ae65db4c86..664f650d0086 100644
> --- a/include/linux/mfd/da9063/core.h
> +++ b/include/linux/mfd/da9063/core.h
> @@ -29,9 +29,7 @@
>  #define DA9063_DRVNAME_RTC		"da9063-rtc"
>  #define DA9063_DRVNAME_VIBRATION	"da9063-vibration"
> 
> -enum da9063_models {
> -	PMIC_DA9063 = 0x61,
> -};
> +#define PMIC_CHIP_ID_DA9063		0x61
> 
>  enum da9063_variant_codes {
>  	PMIC_DA9063_AD = 0x3,
> --
> 2.16.2

Hi Marek,

Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>

Regards,
Stephen

RE: [PATCH v3 05/10] mfd: da9063: Add DA9063L type

[Steve Twiss]

On 02 June 2018 11:12, Marek Vasut wrote,

> Subject: [PATCH v3 05/10] mfd: da9063: Add DA9063L type
> 
> Add type for DA9063L, which is a reduced variant of the DA9063
> without RTC block and with less regulators.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> V2: No change
> V3: No change
> ---
>  include/linux/mfd/da9063/core.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/mfd/da9063/core.h b/include/linux/mfd/da9063/core.h
> index eb234582dcb2..c3e917dd1860 100644
> --- a/include/linux/mfd/da9063/core.h
> +++ b/include/linux/mfd/da9063/core.h
> @@ -33,6 +33,7 @@
> 
>  enum da9063_type {
>  	PMIC_TYPE_DA9063 = 0,
> +	PMIC_TYPE_DA9063L,
>  };
> 
>  enum da9063_variant_codes {
> --
> 2.16.2

Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>

RE: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L

[Steve Twiss]

Hi Marek,

Thanks.

On 02 June 2018 11:12, Marek Vasut wrote,

> Subject: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L
> 
> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> block, the DA9063L does not have an RTC. Add custom IRQ map for DA9063L to
> ignore the Alarm and Tick IRQs from the PMIC.

I've added a request to remove the RTC references from the DA9063L datasheet.

Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
(2v1, 23-Mar-2017) lists the RTC register block" is just pointing out an ambiguity from the
datasheet since the it also identifies those registers in Table 102 on page 126 as
"Reserved".

I would like to suggest, the commit log entry: "Add custom IRQ map for DA9063L to ignore
the Alarm and Tick IRQs from the PMIC".

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V3: New patch
> ---
>  drivers/mfd/da9063-irq.c | 55
> ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
> index 5b406ecfc14a..b6a88861cc2e 100644
> --- a/drivers/mfd/da9063-irq.c
> +++ b/drivers/mfd/da9063-irq.c
> @@ -74,8 +74,55 @@ static const struct regmap_irq_chip da9063_irq_chip = {
>  	.init_ack_masked = true,
>  };
> 
> +static const struct regmap_irq da9063l_irqs[] = {
> +	/* DA9063 event A register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY,
> DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
> +	/* DA9063 event B register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_WAKE,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
> +	REGMAP_IRQ_REG(DA9063_IRQ_TEMP,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
> +	REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
> +	REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
> +	REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
> +	REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
> +	REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
> +	REGMAP_IRQ_REG(DA9063_IRQ_WARN,
> DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
> +	/* DA9063 event C register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI0),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI1),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI2),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI3),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI4),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI5),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI6),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET,
> DA9063_M_GPI7),
> +	/* DA9063 event D register */
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET,
> DA9063_M_GPI8),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET,
> DA9063_M_GPI9),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI10,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI11,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI12,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI13,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI14,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI15,
> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
> +};
> +
> +static const struct regmap_irq_chip da9063l_irq_chip = {
> +	.name = "da9063l-irq",
> +	.irqs = da9063l_irqs,
> +	.num_irqs = DA9063_NUM_IRQ,

Isn't the number of IRQs in the DA9063L different to the standard DA9063
because of the missing RTC?

Regards,
Steve

> +
> +	.num_regs = 4,
> +	.status_base = DA9063_REG_EVENT_A,
> +	.mask_base = DA9063_REG_IRQ_MASK_A,
> +	.ack_base = DA9063_REG_EVENT_A,
> +	.init_ack_masked = true,
> +};
> +
>  int da9063_irq_init(struct da9063 *da9063)
>  {
> +	struct regmap_irq_chip *irq_chip;
>  	int ret;
> 
>  	if (!da9063->chip_irq) {
> @@ -83,10 +130,14 @@ int da9063_irq_init(struct da9063 *da9063)
>  		return -EINVAL;
>  	}
> 
> +	if (da9063->type == PMIC_TYPE_DA9063)
> +		irq_chip = &da9063_irq_chip;
> +	else
> +		irq_chip = &da9063l_irq_chip;
> +
>  	ret = regmap_add_irq_chip(da9063->regmap, da9063->chip_irq,
>  			IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED,
> -			da9063->irq_base, &da9063_irq_chip,
> -			&da9063->regmap_irq);
> +			da9063->irq_base, irq_chip, &da9063->regmap_irq);
>  	if (ret) {
>  		dev_err(da9063->dev, "Failed to reguest IRQ %d: %d\n",
>  				da9063->chip_irq, ret);
> --
> 2.16.2

RE: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Steve Twiss]

Hi Marek and Geert,

On 04 June 2018 17:25 Marek Vasut wrote,

> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
> 
> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
> > Hi Marek, Steve,
> >
> > On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> >> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
> >> prevent access into that register block.

Ok. I've said previously in [v3 07/10], but I'll copy again:
There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
(2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
"Reserved".

Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
Also Dialog do not store a history of Datasheets on their website so once this is updated (although
this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
make the commit message just as misleading as the current datasheet.

How about something simpler?
"The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
into that register block."

> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >
> > Thanks for your patch!
> >
> >> --- a/drivers/mfd/da9063-i2c.c
> >> +++ b/drivers/mfd/da9063-i2c.c
> >> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
> >
> > Note that the line above doesn't check da9063->type, but da9063-
> >variant_code...
> >
> >>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
> >>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
> >>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
> >> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
> >
> > ... so this may be slightly confusing.
> 
> I know.
> 
> >> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
> >> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
> >> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
> >>         } else {
> >>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
> >>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
> >
> > However, da9063->variant_code doesn't seem to have been filled in at this
> > point yet (the call to da9063_device_init() doing so is below, at the end
> > of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
> > support for AD silicon variant") never actually handled the AD silicon variant
> > correctly? Or am I missing something?

Okay ... No. You're not missing anything. I had noticed that.
The AD chip model is not referenced and by default only the BB chip model is used.

> Ha, that is a good point.

Yeah, it's a good point, but it's not an amusing point.
The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
There is no datasheet listing AD registers supported by Dialog, only BB.

But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
and the RTC driver was updated to distinguish between the AD and BB according to
the type of variant detected at run-time during the da9063_device_init() call.

The real problem is that this leads to two competing chip detection methods for the
DA9063. The function da9063_device_init() autodetects the chip variant, but
autodetection cannot define the chip model. It's circular: the chip model cannot be
autodetected because a chip model is needed to access the register used during
autodetection.

Which leads me back to what I said two paragraphs up:
> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
> There is no datasheet listing AD registers supported by Dialog, only BB.

This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
used to print the information to the console during start-up and it is the DT that defines
the chip model based upon "dlg,da9062" or "dlg,da9061".

Steve

Re: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L

[Marek Vasut]

On 06/05/2018 09:52 PM, Steve Twiss wrote:
> Hi Marek,

Hi,

> Thanks.
> 
> On 02 June 2018 11:12, Marek Vasut wrote,
> 
>> Subject: [PATCH v3 07/10] mfd: da9063: Add custom IRQ map for DA9063L
>>
>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>> block, the DA9063L does not have an RTC. Add custom IRQ map for DA9063L to
>> ignore the Alarm and Tick IRQs from the PMIC.
> 
> I've added a request to remove the RTC references from the DA9063L datasheet.
> 
> Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
> (2v1, 23-Mar-2017) lists the RTC register block" is just pointing out an ambiguity from the
> datasheet since the it also identifies those registers in Table 102 on page 126 as
> "Reserved".
> 
> I would like to suggest, the commit log entry: "Add custom IRQ map for DA9063L to ignore
> the Alarm and Tick IRQs from the PMIC".

OK, I'll hide it.

>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>> V3: New patch
>> ---
>>  drivers/mfd/da9063-irq.c | 55
>> ++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/da9063-irq.c b/drivers/mfd/da9063-irq.c
>> index 5b406ecfc14a..b6a88861cc2e 100644
>> --- a/drivers/mfd/da9063-irq.c
>> +++ b/drivers/mfd/da9063-irq.c
>> @@ -74,8 +74,55 @@ static const struct regmap_irq_chip da9063_irq_chip = {
>>  	.init_ack_masked = true,
>>  };
>>
>> +static const struct regmap_irq da9063l_irqs[] = {
>> +	/* DA9063 event A register */
>> +	REGMAP_IRQ_REG(DA9063_IRQ_ONKEY,
>> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ONKEY),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_ADC_RDY,
>> DA9063_REG_EVENT_A_OFFSET, DA9063_M_ADC_RDY),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_SEQ_RDY,
>> DA9063_REG_EVENT_A_OFFSET, DA9063_M_SEQ_RDY),
>> +	/* DA9063 event B register */
>> +	REGMAP_IRQ_REG(DA9063_IRQ_WAKE,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_WAKE),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_TEMP,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_TEMP),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_COMP_1V2,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_COMP_1V2),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_LDO_LIM,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_LDO_LIM),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_REG_UVOV,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_UVOV),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_DVC_RDY,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_DVC_RDY),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_VDD_MON,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_MON),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_WARN,
>> DA9063_REG_EVENT_B_OFFSET, DA9063_M_VDD_WARN),
>> +	/* DA9063 event C register */
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI0, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI0),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI1, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI1),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI2, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI2),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI3, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI3),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI4, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI4),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI5, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI5),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI6, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI6),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI7, DA9063_REG_EVENT_C_OFFSET,
>> DA9063_M_GPI7),
>> +	/* DA9063 event D register */
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI8, DA9063_REG_EVENT_D_OFFSET,
>> DA9063_M_GPI8),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI9, DA9063_REG_EVENT_D_OFFSET,
>> DA9063_M_GPI9),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI10,
>> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI10),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI11,
>> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI11),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI12,
>> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI12),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI13,
>> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI13),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI14,
>> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI14),
>> +	REGMAP_IRQ_REG(DA9063_IRQ_GPI15,
>> DA9063_REG_EVENT_D_OFFSET, DA9063_M_GPI15),
>> +};
>> +
>> +static const struct regmap_irq_chip da9063l_irq_chip = {
>> +	.name = "da9063l-irq",
>> +	.irqs = da9063l_irqs,
>> +	.num_irqs = DA9063_NUM_IRQ,
> 
> Isn't the number of IRQs in the DA9063L different to the standard DA9063
> because of the missing RTC?

I think this can even be replaced by ARRAY_SIZE() ?

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Marek Vasut]

On 06/05/2018 10:17 PM, Steve Twiss wrote:
> Hi Marek and Geert,
> 
> On 04 June 2018 17:25 Marek Vasut wrote,
> 
>> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
>>
>> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
>>> Hi Marek, Steve,
>>>
>>> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>>>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
>>>> prevent access into that register block.
> 
> Ok. I've said previously in [v3 07/10], but I'll copy again:
> There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
> Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
> (2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
> on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
> "Reserved".
> 
> Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
> Also Dialog do not store a history of Datasheets on their website so once this is updated (although
> this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
> make the commit message just as misleading as the current datasheet.
> 
> How about something simpler?
> "The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
> into that register block."
> 
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>
>>> Thanks for your patch!
>>>
>>>> --- a/drivers/mfd/da9063-i2c.c
>>>> +++ b/drivers/mfd/da9063-i2c.c
>>>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
>>>
>>> Note that the line above doesn't check da9063->type, but da9063-
>>> variant_code...
>>>
>>>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>>>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>>>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
>>>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
>>>
>>> ... so this may be slightly confusing.
>>
>> I know.
>>
>>>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
>>>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
>>>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>>>>         } else {
>>>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>>>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
>>>
>>> However, da9063->variant_code doesn't seem to have been filled in at this
>>> point yet (the call to da9063_device_init() doing so is below, at the end
>>> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
>>> support for AD silicon variant") never actually handled the AD silicon variant
>>> correctly? Or am I missing something?
> 
> Okay ... No. You're not missing anything. I had noticed that.
> The AD chip model is not referenced and by default only the BB chip model is used.
> 
>> Ha, that is a good point.
> 
> Yeah, it's a good point, but it's not an amusing point.
> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
> There is no datasheet listing AD registers supported by Dialog, only BB.
> 
> But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
> and the RTC driver was updated to distinguish between the AD and BB according to
> the type of variant detected at run-time during the da9063_device_init() call.
> 
> The real problem is that this leads to two competing chip detection methods for the
> DA9063. The function da9063_device_init() autodetects the chip variant, but
> autodetection cannot define the chip model. It's circular: the chip model cannot be
> autodetected because a chip model is needed to access the register used during
> autodetection.
> 
> Which leads me back to what I said two paragraphs up:
>> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
>> There is no datasheet listing AD registers supported by Dialog, only BB.
> 
> This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
> used to print the information to the console during start-up and it is the DT that defines
> the chip model based upon "dlg,da9062" or "dlg,da9061".

So the AD was broken since forever and noone noticed ? :)

Do you have an AD hardware and can you fix it ?

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Lee Jones]

On Tue, 05 Jun 2018, Marek Vasut wrote:

> On 06/05/2018 09:53 AM, Lee Jones wrote:
> > On Sat, 02 Jun 2018, Marek Vasut wrote:
> > 
> >> The DA9063L does not contain RTC block, unlike the full DA9063.
> >> Split the RTC block into separate mfd cell and register it only
> >> on DA9063.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> Cc: Mark Brown <broonie@kernel.org>
> >> Cc: Steve Twiss <stwiss.opensource@diasemi.com>
> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> ---
> >> V2: No change
> >> V3: Rework of mfd: da9063: Disallow RTC on DA9063L
> >> ---
> >>  drivers/mfd/da9063-core.c | 30 +++++++++++++++++++++++-------
> >>  1 file changed, 23 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c
> >> index eebca3442cf3..b05910c797af 100644
> >> --- a/drivers/mfd/da9063-core.c
> >> +++ b/drivers/mfd/da9063-core.c
> >> @@ -76,7 +76,7 @@ static struct resource da9063_hwmon_resources[] = {
> >>  };
> >>  
> >>  
> >> -static const struct mfd_cell da9063_devs[] = {
> >> +static const struct mfd_cell da9063_common_devs[] = {
> >>  	{
> >>  		.name		= DA9063_DRVNAME_REGULATORS,
> > 
> > Appreciate that these are historical, but these device name defines
> > make me shudder.  They only serve to act as an obfuscation layer when
> > debugging at platform level.  Please consider getting rid of them.
> 
> The macro can be shared between the core and the drivers, so the names
> never run out of sync.

Platform driver name changes are vary rare.  Even if they are changed,
even light testing would uncover the fact that child drivers do not
.probe().  Due to the current obfuscation, I currently have no idea
what this device's name is.  This technique is not allowed for new
drivers - unfortunately I didn't not review this driver in the first
instance.

It doesn't bother me enough to go and change it myself and I'm not
going to have a baby over patches not being submitted to fix it.

> >>  		.num_resources	= ARRAY_SIZE(da9063_regulators_resources),
> >> @@ -100,15 +100,19 @@ static const struct mfd_cell da9063_devs[] = {
> >>  		.resources	= da9063_onkey_resources,
> >>  		.of_compatible = "dlg,da9063-onkey",
> >>  	},
> >> +	{
> >> +		.name		= DA9063_DRVNAME_VIBRATION,
> >> +	},
> > 
> > Place this on a single line please.
> 
> This would only make the style inconsistent with the ie. LEDs entry.
> 
> >         { .name	= DA9063_DRVNAME_VIBRATION },

If that is a one line entry spaced over multiple lines, then that
should also be changed.

Maybe I will go through and stylise this driver a bit after all (but
as time is short at the moment, maybe not!) :)

[...]

> >> +err_mfd_cleanup:
> >> +	mfd_remove_devices(da9063->dev);
> > 
> > Any reason why you can't use devm_*?
> 
> Because we need to undo the MFD setup before the IRQ setup.

Sounds like a good enough reason.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Marek Vasut]

On 06/06/2018 08:16 AM, Lee Jones wrote:
> On Tue, 05 Jun 2018, Marek Vasut wrote:

[...]

>>>> -static const struct mfd_cell da9063_devs[] = {
>>>> +static const struct mfd_cell da9063_common_devs[] = {
>>>>  	{
>>>>  		.name		= DA9063_DRVNAME_REGULATORS,
>>>
>>> Appreciate that these are historical, but these device name defines
>>> make me shudder.  They only serve to act as an obfuscation layer when
>>> debugging at platform level.  Please consider getting rid of them.
>>
>> The macro can be shared between the core and the drivers, so the names
>> never run out of sync.
> 
> Platform driver name changes are vary rare.  Even if they are changed,
> even light testing would uncover the fact that child drivers do not
> .probe().

Sure, while if the macro is used, this problem is avoided altogether.

> Due to the current obfuscation, I currently have no idea
> what this device's name is.

I'm sure ctags or git grep can easily help.

> This technique is not allowed for new
> drivers - unfortunately I didn't not review this driver in the first
> instance.

Why not ? This looks like a step back to me.

> It doesn't bother me enough to go and change it myself and I'm not
> going to have a baby over patches not being submitted to fix it.
> 
>>>>  		.num_resources	= ARRAY_SIZE(da9063_regulators_resources),
>>>> @@ -100,15 +100,19 @@ static const struct mfd_cell da9063_devs[] = {
>>>>  		.resources	= da9063_onkey_resources,
>>>>  		.of_compatible = "dlg,da9063-onkey",
>>>>  	},
>>>> +	{
>>>> +		.name		= DA9063_DRVNAME_VIBRATION,
>>>> +	},
>>>
>>> Place this on a single line please.
>>
>> This would only make the style inconsistent with the ie. LEDs entry.
>>
>>>         { .name	= DA9063_DRVNAME_VIBRATION },
> 
> If that is a one line entry spaced over multiple lines, then that
> should also be changed.
> 
> Maybe I will go through and stylise this driver a bit after all (but
> as time is short at the moment, maybe not!) :)

You'd end up with two entries which look different then the rest, which
triggers my OCD.

> [...]
> 
>>>> +err_mfd_cleanup:
>>>> +	mfd_remove_devices(da9063->dev);
>>>
>>> Any reason why you can't use devm_*?
>>
>> Because we need to undo the MFD setup before the IRQ setup.
> 
> Sounds like a good enough reason.

Or the da9063_irq_init() could use devm_regmap_add_irq_chip().

-- 
Best regards,
Marek Vasut

RE: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Steve Twiss]

Hi Marek and Geert,

On 06 June 2018 00:02 Marek Vasut wrote,

> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
> 
> On 06/05/2018 10:17 PM, Steve Twiss wrote:
> > Hi Marek and Geert,
> >
> > On 04 June 2018 17:25 Marek Vasut wrote,
> >
> >> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
> >>
> >> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
> >>> Hi Marek, Steve,
> >>>
> >>> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> >>>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
> >>>> prevent access into that register block.
> >
> > Ok. I've said previously in [v3 07/10], but I'll copy again:
> > There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
> > Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
> > (2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
> > on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
> > "Reserved".
> >
> > Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
> > Also Dialog do not store a history of Datasheets on their website so once this is updated (although
> > this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
> > make the commit message just as misleading as the current datasheet.
> >
> > How about something simpler?
> > "The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
> > into that register block."
> >
> >>>>
> >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>
> >>> Thanks for your patch!
> >>>
> >>>> --- a/drivers/mfd/da9063-i2c.c
> >>>> +++ b/drivers/mfd/da9063-i2c.c
> >>>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
> >>>
> >>> Note that the line above doesn't check da9063->type, but da9063-
> >>> variant_code...
> >>>
> >>>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
> >>>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
> >>>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
> >>>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
> >>>
> >>> ... so this may be slightly confusing.
> >>
> >> I know.
> >>
> >>>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
> >>>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
> >>>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
> >>>>         } else {
> >>>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
> >>>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
> >>>
> >>> However, da9063->variant_code doesn't seem to have been filled in at this
> >>> point yet (the call to da9063_device_init() doing so is below, at the end
> >>> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
> >>> support for AD silicon variant") never actually handled the AD silicon variant
> >>> correctly? Or am I missing something?
> >
> > Okay ... No. You're not missing anything. I had noticed that.
> > The AD chip model is not referenced and by default only the BB chip model is used.
> >
> >> Ha, that is a good point.
> >
> > Yeah, it's a good point, but it's not an amusing point.
> > The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
> > There is no datasheet listing AD registers supported by Dialog, only BB.
> >
> > But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
> > and the RTC driver was updated to distinguish between the AD and BB according to
> > the type of variant detected at run-time during the da9063_device_init() call.
> >
> > The real problem is that this leads to two competing chip detection methods for the
> > DA9063. The function da9063_device_init() autodetects the chip variant, but
> > autodetection cannot define the chip model. It's circular: the chip model cannot be
> > autodetected because a chip model is needed to access the register used during
> > autodetection.
> >
> > Which leads me back to what I said two paragraphs up:
> >> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
> >> There is no datasheet listing AD registers supported by Dialog, only BB.
> >
> > This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
> > used to print the information to the console during start-up and it is the DT that defines
> > the chip model based upon "dlg,da9062" or "dlg,da9061".
> 
> So the AD was broken since forever and noone noticed ? :)

Not quite. 
The AD support is working, but the driver doesn't work like everybody
expects because it uses the BB chip model. But it does work because the chip
model for BB is valid for AD; in this case BB represents a superset of AD
registers (and any mismatches are never accessed or mean anything in AD).

> Do you have an AD hardware and can you fix it ?

Part of my work is to support the community and I think this is fixable.

But all of this shouldn't affect your DA9063L submission should it?

Regards,
Steve

Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

[Marek Vasut]

On 06/06/2018 11:47 AM, Steve Twiss wrote:
> Hi Marek and Geert,
> 
> On 06 June 2018 00:02 Marek Vasut wrote,
> 
>> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
>>
>> On 06/05/2018 10:17 PM, Steve Twiss wrote:
>>> Hi Marek and Geert,
>>>
>>> On 04 June 2018 17:25 Marek Vasut wrote,
>>>
>>>> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
>>>>
>>>> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
>>>>> Hi Marek, Steve,
>>>>>
>>>>> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>>>>>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
>>>>>> prevent access into that register block.
>>>
>>> Ok. I've said previously in [v3 07/10], but I'll copy again:
>>> There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
>>> Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
>>> (2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
>>> on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
>>> "Reserved".
>>>
>>> Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
>>> Also Dialog do not store a history of Datasheets on their website so once this is updated (although
>>> this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
>>> make the commit message just as misleading as the current datasheet.
>>>
>>> How about something simpler?
>>> "The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
>>> into that register block."
>>>
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/drivers/mfd/da9063-i2c.c
>>>>>> +++ b/drivers/mfd/da9063-i2c.c
>>>>>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
>>>>>
>>>>> Note that the line above doesn't check da9063->type, but da9063-
>>>>> variant_code...
>>>>>
>>>>>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>>>>>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>>>>>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
>>>>>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
>>>>>
>>>>> ... so this may be slightly confusing.
>>>>
>>>> I know.
>>>>
>>>>>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
>>>>>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
>>>>>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>>>>>>         } else {
>>>>>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>>>>>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
>>>>>
>>>>> However, da9063->variant_code doesn't seem to have been filled in at this
>>>>> point yet (the call to da9063_device_init() doing so is below, at the end
>>>>> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
>>>>> support for AD silicon variant") never actually handled the AD silicon variant
>>>>> correctly? Or am I missing something?
>>>
>>> Okay ... No. You're not missing anything. I had noticed that.
>>> The AD chip model is not referenced and by default only the BB chip model is used.
>>>
>>>> Ha, that is a good point.
>>>
>>> Yeah, it's a good point, but it's not an amusing point.
>>> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
>>> There is no datasheet listing AD registers supported by Dialog, only BB.
>>>
>>> But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
>>> and the RTC driver was updated to distinguish between the AD and BB according to
>>> the type of variant detected at run-time during the da9063_device_init() call.
>>>
>>> The real problem is that this leads to two competing chip detection methods for the
>>> DA9063. The function da9063_device_init() autodetects the chip variant, but
>>> autodetection cannot define the chip model. It's circular: the chip model cannot be
>>> autodetected because a chip model is needed to access the register used during
>>> autodetection.
>>>
>>> Which leads me back to what I said two paragraphs up:
>>>> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
>>>> There is no datasheet listing AD registers supported by Dialog, only BB.
>>>
>>> This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
>>> used to print the information to the console during start-up and it is the DT that defines
>>> the chip model based upon "dlg,da9062" or "dlg,da9061".
>>
>> So the AD was broken since forever and noone noticed ? :)
> 
> Not quite. 
> The AD support is working, but the driver doesn't work like everybody
> expects because it uses the BB chip model. But it does work because the chip
> model for BB is valid for AD; in this case BB represents a superset of AD
> registers (and any mismatches are never accessed or mean anything in AD).
> 
>> Do you have an AD hardware and can you fix it ?
> 
> Part of my work is to support the community and I think this is fixable.
> 
> But all of this shouldn't affect your DA9063L submission should it?

I think there might be conflict between those patchsets, so let me send
out a V5 so you can play around with the AD and fix that too.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Lee Jones]

On Wed, 06 Jun 2018, Marek Vasut wrote:
> On 06/06/2018 08:16 AM, Lee Jones wrote:
> > On Tue, 05 Jun 2018, Marek Vasut wrote:
> 
> [...]
> 
> >>>> -static const struct mfd_cell da9063_devs[] = {
> >>>> +static const struct mfd_cell da9063_common_devs[] = {
> >>>>  	{
> >>>>  		.name		= DA9063_DRVNAME_REGULATORS,
> >>>
> >>> Appreciate that these are historical, but these device name defines
> >>> make me shudder.  They only serve to act as an obfuscation layer when
> >>> debugging at platform level.  Please consider getting rid of them.
> >>
> >> The macro can be shared between the core and the drivers, so the names
> >> never run out of sync.
> > 
> > Platform driver name changes are vary rare.  Even if they are changed,
> > even light testing would uncover the fact that child drivers do not
> > .probe().
> 
> Sure, while if the macro is used, this problem is avoided altogether.
> 
> > Due to the current obfuscation, I currently have no idea
> > what this device's name is.
> 
> I'm sure ctags or git grep can easily help.

I'm aware how to get around the 'issue', but it's an additional step
which is avoidable.  For me personally it comes from doing *a lot* of
platform level work and being irritated by the extra grepping.  Macros
for driver names does not sit right with me at all.  There are even
worse examples of people defining the MACROs *inside* the driver,
which doesn't even benefit from the small redeeming feature you
mention above.

Anyway, I'm happy with you not wanting to change it.  Just leave them
as they are for now.

> >>>> +	{
> >>>> +		.name		= DA9063_DRVNAME_VIBRATION,
> >>>> +	},
> >>>
> >>> Place this on a single line please.
> >>
> >> This would only make the style inconsistent with the ie. LEDs entry.
> >>
> >>>         { .name	= DA9063_DRVNAME_VIBRATION },
> > 
> > If that is a one line entry spaced over multiple lines, then that
> > should also be changed.
> > 
> > Maybe I will go through and stylise this driver a bit after all (but
> > as time is short at the moment, maybe not!) :)
> 
> You'd end up with two entries which look different then the rest, which
> triggers my OCD.

OCD or not, it's never okay to waste lines.  If ordering it not
important (which it should not be -- it's fragile to rely on device
ordering in MFD cells), the multi-line entries go at the top, followed
by the single line entries.  If done right, it looks the opposite of
bad/out of place.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Marek Vasut]

On 06/07/2018 07:04 AM, Lee Jones wrote:
> On Wed, 06 Jun 2018, Marek Vasut wrote:
>> On 06/06/2018 08:16 AM, Lee Jones wrote:
>>> On Tue, 05 Jun 2018, Marek Vasut wrote:
>>
>> [...]
>>
>>>>>> -static const struct mfd_cell da9063_devs[] = {
>>>>>> +static const struct mfd_cell da9063_common_devs[] = {
>>>>>>  	{
>>>>>>  		.name		= DA9063_DRVNAME_REGULATORS,
>>>>>
>>>>> Appreciate that these are historical, but these device name defines
>>>>> make me shudder.  They only serve to act as an obfuscation layer when
>>>>> debugging at platform level.  Please consider getting rid of them.
>>>>
>>>> The macro can be shared between the core and the drivers, so the names
>>>> never run out of sync.
>>>
>>> Platform driver name changes are vary rare.  Even if they are changed,
>>> even light testing would uncover the fact that child drivers do not
>>> .probe().
>>
>> Sure, while if the macro is used, this problem is avoided altogether.
>>
>>> Due to the current obfuscation, I currently have no idea
>>> what this device's name is.
>>
>> I'm sure ctags or git grep can easily help.
> 
> I'm aware how to get around the 'issue', but it's an additional step
> which is avoidable.  For me personally it comes from doing *a lot* of
> platform level work and being irritated by the extra grepping.  Macros
> for driver names does not sit right with me at all.  There are even
> worse examples of people defining the MACROs *inside* the driver,
> which doesn't even benefit from the small redeeming feature you
> mention above.

If we follow this line of thinking, we could just run cpp and expand all
macros. Then there's no need for grepping at all. That probably won't be
the result anyone would like though.

> Anyway, I'm happy with you not wanting to change it.  Just leave them
> as they are for now.
> 
>>>>>> +	{
>>>>>> +		.name		= DA9063_DRVNAME_VIBRATION,
>>>>>> +	},
>>>>>
>>>>> Place this on a single line please.
>>>>
>>>> This would only make the style inconsistent with the ie. LEDs entry.
>>>>
>>>>>         { .name	= DA9063_DRVNAME_VIBRATION },
>>>
>>> If that is a one line entry spaced over multiple lines, then that
>>> should also be changed.
>>>
>>> Maybe I will go through and stylise this driver a bit after all (but
>>> as time is short at the moment, maybe not!) :)
>>
>> You'd end up with two entries which look different then the rest, which
>> triggers my OCD.
> 
> OCD or not, it's never okay to waste lines.  If ordering it not
> important (which it should not be -- it's fragile to rely on device
> ordering in MFD cells), the multi-line entries go at the top, followed
> by the single line entries.  If done right, it looks the opposite of
> bad/out of place.

My point is, the style should at least be consistent. But anyway.

-- 
Best regards,
Marek Vasut

Re: [PATCH v3 08/10] mfd: da9063: Register RTC only on DA9063L

[Lee Jones]

On Thu, 07 Jun 2018, Marek Vasut wrote:

> On 06/07/2018 07:04 AM, Lee Jones wrote:
> > On Wed, 06 Jun 2018, Marek Vasut wrote:
> >> On 06/06/2018 08:16 AM, Lee Jones wrote:
> >>> On Tue, 05 Jun 2018, Marek Vasut wrote:
> >>
> >> [...]
> >>
> >>>>>> -static const struct mfd_cell da9063_devs[] = {
> >>>>>> +static const struct mfd_cell da9063_common_devs[] = {
> >>>>>>  	{
> >>>>>>  		.name		= DA9063_DRVNAME_REGULATORS,
> >>>>>
> >>>>> Appreciate that these are historical, but these device name defines
> >>>>> make me shudder.  They only serve to act as an obfuscation layer when
> >>>>> debugging at platform level.  Please consider getting rid of them.
> >>>>
> >>>> The macro can be shared between the core and the drivers, so the names
> >>>> never run out of sync.
> >>>
> >>> Platform driver name changes are vary rare.  Even if they are changed,
> >>> even light testing would uncover the fact that child drivers do not
> >>> .probe().
> >>
> >> Sure, while if the macro is used, this problem is avoided altogether.
> >>
> >>> Due to the current obfuscation, I currently have no idea
> >>> what this device's name is.
> >>
> >> I'm sure ctags or git grep can easily help.
> > 
> > I'm aware how to get around the 'issue', but it's an additional step
> > which is avoidable.  For me personally it comes from doing *a lot* of
> > platform level work and being irritated by the extra grepping.  Macros
> > for driver names does not sit right with me at all.  There are even
> > worse examples of people defining the MACROs *inside* the driver,
> > which doesn't even benefit from the small redeeming feature you
> > mention above.
> 
> If we follow this line of thinking, we could just run cpp and expand all
> macros. Then there's no need for grepping at all. That probably won't be
> the result anyone would like though.

Hmm ... yes, that's the same! :D

> > Anyway, I'm happy with you not wanting to change it.  Just leave them
> > as they are for now.
> >>>>>> +	{
> >>>>>> +		.name		= DA9063_DRVNAME_VIBRATION,
> >>>>>> +	},
> >>>>>
> >>>>> Place this on a single line please.
> >>>>
> >>>> This would only make the style inconsistent with the ie. LEDs entry.
> >>>>
> >>>>>         { .name	= DA9063_DRVNAME_VIBRATION },
> >>>
> >>> If that is a one line entry spaced over multiple lines, then that
> >>> should also be changed.
> >>>
> >>> Maybe I will go through and stylise this driver a bit after all (but
> >>> as time is short at the moment, maybe not!) :)
> >>
> >> You'd end up with two entries which look different then the rest, which
> >> triggers my OCD.
> > 
> > OCD or not, it's never okay to waste lines.  If ordering it not
> > important (which it should not be -- it's fragile to rely on device
> > ordering in MFD cells), the multi-line entries go at the top, followed
> > by the single line entries.  If done right, it looks the opposite of
> > bad/out of place.
> 
> My point is, the style should at least be consistent. But anyway.

It is consistent.

 - Multi-line entries go on multiple lines.
 - Single line entries go on single lines.

See drivers/mfd/max77620.c for how it should look.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog