[PATCH v3 00/10] mfd: tps65218: Clean ups

[PATCH v3 00/10] mfd: tps65218: Clean ups

[Keerthy]

The series cleans up mainly the regulator driver and implements
the device tree parsing using the regulator framework. Removes
all the redundant compatibles for the individual regulators.
Removes compatible for gpio driver as well and adds it to
the mfd_cell.

One of the patch removes redundant read wrapper and makes
use of regmap_read wherever necessary.

The series is checked for all the regulator registrations on
am437x-gp-evm and am437x-sk-evm.

Changes in v3:

Missed making corresponding changes in the power button and gpio
drivers. Added couple of patches to fix them to use mfd_cell way
of parsing.


Keerthy (10):
  mfd: tps65218: Remove redundant read wrapper
  Documentation: regulator: tps65218: Update examples
  input: tps65218-pwrbutton: Add platform_device_id table
  mfd: tps65218: Use mfd_add_devices instead of of_platform_populate
  regulator: tps65218: Remove all the compatibles
  gpio: tps65218: Remove the compatible
  ARM: dts: AM437X-GP-EVM: Remove redundant regulator compatibles
  ARM: dts: AM437X-SK-EVM: Remove redundant regulator compatibles
  ARM: dts: AM437X-CM-T43: Remove redundant regulator compatibles
  ARM: dts: AM43X-EPOS-EVM: Remove redundant regulator compatibles

 .../devicetree/bindings/regulator/tps65218.txt     |  87 ++++++++++---
 arch/arm/boot/dts/am437x-cm-t43.dts                |   6 -
 arch/arm/boot/dts/am437x-gp-evm.dts                |   6 -
 arch/arm/boot/dts/am437x-sk-evm.dts                |   5 -
 arch/arm/boot/dts/am43x-epos-evm.dts               |   7 --
 drivers/gpio/gpio-tps65218.c                       |  13 +-
 drivers/input/misc/tps65218-pwrbutton.c            |  10 +-
 drivers/mfd/tps65218.c                             |  29 ++---
 drivers/regulator/tps65218-regulator.c             | 134 ++++++++-------------
 include/linux/mfd/tps65218.h                       |   2 -
 10 files changed, 147 insertions(+), 152 deletions(-)

-- 
1.9.1

[PATCH v3 01/10] mfd: tps65218: Remove redundant read wrapper

[Keerthy]

Currently read directly calls the repmap read function. Hence
remove the redundant wrapper and use regmap read wherever
needed.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---

Changes in v3:

  * Cleaned up tps65218-pwrbutton and gpio drivers to remove
    tps65218_reg_read instances.

 drivers/gpio/gpio-tps65218.c            |  3 ++-
 drivers/input/misc/tps65218-pwrbutton.c |  3 ++-
 drivers/mfd/tps65218.c                  | 16 +---------------
 drivers/regulator/tps65218-regulator.c  |  3 ++-
 include/linux/mfd/tps65218.h            |  2 --
 5 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-tps65218.c b/drivers/gpio/gpio-tps65218.c
index 0eaeac8..0f9d9bd 100644
--- a/drivers/gpio/gpio-tps65218.c
+++ b/drivers/gpio/gpio-tps65218.c
@@ -16,6 +16,7 @@
 #include <linux/errno.h>
 #include <linux/gpio/driver.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/mfd/tps65218.h>
 
 struct tps65218_gpio {
@@ -30,7 +31,7 @@ static int tps65218_gpio_get(struct gpio_chip *gc, unsigned offset)
 	unsigned int val;
 	int ret;
 
-	ret = tps65218_reg_read(tps65218, TPS65218_REG_ENABLE2, &val);
+	ret = regmap_read(tps65218->regmap, TPS65218_REG_ENABLE2, &val);
 	if (ret)
 		return ret;
 
diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
index a39b626..2bba8de 100644
--- a/drivers/input/misc/tps65218-pwrbutton.c
+++ b/drivers/input/misc/tps65218-pwrbutton.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 struct tps65218_pwrbutton {
@@ -36,7 +37,7 @@ static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr)
 	unsigned int reg;
 	int error;
 
-	error = tps65218_reg_read(pwr->tps, TPS65218_REG_STATUS, &reg);
+	error = regmap_read(pwr->tps->regmap, TPS65218_REG_STATUS, &reg);
 	if (error) {
 		dev_err(pwr->dev, "can't read register: %d\n", error);
 		goto out;
diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
index 80b9dc3..f20a531 100644
--- a/drivers/mfd/tps65218.c
+++ b/drivers/mfd/tps65218.c
@@ -34,20 +34,6 @@
 #define TPS65218_PASSWORD_REGS_UNLOCK   0x7D
 
 /**
- * tps65218_reg_read: Read a single tps65218 register.
- *
- * @tps: Device to read from.
- * @reg: Register to read.
- * @val: Contians the value
- */
-int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
-			unsigned int *val)
-{
-	return regmap_read(tps->regmap, reg, val);
-}
-EXPORT_SYMBOL_GPL(tps65218_reg_read);
-
-/**
  * tps65218_reg_write: Write a single tps65218 register.
  *
  * @tps65218: Device to write to.
@@ -93,7 +79,7 @@ static int tps65218_update_bits(struct tps65218 *tps, unsigned int reg,
 	int ret;
 	unsigned int data;
 
-	ret = tps65218_reg_read(tps, reg, &data);
+	ret = regmap_read(tps->regmap, reg, &data);
 	if (ret) {
 		dev_err(tps->dev, "Read from reg 0x%x failed\n", reg);
 		return ret;
diff --git a/drivers/regulator/tps65218-regulator.c b/drivers/regulator/tps65218-regulator.c
index a5e5634..679ab2f 100644
--- a/drivers/regulator/tps65218-regulator.c
+++ b/drivers/regulator/tps65218-regulator.c
@@ -22,6 +22,7 @@
 #include <linux/err.h>
 #include <linux/platform_device.h>
 #include <linux/of_device.h>
+#include <linux/regmap.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
@@ -224,7 +225,7 @@ static int tps65218_pmic_get_current_limit(struct regulator_dev *dev)
 	unsigned int index;
 	struct tps65218 *tps = rdev_get_drvdata(dev);
 
-	retval = tps65218_reg_read(tps, dev->desc->csel_reg, &index);
+	retval = regmap_read(tps->regmap, dev->desc->csel_reg, &index);
 	if (retval < 0)
 		return retval;
 
diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
index d58f3b5..7c9241f 100644
--- a/include/linux/mfd/tps65218.h
+++ b/include/linux/mfd/tps65218.h
@@ -276,8 +276,6 @@ struct tps65218 {
 	struct regmap *regmap;
 };
 
-int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
-					unsigned int *val);
 int tps65218_reg_write(struct tps65218 *tps, unsigned int reg,
 			unsigned int val, unsigned int level);
 int tps65218_set_bits(struct tps65218 *tps, unsigned int reg,
-- 
1.9.1

[PATCH v3 02/10] Documentation: regulator: tps65218: Update examples

[Keerthy]

This updates the device tree according to the preferred way of parsing
the nodes using the regulator framework.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 .../devicetree/bindings/regulator/tps65218.txt     | 87 ++++++++++++++++++----
 1 file changed, 71 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
index fccc1d2..5e1888f 100644
--- a/Documentation/devicetree/bindings/regulator/tps65218.txt
+++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
@@ -1,23 +1,78 @@
 TPS65218 family of regulators
 
 Required properties:
-For tps65218 regulators/LDOs
-- compatible:
-  - "ti,tps65218-dcdc1" for DCDC1
-  - "ti,tps65218-dcdc2" for DCDC2
-  - "ti,tps65218-dcdc3" for DCDC3
-  - "ti,tps65218-dcdc4" for DCDC4
-  - "ti,tps65218-dcdc5" for DCDC5
-  - "ti,tps65218-dcdc6" for DCDC6
-  - "ti,tps65218-ldo1" for LDO1
-
-Optional properties:
-- Any optional property defined in bindings/regulator/regulator.txt
+- compatible: "ti,tps65218"
+- reg: I2C slave address
+- list of regulators provided by this controller, must be named
+  after their hardware counterparts: dcdc[1-6] and ldo1
+- This is the list of child nodes that specify the regulator
+  initialization data for defined regulators. Not all regulators for the given
+  device need to be present. The definition for each of these nodes is defined
+  using the standard binding for regulators found at
+  Documentation/devicetree/bindings/regulator/regulator.txt.
+
+  The valid names for regulators are:
+  tps65217: regulator-dcdc1, regulator-dcdc2, regulator-dcdc3, regulator-dcdc4,
+  regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3
+  Each regulator is defined using the standard binding for regulators.
 
 Example:
+tps65218: tps65218@24 {
+	reg = <0x24>;
+	compatible = "ti,tps65218";
+	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
+	interrupt-controller;
+	#interrupt-cells = <2>;
+
+	dcdc1: regulator-dcdc1 {
+		regulator-name = "vdd_core";
+		regulator-min-microvolt = <912000>;
+		regulator-max-microvolt = <1144000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	dcdc2: regulator-dcdc2 {
+		regulator-name = "vdd_mpu";
+		regulator-min-microvolt = <912000>;
+		regulator-max-microvolt = <1378000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	dcdc3: regulator-dcdc3 {
+		regulator-name = "vdcdc3";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <1500000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	dcdc5: regulator-dcdc5 {
+		regulator-name = "v1_0bat";
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	dcdc6: regulator-dcdc6 {
+		regulator-name = "v1_8bat";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	ldo1: regulator-ldo1 {
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 
-	xyz: regulator@0 {
-		compatible = "ti,tps65218-dcdc1";
-		regulator-min-microvolt  = <1000000>;
-		regulator-max-microvolt  = <3000000>;
+	ls3: regulator-ls3 {
+		regulator-min-microvolt = <100000>;
+		regulator-max-microvolt = <1000000>;
 	};
+};
-- 
1.9.1

[PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[Keerthy]

platform_device_id table is needed for adding the tps65218-pwrbutton
module to the mfd_cell array.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/input/misc/tps65218-pwrbutton.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
index 2bba8de..a0cb7d2 100644
--- a/drivers/input/misc/tps65218-pwrbutton.c
+++ b/drivers/input/misc/tps65218-pwrbutton.c
@@ -113,12 +113,19 @@ static const struct of_device_id of_tps65218_pwr_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
 
+static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
+	{ "tps65218-pwrbutton", },
+	{ /* sentinel */ }
+}
+MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
+
 static struct platform_driver tps65218_pwron_driver = {
 	.probe	= tps65218_pwron_probe,
 	.driver	= {
 		.name	= "tps65218_pwrbutton",
 		.of_match_table = of_tps65218_pwr_match,
 	},
+	.id_table = tps65218_pwrbtn_id_table,
 };
 module_platform_driver(tps65218_pwron_driver);
 
-- 
1.9.1

[PATCH v3 04/10] mfd: tps65218: Use mfd_add_devices instead of of_platform_populate

[Keerthy]

mfd_add_devices enables parsing device tree nodes without compatibles
for regulators and gpio modules. Replace of_platform_populate with
mfd_add_devices. mfd_cell currently is populated with regulators,
gpio and powerbutton.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/tps65218.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
index f20a531..107d695 100644
--- a/drivers/mfd/tps65218.c
+++ b/drivers/mfd/tps65218.c
@@ -33,6 +33,15 @@
 
 #define TPS65218_PASSWORD_REGS_UNLOCK   0x7D
 
+static const struct mfd_cell tps65218_cells[] = {
+	{ .name = "tps65218-regulator", },
+	{
+		.name = "tps65218-pwrbutton",
+		.of_compatible = "ti,tps65218-pwrbutton",
+	},
+	{ .name = "tps65218-gpio", },
+};
+
 /**
  * tps65218_reg_write: Write a single tps65218 register.
  *
@@ -236,8 +245,10 @@ static int tps65218_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	ret = of_platform_populate(client->dev.of_node, NULL, NULL,
-				   &client->dev);
+	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
+			      ARRAY_SIZE(tps65218_cells), NULL, 0,
+			      regmap_irq_get_domain(tps->irq_data));
+
 	if (ret < 0)
 		goto err_irq;
 
-- 
1.9.1

[PATCH v3 05/10] regulator: tps65218: Remove all the compatibles

[Keerthy]

Remove all the individual compatibles for all the regulators
and introduce id_table and update the driver accordingly
to parse device tree nodes using the regulator framework.

Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/regulator/tps65218-regulator.c | 131 +++++++++++----------------------
 1 file changed, 45 insertions(+), 86 deletions(-)

diff --git a/drivers/regulator/tps65218-regulator.c b/drivers/regulator/tps65218-regulator.c
index 679ab2f..8106cc4 100644
--- a/drivers/regulator/tps65218-regulator.c
+++ b/drivers/regulator/tps65218-regulator.c
@@ -31,10 +31,11 @@
 enum tps65218_regulators { DCDC1, DCDC2, DCDC3, DCDC4,
 			   DCDC5, DCDC6, LDO1, LS3 };
 
-#define TPS65218_REGULATOR(_name, _id, _type, _ops, _n, _vr, _vm, _er, _em, \
-			    _cr, _cm, _lr, _nlr, _delay, _fuv)		\
+#define TPS65218_REGULATOR(_name, _of,  _id, _type, _ops, _n, _vr, _vm, _er,\
+			   _em,  _cr, _cm, _lr, _nlr, _delay, _fuv)	\
 	{							\
 		.name			= _name,		\
+		.of_match		= _of,			\
 		.id			= _id,			\
 		.ops			= &_ops,		\
 		.n_voltages		= _n,			\
@@ -53,14 +54,6 @@ enum tps65218_regulators { DCDC1, DCDC2, DCDC3, DCDC4,
 		.fixed_uV		= _fuv			\
 	}							\
 
-#define TPS65218_INFO(_id, _nm, _min, _max)	\
-	[_id] = {					\
-		.id		= _id,			\
-		.name		= _nm,			\
-		.min_uV		= _min,			\
-		.max_uV		= _max,			\
-	}
-
 static const struct regulator_linear_range dcdc1_dcdc2_ranges[] = {
 	REGULATOR_LINEAR_RANGE(850000, 0x0, 0x32, 10000),
 	REGULATOR_LINEAR_RANGE(1375000, 0x33, 0x3f, 25000),
@@ -76,36 +69,6 @@ static const struct regulator_linear_range dcdc4_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1600000, 0x10, 0x34, 50000),
 };
 
-static struct tps_info tps65218_pmic_regs[] = {
-	TPS65218_INFO(DCDC1, "DCDC1", 850000, 1675000),
-	TPS65218_INFO(DCDC2, "DCDC2", 850000, 1675000),
-	TPS65218_INFO(DCDC3, "DCDC3", 900000, 3400000),
-	TPS65218_INFO(DCDC4, "DCDC4", 1175000, 3400000),
-	TPS65218_INFO(DCDC5, "DCDC5", 1000000, 1000000),
-	TPS65218_INFO(DCDC6, "DCDC6", 1800000, 1800000),
-	TPS65218_INFO(LDO1, "LDO1", 900000, 3400000),
-	TPS65218_INFO(LS3, "LS3", -1, -1),
-};
-
-#define TPS65218_OF_MATCH(comp, label) \
-	{ \
-		.compatible = comp, \
-		.data = &label, \
-	}
-
-static const struct of_device_id tps65218_of_match[] = {
-	TPS65218_OF_MATCH("ti,tps65218-dcdc1", tps65218_pmic_regs[DCDC1]),
-	TPS65218_OF_MATCH("ti,tps65218-dcdc2", tps65218_pmic_regs[DCDC2]),
-	TPS65218_OF_MATCH("ti,tps65218-dcdc3", tps65218_pmic_regs[DCDC3]),
-	TPS65218_OF_MATCH("ti,tps65218-dcdc4", tps65218_pmic_regs[DCDC4]),
-	TPS65218_OF_MATCH("ti,tps65218-dcdc5", tps65218_pmic_regs[DCDC5]),
-	TPS65218_OF_MATCH("ti,tps65218-dcdc6", tps65218_pmic_regs[DCDC6]),
-	TPS65218_OF_MATCH("ti,tps65218-ldo1", tps65218_pmic_regs[LDO1]),
-	TPS65218_OF_MATCH("ti,tps65218-ls3", tps65218_pmic_regs[LS3]),
-	{ }
-};
-MODULE_DEVICE_TABLE(of, tps65218_of_match);
-
 static int tps65218_pmic_set_voltage_sel(struct regulator_dev *dev,
 					 unsigned selector)
 {
@@ -251,92 +214,88 @@ static struct regulator_ops tps65218_dcdc56_pmic_ops = {
 };
 
 static const struct regulator_desc regulators[] = {
-	TPS65218_REGULATOR("DCDC1", TPS65218_DCDC_1, REGULATOR_VOLTAGE,
-			   tps65218_dcdc12_ops, 64, TPS65218_REG_CONTROL_DCDC1,
+	TPS65218_REGULATOR("DCDC1", "regulator-dcdc1", TPS65218_DCDC_1,
+			   REGULATOR_VOLTAGE, tps65218_dcdc12_ops, 64,
+			   TPS65218_REG_CONTROL_DCDC1,
 			   TPS65218_CONTROL_DCDC1_MASK, TPS65218_REG_ENABLE1,
 			   TPS65218_ENABLE1_DC1_EN, 0, 0, dcdc1_dcdc2_ranges,
 			   2, 4000, 0),
-	TPS65218_REGULATOR("DCDC2", TPS65218_DCDC_2, REGULATOR_VOLTAGE,
-			   tps65218_dcdc12_ops, 64, TPS65218_REG_CONTROL_DCDC2,
+	TPS65218_REGULATOR("DCDC2", "regulator-dcdc2", TPS65218_DCDC_2,
+			   REGULATOR_VOLTAGE, tps65218_dcdc12_ops, 64,
+			   TPS65218_REG_CONTROL_DCDC2,
 			   TPS65218_CONTROL_DCDC2_MASK, TPS65218_REG_ENABLE1,
 			   TPS65218_ENABLE1_DC2_EN, 0, 0, dcdc1_dcdc2_ranges,
 			   2, 4000, 0),
-	TPS65218_REGULATOR("DCDC3", TPS65218_DCDC_3, REGULATOR_VOLTAGE,
-			   tps65218_ldo1_dcdc34_ops, 64,
+	TPS65218_REGULATOR("DCDC3", "regulator-dcdc3", TPS65218_DCDC_3,
+			   REGULATOR_VOLTAGE, tps65218_ldo1_dcdc34_ops, 64,
 			   TPS65218_REG_CONTROL_DCDC3,
 			   TPS65218_CONTROL_DCDC3_MASK, TPS65218_REG_ENABLE1,
 			   TPS65218_ENABLE1_DC3_EN, 0, 0, ldo1_dcdc3_ranges, 2,
 			   0, 0),
-	TPS65218_REGULATOR("DCDC4", TPS65218_DCDC_4, REGULATOR_VOLTAGE,
-			   tps65218_ldo1_dcdc34_ops, 53,
+	TPS65218_REGULATOR("DCDC4", "regulator-dcdc4", TPS65218_DCDC_4,
+			   REGULATOR_VOLTAGE, tps65218_ldo1_dcdc34_ops, 53,
 			   TPS65218_REG_CONTROL_DCDC4,
 			   TPS65218_CONTROL_DCDC4_MASK, TPS65218_REG_ENABLE1,
 			   TPS65218_ENABLE1_DC4_EN, 0, 0, dcdc4_ranges, 2,
 			   0, 0),
-	TPS65218_REGULATOR("DCDC5", TPS65218_DCDC_5, REGULATOR_VOLTAGE,
-			   tps65218_dcdc56_pmic_ops, 1, -1, -1,
-			   TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC5_EN, 0, 0,
-			   NULL, 0, 0, 1000000),
-	TPS65218_REGULATOR("DCDC6", TPS65218_DCDC_6, REGULATOR_VOLTAGE,
-			   tps65218_dcdc56_pmic_ops, 1, -1, -1,
-			   TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC6_EN, 0, 0,
-			   NULL, 0, 0, 1800000),
-	TPS65218_REGULATOR("LDO1", TPS65218_LDO_1, REGULATOR_VOLTAGE,
-			   tps65218_ldo1_dcdc34_ops, 64,
+	TPS65218_REGULATOR("DCDC5", "regulator-dcdc5", TPS65218_DCDC_5,
+			   REGULATOR_VOLTAGE, tps65218_dcdc56_pmic_ops, 1, -1,
+			   -1, TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC5_EN, 0,
+			   0, NULL, 0, 0, 1000000),
+	TPS65218_REGULATOR("DCDC6", "regulator-dcdc6", TPS65218_DCDC_6,
+			   REGULATOR_VOLTAGE, tps65218_dcdc56_pmic_ops, 1, -1,
+			   -1, TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC6_EN, 0,
+			   0, NULL, 0, 0, 1800000),
+	TPS65218_REGULATOR("LDO1", "regulator-ldo1", TPS65218_LDO_1,
+			   REGULATOR_VOLTAGE, tps65218_ldo1_dcdc34_ops, 64,
 			   TPS65218_REG_CONTROL_LDO1,
 			   TPS65218_CONTROL_LDO1_MASK, TPS65218_REG_ENABLE2,
 			   TPS65218_ENABLE2_LDO1_EN, 0, 0, ldo1_dcdc3_ranges,
 			   2, 0, 0),
-	TPS65218_REGULATOR("LS3", TPS65218_LS_3, REGULATOR_CURRENT,
-			   tps65218_ls3_ops, 0, 0, 0, TPS65218_REG_ENABLE2,
-			   TPS65218_ENABLE2_LS3_EN, TPS65218_REG_CONFIG2,
-			   TPS65218_CONFIG2_LS3ILIM_MASK, NULL, 0, 0, 0),
+	TPS65218_REGULATOR("LS3", "regulator-ls3", TPS65218_LS_3,
+			   REGULATOR_CURRENT, tps65218_ls3_ops, 0, 0, 0,
+			   TPS65218_REG_ENABLE2, TPS65218_ENABLE2_LS3_EN,
+			   TPS65218_REG_CONFIG2, TPS65218_CONFIG2_LS3ILIM_MASK,
+			   NULL, 0, 0, 0),
 };
 
 static int tps65218_regulator_probe(struct platform_device *pdev)
 {
 	struct tps65218 *tps = dev_get_drvdata(pdev->dev.parent);
-	struct regulator_init_data *init_data;
-	const struct tps_info	*template;
 	struct regulator_dev *rdev;
-	const struct of_device_id	*match;
 	struct regulator_config config = { };
-	int id;
-
-	match = of_match_device(tps65218_of_match, &pdev->dev);
-	if (!match)
-		return -ENODEV;
-
-	template = match->data;
-	id = template->id;
-	init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
-					       &regulators[id]);
+	int i;
 
-	platform_set_drvdata(pdev, tps);
-
-	tps->info[id] = &tps65218_pmic_regs[id];
 	config.dev = &pdev->dev;
-	config.init_data = init_data;
+	config.dev->of_node = tps->dev->of_node;
 	config.driver_data = tps;
 	config.regmap = tps->regmap;
-	config.of_node = pdev->dev.of_node;
 
-	rdev = devm_regulator_register(&pdev->dev, &regulators[id], &config);
-	if (IS_ERR(rdev)) {
-		dev_err(tps->dev, "failed to register %s regulator\n",
-			pdev->name);
-		return PTR_ERR(rdev);
+	for (i = 0; i < ARRAY_SIZE(regulators); i++) {
+		rdev = devm_regulator_register(&pdev->dev, &regulators[i],
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(tps->dev, "failed to register %s regulator\n",
+				pdev->name);
+			return PTR_ERR(rdev);
+		}
 	}
 
 	return 0;
 }
 
+static const struct platform_device_id tps65218_regulator_id_table[] = {
+	{ "tps65218-regulator", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, tps65218_regulator_id_table);
+
 static struct platform_driver tps65218_regulator_driver = {
 	.driver = {
 		.name = "tps65218-pmic",
-		.of_match_table = tps65218_of_match,
 	},
 	.probe = tps65218_regulator_probe,
+	.id_table = tps65218_regulator_id_table,
 };
 
 module_platform_driver(tps65218_regulator_driver);
-- 
1.9.1

[PATCH v3 06/10] gpio: tps65218: Remove the compatible

[Keerthy]

Remove the individual compatible for the gpio module.
Introduce id_table and update the driver accordingly
to parse device tree nodes by just using the mfd compatible.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/gpio/gpio-tps65218.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-tps65218.c b/drivers/gpio/gpio-tps65218.c
index 0f9d9bd..06fd61f 100644
--- a/drivers/gpio/gpio-tps65218.c
+++ b/drivers/gpio/gpio-tps65218.c
@@ -225,19 +225,19 @@ static int tps65218_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id tps65218_dt_match[] = {
-	{ .compatible = "ti,tps65218-gpio" },
-	{  }
+static const struct platform_device_id tps65218_gpio_id_table[] = {
+	{ "tps65218-gpio", },
+	{ /* sentinel */ }
 };
-MODULE_DEVICE_TABLE(of, tps65218_dt_match);
+MODULE_DEVICE_TABLE(platform, tps65218_gpio_id_table);
 
 static struct platform_driver tps65218_gpio_driver = {
 	.driver = {
 		.name = "tps65218-gpio",
-		.of_match_table = of_match_ptr(tps65218_dt_match)
 	},
 	.probe = tps65218_gpio_probe,
 	.remove = tps65218_gpio_remove,
+	.id_table = tps65218_gpio_id_table,
 };
 
 module_platform_driver(tps65218_gpio_driver);
-- 
1.9.1

[PATCH v3 07/10] ARM: dts: AM437X-GP-EVM: Remove redundant regulator compatibles

[Keerthy]

With the device tree parsing using the regulator framework
there is a no longer a need for separate compatibles for
individual regulator nodes. Hence removing them all.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 arch/arm/boot/dts/am437x-gp-evm.dts | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index 84832bf..dd974c7 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -513,7 +513,6 @@
 		#interrupt-cells = <2>;
 
 		dcdc1: regulator-dcdc1 {
-			compatible = "ti,tps65218-dcdc1";
 			regulator-name = "vdd_core";
 			regulator-min-microvolt = <912000>;
 			regulator-max-microvolt = <1144000>;
@@ -522,7 +521,6 @@
 		};
 
 		dcdc2: regulator-dcdc2 {
-			compatible = "ti,tps65218-dcdc2";
 			regulator-name = "vdd_mpu";
 			regulator-min-microvolt = <912000>;
 			regulator-max-microvolt = <1378000>;
@@ -531,7 +529,6 @@
 		};
 
 		dcdc3: regulator-dcdc3 {
-			compatible = "ti,tps65218-dcdc3";
 			regulator-name = "vdcdc3";
 			regulator-min-microvolt = <1500000>;
 			regulator-max-microvolt = <1500000>;
@@ -539,7 +536,6 @@
 			regulator-always-on;
 		};
 		dcdc5: regulator-dcdc5 {
-			compatible = "ti,tps65218-dcdc5";
 			regulator-name = "v1_0bat";
 			regulator-min-microvolt = <1000000>;
 			regulator-max-microvolt = <1000000>;
@@ -548,7 +544,6 @@
 		};
 
 		dcdc6: regulator-dcdc6 {
-			compatible = "ti,tps65218-dcdc6";
 			regulator-name = "v1_8bat";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
@@ -557,7 +552,6 @@
 		};
 
 		ldo1: regulator-ldo1 {
-			compatible = "ti,tps65218-ldo1";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
 			regulator-boot-on;
-- 
1.9.1

[PATCH v3 08/10] ARM: dts: AM437X-SK-EVM: Remove redundant regulator compatibles

[Keerthy]

With the device tree parsing using the regulator framework
there is a no longer a need for separate compatibles for
individual regulator nodes. Hence removing them all.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 arch/arm/boot/dts/am437x-sk-evm.dts | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
index 5687d6b..9da2173 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -428,7 +428,6 @@
 		#interrupt-cells = <2>;
 
 		dcdc1: regulator-dcdc1 {
-			compatible = "ti,tps65218-dcdc1";
 			/* VDD_CORE limits min of OPP50 and max of OPP100 */
 			regulator-name = "vdd_core";
 			regulator-min-microvolt = <912000>;
@@ -438,7 +437,6 @@
 		};
 
 		dcdc2: regulator-dcdc2 {
-			compatible = "ti,tps65218-dcdc2";
 			/* VDD_MPU limits min of OPP50 and max of OPP_NITRO */
 			regulator-name = "vdd_mpu";
 			regulator-min-microvolt = <912000>;
@@ -448,7 +446,6 @@
 		};
 
 		dcdc3: regulator-dcdc3 {
-			compatible = "ti,tps65218-dcdc3";
 			regulator-name = "vdds_ddr";
 			regulator-min-microvolt = <1500000>;
 			regulator-max-microvolt = <1500000>;
@@ -457,7 +454,6 @@
 		};
 
 		dcdc4: regulator-dcdc4 {
-			compatible = "ti,tps65218-dcdc4";
 			regulator-name = "v3_3d";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
@@ -466,7 +462,6 @@
 		};
 
 		ldo1: regulator-ldo1 {
-			compatible = "ti,tps65218-ldo1";
 			regulator-name = "v1_8d";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
-- 
1.9.1

[PATCH v3 09/10] ARM: dts: AM437X-CM-T43: Remove redundant regulator compatibles

[Keerthy]

With the device tree parsing using the regulator framework
there is a no longer a need for separate compatibles for
individual regulator nodes. Hence removing them all.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 arch/arm/boot/dts/am437x-cm-t43.dts | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-cm-t43.dts b/arch/arm/boot/dts/am437x-cm-t43.dts
index 9551c47..9e92d48 100644
--- a/arch/arm/boot/dts/am437x-cm-t43.dts
+++ b/arch/arm/boot/dts/am437x-cm-t43.dts
@@ -209,7 +209,6 @@
 		#interrupt-cells = <2>;
 
 		dcdc1: regulator-dcdc1 {
-			compatible = "ti,tps65218-dcdc1";
 			regulator-name = "vdd_core";
 			regulator-min-microvolt = <912000>;
 			regulator-max-microvolt = <1144000>;
@@ -218,7 +217,6 @@
 		};
 
 		dcdc2: regulator-dcdc2 {
-			compatible = "ti,tps65218-dcdc2";
 			regulator-name = "vdd_mpu";
 			regulator-min-microvolt = <912000>;
 			regulator-max-microvolt = <1378000>;
@@ -227,7 +225,6 @@
 		};
 
 		dcdc3: regulator-dcdc3 {
-			compatible = "ti,tps65218-dcdc3";
 			regulator-name = "vdcdc3";
 			regulator-suspend-enable;
 			regulator-min-microvolt = <1500000>;
@@ -237,7 +234,6 @@
 		};
 
 		dcdc5: regulator-dcdc5 {
-			compatible = "ti,tps65218-dcdc5";
 			regulator-name = "v1_0bat";
 			regulator-min-microvolt = <1000000>;
 			regulator-max-microvolt = <1000000>;
@@ -246,7 +242,6 @@
 		};
 
 		dcdc6: regulator-dcdc6 {
-			compatible = "ti,tps65218-dcdc6";
 			regulator-name = "v1_8bat";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
@@ -255,7 +250,6 @@
 		};
 
 		ldo1: regulator-ldo1 {
-			compatible = "ti,tps65218-ldo1";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
 			regulator-boot-on;
-- 
1.9.1

[PATCH v3 10/10] ARM: dts: AM43X-EPOS-EVM: Remove redundant regulator compatibles

[Keerthy]

With the device tree parsing using the regulator framework
there is a no longer a need for separate compatibles for
individual regulator nodes. Hence removing them all.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 arch/arm/boot/dts/am43x-epos-evm.dts | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 3549b8c..6a2eecd 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -421,7 +421,6 @@
 		#interrupt-cells = <2>;
 
 		dcdc1: regulator-dcdc1 {
-			compatible = "ti,tps65218-dcdc1";
 			regulator-name = "vdd_core";
 			regulator-min-microvolt = <912000>;
 			regulator-max-microvolt = <1144000>;
@@ -430,7 +429,6 @@
 		};
 
 		dcdc2: regulator-dcdc2 {
-			compatible = "ti,tps65218-dcdc2";
 			regulator-name = "vdd_mpu";
 			regulator-min-microvolt = <912000>;
 			regulator-max-microvolt = <1378000>;
@@ -439,7 +437,6 @@
 		};
 
 		dcdc3: regulator-dcdc3 {
-			compatible = "ti,tps65218-dcdc3";
 			regulator-name = "vdcdc3";
 			regulator-min-microvolt = <1500000>;
 			regulator-max-microvolt = <1500000>;
@@ -448,7 +445,6 @@
 		};
 
 		dcdc4: regulator-dcdc4 {
-			compatible = "ti,tps65218-dcdc4";
 			regulator-name = "vdcdc4";
 			regulator-min-microvolt = <3300000>;
 			regulator-max-microvolt = <3300000>;
@@ -457,21 +453,18 @@
 		};
 
 		dcdc5: regulator-dcdc5 {
-			compatible = "ti,tps65218-dcdc5";
 			regulator-name = "v1_0bat";
 			regulator-min-microvolt = <1000000>;
 			regulator-max-microvolt = <1000000>;
 		};
 
 		dcdc6: regulator-dcdc6 {
-			compatible = "ti,tps65218-dcdc6";
 			regulator-name = "v1_8bat";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
 		};
 
 		ldo1: regulator-ldo1 {
-			compatible = "ti,tps65218-ldo1";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
 			regulator-boot-on;
-- 
1.9.1

Re: [PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[kbuild test robot]

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

Hi,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.7-rc5 next-20160627]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Keerthy/mfd-tps65218-Clean-ups/20160628-141354
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=tile 

All errors (new ones prefixed by >>):

>> drivers/input/misc/tps65218-pwrbutton.c:120:1: error: expected ',' or ';' before 'extern'

vim +120 drivers/input/misc/tps65218-pwrbutton.c

   114	MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
   115	
   116	static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
   117		{ "tps65218-pwrbutton", },
   118		{ /* sentinel */ }
   119	}
 > 120	MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
   121	
   122	static struct platform_driver tps65218_pwron_driver = {
   123		.probe	= tps65218_pwron_probe,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 44836 bytes --]

Re: [PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[kbuild test robot]

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

Hi,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.7-rc5 next-20160627]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Keerthy/mfd-tps65218-Clean-ups/20160628-141354
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/input/misc/tps65218-pwrbutton.c:22:0:
>> include/linux/module.h:223:1: error: expected ',' or ';' before 'extern'
    extern const typeof(name) __mod_##type##__##name##_device_table  \
    ^
>> drivers/input/misc/tps65218-pwrbutton.c:120:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
    ^

vim +/MODULE_DEVICE_TABLE +120 drivers/input/misc/tps65218-pwrbutton.c

    16	
    17	#include <linux/init.h>
    18	#include <linux/input.h>
    19	#include <linux/interrupt.h>
    20	#include <linux/kernel.h>
    21	#include <linux/mfd/tps65218.h>
  > 22	#include <linux/module.h>
    23	#include <linux/of.h>
    24	#include <linux/platform_device.h>
    25	#include <linux/regmap.h>
    26	#include <linux/slab.h>
    27	
    28	struct tps65218_pwrbutton {
    29		struct device *dev;
    30		struct tps65218 *tps;
    31		struct input_dev *idev;
    32	};
    33	
    34	static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr)
    35	{
    36		struct tps65218_pwrbutton *pwr = _pwr;
    37		unsigned int reg;
    38		int error;
    39	
    40		error = regmap_read(pwr->tps->regmap, TPS65218_REG_STATUS, &reg);
    41		if (error) {
    42			dev_err(pwr->dev, "can't read register: %d\n", error);
    43			goto out;
    44		}
    45	
    46		if (reg & TPS65218_STATUS_PB_STATE) {
    47			input_report_key(pwr->idev, KEY_POWER, 1);
    48			pm_wakeup_event(pwr->dev, 0);
    49		} else {
    50			input_report_key(pwr->idev, KEY_POWER, 0);
    51		}
    52	
    53		input_sync(pwr->idev);
    54	
    55	out:
    56		return IRQ_HANDLED;
    57	}
    58	
    59	static int tps65218_pwron_probe(struct platform_device *pdev)
    60	{
    61		struct tps65218 *tps = dev_get_drvdata(pdev->dev.parent);
    62		struct device *dev = &pdev->dev;
    63		struct tps65218_pwrbutton *pwr;
    64		struct input_dev *idev;
    65		int error;
    66		int irq;
    67	
    68		pwr = devm_kzalloc(dev, sizeof(*pwr), GFP_KERNEL);
    69		if (!pwr)
    70			return -ENOMEM;
    71	
    72		idev = devm_input_allocate_device(dev);
    73		if (!idev)
    74			return -ENOMEM;
    75	
    76		idev->name = "tps65218_pwrbutton";
    77		idev->phys = "tps65218_pwrbutton/input0";
    78		idev->dev.parent = dev;
    79		idev->id.bustype = BUS_I2C;
    80	
    81		input_set_capability(idev, EV_KEY, KEY_POWER);
    82	
    83		pwr->tps = tps;
    84		pwr->dev = dev;
    85		pwr->idev = idev;
    86		platform_set_drvdata(pdev, pwr);
    87		device_init_wakeup(dev, true);
    88	
    89		irq = platform_get_irq(pdev, 0);
    90		error = devm_request_threaded_irq(dev, irq, NULL, tps65218_pwr_irq,
    91						  IRQF_TRIGGER_RISING |
    92							IRQF_TRIGGER_FALLING |
    93							IRQF_ONESHOT,
    94						  "tps65218-pwrbutton", pwr);
    95		if (error) {
    96			dev_err(dev, "failed to request IRQ #%d: %d\n",
    97				irq, error);
    98			return error;
    99		}
   100	
   101		error= input_register_device(idev);
   102		if (error) {
   103			dev_err(dev, "Can't register power button: %d\n", error);
   104			return error;
   105		}
   106	
   107		return 0;
   108	}
   109	
   110	static const struct of_device_id of_tps65218_pwr_match[] = {
   111		{ .compatible = "ti,tps65218-pwrbutton" },
   112		{ },
   113	};
   114	MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
   115	
   116	static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
   117		{ "tps65218-pwrbutton", },
   118		{ /* sentinel */ }
   119	}
 > 120	MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
   121	
   122	static struct platform_driver tps65218_pwron_driver = {
   123		.probe	= tps65218_pwron_probe,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 46347 bytes --]

Re: [PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[Lee Jones]

On Tue, 28 Jun 2016, Keerthy wrote:

> platform_device_id table is needed for adding the tps65218-pwrbutton
> module to the mfd_cell array.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/input/misc/tps65218-pwrbutton.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
> index 2bba8de..a0cb7d2 100644
> --- a/drivers/input/misc/tps65218-pwrbutton.c
> +++ b/drivers/input/misc/tps65218-pwrbutton.c
> @@ -113,12 +113,19 @@ static const struct of_device_id of_tps65218_pwr_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
>  
> +static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
> +	{ "tps65218-pwrbutton", },
> +	{ /* sentinel */ }
> +}

Missing ';'.  Did you build test this?

> +MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
> +
>  static struct platform_driver tps65218_pwron_driver = {
>  	.probe	= tps65218_pwron_probe,
>  	.driver	= {
>  		.name	= "tps65218_pwrbutton",
>  		.of_match_table = of_tps65218_pwr_match,
>  	},
> +	.id_table = tps65218_pwrbtn_id_table,
>  };
>  module_platform_driver(tps65218_pwron_driver);
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 06/10] gpio: tps65218: Remove the compatible

[Lee Jones]

On Tue, 28 Jun 2016, Keerthy wrote:

> Remove the individual compatible for the gpio module.
> Introduce id_table and update the driver accordingly
> to parse device tree nodes by just using the mfd compatible.

Aren't there any OF calls in this driver?

You can use the MFD API to pass through compatible strings in order to
interrogate the DT in the normal way.  There isn't usually a reason to
remove DT support just because you start using the MFD API.

> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/gpio/gpio-tps65218.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-tps65218.c b/drivers/gpio/gpio-tps65218.c
> index 0f9d9bd..06fd61f 100644
> --- a/drivers/gpio/gpio-tps65218.c
> +++ b/drivers/gpio/gpio-tps65218.c
> @@ -225,19 +225,19 @@ static int tps65218_gpio_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct of_device_id tps65218_dt_match[] = {
> -	{ .compatible = "ti,tps65218-gpio" },
> -	{  }
> +static const struct platform_device_id tps65218_gpio_id_table[] = {
> +	{ "tps65218-gpio", },
> +	{ /* sentinel */ }
>  };
> -MODULE_DEVICE_TABLE(of, tps65218_dt_match);
> +MODULE_DEVICE_TABLE(platform, tps65218_gpio_id_table);
>  
>  static struct platform_driver tps65218_gpio_driver = {
>  	.driver = {
>  		.name = "tps65218-gpio",
> -		.of_match_table = of_match_ptr(tps65218_dt_match)
>  	},
>  	.probe = tps65218_gpio_probe,
>  	.remove = tps65218_gpio_remove,
> +	.id_table = tps65218_gpio_id_table,
>  };
>  
>  module_platform_driver(tps65218_gpio_driver);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 04/10] mfd: tps65218: Use mfd_add_devices instead of of_platform_populate

[Lee Jones]

On Tue, 28 Jun 2016, Keerthy wrote:

> mfd_add_devices enables parsing device tree nodes without compatibles
> for regulators and gpio modules. Replace of_platform_populate with
> mfd_add_devices. mfd_cell currently is populated with regulators,
> gpio and powerbutton.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/mfd/tps65218.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> index f20a531..107d695 100644
> --- a/drivers/mfd/tps65218.c
> +++ b/drivers/mfd/tps65218.c
> @@ -33,6 +33,15 @@
>  
>  #define TPS65218_PASSWORD_REGS_UNLOCK   0x7D
>  
> +static const struct mfd_cell tps65218_cells[] = {
> +	{ .name = "tps65218-regulator", },
> +	{
> +		.name = "tps65218-pwrbutton",
> +		.of_compatible = "ti,tps65218-pwrbutton",
> +	},
> +	{ .name = "tps65218-gpio", },
> +};

If there are no ordering dependencies, please move and group the one
line entries to the bottom of the table.

>  /**
>   * tps65218_reg_write: Write a single tps65218 register.
>   *
> @@ -236,8 +245,10 @@ static int tps65218_probe(struct i2c_client *client,
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = of_platform_populate(client->dev.of_node, NULL, NULL,
> -				   &client->dev);
> +	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
> +			      ARRAY_SIZE(tps65218_cells), NULL, 0,
> +			      regmap_irq_get_domain(tps->irq_data));
> +
>  	if (ret < 0)
>  		goto err_irq;
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[Keerthy]

On Tuesday 28 June 2016 12:19 PM, Lee Jones wrote:
> On Tue, 28 Jun 2016, Keerthy wrote:
>
>> platform_device_id table is needed for adding the tps65218-pwrbutton
>> module to the mfd_cell array.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>   drivers/input/misc/tps65218-pwrbutton.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
>> index 2bba8de..a0cb7d2 100644
>> --- a/drivers/input/misc/tps65218-pwrbutton.c
>> +++ b/drivers/input/misc/tps65218-pwrbutton.c
>> @@ -113,12 +113,19 @@ static const struct of_device_id of_tps65218_pwr_match[] = {
>>   };
>>   MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
>>
>> +static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
>> +	{ "tps65218-pwrbutton", },
>> +	{ /* sentinel */ }
>> +}
>
> Missing ';'.  Did you build test this?

Oops sorry for the mess. Yes i built and booted. While reviewing and 
just before sending i have accidentally deleted. I will send a v4 in a 
bit. I will send v4 of this patch.

I just checked now. It somehow compiles silently even with the ';' 
missing! I will send a v4.


>
>> +MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
>> +
>>   static struct platform_driver tps65218_pwron_driver = {
>>   	.probe	= tps65218_pwron_probe,
>>   	.driver	= {
>>   		.name	= "tps65218_pwrbutton",
>>   		.of_match_table = of_tps65218_pwr_match,
>>   	},
>> +	.id_table = tps65218_pwrbtn_id_table,
>>   };
>>   module_platform_driver(tps65218_pwron_driver);
>>
>

Re: [PATCH v3 02/10] Documentation: regulator: tps65218: Update examples

[Lee Jones]

This is a more common format for DT bindings:

  dt-bindings: regulator: 

On Tue, 28 Jun 2016, Keerthy wrote:
> This updates the device tree according to the preferred way of parsing

Nit: Device Tree

> the nodes using the regulator framework.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  .../devicetree/bindings/regulator/tps65218.txt     | 87 ++++++++++++++++++----
>  1 file changed, 71 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
> index fccc1d2..5e1888f 100644
> --- a/Documentation/devicetree/bindings/regulator/tps65218.txt
> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
> @@ -1,23 +1,78 @@
>  TPS65218 family of regulators
>  
>  Required properties:
> -For tps65218 regulators/LDOs
> -- compatible:
> -  - "ti,tps65218-dcdc1" for DCDC1
> -  - "ti,tps65218-dcdc2" for DCDC2
> -  - "ti,tps65218-dcdc3" for DCDC3
> -  - "ti,tps65218-dcdc4" for DCDC4
> -  - "ti,tps65218-dcdc5" for DCDC5
> -  - "ti,tps65218-dcdc6" for DCDC6
> -  - "ti,tps65218-ldo1" for LDO1
> -
> -Optional properties:
> -- Any optional property defined in bindings/regulator/regulator.txt
> +- compatible: "ti,tps65218"
> +- reg: I2C slave address

'\n' here for clarity.

> +- list of regulators provided by this controller, must be named

Sentences start with an uppercase char.

> +  after their hardware counterparts: dcdc[1-6] and ldo1
> +- This is the list of child nodes that specify the regulator
> +  initialization data for defined regulators. Not all regulators for the given
> +  device need to be present. The definition for each of these nodes is defined
> +  using the standard binding for regulators found at
> +  Documentation/devicetree/bindings/regulator/regulator.txt.

Use relative path names for brevity.

./regulator.txt

> +  The valid names for regulators are:
> +  tps65217: regulator-dcdc1, regulator-dcdc2, regulator-dcdc3, regulator-dcdc4,
> +  regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3
> +  Each regulator is defined using the standard binding for regulators.
>  
>  Example:
> +tps65218: tps65218@24 {

Is this label used?

> +	reg = <0x24>;
> +	compatible = "ti,tps65218";
> +	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
> +	interrupt-controller;
> +	#interrupt-cells = <2>;

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 06/10] gpio: tps65218: Remove the compatible

[Keerthy]

On Tuesday 28 June 2016 12:22 PM, Lee Jones wrote:
> On Tue, 28 Jun 2016, Keerthy wrote:
>
>> Remove the individual compatible for the gpio module.
>> Introduce id_table and update the driver accordingly
>> to parse device tree nodes by just using the mfd compatible.
>
> Aren't there any OF calls in this driver?
>

There is one assignment.

ps65218_gpio->gpio_chip.of_node = pdev->dev.of_node;


> You can use the MFD API to pass through compatible strings in order to
> interrogate the DT in the normal way.  There isn't usually a reason to
> remove DT support just because you start using the MFD API.

Okay. I will do it the way i am doing for tps65218-pwrbutton.

Thanks for the review.

>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>   drivers/gpio/gpio-tps65218.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-tps65218.c b/drivers/gpio/gpio-tps65218.c
>> index 0f9d9bd..06fd61f 100644
>> --- a/drivers/gpio/gpio-tps65218.c
>> +++ b/drivers/gpio/gpio-tps65218.c
>> @@ -225,19 +225,19 @@ static int tps65218_gpio_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>
>> -static const struct of_device_id tps65218_dt_match[] = {
>> -	{ .compatible = "ti,tps65218-gpio" },
>> -	{  }
>> +static const struct platform_device_id tps65218_gpio_id_table[] = {
>> +	{ "tps65218-gpio", },
>> +	{ /* sentinel */ }
>>   };
>> -MODULE_DEVICE_TABLE(of, tps65218_dt_match);
>> +MODULE_DEVICE_TABLE(platform, tps65218_gpio_id_table);
>>
>>   static struct platform_driver tps65218_gpio_driver = {
>>   	.driver = {
>>   		.name = "tps65218-gpio",
>> -		.of_match_table = of_match_ptr(tps65218_dt_match)
>>   	},
>>   	.probe = tps65218_gpio_probe,
>>   	.remove = tps65218_gpio_remove,
>> +	.id_table = tps65218_gpio_id_table,
>>   };
>>
>>   module_platform_driver(tps65218_gpio_driver);
>

Re: [PATCH v3 01/10] mfd: tps65218: Remove redundant read wrapper

[Lee Jones]

On Tue, 28 Jun 2016, Keerthy wrote:

> Currently read directly calls the repmap read function. Hence
> remove the redundant wrapper and use regmap read wherever
> needed.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> 
> Changes in v3:
> 
>   * Cleaned up tps65218-pwrbutton and gpio drivers to remove
>     tps65218_reg_read instances.
> 
>  drivers/gpio/gpio-tps65218.c            |  3 ++-
>  drivers/input/misc/tps65218-pwrbutton.c |  3 ++-
>  drivers/mfd/tps65218.c                  | 16 +---------------
>  drivers/regulator/tps65218-regulator.c  |  3 ++-
>  include/linux/mfd/tps65218.h            |  2 --
>  5 files changed, 7 insertions(+), 20 deletions(-)

I like the idea.  So, as long as the patch is properly build tested:

For the MFD part:
  Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/gpio/gpio-tps65218.c b/drivers/gpio/gpio-tps65218.c
> index 0eaeac8..0f9d9bd 100644
> --- a/drivers/gpio/gpio-tps65218.c
> +++ b/drivers/gpio/gpio-tps65218.c
> @@ -16,6 +16,7 @@
>  #include <linux/errno.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/mfd/tps65218.h>
>  
>  struct tps65218_gpio {
> @@ -30,7 +31,7 @@ static int tps65218_gpio_get(struct gpio_chip *gc, unsigned offset)
>  	unsigned int val;
>  	int ret;
>  
> -	ret = tps65218_reg_read(tps65218, TPS65218_REG_ENABLE2, &val);
> +	ret = regmap_read(tps65218->regmap, TPS65218_REG_ENABLE2, &val);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
> index a39b626..2bba8de 100644
> --- a/drivers/input/misc/tps65218-pwrbutton.c
> +++ b/drivers/input/misc/tps65218-pwrbutton.c
> @@ -22,6 +22,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  
>  struct tps65218_pwrbutton {
> @@ -36,7 +37,7 @@ static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr)
>  	unsigned int reg;
>  	int error;
>  
> -	error = tps65218_reg_read(pwr->tps, TPS65218_REG_STATUS, &reg);
> +	error = regmap_read(pwr->tps->regmap, TPS65218_REG_STATUS, &reg);
>  	if (error) {
>  		dev_err(pwr->dev, "can't read register: %d\n", error);
>  		goto out;
> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> index 80b9dc3..f20a531 100644
> --- a/drivers/mfd/tps65218.c
> +++ b/drivers/mfd/tps65218.c
> @@ -34,20 +34,6 @@
>  #define TPS65218_PASSWORD_REGS_UNLOCK   0x7D
>  
>  /**
> - * tps65218_reg_read: Read a single tps65218 register.
> - *
> - * @tps: Device to read from.
> - * @reg: Register to read.
> - * @val: Contians the value
> - */
> -int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
> -			unsigned int *val)
> -{
> -	return regmap_read(tps->regmap, reg, val);
> -}
> -EXPORT_SYMBOL_GPL(tps65218_reg_read);
> -
> -/**
>   * tps65218_reg_write: Write a single tps65218 register.
>   *
>   * @tps65218: Device to write to.
> @@ -93,7 +79,7 @@ static int tps65218_update_bits(struct tps65218 *tps, unsigned int reg,
>  	int ret;
>  	unsigned int data;
>  
> -	ret = tps65218_reg_read(tps, reg, &data);
> +	ret = regmap_read(tps->regmap, reg, &data);
>  	if (ret) {
>  		dev_err(tps->dev, "Read from reg 0x%x failed\n", reg);
>  		return ret;
> diff --git a/drivers/regulator/tps65218-regulator.c b/drivers/regulator/tps65218-regulator.c
> index a5e5634..679ab2f 100644
> --- a/drivers/regulator/tps65218-regulator.c
> +++ b/drivers/regulator/tps65218-regulator.c
> @@ -22,6 +22,7 @@
>  #include <linux/err.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_device.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/of_regulator.h>
>  #include <linux/regulator/driver.h>
>  #include <linux/regulator/machine.h>
> @@ -224,7 +225,7 @@ static int tps65218_pmic_get_current_limit(struct regulator_dev *dev)
>  	unsigned int index;
>  	struct tps65218 *tps = rdev_get_drvdata(dev);
>  
> -	retval = tps65218_reg_read(tps, dev->desc->csel_reg, &index);
> +	retval = regmap_read(tps->regmap, dev->desc->csel_reg, &index);
>  	if (retval < 0)
>  		return retval;
>  
> diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
> index d58f3b5..7c9241f 100644
> --- a/include/linux/mfd/tps65218.h
> +++ b/include/linux/mfd/tps65218.h
> @@ -276,8 +276,6 @@ struct tps65218 {
>  	struct regmap *regmap;
>  };
>  
> -int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
> -					unsigned int *val);
>  int tps65218_reg_write(struct tps65218 *tps, unsigned int reg,
>  			unsigned int val, unsigned int level);
>  int tps65218_set_bits(struct tps65218 *tps, unsigned int reg,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 01/10] mfd: tps65218: Remove redundant read wrapper

[Keerthy]

On Tuesday 28 June 2016 12:30 PM, Lee Jones wrote:
> On Tue, 28 Jun 2016, Keerthy wrote:
>
>> Currently read directly calls the repmap read function. Hence
>> remove the redundant wrapper and use regmap read wherever
>> needed.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>
>> Changes in v3:
>>
>>    * Cleaned up tps65218-pwrbutton and gpio drivers to remove
>>      tps65218_reg_read instances.
>>
>>   drivers/gpio/gpio-tps65218.c            |  3 ++-
>>   drivers/input/misc/tps65218-pwrbutton.c |  3 ++-
>>   drivers/mfd/tps65218.c                  | 16 +---------------
>>   drivers/regulator/tps65218-regulator.c  |  3 ++-
>>   include/linux/mfd/tps65218.h            |  2 --
>>   5 files changed, 7 insertions(+), 20 deletions(-)
>
> I like the idea.  So, as long as the patch is properly build tested:
>
> For the MFD part:
>    Acked-by: Lee Jones <lee.jones@linaro.org>

I will do the testing again. Post a v4. With all the suggested changes. 
Thanks for the review.

>
>> diff --git a/drivers/gpio/gpio-tps65218.c b/drivers/gpio/gpio-tps65218.c
>> index 0eaeac8..0f9d9bd 100644
>> --- a/drivers/gpio/gpio-tps65218.c
>> +++ b/drivers/gpio/gpio-tps65218.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/errno.h>
>>   #include <linux/gpio/driver.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>>   #include <linux/mfd/tps65218.h>
>>
>>   struct tps65218_gpio {
>> @@ -30,7 +31,7 @@ static int tps65218_gpio_get(struct gpio_chip *gc, unsigned offset)
>>   	unsigned int val;
>>   	int ret;
>>
>> -	ret = tps65218_reg_read(tps65218, TPS65218_REG_ENABLE2, &val);
>> +	ret = regmap_read(tps65218->regmap, TPS65218_REG_ENABLE2, &val);
>>   	if (ret)
>>   		return ret;
>>
>> diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
>> index a39b626..2bba8de 100644
>> --- a/drivers/input/misc/tps65218-pwrbutton.c
>> +++ b/drivers/input/misc/tps65218-pwrbutton.c
>> @@ -22,6 +22,7 @@
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>>   #include <linux/slab.h>
>>
>>   struct tps65218_pwrbutton {
>> @@ -36,7 +37,7 @@ static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr)
>>   	unsigned int reg;
>>   	int error;
>>
>> -	error = tps65218_reg_read(pwr->tps, TPS65218_REG_STATUS, &reg);
>> +	error = regmap_read(pwr->tps->regmap, TPS65218_REG_STATUS, &reg);
>>   	if (error) {
>>   		dev_err(pwr->dev, "can't read register: %d\n", error);
>>   		goto out;
>> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
>> index 80b9dc3..f20a531 100644
>> --- a/drivers/mfd/tps65218.c
>> +++ b/drivers/mfd/tps65218.c
>> @@ -34,20 +34,6 @@
>>   #define TPS65218_PASSWORD_REGS_UNLOCK   0x7D
>>
>>   /**
>> - * tps65218_reg_read: Read a single tps65218 register.
>> - *
>> - * @tps: Device to read from.
>> - * @reg: Register to read.
>> - * @val: Contians the value
>> - */
>> -int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
>> -			unsigned int *val)
>> -{
>> -	return regmap_read(tps->regmap, reg, val);
>> -}
>> -EXPORT_SYMBOL_GPL(tps65218_reg_read);
>> -
>> -/**
>>    * tps65218_reg_write: Write a single tps65218 register.
>>    *
>>    * @tps65218: Device to write to.
>> @@ -93,7 +79,7 @@ static int tps65218_update_bits(struct tps65218 *tps, unsigned int reg,
>>   	int ret;
>>   	unsigned int data;
>>
>> -	ret = tps65218_reg_read(tps, reg, &data);
>> +	ret = regmap_read(tps->regmap, reg, &data);
>>   	if (ret) {
>>   		dev_err(tps->dev, "Read from reg 0x%x failed\n", reg);
>>   		return ret;
>> diff --git a/drivers/regulator/tps65218-regulator.c b/drivers/regulator/tps65218-regulator.c
>> index a5e5634..679ab2f 100644
>> --- a/drivers/regulator/tps65218-regulator.c
>> +++ b/drivers/regulator/tps65218-regulator.c
>> @@ -22,6 +22,7 @@
>>   #include <linux/err.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/of_device.h>
>> +#include <linux/regmap.h>
>>   #include <linux/regulator/of_regulator.h>
>>   #include <linux/regulator/driver.h>
>>   #include <linux/regulator/machine.h>
>> @@ -224,7 +225,7 @@ static int tps65218_pmic_get_current_limit(struct regulator_dev *dev)
>>   	unsigned int index;
>>   	struct tps65218 *tps = rdev_get_drvdata(dev);
>>
>> -	retval = tps65218_reg_read(tps, dev->desc->csel_reg, &index);
>> +	retval = regmap_read(tps->regmap, dev->desc->csel_reg, &index);
>>   	if (retval < 0)
>>   		return retval;
>>
>> diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
>> index d58f3b5..7c9241f 100644
>> --- a/include/linux/mfd/tps65218.h
>> +++ b/include/linux/mfd/tps65218.h
>> @@ -276,8 +276,6 @@ struct tps65218 {
>>   	struct regmap *regmap;
>>   };
>>
>> -int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
>> -					unsigned int *val);
>>   int tps65218_reg_write(struct tps65218 *tps, unsigned int reg,
>>   			unsigned int val, unsigned int level);
>>   int tps65218_set_bits(struct tps65218 *tps, unsigned int reg,
>

Re: [PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[Lee Jones]

On Tue, 28 Jun 2016, Keerthy wrote:

> 
> 
> On Tuesday 28 June 2016 12:19 PM, Lee Jones wrote:
> >On Tue, 28 Jun 2016, Keerthy wrote:
> >
> >>platform_device_id table is needed for adding the tps65218-pwrbutton
> >>module to the mfd_cell array.
> >>
> >>Signed-off-by: Keerthy <j-keerthy@ti.com>
> >>---
> >>  drivers/input/misc/tps65218-pwrbutton.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >>diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
> >>index 2bba8de..a0cb7d2 100644
> >>--- a/drivers/input/misc/tps65218-pwrbutton.c
> >>+++ b/drivers/input/misc/tps65218-pwrbutton.c
> >>@@ -113,12 +113,19 @@ static const struct of_device_id of_tps65218_pwr_match[] = {
> >>  };
> >>  MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
> >>
> >>+static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
> >>+	{ "tps65218-pwrbutton", },
> >>+	{ /* sentinel */ }
> >>+}
> >
> >Missing ';'.  Did you build test this?
> 
> Oops sorry for the mess. Yes i built and booted. While reviewing and just
> before sending i have accidentally deleted. I will send a v4 in a bit. I
> will send v4 of this patch.
> 
> I just checked now. It somehow compiles silently even with the ';' missing!
> I will send a v4.

Are you sure this driver is enable in the CONFIG?

> >>+MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
> >>+
> >>  static struct platform_driver tps65218_pwron_driver = {
> >>  	.probe	= tps65218_pwron_probe,
> >>  	.driver	= {
> >>  		.name	= "tps65218_pwrbutton",
> >>  		.of_match_table = of_tps65218_pwr_match,
> >>  	},
> >>+	.id_table = tps65218_pwrbtn_id_table,
> >>  };
> >>  module_platform_driver(tps65218_pwron_driver);
> >>
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 03/10] input: tps65218-pwrbutton: Add platform_device_id table

[Keerthy]

On Tuesday 28 June 2016 01:11 PM, Lee Jones wrote:
> On Tue, 28 Jun 2016, Keerthy wrote:
>
>>
>>
>> On Tuesday 28 June 2016 12:19 PM, Lee Jones wrote:
>>> On Tue, 28 Jun 2016, Keerthy wrote:
>>>
>>>> platform_device_id table is needed for adding the tps65218-pwrbutton
>>>> module to the mfd_cell array.
>>>>
>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>> ---
>>>>   drivers/input/misc/tps65218-pwrbutton.c | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/input/misc/tps65218-pwrbutton.c b/drivers/input/misc/tps65218-pwrbutton.c
>>>> index 2bba8de..a0cb7d2 100644
>>>> --- a/drivers/input/misc/tps65218-pwrbutton.c
>>>> +++ b/drivers/input/misc/tps65218-pwrbutton.c
>>>> @@ -113,12 +113,19 @@ static const struct of_device_id of_tps65218_pwr_match[] = {
>>>>   };
>>>>   MODULE_DEVICE_TABLE(of, of_tps65218_pwr_match);
>>>>
>>>> +static const struct platform_device_id tps65218_pwrbtn_id_table[] = {
>>>> +	{ "tps65218-pwrbutton", },
>>>> +	{ /* sentinel */ }
>>>> +}
>>>
>>> Missing ';'.  Did you build test this?
>>
>> Oops sorry for the mess. Yes i built and booted. While reviewing and just
>> before sending i have accidentally deleted. I will send a v4 in a bit. I
>> will send v4 of this patch.
>>
>> I just checked now. It somehow compiles silently even with the ';' missing!
>> I will send a v4.
>
> Are you sure this driver is enable in the CONFIG?

Yes! I think the MACRO Below covers up for the missing ';' and hence 
went unnoticed :-/

>
>>>> +MODULE_DEVICE_TABLE(platform, tps65218_pwrbtn_id_table);
>>>> +
>>>>   static struct platform_driver tps65218_pwron_driver = {
>>>>   	.probe	= tps65218_pwron_probe,
>>>>   	.driver	= {
>>>>   		.name	= "tps65218_pwrbutton",
>>>>   		.of_match_table = of_tps65218_pwr_match,
>>>>   	},
>>>> +	.id_table = tps65218_pwrbtn_id_table,
>>>>   };
>>>>   module_platform_driver(tps65218_pwron_driver);
>>>>
>>>
>

Re: [PATCH v3 02/10] Documentation: regulator: tps65218: Update examples

[Keerthy]

On Tuesday 28 June 2016 12:29 PM, Lee Jones wrote:
> This is a more common format for DT bindings:
>
>    dt-bindings: regulator:
>
> On Tue, 28 Jun 2016, Keerthy wrote:
>> This updates the device tree according to the preferred way of parsing
>
> Nit: Device Tree

Okay.

>
>> the nodes using the regulator framework.
>>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>   .../devicetree/bindings/regulator/tps65218.txt     | 87 ++++++++++++++++++----
>>   1 file changed, 71 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
>> index fccc1d2..5e1888f 100644
>> --- a/Documentation/devicetree/bindings/regulator/tps65218.txt
>> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
>> @@ -1,23 +1,78 @@
>>   TPS65218 family of regulators
>>
>>   Required properties:
>> -For tps65218 regulators/LDOs
>> -- compatible:
>> -  - "ti,tps65218-dcdc1" for DCDC1
>> -  - "ti,tps65218-dcdc2" for DCDC2
>> -  - "ti,tps65218-dcdc3" for DCDC3
>> -  - "ti,tps65218-dcdc4" for DCDC4
>> -  - "ti,tps65218-dcdc5" for DCDC5
>> -  - "ti,tps65218-dcdc6" for DCDC6
>> -  - "ti,tps65218-ldo1" for LDO1
>> -
>> -Optional properties:
>> -- Any optional property defined in bindings/regulator/regulator.txt
>> +- compatible: "ti,tps65218"
>> +- reg: I2C slave address
>
> '\n' here for clarity.

Okay.

>
>> +- list of regulators provided by this controller, must be named
>
> Sentences start with an uppercase char.

Okay.

>
>> +  after their hardware counterparts: dcdc[1-6] and ldo1
>> +- This is the list of child nodes that specify the regulator
>> +  initialization data for defined regulators. Not all regulators for the given
>> +  device need to be present. The definition for each of these nodes is defined
>> +  using the standard binding for regulators found at
>> +  Documentation/devicetree/bindings/regulator/regulator.txt.
>
> Use relative path names for brevity.

Okay.

>
> ./regulator.txt
>
>> +  The valid names for regulators are:
>> +  tps65217: regulator-dcdc1, regulator-dcdc2, regulator-dcdc3, regulator-dcdc4,
>> +  regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3
>> +  Each regulator is defined using the standard binding for regulators.
>>
>>   Example:
>> +tps65218: tps65218@24 {
>
> Is this label used?

Yes. This can be used in the dts files.

>
>> +	reg = <0x24>;
>> +	compatible = "ti,tps65218";
>> +	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
>> +	interrupt-controller;
>> +	#interrupt-cells = <2>;
>
> [...]
>