linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] regulator: max8998: Add support for Device Tree
@ 2013-06-24 12:39 Tomasz Figa
  2013-06-24 12:39 ` [PATCH v3 1/3] mfd: Add irq domain support for max8998 interrupts Tomasz Figa
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Tomasz Figa @ 2013-06-24 12:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes, Tomasz Figa

This series adds Device Tree support to max8998 MFD driver.

First patch reworks max8998-irq driver to use IRQ domains. Second patch
prepares platform data structure to ease generating it at runtime from
data parsed from device tree. Third patch implements Device Tree
binding and adds necessary documentation.

Tested on Universal C210 board.

Changes since v2:
 - switched to simple domains to retain compatibility with platforms
   that do not support Device Tree
 - removed clocks from DT regulator bindings (will be moved to common
   clock framework in further patch)
 - beautified the code a bit
Changes since v1:
 - rebased to current for-next of regulator tree

Tomasz Figa (3):
  mfd: Add irq domain support for max8998 interrupts
  regulator: max8998: Use arrays for specifying voltages in platform
    data
  mfd: max8998: Add support for Device Tree

 Documentation/devicetree/bindings/mfd/max8998.txt | 108 ++++++++++
 arch/arm/mach-exynos/mach-universal_c210.c        |   8 +-
 arch/arm/mach-s5pv210/mach-aquila.c               |   8 +-
 arch/arm/mach-s5pv210/mach-goni.c                 |   8 +-
 drivers/mfd/Kconfig                               |   1 +
 drivers/mfd/max8998-irq.c                         |  65 +++---
 drivers/mfd/max8998.c                             |  75 ++++++-
 drivers/regulator/max8998.c                       | 239 +++++++++++++++-------
 drivers/rtc/rtc-max8998.c                         |  14 +-
 include/linux/mfd/max8998-private.h               |   7 +-
 include/linux/mfd/max8998.h                       |  20 +-
 11 files changed, 421 insertions(+), 132 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

-- 
1.8.2.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/3] mfd: Add irq domain support for max8998 interrupts
  2013-06-24 12:39 [PATCH v3 0/3] regulator: max8998: Add support for Device Tree Tomasz Figa
@ 2013-06-24 12:39 ` Tomasz Figa
  2013-06-24 12:39 ` [PATCH v3 2/3] regulator: max8998: Use arrays for specifying voltages in platform data Tomasz Figa
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Tomasz Figa @ 2013-06-24 12:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes, Tomasz Figa

This patch adds irq domain support for max8998 interrupts.

To keep both non-DT and DT worlds happy, simple domain is used, which is
linear when no explicit IRQ base is specified and legacy, with static
mapping, otherwise.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 drivers/mfd/Kconfig                 |  1 +
 drivers/mfd/max8998-irq.c           | 65 +++++++++++++++++++++++--------------
 drivers/rtc/rtc-max8998.c           | 12 ++++++-
 include/linux/mfd/max8998-private.h |  5 ++-
 include/linux/mfd/max8998.h         |  2 +-
 5 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d54e985..360be92 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1002,6 +1002,7 @@ config MFD_TC6393XB
 config MFD_VX855
 	tristate "VIA VX855/VX875 integrated south bridge"
 	depends on PCI && GENERIC_HARDIRQS
+	select IRQ_DOMAIN
 	select MFD_CORE
 	help
 	  Say yes here to enable support for various functions of the
diff --git a/drivers/mfd/max8998-irq.c b/drivers/mfd/max8998-irq.c
index 5919710..c469477 100644
--- a/drivers/mfd/max8998-irq.c
+++ b/drivers/mfd/max8998-irq.c
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/mfd/max8998-private.h>
 
 struct max8998_irq_data {
@@ -99,7 +100,8 @@ static struct max8998_irq_data max8998_irqs[] = {
 static inline struct max8998_irq_data *
 irq_to_max8998_irq(struct max8998_dev *max8998, int irq)
 {
-	return &max8998_irqs[irq - max8998->irq_base];
+	struct irq_data *data = irq_get_irq_data(irq);
+	return &max8998_irqs[data->hwirq];
 }
 
 static void max8998_irq_lock(struct irq_data *data)
@@ -176,8 +178,14 @@ static irqreturn_t max8998_irq_thread(int irq, void *data)
 
 	/* Report */
 	for (i = 0; i < MAX8998_IRQ_NR; i++) {
-		if (irq_reg[max8998_irqs[i].reg - 1] & max8998_irqs[i].mask)
-			handle_nested_irq(max8998->irq_base + i);
+		if (irq_reg[max8998_irqs[i].reg - 1] & max8998_irqs[i].mask) {
+			irq = irq_find_mapping(max8998->irq_domain, i);
+			if (WARN_ON(!irq)) {
+				disable_irq_nosync(max8998->irq);
+				return IRQ_NONE;
+			}
+			handle_nested_irq(irq);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -185,27 +193,40 @@ static irqreturn_t max8998_irq_thread(int irq, void *data)
 
 int max8998_irq_resume(struct max8998_dev *max8998)
 {
-	if (max8998->irq && max8998->irq_base)
-		max8998_irq_thread(max8998->irq_base, max8998);
+	if (max8998->irq && max8998->irq_domain)
+		max8998_irq_thread(max8998->irq, max8998);
+	return 0;
+}
+
+static int max8998_irq_domain_map(struct irq_domain *d, unsigned int irq,
+					irq_hw_number_t hw)
+{
+	struct max8997_dev *max8998 = d->host_data;
+
+	irq_set_chip_data(irq, max8998);
+	irq_set_chip_and_handler(irq, &max8998_irq_chip, handle_edge_irq);
+	irq_set_nested_thread(irq, 1);
+#ifdef CONFIG_ARM
+	set_irq_flags(irq, IRQF_VALID);
+#else
+	irq_set_noprobe(irq);
+#endif
 	return 0;
 }
 
+static struct irq_domain_ops max8998_irq_domain_ops = {
+	.map = max8998_irq_domain_map,
+};
+
 int max8998_irq_init(struct max8998_dev *max8998)
 {
 	int i;
-	int cur_irq;
 	int ret;
+	struct irq_domain *domain;
 
 	if (!max8998->irq) {
 		dev_warn(max8998->dev,
 			 "No interrupt specified, no interrupts\n");
-		max8998->irq_base = 0;
-		return 0;
-	}
-
-	if (!max8998->irq_base) {
-		dev_err(max8998->dev,
-			"No interrupt base specified, no interrupts\n");
 		return 0;
 	}
 
@@ -221,19 +242,13 @@ int max8998_irq_init(struct max8998_dev *max8998)
 	max8998_write_reg(max8998->i2c, MAX8998_REG_STATUSM1, 0xff);
 	max8998_write_reg(max8998->i2c, MAX8998_REG_STATUSM2, 0xff);
 
-	/* register with genirq */
-	for (i = 0; i < MAX8998_IRQ_NR; i++) {
-		cur_irq = i + max8998->irq_base;
-		irq_set_chip_data(cur_irq, max8998);
-		irq_set_chip_and_handler(cur_irq, &max8998_irq_chip,
-					 handle_edge_irq);
-		irq_set_nested_thread(cur_irq, 1);
-#ifdef CONFIG_ARM
-		set_irq_flags(cur_irq, IRQF_VALID);
-#else
-		irq_set_noprobe(cur_irq);
-#endif
+	domain = irq_domain_add_simple(NULL, MAX8998_IRQ_NR,
+			max8998->irq_base, &max8998_irq_domain_ops, max8998);
+	if (!domain) {
+		dev_err(max8998->dev, "could not create irq domain\n");
+		return -ENODEV;
 	}
+	max8998->irq_domain = domain;
 
 	ret = request_threaded_irq(max8998->irq, NULL, max8998_irq_thread,
 				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index d5af7ba..46f2301 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -16,6 +16,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/bcd.h>
+#include <linux/irqdomain.h>
 #include <linux/rtc.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/max8998.h>
@@ -264,7 +265,6 @@ static int max8998_rtc_probe(struct platform_device *pdev)
 	info->dev = &pdev->dev;
 	info->max8998 = max8998;
 	info->rtc = max8998->rtc;
-	info->irq = max8998->irq_base + MAX8998_IRQ_ALARM0;
 
 	platform_set_drvdata(pdev, info);
 
@@ -277,6 +277,15 @@ static int max8998_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
+	if (!max8998->irq_domain)
+		goto no_irq;
+
+	info->irq = irq_create_mapping(max8998->irq_domain, MAX8998_IRQ_ALARM0);
+	if (!info->irq) {
+		dev_warn(&pdev->dev, "Failed to map alarm IRQ\n");
+		goto no_irq;
+	}
+
 	ret = devm_request_threaded_irq(&pdev->dev, info->irq, NULL,
 				max8998_rtc_alarm_irq, 0, "rtc-alarm0", info);
 
@@ -284,6 +293,7 @@ static int max8998_rtc_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->irq, ret);
 
+no_irq:
 	dev_info(&pdev->dev, "RTC CHIP NAME: %s\n", pdev->id_entry->name);
 	if (pdata && pdata->rtc_delay) {
 		info->lp3974_bug_workaround = true;
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index effa5d3..bfb48b6 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -132,6 +132,8 @@ enum {
 
 #define MAX8998_ENRAMP                  (1 << 4)
 
+struct irq_domain;
+
 /**
  * struct max8998_dev - max8998 master device for sub-drivers
  * @dev: master device of the chip (can be used to access platform data)
@@ -153,7 +155,8 @@ struct max8998_dev {
 	struct mutex iolock;
 	struct mutex irqlock;
 
-	int irq_base;
+	unsigned int irq_base;
+	struct irq_domain *irq_domain;
 	int irq;
 	int ono;
 	u8 irq_masks_cur[MAX8998_NUM_IRQ_REGS];
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 6823548..7547118 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -100,7 +100,7 @@ struct max8998_regulator_data {
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
 	int				num_regulators;
-	int				irq_base;
+	unsigned int			irq_base;
 	int				ono;
 	bool				buck_voltage_lock;
 	int				buck1_voltage1;
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/3] regulator: max8998: Use arrays for specifying voltages in platform data
  2013-06-24 12:39 [PATCH v3 0/3] regulator: max8998: Add support for Device Tree Tomasz Figa
  2013-06-24 12:39 ` [PATCH v3 1/3] mfd: Add irq domain support for max8998 interrupts Tomasz Figa
@ 2013-06-24 12:39 ` Tomasz Figa
  2013-06-24 14:54   ` Mark Brown
  2013-06-24 12:39 ` [PATCH v3 3/3] mfd: max8998: Add support for Device Tree Tomasz Figa
  2013-06-27  8:01 ` [PATCH v3 0/3] regulator: " Samuel Ortiz
  3 siblings, 1 reply; 11+ messages in thread
From: Tomasz Figa @ 2013-06-24 12:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes, Tomasz Figa

This patch modifies the platform data of max8998 to use arrays for
specifying predefined voltages of buck1 and buck2 instead of separate
field for each voltage.

This allows to simplify the code a bit and will help in adding support
for Device Tree, which will be introduced in further patch.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 arch/arm/mach-exynos/mach-universal_c210.c |  8 +--
 arch/arm/mach-s5pv210/mach-aquila.c        |  8 +--
 arch/arm/mach-s5pv210/mach-goni.c          |  8 +--
 drivers/regulator/max8998.c                | 96 +++++++++---------------------
 include/linux/mfd/max8998.h                | 16 ++---
 5 files changed, 39 insertions(+), 97 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-universal_c210.c b/arch/arm/mach-exynos/mach-universal_c210.c
index 74ddb2b5..f912444 100644
--- a/arch/arm/mach-exynos/mach-universal_c210.c
+++ b/arch/arm/mach-exynos/mach-universal_c210.c
@@ -540,15 +540,11 @@ static struct max8998_regulator_data lp3974_regulators[] = {
 static struct max8998_platform_data universal_lp3974_pdata = {
 	.num_regulators		= ARRAY_SIZE(lp3974_regulators),
 	.regulators		= lp3974_regulators,
-	.buck1_voltage1		= 1100000,	/* INT */
-	.buck1_voltage2		= 1000000,
-	.buck1_voltage3		= 1100000,
-	.buck1_voltage4		= 1000000,
+	.buck1_voltage		= { 1100000, 1000000, 1100000, 1000000 },
 	.buck1_set1		= EXYNOS4_GPX0(5),
 	.buck1_set2		= EXYNOS4_GPX0(6),
-	.buck2_voltage1		= 1200000,	/* G3D */
-	.buck2_voltage2		= 1100000,
 	.buck1_default_idx	= 0,
+	.buck2_voltage		= { 1200000, 1100000 },
 	.buck2_set3		= EXYNOS4_GPE2(0),
 	.buck2_default_idx	= 0,
 	.wakeup			= true,
diff --git a/arch/arm/mach-s5pv210/mach-aquila.c b/arch/arm/mach-s5pv210/mach-aquila.c
index ed2b854..ad40ab0 100644
--- a/arch/arm/mach-s5pv210/mach-aquila.c
+++ b/arch/arm/mach-s5pv210/mach-aquila.c
@@ -377,12 +377,8 @@ static struct max8998_platform_data aquila_max8998_pdata = {
 	.buck1_set1	= S5PV210_GPH0(3),
 	.buck1_set2	= S5PV210_GPH0(4),
 	.buck2_set3	= S5PV210_GPH0(5),
-	.buck1_voltage1	= 1200000,
-	.buck1_voltage2	= 1200000,
-	.buck1_voltage3	= 1200000,
-	.buck1_voltage4	= 1200000,
-	.buck2_voltage1	= 1200000,
-	.buck2_voltage2	= 1200000,
+	.buck1_voltage	= { 1200000, 1200000, 1200000, 1200000 },
+	.buck2_voltage	= { 1200000, 1200000 },
 };
 #endif
 
diff --git a/arch/arm/mach-s5pv210/mach-goni.c b/arch/arm/mach-s5pv210/mach-goni.c
index 30b24ad..e5cd9fb 100644
--- a/arch/arm/mach-s5pv210/mach-goni.c
+++ b/arch/arm/mach-s5pv210/mach-goni.c
@@ -580,12 +580,8 @@ static struct max8998_platform_data goni_max8998_pdata = {
 	.buck1_set1	= S5PV210_GPH0(3),
 	.buck1_set2	= S5PV210_GPH0(4),
 	.buck2_set3	= S5PV210_GPH0(5),
-	.buck1_voltage1	= 1200000,
-	.buck1_voltage2	= 1200000,
-	.buck1_voltage3	= 1200000,
-	.buck1_voltage4	= 1200000,
-	.buck2_voltage1	= 1200000,
-	.buck2_voltage2	= 1200000,
+	.buck1_voltage	= { 1200000, 1200000, 1200000, 1200000 },
+	.buck2_voltage	= { 1200000, 1200000 },
 };
 #endif
 
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index a57a1b1..8c45b93 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -630,6 +630,7 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 	struct max8998_data *max8998;
 	struct i2c_client *i2c;
 	int i, ret, size;
+	unsigned int v;
 
 	if (!pdata) {
 		dev_err(pdev->dev.parent, "No platform init data supplied\n");
@@ -688,53 +689,21 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		gpio_request(pdata->buck1_set2, "MAX8998 BUCK1_SET2");
 		gpio_direction_output(pdata->buck1_set2,
 				      (max8998->buck1_idx >> 1) & 0x1);
-		/* Set predefined value for BUCK1 register 1 */
-		i = 0;
-		while (buck12_voltage_map_desc.min +
-		       buck12_voltage_map_desc.step*i
-		       < pdata->buck1_voltage1)
-			i++;
-		max8998->buck1_vol[0] = i;
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE1, i);
-		if (ret)
-			goto err_out;
-
-		/* Set predefined value for BUCK1 register 2 */
-		i = 0;
-		while (buck12_voltage_map_desc.min +
-		       buck12_voltage_map_desc.step*i
-		       < pdata->buck1_voltage2)
-			i++;
-
-		max8998->buck1_vol[1] = i;
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE2, i);
-		if (ret)
-			goto err_out;
-
-		/* Set predefined value for BUCK1 register 3 */
-		i = 0;
-		while (buck12_voltage_map_desc.min +
-		       buck12_voltage_map_desc.step*i
-		       < pdata->buck1_voltage3)
-			i++;
-
-		max8998->buck1_vol[2] = i;
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE3, i);
-		if (ret)
-			goto err_out;
-
-		/* Set predefined value for BUCK1 register 4 */
-		i = 0;
-		while (buck12_voltage_map_desc.min +
-		       buck12_voltage_map_desc.step*i
-		       < pdata->buck1_voltage4)
-			i++;
-
-		max8998->buck1_vol[3] = i;
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE4, i);
-		if (ret)
-			goto err_out;
 
+		/* Set predefined values for BUCK1 registers */
+		for (v = 0; v < ARRAY_SIZE(pdata->buck1_voltage); ++v) {
+			i = 0;
+			while (buck12_voltage_map_desc.min +
+			       buck12_voltage_map_desc.step*i
+			       < pdata->buck1_voltage[v])
+				i++;
+
+			max8998->buck1_vol[v] = i;
+			ret = max8998_write_reg(i2c,
+					MAX8998_REG_BUCK1_VOLTAGE1 + v, i);
+			if (ret)
+				goto err_out;
+		}
 	}
 
 	if (gpio_is_valid(pdata->buck2_set3)) {
@@ -750,27 +719,20 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		gpio_direction_output(pdata->buck2_set3,
 				      max8998->buck2_idx & 0x1);
 
-		/* BUCK2 register 1 */
-		i = 0;
-		while (buck12_voltage_map_desc.min +
-		       buck12_voltage_map_desc.step*i
-		       < pdata->buck2_voltage1)
-			i++;
-		max8998->buck2_vol[0] = i;
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE1, i);
-		if (ret)
-			goto err_out;
-
-		/* BUCK2 register 2 */
-		i = 0;
-		while (buck12_voltage_map_desc.min +
-		       buck12_voltage_map_desc.step*i
-		       < pdata->buck2_voltage2)
-			i++;
-		max8998->buck2_vol[1] = i;
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE2, i);
-		if (ret)
-			goto err_out;
+		/* Set predefined values for BUCK2 registers */
+		for (v = 0; v < ARRAY_SIZE(pdata->buck2_voltage); ++v) {
+			i = 0;
+			while (buck12_voltage_map_desc.min +
+			       buck12_voltage_map_desc.step*i
+			       < pdata->buck2_voltage[v])
+				i++;
+
+			max8998->buck2_vol[v] = i;
+			ret = max8998_write_reg(i2c,
+					MAX8998_REG_BUCK2_VOLTAGE1 + v, i);
+			if (ret)
+				goto err_out;
+		}
 	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 7547118..ca56bb0 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -73,12 +73,8 @@ struct max8998_regulator_data {
  * @buck_voltage_lock: Do NOT change the values of the following six
  *   registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
  *   be other than the preset values.
- * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
- * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
- * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
- * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
- * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
- * @buck2_voltage2: BUCK2 DVS mode 2 voltage register
+ * @buck1_voltage: BUCK1 DVS mode 1 voltage registers
+ * @buck2_voltage: BUCK2 DVS mode 2 voltage registers
  * @buck1_set1: BUCK1 gpio pin 1 to set output voltage
  * @buck1_set2: BUCK1 gpio pin 2 to set output voltage
  * @buck1_default_idx: Default for BUCK1 gpio pin 1, 2
@@ -103,12 +99,8 @@ struct max8998_platform_data {
 	unsigned int			irq_base;
 	int				ono;
 	bool				buck_voltage_lock;
-	int				buck1_voltage1;
-	int				buck1_voltage2;
-	int				buck1_voltage3;
-	int				buck1_voltage4;
-	int				buck2_voltage1;
-	int				buck2_voltage2;
+	int				buck1_voltage[4];
+	int				buck2_voltage[2];
 	int				buck1_set1;
 	int				buck1_set2;
 	int				buck1_default_idx;
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/3] mfd: max8998: Add support for Device Tree
  2013-06-24 12:39 [PATCH v3 0/3] regulator: max8998: Add support for Device Tree Tomasz Figa
  2013-06-24 12:39 ` [PATCH v3 1/3] mfd: Add irq domain support for max8998 interrupts Tomasz Figa
  2013-06-24 12:39 ` [PATCH v3 2/3] regulator: max8998: Use arrays for specifying voltages in platform data Tomasz Figa
@ 2013-06-24 12:39 ` Tomasz Figa
  2013-06-24 14:54   ` Sachin Kamat
  2013-06-24 15:04   ` [PATCH v3 " Mark Brown
  2013-06-27  8:01 ` [PATCH v3 0/3] regulator: " Samuel Ortiz
  3 siblings, 2 replies; 11+ messages in thread
From: Tomasz Figa @ 2013-06-24 12:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes, Tomasz Figa

This patch adds Device Tree support to max8998 driver.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max8998.txt | 108 ++++++++++++++++
 drivers/mfd/max8998.c                             |  75 +++++++++++-
 drivers/regulator/max8998.c                       | 143 +++++++++++++++++++++-
 drivers/rtc/rtc-max8998.c                         |   2 +-
 include/linux/mfd/max8998-private.h               |   2 +
 include/linux/mfd/max8998.h                       |   2 +
 6 files changed, 325 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
new file mode 100644
index 0000000..5304558
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max8998.txt
@@ -0,0 +1,108 @@
+* Maxim MAX8998, National/TI LP3974 Voltage and Current Regulator
+
+The Maxim MAX8998 is a multi-function device which includes volatage and
+current regulators, rtc, charger controller and other sub-blocks. It is
+interfaced to the host controller using a i2c interface. Each sub-block is
+addressed by the host system using different i2c slave address. This document
+describes the bindings for 'pmic' sub-block of max8998.
+
+Required properties:
+- compatible: Should be one of the following:
+    - "maxim,max8998" for Maxim MAX8998
+    - "national,lp3974" or "ti,lp3974" for National/TI LP3974.
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+  the interrupts from max8998 are delivered to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+  - First interrupt specifier is for main interrupt.
+  - Second interrupt specifier is for power-on/-off interrupt.
+- max8998,pmic-buck1-dvs-gpios: GPIO specifiers for two host gpios used
+  for buck 1 dvs. The format of the gpio specifier depends in the gpio
+  controller.
+- max8998,pmic-buck2-dvs-gpio: GPIO specifier for host gpio used
+  for buck 2 dvs. The format of the gpio specifier depends in the gpio
+  controller.
+- max8998,pmic-buck1-default-dvs-idx: Default voltage setting selected from
+  the possible 4 options selectable by the dvs gpios. The value of this
+  property should be between 0 and 3. If not specified or if out of range, the
+  default value of this property is set to 0.
+- max8998,pmic-buck2-default-dvs-idx: Default voltage setting selected from
+  the possible 2 options selectable by the dvs gpios. The value of this
+  property should be between 0 and 1. If not specified or if out of range, the
+  default value of this property is set to 0.
+- max8998,pmic-buck-voltage-lock: If present, disallows changing of
+  preprogrammed buck dvfs voltages.
+
+Additional properties required if max8998,pmic-buck1-dvs-gpios is defined:
+- max8998,pmic-buck1-dvs-voltage: A set of 4 voltage values in micro-volt (uV)
+  units for buck1 when changing voltage using gpio dvs.
+
+Additional properties required if max8998,pmic-buck2-dvs-gpio is defined:
+- max8998,pmic-buck2-dvs-voltage: A set of 2 voltage values in micro-volt (uV)
+  units for buck2 when changing voltage using gpio dvs.
+
+Regulators: The regulators of max8998 that have to be instantiated should be
+included in a sub-node named 'regulators'. Regulator nodes included in this
+sub-node should be of the format as listed below.
+
+	regulator_name {
+		standard regulator bindings here
+	};
+
+The following are the names of the regulators that the max8998 pmic block
+supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
+as per the datasheet of max8998.
+
+	- LDOn
+		  - valid values for n are 2 to 17
+		  - Example: LDO2, LDO10, LDO17
+	- BUCKn
+		  - valid values for n are 1 to 4.
+		  - Example: BUCK1, BUCK2, BUCK3, BUCK4
+
+	- ENVICHG: Battery Charging Current Monitor Output. This is a fixed
+		   voltage type regulator
+
+	- ESAFEOUT1: (ldo19)
+	- ESAFEOUT2: (ld020)
+
+The bindings inside the regulator nodes use the standard regulator bindings
+which are documented elsewhere.
+
+Example:
+
+	max8998_pmic@66 {
+		compatible = "maxim,max8998-pmic";
+		interrupt-parent = <&wakeup_eint>;
+		reg = <0x66>;
+		interrupts = <4 0>, <3 0>;
+
+		max8998,pmic-buck1-default-dvs-idx = <0>;
+		max8998,pmic-buck1-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
+						 <&gpx0 1 1 0 0>; /* SET2 */
+		max8998,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
+						 <1000000>, <950000>;
+
+		max8998,pmic-buck2-default-dvs-idx = <0>;
+		max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */
+		max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>;
+
+		regulators {
+			ldo2_reg: LDO2 {
+				regulator-name = "VDD_ALIVE_1.1V";
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "VDD_ARM_1.2V";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+		};
+	};
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index d7218cc..a92d8d4 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -20,12 +20,14 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include <linux/err.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/of_irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/mutex.h>
 #include <linux/mfd/core.h>
@@ -128,6 +130,65 @@ int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
 }
 EXPORT_SYMBOL(max8998_update_reg);
 
+#ifdef CONFIG_OF
+static struct of_device_id max8998_dt_match[] = {
+	{ .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
+	{ .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
+	{ .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max8998_dt_match);
+
+/*
+ * Only the common platform data elements for max8998 are parsed here from the
+ * device tree. Other sub-modules of max8998 such as pmic, rtc and others have
+ * to parse their own platform data elements from device tree.
+ *
+ * The max8998 platform data structure is instantiated here and the drivers for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
+							struct device *dev)
+{
+	struct max8998_platform_data *pd;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		dev_err(dev, "could not allocate memory for pdata\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	pd->ono = irq_of_parse_and_map(dev->of_node, 1);
+
+	/*
+	 * ToDo: the 'wakeup' member in the platform data is more of a linux
+	 * specfic information. Hence, there is no binding for that yet and
+	 * not parsed here.
+	 */
+
+	return pd;
+}
+#else
+static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
+							struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static inline int max8998_i2c_get_driver_data(struct i2c_client *i2c,
+						const struct i2c_device_id *id)
+{
+	if (i2c->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(max8998_dt_match, i2c->dev.of_node);
+		return (int)match->data;
+	}
+
+	return (int)id->driver_data;
+}
+
 static int max8998_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
@@ -139,11 +200,20 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 	if (max8998 == NULL)
 		return -ENOMEM;
 
+	if (i2c->dev.of_node) {
+		pdata = max8998_i2c_parse_dt_pdata(&i2c->dev);
+		if (IS_ERR(pdata)) {
+			ret = PTR_ERR(pdata);
+			goto err;
+		}
+	}
+
 	i2c_set_clientdata(i2c, max8998);
 	max8998->dev = &i2c->dev;
 	max8998->i2c = i2c;
 	max8998->irq = i2c->irq;
-	max8998->type = id->driver_data;
+	max8998->type = max8998_i2c_get_driver_data(i2c, id);
+	max8998->pdata = pdata;
 	if (pdata) {
 		max8998->ono = pdata->ono;
 		max8998->irq_base = pdata->irq_base;
@@ -158,7 +228,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 
 	pm_runtime_set_active(max8998->dev);
 
-	switch (id->driver_data) {
+	switch (max8998->type) {
 	case TYPE_LP3974:
 		ret = mfd_add_devices(max8998->dev, -1,
 				      lp3974_devs, ARRAY_SIZE(lp3974_devs),
@@ -314,6 +384,7 @@ static struct i2c_driver max8998_i2c_driver = {
 		   .name = "max8998",
 		   .owner = THIS_MODULE,
 		   .pm = &max8998_pm,
+		   .of_match_table = of_match_ptr(max8998_dt_match),
 	},
 	.probe = max8998_i2c_probe,
 	.remove = max8998_i2c_remove,
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 8c45b93..ca16d6a 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -28,8 +28,10 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/mutex.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/mfd/max8998.h>
 #include <linux/mfd/max8998-private.h>
 
@@ -589,13 +591,13 @@ static struct regulator_desc regulators[] = {
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
 	}, {
-		.name		= "EN32KHz AP",
+		.name		= "EN32KHz-AP",
 		.id		= MAX8998_EN32KHZ_AP,
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
 	}, {
-		.name		= "EN32KHz CP",
+		.name		= "EN32KHz-CP",
 		.id		= MAX8998_EN32KHZ_CP,
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
@@ -621,10 +623,135 @@ static struct regulator_desc regulators[] = {
 	}
 };
 
+#ifdef CONFIG_OF
+static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
+			struct max8998_platform_data *pdata,
+			struct device_node *pmic_np)
+{
+	int gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 0);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck1 gpio[0]: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck1_set1 = gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 1);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck1 gpio[1]: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck1_set2 = gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck2-dvs-gpio", 0);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck 2 gpio: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck2_set3 = gpio;
+
+	return 0;
+}
+
+static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
+					struct max8998_platform_data *pdata)
+{
+	struct device_node *pmic_np = iodev->dev->of_node;
+	struct device_node *regulators_np, *reg_np;
+	struct max8998_regulator_data *rdata;
+	unsigned int i;
+	int ret;
+
+	regulators_np = of_get_child_by_name(pmic_np, "regulators");
+	if (!regulators_np) {
+		dev_err(iodev->dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	/* count the number of regulators to be supported in pmic */
+	pdata->num_regulators = of_get_child_count(regulators_np);
+
+	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata)
+		return -ENOMEM;
+
+	pdata->regulators = rdata;
+	for_each_child_of_node(regulators_np, reg_np) {
+		for (i = 0; i < ARRAY_SIZE(regulators); ++i)
+			if (!of_node_cmp(reg_np->name, regulators[i].name))
+				break;
+
+		if (i == ARRAY_SIZE(regulators)) {
+			dev_warn(iodev->dev,
+				"invalid regulator name '%s'\n", reg_np->name);
+			--pdata->num_regulators;
+			continue;
+		}
+
+		rdata->id = regulators[i].id;
+		rdata->initdata = of_get_regulator_init_data(
+							iodev->dev, reg_np);
+		rdata->reg_node = reg_np;
+		++rdata;
+	}
+
+	ret = max8998_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
+	if (ret)
+		return -EINVAL;
+
+	if (of_find_property(pmic_np, "max8998,pmic-buck-voltage-lock", NULL))
+		pdata->buck_voltage_lock = true;
+
+	ret = of_property_read_u32(pmic_np,
+					"max8998,pmic-buck1-default-dvs-idx",
+					&pdata->buck1_default_idx);
+	if (!ret && pdata->buck1_default_idx >= 4) {
+		pdata->buck1_default_idx = 0;
+		dev_warn(iodev->dev, "invalid value for default dvs index, using 0 instead\n");
+	}
+
+	ret = of_property_read_u32(pmic_np,
+					"max8998,pmic-buck2-default-dvs-idx",
+					&pdata->buck2_default_idx);
+	if (!ret && pdata->buck2_default_idx >= 2) {
+		pdata->buck2_default_idx = 0;
+		dev_warn(iodev->dev, "invalid value for default dvs index, using 0 instead\n");
+	}
+
+	ret = of_property_read_u32_array(pmic_np,
+					"max8998,pmic-buck1-dvs-voltage",
+					pdata->buck1_voltage,
+					ARRAY_SIZE(pdata->buck1_voltage));
+	if (ret) {
+		dev_err(iodev->dev, "buck1 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(pmic_np,
+					"max8998,pmic-buck2-dvs-voltage",
+					pdata->buck2_voltage,
+					ARRAY_SIZE(pdata->buck2_voltage));
+	if (ret) {
+		dev_err(iodev->dev, "buck2 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+#else
+static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
+					struct max8998_platform_data *pdata)
+{
+	return 0;
+}
+#endif /* CONFIG_OF */
+
 static int max8998_pmic_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max8998_platform_data *pdata = iodev->pdata;
 	struct regulator_config config = { };
 	struct regulator_dev **rdev;
 	struct max8998_data *max8998;
@@ -637,6 +764,12 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (IS_ENABLED(CONFIG_OF) && iodev->dev->of_node) {
+		ret = max8998_pmic_dt_parse_pdata(iodev, pdata);
+		if (ret)
+			return ret;
+	}
+
 	max8998 = devm_kzalloc(&pdev->dev, sizeof(struct max8998_data),
 			       GFP_KERNEL);
 	if (!max8998)
@@ -750,13 +883,15 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		}
 
 		config.dev = max8998->dev;
+		config.of_node = pdata->regulators[i].reg_node;
 		config.init_data = pdata->regulators[i].initdata;
 		config.driver_data = max8998;
 
 		rdev[i] = regulator_register(&regulators[index], &config);
 		if (IS_ERR(rdev[i])) {
 			ret = PTR_ERR(rdev[i]);
-			dev_err(max8998->dev, "regulator init failed\n");
+			dev_err(max8998->dev, "regulator %s init failed (%d)\n",
+						regulators[index].name, ret);
 			rdev[i] = NULL;
 			goto err;
 		}
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index 46f2301..042a873 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -253,7 +253,7 @@ static const struct rtc_class_ops max8998_rtc_ops = {
 static int max8998_rtc_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *max8998 = dev_get_drvdata(pdev->dev.parent);
-	struct max8998_platform_data *pdata = dev_get_platdata(max8998->dev);
+	struct max8998_platform_data *pdata = max8998->pdata;
 	struct max8998_rtc_info *info;
 	int ret;
 
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index bfb48b6..84844e0 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -137,6 +137,7 @@ struct irq_domain;
 /**
  * struct max8998_dev - max8998 master device for sub-drivers
  * @dev: master device of the chip (can be used to access platform data)
+ * @pdata: platform data for the driver and subdrivers
  * @i2c: i2c client private data for regulator
  * @rtc: i2c client private data for rtc
  * @iolock: mutex for serializing io access
@@ -150,6 +151,7 @@ struct irq_domain;
  */
 struct max8998_dev {
 	struct device *dev;
+	struct max8998_platform_data *pdata;
 	struct i2c_client *i2c;
 	struct i2c_client *rtc;
 	struct mutex iolock;
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index ca56bb0..e3956a6 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -58,10 +58,12 @@ enum {
  * max8998_regulator_data - regulator data
  * @id: regulator id
  * @initdata: regulator init data (contraints, supplies, ...)
+ * @reg_node: DT node of regulator (unused on non-DT platforms)
  */
 struct max8998_regulator_data {
 	int				id;
 	struct regulator_init_data	*initdata;
+	struct device_node		*reg_node;
 };
 
 /**
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/3] mfd: max8998: Add support for Device Tree
  2013-06-24 12:39 ` [PATCH v3 3/3] mfd: max8998: Add support for Device Tree Tomasz Figa
@ 2013-06-24 14:54   ` Sachin Kamat
  2013-06-25 13:46     ` [PATCH v4 " Tomasz Figa
  2013-06-25 14:08     ` [PATCH RESEND " Tomasz Figa
  2013-06-24 15:04   ` [PATCH v3 " Mark Brown
  1 sibling, 2 replies; 11+ messages in thread
From: Sachin Kamat @ 2013-06-24 14:54 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes

Hi Tomasz,

On 24 June 2013 18:09, Tomasz Figa <t.figa@samsung.com> wrote:
> This patch adds Device Tree support to max8998 driver.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max8998.txt | 108 ++++++++++++++++
>  drivers/mfd/max8998.c                             |  75 +++++++++++-
>  drivers/regulator/max8998.c                       | 143 +++++++++++++++++++++-
>  drivers/rtc/rtc-max8998.c                         |   2 +-
>  include/linux/mfd/max8998-private.h               |   2 +
>  include/linux/mfd/max8998.h                       |   2 +
>  6 files changed, 325 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
> new file mode 100644
> index 0000000..5304558
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/max8998.txt
> @@ -0,0 +1,108 @@
> +* Maxim MAX8998, National/TI LP3974 Voltage and Current Regulator
> +
> +The Maxim MAX8998 is a multi-function device which includes volatage and

s/volatage/voltage

> +current regulators, rtc, charger controller and other sub-blocks. It is
> +interfaced to the host controller using a i2c interface. Each sub-block is
> +addressed by the host system using different i2c slave address. This document
> +describes the bindings for 'pmic' sub-block of max8998.
> +
> +Required properties:
> +- compatible: Should be one of the following:
> +    - "maxim,max8998" for Maxim MAX8998
> +    - "national,lp3974" or "ti,lp3974" for National/TI LP3974.
> +- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
> +
> +Optional properties:
> +- interrupt-parent: Specifies the phandle of the interrupt controller to which
> +  the interrupts from max8998 are delivered to.
> +- interrupts: Interrupt specifiers for two interrupt sources.
> +  - First interrupt specifier is for main interrupt.
> +  - Second interrupt specifier is for power-on/-off interrupt.
> +- max8998,pmic-buck1-dvs-gpios: GPIO specifiers for two host gpios used
> +  for buck 1 dvs. The format of the gpio specifier depends in the gpio

s/in/on

> +  controller.
> +- max8998,pmic-buck2-dvs-gpio: GPIO specifier for host gpio used
> +  for buck 2 dvs. The format of the gpio specifier depends in the gpio

ditto

> +  controller.
> +- max8998,pmic-buck1-default-dvs-idx: Default voltage setting selected from
> +  the possible 4 options selectable by the dvs gpios. The value of this
> +  property should be between 0 and 3. If not specified or if out of range, the
> +  default value of this property is set to 0.

[snip]

> +static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
> +                                                       struct device *dev)
> +{
> +       struct max8998_platform_data *pd;
> +
> +       pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +       if (!pd) {
> +               dev_err(dev, "could not allocate memory for pdata\n");

Redundant print.

> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       pd->ono = irq_of_parse_and_map(dev->of_node, 1);
> +
> +       /*
> +        * ToDo: the 'wakeup' member in the platform data is more of a linux
> +        * specfic information. Hence, there is no binding for that yet and
> +        * not parsed here.
> +        */
> +
> +       return pd;
> +}
> +#else
> +static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
> +                                                       struct device *dev)
> +{
> +       return 0;

return NULL for syntactic correctness.

> +}
> +#endif
> +
> +static inline int max8998_i2c_get_driver_data(struct i2c_client *i2c,
> +                                               const struct i2c_device_id *id)
> +{
> +       if (i2c->dev.of_node) {
> +               const struct of_device_id *match;
> +               match = of_match_node(max8998_dt_match, i2c->dev.of_node);
> +               return (int)match->data;
> +       }
> +
> +       return (int)id->driver_data;
> +}
> +
>  static int max8998_i2c_probe(struct i2c_client *i2c,
>                             const struct i2c_device_id *id)
>  {
> @@ -139,11 +200,20 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
>         if (max8998 == NULL)
>                 return -ENOMEM;
>
> +       if (i2c->dev.of_node) {
> +               pdata = max8998_i2c_parse_dt_pdata(&i2c->dev);
> +               if (IS_ERR(pdata)) {
> +                       ret = PTR_ERR(pdata);
> +                       goto err;
> +               }
> +       }
> +
>         i2c_set_clientdata(i2c, max8998);
>         max8998->dev = &i2c->dev;
>         max8998->i2c = i2c;
>         max8998->irq = i2c->irq;
> -       max8998->type = id->driver_data;
> +       max8998->type = max8998_i2c_get_driver_data(i2c, id);
> +       max8998->pdata = pdata;
>         if (pdata) {
>                 max8998->ono = pdata->ono;
>                 max8998->irq_base = pdata->irq_base;
> @@ -158,7 +228,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
>
>         pm_runtime_set_active(max8998->dev);
>
> -       switch (id->driver_data) {
> +       switch (max8998->type) {
>         case TYPE_LP3974:
>                 ret = mfd_add_devices(max8998->dev, -1,
>                                       lp3974_devs, ARRAY_SIZE(lp3974_devs),
> @@ -314,6 +384,7 @@ static struct i2c_driver max8998_i2c_driver = {
>                    .name = "max8998",
>                    .owner = THIS_MODULE,
>                    .pm = &max8998_pm,
> +                  .of_match_table = of_match_ptr(max8998_dt_match),

Need to include <linux/of.h>.

-- 
With warm regards,
Sachin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/3] regulator: max8998: Use arrays for specifying voltages in platform data
  2013-06-24 12:39 ` [PATCH v3 2/3] regulator: max8998: Use arrays for specifying voltages in platform data Tomasz Figa
@ 2013-06-24 14:54   ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2013-06-24 14:54 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Masanari Iida, Rob Herring, Rob Landley, rtc-linux, Russell King,
	Samuel Ortiz, Rasmus Villemoes

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

On Mon, Jun 24, 2013 at 02:39:53PM +0200, Tomasz Figa wrote:
> This patch modifies the platform data of max8998 to use arrays for
> specifying predefined voltages of buck1 and buck2 instead of separate
> field for each voltage.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/3] mfd: max8998: Add support for Device Tree
  2013-06-24 12:39 ` [PATCH v3 3/3] mfd: max8998: Add support for Device Tree Tomasz Figa
  2013-06-24 14:54   ` Sachin Kamat
@ 2013-06-24 15:04   ` Mark Brown
  1 sibling, 0 replies; 11+ messages in thread
From: Mark Brown @ 2013-06-24 15:04 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Masanari Iida, Rob Herring, Rob Landley, rtc-linux, Russell King,
	Samuel Ortiz, Rasmus Villemoes

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

On Mon, Jun 24, 2013 at 02:39:54PM +0200, Tomasz Figa wrote:
> This patch adds Device Tree support to max8998 driver.

Acked-by: Mark Brown <broonie@linaro.org>

If applying please also apply the second patch, I'll drop it from the
regulator tree.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] mfd: max8998: Add support for Device Tree
  2013-06-24 14:54   ` Sachin Kamat
@ 2013-06-25 13:46     ` Tomasz Figa
  2013-06-25 14:07       ` Tomasz Figa
  2013-06-25 14:08     ` [PATCH RESEND " Tomasz Figa
  1 sibling, 1 reply; 11+ messages in thread
From: Tomasz Figa @ 2013-06-25 13:46 UTC (permalink / raw)
  To: Sachin Kamat
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes

This patch adds Device Tree support to max8998 driver.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max8998.txt | 119 ++++++++++++++++++++
 drivers/mfd/max8998.c                             |  67 ++++++++++-
 drivers/regulator/max8998.c                       | 130 +++++++++++++++++++++-
 drivers/rtc/rtc-max8998.c                         |   2 +-
 include/linux/mfd/max8998-private.h               |   2 +
 include/linux/mfd/max8998.h                       |   2 +
 6 files changed, 315 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

Changes since v3:
 - Addressed Sachin's comments:
    - fixed typos in documentation
    - removed redundant error messages
    - and other minor stylistic fixes
 - Reworded parts of the documentation and improved formatting a bit
 - Simplified device tree parsing code even more
 - Replaced most of #ifdef CONFIG_OF with if (IS_ENABLED(CONFIG_OF))
    to move responsibility of dropping unused code to compiler and avoid
    unnecessary function stubs

diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
new file mode 100644
index 0000000..23a3650
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max8998.txt
@@ -0,0 +1,119 @@
+* Maxim MAX8998, National/TI LP3974 multi-function device
+
+The Maxim MAX8998 is a multi-function device which includes voltage/current
+regulators, real time clock, battery charging controller and several
+other sub-blocks. It is interfaced using an I2C interface. Each sub-block
+is addressed by the host system using different i2c slave address.
+
+PMIC sub-block
+--------------
+
+The PMIC sub-block contains a number of voltage and current regulators,
+with controllable parameters and dynamic voltage scaling capability.
+In addition, it includes a real time clock and battery charging controller
+as well. It is accessible at I2C address 0x66.
+
+Required properties:
+- compatible: Should be one of the following:
+    - "maxim,max8998" for Maxim MAX8998
+    - "national,lp3974" or "ti,lp3974" for National/TI LP3974.
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+  the interrupts from MAX8998 are routed to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+  - First interrupt specifier is for main interrupt.
+  - Second interrupt specifier is for power-on/-off interrupt.
+- max8998,pmic-buck1-dvs-gpios: GPIO specifiers for two host gpios used
+  for buck 1 dvs. The format of the gpio specifier depends on the gpio
+  controller.
+- max8998,pmic-buck2-dvs-gpio: GPIO specifier for host gpio used
+  for buck 2 dvs. The format of the gpio specifier depends on the gpio
+  controller.
+- max8998,pmic-buck1-default-dvs-idx: Default voltage setting selected from
+  the possible 4 options selectable by the dvs gpios. The value of this
+  property should be 0, 1, 2 or 3. If not specified or out of range,
+  a default value of 0 is taken.
+- max8998,pmic-buck2-default-dvs-idx: Default voltage setting selected from
+  the possible 2 options selectable by the dvs gpios. The value of this
+  property should be 0 or 1. If not specified or out of range, a default
+  value of 0 is taken.
+- max8998,pmic-buck-voltage-lock: If present, disallows changing of
+  preprogrammed buck dvfs voltages.
+
+Additional properties required if max8998,pmic-buck1-dvs-gpios is defined:
+- max8998,pmic-buck1-dvs-voltage: An array of 4 voltage values in microvolts
+  for buck1 regulator that can be selected using dvs gpio.
+
+Additional properties required if max8998,pmic-buck2-dvs-gpio is defined:
+- max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts
+  for buck2 regulator that can be selected using dvs gpio.
+
+Regulators: All the regulators of MAX8998 to be instantiated shall be
+listed in a child node named 'regulators'. Each regulator is represented
+by a child node of the 'regulators' node.
+
+	regulator-name {
+		/* standard regulator bindings here */
+	};
+
+Following regulators of the MAX8998 PMIC block are supported. Note that
+the 'n' in regulator name, as in LDOn or BUCKn, represents the LDO or BUCK
+number as described in MAX8998 datasheet.
+
+	- LDOn
+		  - valid values for n are 2 to 17
+		  - Example: LDO2, LDO10, LDO17
+	- BUCKn
+		  - valid values for n are 1 to 4.
+		  - Example: BUCK1, BUCK2, BUCK3, BUCK4
+
+	- ENVICHG: Battery Charging Current Monitor Output. This is a fixed
+		   voltage type regulator
+
+	- ESAFEOUT1: (ldo19)
+	- ESAFEOUT2: (ld020)
+
+Standard regulator bindings are used inside regulator subnodes. Check
+  Documentation/devicetree/bindings/regulator/regulator.txt
+for more details.
+
+Example:
+
+	pmic@66 {
+		compatible = "maxim,max8998-pmic";
+		reg = <0x66>;
+		interrupt-parent = <&wakeup_eint>;
+		interrupts = <4 0>, <3 0>;
+
+		/* Buck 1 DVS settings */
+		max8998,pmic-buck1-default-dvs-idx = <0>;
+		max8998,pmic-buck1-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
+					       <&gpx0 1 1 0 0>; /* SET2 */
+		max8998,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
+						 <1000000>, <950000>;
+
+		/* Buck 2 DVS settings */
+		max8998,pmic-buck2-default-dvs-idx = <0>;
+		max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */
+		max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>;
+
+		/* Regulators to instantiate */
+		regulators {
+			ldo2_reg: LDO2 {
+				regulator-name = "VDD_ALIVE_1.1V";
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "VDD_ARM_1.2V";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+		};
+	};
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index d7218cc..21af51a 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -20,12 +20,15 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include <linux/err.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/mutex.h>
 #include <linux/mfd/core.h>
@@ -128,6 +131,56 @@ int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
 }
 EXPORT_SYMBOL(max8998_update_reg);
 
+#ifdef CONFIG_OF
+static struct of_device_id max8998_dt_match[] = {
+	{ .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
+	{ .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
+	{ .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max8998_dt_match);
+#endif
+
+/*
+ * Only the common platform data elements for max8998 are parsed here from the
+ * device tree. Other sub-modules of max8998 such as pmic, rtc and others have
+ * to parse their own platform data elements from device tree.
+ *
+ * The max8998 platform data structure is instantiated here and the drivers for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
+							struct device *dev)
+{
+	struct max8998_platform_data *pd;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return ERR_PTR(-ENOMEM);
+
+	pd->ono = irq_of_parse_and_map(dev->of_node, 1);
+
+	/*
+	 * ToDo: the 'wakeup' member in the platform data is more of a linux
+	 * specfic information. Hence, there is no binding for that yet and
+	 * not parsed here.
+	 */
+	return pd;
+}
+
+static inline int max8998_i2c_get_driver_data(struct i2c_client *i2c,
+						const struct i2c_device_id *id)
+{
+	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(max8998_dt_match, i2c->dev.of_node);
+		return (int)match->data;
+	}
+
+	return (int)id->driver_data;
+}
+
 static int max8998_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
@@ -139,11 +192,20 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 	if (max8998 == NULL)
 		return -ENOMEM;
 
+	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
+		pdata = max8998_i2c_parse_dt_pdata(&i2c->dev);
+		if (IS_ERR(pdata)) {
+			ret = PTR_ERR(pdata);
+			goto err;
+		}
+	}
+
 	i2c_set_clientdata(i2c, max8998);
 	max8998->dev = &i2c->dev;
 	max8998->i2c = i2c;
 	max8998->irq = i2c->irq;
-	max8998->type = id->driver_data;
+	max8998->type = max8998_i2c_get_driver_data(i2c, id);
+	max8998->pdata = pdata;
 	if (pdata) {
 		max8998->ono = pdata->ono;
 		max8998->irq_base = pdata->irq_base;
@@ -158,7 +220,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 
 	pm_runtime_set_active(max8998->dev);
 
-	switch (id->driver_data) {
+	switch (max8998->type) {
 	case TYPE_LP3974:
 		ret = mfd_add_devices(max8998->dev, -1,
 				      lp3974_devs, ARRAY_SIZE(lp3974_devs),
@@ -314,6 +376,7 @@ static struct i2c_driver max8998_i2c_driver = {
 		   .name = "max8998",
 		   .owner = THIS_MODULE,
 		   .pm = &max8998_pm,
+		   .of_match_table = of_match_ptr(max8998_dt_match),
 	},
 	.probe = max8998_i2c_probe,
 	.remove = max8998_i2c_remove,
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 8c45b93..e87bc80 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -28,8 +28,11 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/mfd/max8998.h>
 #include <linux/mfd/max8998-private.h>
 
@@ -589,13 +592,13 @@ static struct regulator_desc regulators[] = {
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
 	}, {
-		.name		= "EN32KHz AP",
+		.name		= "EN32KHz-AP",
 		.id		= MAX8998_EN32KHZ_AP,
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
 	}, {
-		.name		= "EN32KHz CP",
+		.name		= "EN32KHz-CP",
 		.id		= MAX8998_EN32KHZ_CP,
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
@@ -621,10 +624,121 @@ static struct regulator_desc regulators[] = {
 	}
 };
 
+static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
+			struct max8998_platform_data *pdata,
+			struct device_node *pmic_np)
+{
+	int gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 0);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck1 gpio[0]: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck1_set1 = gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 1);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck1 gpio[1]: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck1_set2 = gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck2-dvs-gpio", 0);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck 2 gpio: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck2_set3 = gpio;
+
+	return 0;
+}
+
+static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
+					struct max8998_platform_data *pdata)
+{
+	struct device_node *pmic_np = iodev->dev->of_node;
+	struct device_node *regulators_np, *reg_np;
+	struct max8998_regulator_data *rdata;
+	unsigned int i;
+	int ret;
+
+	regulators_np = of_get_child_by_name(pmic_np, "regulators");
+	if (!regulators_np) {
+		dev_err(iodev->dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	/* count the number of regulators to be supported in pmic */
+	pdata->num_regulators = of_get_child_count(regulators_np);
+
+	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata)
+		return -ENOMEM;
+
+	pdata->regulators = rdata;
+	for (i = 0; i < ARRAY_SIZE(regulators); ++i) {
+		reg_np = of_get_child_by_name(regulators_np,
+							regulators[i].name);
+		if (!reg_np)
+			continue;
+
+		rdata->id = regulators[i].id;
+		rdata->initdata = of_get_regulator_init_data(
+							iodev->dev, reg_np);
+		rdata->reg_node = reg_np;
+		++rdata;
+	}
+
+	ret = max8998_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
+	if (ret)
+		return -EINVAL;
+
+	if (of_find_property(pmic_np, "max8998,pmic-buck-voltage-lock", NULL))
+		pdata->buck_voltage_lock = true;
+
+	ret = of_property_read_u32(pmic_np,
+					"max8998,pmic-buck1-default-dvs-idx",
+					&pdata->buck1_default_idx);
+	if (!ret && pdata->buck1_default_idx >= 4) {
+		pdata->buck1_default_idx = 0;
+		dev_warn(iodev->dev, "invalid value for default dvs index, using 0 instead\n");
+	}
+
+	ret = of_property_read_u32(pmic_np,
+					"max8998,pmic-buck2-default-dvs-idx",
+					&pdata->buck2_default_idx);
+	if (!ret && pdata->buck2_default_idx >= 2) {
+		pdata->buck2_default_idx = 0;
+		dev_warn(iodev->dev, "invalid value for default dvs index, using 0 instead\n");
+	}
+
+	ret = of_property_read_u32_array(pmic_np,
+					"max8998,pmic-buck1-dvs-voltage",
+					pdata->buck1_voltage,
+					ARRAY_SIZE(pdata->buck1_voltage));
+	if (ret) {
+		dev_err(iodev->dev, "buck1 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(pmic_np,
+					"max8998,pmic-buck2-dvs-voltage",
+					pdata->buck2_voltage,
+					ARRAY_SIZE(pdata->buck2_voltage));
+	if (ret) {
+		dev_err(iodev->dev, "buck2 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int max8998_pmic_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max8998_platform_data *pdata = iodev->pdata;
 	struct regulator_config config = { };
 	struct regulator_dev **rdev;
 	struct max8998_data *max8998;
@@ -637,6 +751,12 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (IS_ENABLED(CONFIG_OF) && iodev->dev->of_node) {
+		ret = max8998_pmic_dt_parse_pdata(iodev, pdata);
+		if (ret)
+			return ret;
+	}
+
 	max8998 = devm_kzalloc(&pdev->dev, sizeof(struct max8998_data),
 			       GFP_KERNEL);
 	if (!max8998)
@@ -750,13 +870,15 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		}
 
 		config.dev = max8998->dev;
+		config.of_node = pdata->regulators[i].reg_node;
 		config.init_data = pdata->regulators[i].initdata;
 		config.driver_data = max8998;
 
 		rdev[i] = regulator_register(&regulators[index], &config);
 		if (IS_ERR(rdev[i])) {
 			ret = PTR_ERR(rdev[i]);
-			dev_err(max8998->dev, "regulator init failed\n");
+			dev_err(max8998->dev, "regulator %s init failed (%d)\n",
+						regulators[index].name, ret);
 			rdev[i] = NULL;
 			goto err;
 		}
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index 46f2301..042a873 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -253,7 +253,7 @@ static const struct rtc_class_ops max8998_rtc_ops = {
 static int max8998_rtc_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *max8998 = dev_get_drvdata(pdev->dev.parent);
-	struct max8998_platform_data *pdata = dev_get_platdata(max8998->dev);
+	struct max8998_platform_data *pdata = max8998->pdata;
 	struct max8998_rtc_info *info;
 	int ret;
 
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index bfb48b6..84844e0 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -137,6 +137,7 @@ struct irq_domain;
 /**
  * struct max8998_dev - max8998 master device for sub-drivers
  * @dev: master device of the chip (can be used to access platform data)
+ * @pdata: platform data for the driver and subdrivers
  * @i2c: i2c client private data for regulator
  * @rtc: i2c client private data for rtc
  * @iolock: mutex for serializing io access
@@ -150,6 +151,7 @@ struct irq_domain;
  */
 struct max8998_dev {
 	struct device *dev;
+	struct max8998_platform_data *pdata;
 	struct i2c_client *i2c;
 	struct i2c_client *rtc;
 	struct mutex iolock;
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index ca56bb0..e3956a6 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -58,10 +58,12 @@ enum {
  * max8998_regulator_data - regulator data
  * @id: regulator id
  * @initdata: regulator init data (contraints, supplies, ...)
+ * @reg_node: DT node of regulator (unused on non-DT platforms)
  */
 struct max8998_regulator_data {
 	int				id;
 	struct regulator_init_data	*initdata;
+	struct device_node		*reg_node;
 };
 
 /**
-- 
1.8.2.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] mfd: max8998: Add support for Device Tree
  2013-06-25 13:46     ` [PATCH v4 " Tomasz Figa
@ 2013-06-25 14:07       ` Tomasz Figa
  0 siblings, 0 replies; 11+ messages in thread
From: Tomasz Figa @ 2013-06-25 14:07 UTC (permalink / raw)
  To: Sachin Kamat
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes

Hi,

Oops, I have posted wrong patch by mistake. Sorry for the noise. Correct 
one is on the way.

Best regards,
Tomasz

On Tuesday 25 of June 2013 15:46:45 Tomasz Figa wrote:
> This patch adds Device Tree support to max8998 driver.
> 
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max8998.txt | 119
> ++++++++++++++++++++ drivers/mfd/max8998.c                             |
>  67 ++++++++++- drivers/regulator/max8998.c                       | 130
> +++++++++++++++++++++- drivers/rtc/rtc-max8998.c                        
> |   2 +-
>  include/linux/mfd/max8998-private.h               |   2 +
>  include/linux/mfd/max8998.h                       |   2 +
>  6 files changed, 315 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt
> 
> Changes since v3:
>  - Addressed Sachin's comments:
>     - fixed typos in documentation
>     - removed redundant error messages
>     - and other minor stylistic fixes
>  - Reworded parts of the documentation and improved formatting a bit
>  - Simplified device tree parsing code even more
>  - Replaced most of #ifdef CONFIG_OF with if (IS_ENABLED(CONFIG_OF))
>     to move responsibility of dropping unused code to compiler and avoid
>     unnecessary function stubs
> 
> diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt
> b/Documentation/devicetree/bindings/mfd/max8998.txt new file mode 100644
> index 0000000..23a3650
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/max8998.txt
> @@ -0,0 +1,119 @@
> +* Maxim MAX8998, National/TI LP3974 multi-function device
> +
> +The Maxim MAX8998 is a multi-function device which includes
> voltage/current +regulators, real time clock, battery charging
> controller and several +other sub-blocks. It is interfaced using an I2C
> interface. Each sub-block +is addressed by the host system using
> different i2c slave address. +
> +PMIC sub-block
> +--------------
> +
> +The PMIC sub-block contains a number of voltage and current regulators,
> +with controllable parameters and dynamic voltage scaling capability.
> +In addition, it includes a real time clock and battery charging
> controller +as well. It is accessible at I2C address 0x66.
> +
> +Required properties:
> +- compatible: Should be one of the following:
> +    - "maxim,max8998" for Maxim MAX8998
> +    - "national,lp3974" or "ti,lp3974" for National/TI LP3974.
> +- reg: Specifies the i2c slave address of the pmic block. It should be
> 0x66. +
> +Optional properties:
> +- interrupt-parent: Specifies the phandle of the interrupt controller to
> which +  the interrupts from MAX8998 are routed to.
> +- interrupts: Interrupt specifiers for two interrupt sources.
> +  - First interrupt specifier is for main interrupt.
> +  - Second interrupt specifier is for power-on/-off interrupt.
> +- max8998,pmic-buck1-dvs-gpios: GPIO specifiers for two host gpios used
> +  for buck 1 dvs. The format of the gpio specifier depends on the gpio
> +  controller.
> +- max8998,pmic-buck2-dvs-gpio: GPIO specifier for host gpio used
> +  for buck 2 dvs. The format of the gpio specifier depends on the gpio
> +  controller.
> +- max8998,pmic-buck1-default-dvs-idx: Default voltage setting selected
> from +  the possible 4 options selectable by the dvs gpios. The value of
> this +  property should be 0, 1, 2 or 3. If not specified or out of
> range, +  a default value of 0 is taken.
> +- max8998,pmic-buck2-default-dvs-idx: Default voltage setting selected
> from +  the possible 2 options selectable by the dvs gpios. The value of
> this +  property should be 0 or 1. If not specified or out of range, a
> default +  value of 0 is taken.
> +- max8998,pmic-buck-voltage-lock: If present, disallows changing of
> +  preprogrammed buck dvfs voltages.
> +
> +Additional properties required if max8998,pmic-buck1-dvs-gpios is
> defined: +- max8998,pmic-buck1-dvs-voltage: An array of 4 voltage values
> in microvolts +  for buck1 regulator that can be selected using dvs
> gpio.
> +
> +Additional properties required if max8998,pmic-buck2-dvs-gpio is
> defined: +- max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values
> in microvolts +  for buck2 regulator that can be selected using dvs
> gpio.
> +
> +Regulators: All the regulators of MAX8998 to be instantiated shall be
> +listed in a child node named 'regulators'. Each regulator is represented
> +by a child node of the 'regulators' node.
> +
> +	regulator-name {
> +		/* standard regulator bindings here */
> +	};
> +
> +Following regulators of the MAX8998 PMIC block are supported. Note that
> +the 'n' in regulator name, as in LDOn or BUCKn, represents the LDO or
> BUCK +number as described in MAX8998 datasheet.
> +
> +	- LDOn
> +		  - valid values for n are 2 to 17
> +		  - Example: LDO2, LDO10, LDO17
> +	- BUCKn
> +		  - valid values for n are 1 to 4.
> +		  - Example: BUCK1, BUCK2, BUCK3, BUCK4
> +
> +	- ENVICHG: Battery Charging Current Monitor Output. This is a fixed
> +		   voltage type regulator
> +
> +	- ESAFEOUT1: (ldo19)
> +	- ESAFEOUT2: (ld020)
> +
> +Standard regulator bindings are used inside regulator subnodes. Check
> +  Documentation/devicetree/bindings/regulator/regulator.txt
> +for more details.
> +
> +Example:
> +
> +	pmic@66 {
> +		compatible = "maxim,max8998-pmic";
> +		reg = <0x66>;
> +		interrupt-parent = <&wakeup_eint>;
> +		interrupts = <4 0>, <3 0>;
> +
> +		/* Buck 1 DVS settings */
> +		max8998,pmic-buck1-default-dvs-idx = <0>;
> +		max8998,pmic-buck1-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
> +					       <&gpx0 1 1 0 0>; /* SET2 */
> +		max8998,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
> +						 <1000000>, <950000>;
> +
> +		/* Buck 2 DVS settings */
> +		max8998,pmic-buck2-default-dvs-idx = <0>;
> +		max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */
> +		max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>;
> +
> +		/* Regulators to instantiate */
> +		regulators {
> +			ldo2_reg: LDO2 {
> +				regulator-name = "VDD_ALIVE_1.1V";
> +				regulator-min-microvolt = <1100000>;
> +				regulator-max-microvolt = <1100000>;
> +				regulator-always-on;
> +			};
> +
> +			buck1_reg: BUCK1 {
> +				regulator-name = "VDD_ARM_1.2V";
> +				regulator-min-microvolt = <950000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-always-on;
> +				regulator-boot-on;
> +			};
> +		};
> +	};
> diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
> index d7218cc..21af51a 100644
> --- a/drivers/mfd/max8998.c
> +++ b/drivers/mfd/max8998.c
> @@ -20,12 +20,15 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 
> USA */
> 
> +#include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/mutex.h>
>  #include <linux/mfd/core.h>
> @@ -128,6 +131,56 @@ int max8998_update_reg(struct i2c_client *i2c, u8
> reg, u8 val, u8 mask) }
>  EXPORT_SYMBOL(max8998_update_reg);
> 
> +#ifdef CONFIG_OF
> +static struct of_device_id max8998_dt_match[] = {
> +	{ .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
> +	{ .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
> +	{ .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, max8998_dt_match);
> +#endif
> +
> +/*
> + * Only the common platform data elements for max8998 are parsed here
> from the + * device tree. Other sub-modules of max8998 such as pmic, rtc
> and others have + * to parse their own platform data elements from
> device tree. + *
> + * The max8998 platform data structure is instantiated here and the
> drivers for + * the sub-modules need not instantiate another instance
> while parsing their + * platform data.
> + */
> +static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
> +							struct device *dev)
> +{
> +	struct max8998_platform_data *pd;
> +
> +	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +	if (!pd)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pd->ono = irq_of_parse_and_map(dev->of_node, 1);
> +
> +	/*
> +	 * ToDo: the 'wakeup' member in the platform data is more of a linux
> +	 * specfic information. Hence, there is no binding for that yet and
> +	 * not parsed here.
> +	 */
> +	return pd;
> +}
> +
> +static inline int max8998_i2c_get_driver_data(struct i2c_client *i2c,
> +						const struct i2c_device_id *id)
> +{
> +	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_node(max8998_dt_match, i2c->dev.of_node);
> +		return (int)match->data;
> +	}
> +
> +	return (int)id->driver_data;
> +}
> +
>  static int max8998_i2c_probe(struct i2c_client *i2c,
>  			    const struct i2c_device_id *id)
>  {
> @@ -139,11 +192,20 @@ static int max8998_i2c_probe(struct i2c_client
> *i2c, if (max8998 == NULL)
>  		return -ENOMEM;
> 
> +	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
> +		pdata = max8998_i2c_parse_dt_pdata(&i2c->dev);
> +		if (IS_ERR(pdata)) {
> +			ret = PTR_ERR(pdata);
> +			goto err;
> +		}
> +	}
> +
>  	i2c_set_clientdata(i2c, max8998);
>  	max8998->dev = &i2c->dev;
>  	max8998->i2c = i2c;
>  	max8998->irq = i2c->irq;
> -	max8998->type = id->driver_data;
> +	max8998->type = max8998_i2c_get_driver_data(i2c, id);
> +	max8998->pdata = pdata;
>  	if (pdata) {
>  		max8998->ono = pdata->ono;
>  		max8998->irq_base = pdata->irq_base;
> @@ -158,7 +220,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
> 
>  	pm_runtime_set_active(max8998->dev);
> 
> -	switch (id->driver_data) {
> +	switch (max8998->type) {
>  	case TYPE_LP3974:
>  		ret = mfd_add_devices(max8998->dev, -1,
>  				      lp3974_devs, ARRAY_SIZE(lp3974_devs),
> @@ -314,6 +376,7 @@ static struct i2c_driver max8998_i2c_driver = {
>  		   .name = "max8998",
>  		   .owner = THIS_MODULE,
>  		   .pm = &max8998_pm,
> +		   .of_match_table = of_match_ptr(max8998_dt_match),
>  	},
>  	.probe = max8998_i2c_probe,
>  	.remove = max8998_i2c_remove,
> diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
> index 8c45b93..e87bc80 100644
> --- a/drivers/regulator/max8998.c
> +++ b/drivers/regulator/max8998.c
> @@ -28,8 +28,11 @@
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>
>  #include <linux/mfd/max8998.h>
>  #include <linux/mfd/max8998-private.h>
> 
> @@ -589,13 +592,13 @@ static struct regulator_desc regulators[] = {
>  		.type		= REGULATOR_VOLTAGE,
>  		.owner		= THIS_MODULE,
>  	}, {
> -		.name		= "EN32KHz AP",
> +		.name		= "EN32KHz-AP",
>  		.id		= MAX8998_EN32KHZ_AP,
>  		.ops		= &max8998_others_ops,
>  		.type		= REGULATOR_VOLTAGE,
>  		.owner		= THIS_MODULE,
>  	}, {
> -		.name		= "EN32KHz CP",
> +		.name		= "EN32KHz-CP",
>  		.id		= MAX8998_EN32KHZ_CP,
>  		.ops		= &max8998_others_ops,
>  		.type		= REGULATOR_VOLTAGE,
> @@ -621,10 +624,121 @@ static struct regulator_desc regulators[] = {
>  	}
>  };
> 
> +static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
> +			struct max8998_platform_data *pdata,
> +			struct device_node *pmic_np)
> +{
> +	int gpio;
> +
> +	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 0);
> +	if (!gpio_is_valid(gpio)) {
> +		dev_err(iodev->dev, "invalid buck1 gpio[0]: %d\n", gpio);
> +		return -EINVAL;
> +	}
> +	pdata->buck1_set1 = gpio;
> +
> +	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 1);
> +	if (!gpio_is_valid(gpio)) {
> +		dev_err(iodev->dev, "invalid buck1 gpio[1]: %d\n", gpio);
> +		return -EINVAL;
> +	}
> +	pdata->buck1_set2 = gpio;
> +
> +	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck2-dvs-gpio", 0);
> +	if (!gpio_is_valid(gpio)) {
> +		dev_err(iodev->dev, "invalid buck 2 gpio: %d\n", gpio);
> +		return -EINVAL;
> +	}
> +	pdata->buck2_set3 = gpio;
> +
> +	return 0;
> +}
> +
> +static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
> +					struct max8998_platform_data *pdata)
> +{
> +	struct device_node *pmic_np = iodev->dev->of_node;
> +	struct device_node *regulators_np, *reg_np;
> +	struct max8998_regulator_data *rdata;
> +	unsigned int i;
> +	int ret;
> +
> +	regulators_np = of_get_child_by_name(pmic_np, "regulators");
> +	if (!regulators_np) {
> +		dev_err(iodev->dev, "could not find regulators sub-node\n");
> +		return -EINVAL;
> +	}
> +
> +	/* count the number of regulators to be supported in pmic */
> +	pdata->num_regulators = of_get_child_count(regulators_np);
> +
> +	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
> +				pdata->num_regulators, GFP_KERNEL);
> +	if (!rdata)
> +		return -ENOMEM;
> +
> +	pdata->regulators = rdata;
> +	for (i = 0; i < ARRAY_SIZE(regulators); ++i) {
> +		reg_np = of_get_child_by_name(regulators_np,
> +							regulators[i].name);
> +		if (!reg_np)
> +			continue;
> +
> +		rdata->id = regulators[i].id;
> +		rdata->initdata = of_get_regulator_init_data(
> +							iodev->dev, reg_np);
> +		rdata->reg_node = reg_np;
> +		++rdata;
> +	}
> +
> +	ret = max8998_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
> +	if (ret)
> +		return -EINVAL;
> +
> +	if (of_find_property(pmic_np, "max8998,pmic-buck-voltage-lock", NULL))
> +		pdata->buck_voltage_lock = true;
> +
> +	ret = of_property_read_u32(pmic_np,
> +					"max8998,pmic-buck1-default-dvs-idx",
> +					&pdata->buck1_default_idx);
> +	if (!ret && pdata->buck1_default_idx >= 4) {
> +		pdata->buck1_default_idx = 0;
> +		dev_warn(iodev->dev, "invalid value for default dvs index, using 0
> instead\n"); +	}
> +
> +	ret = of_property_read_u32(pmic_np,
> +					"max8998,pmic-buck2-default-dvs-idx",
> +					&pdata->buck2_default_idx);
> +	if (!ret && pdata->buck2_default_idx >= 2) {
> +		pdata->buck2_default_idx = 0;
> +		dev_warn(iodev->dev, "invalid value for default dvs index, using 0
> instead\n"); +	}
> +
> +	ret = of_property_read_u32_array(pmic_np,
> +					"max8998,pmic-buck1-dvs-voltage",
> +					pdata->buck1_voltage,
> +					ARRAY_SIZE(pdata->buck1_voltage));
> +	if (ret) {
> +		dev_err(iodev->dev, "buck1 voltages not specified\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32_array(pmic_np,
> +					"max8998,pmic-buck2-dvs-voltage",
> +					pdata->buck2_voltage,
> +					ARRAY_SIZE(pdata->buck2_voltage));
> +	if (ret) {
> +		dev_err(iodev->dev, "buck2 voltages not specified\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int max8998_pmic_probe(struct platform_device *pdev)
>  {
>  	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> -	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
> +	struct max8998_platform_data *pdata = iodev->pdata;
>  	struct regulator_config config = { };
>  	struct regulator_dev **rdev;
>  	struct max8998_data *max8998;
> @@ -637,6 +751,12 @@ static int max8998_pmic_probe(struct platform_device
> *pdev) return -ENODEV;
>  	}
> 
> +	if (IS_ENABLED(CONFIG_OF) && iodev->dev->of_node) {
> +		ret = max8998_pmic_dt_parse_pdata(iodev, pdata);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	max8998 = devm_kzalloc(&pdev->dev, sizeof(struct max8998_data),
>  			       GFP_KERNEL);
>  	if (!max8998)
> @@ -750,13 +870,15 @@ static int max8998_pmic_probe(struct
> platform_device *pdev) }
> 
>  		config.dev = max8998->dev;
> +		config.of_node = pdata->regulators[i].reg_node;
>  		config.init_data = pdata->regulators[i].initdata;
>  		config.driver_data = max8998;
> 
>  		rdev[i] = regulator_register(&regulators[index], &config);
>  		if (IS_ERR(rdev[i])) {
>  			ret = PTR_ERR(rdev[i]);
> -			dev_err(max8998->dev, "regulator init failed\n");
> +			dev_err(max8998->dev, "regulator %s init failed (%d)\n",
> +						regulators[index].name, ret);
>  			rdev[i] = NULL;
>  			goto err;
>  		}
> diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
> index 46f2301..042a873 100644
> --- a/drivers/rtc/rtc-max8998.c
> +++ b/drivers/rtc/rtc-max8998.c
> @@ -253,7 +253,7 @@ static const struct rtc_class_ops max8998_rtc_ops = {
> static int max8998_rtc_probe(struct platform_device *pdev)
>  {
>  	struct max8998_dev *max8998 = dev_get_drvdata(pdev->dev.parent);
> -	struct max8998_platform_data *pdata = dev_get_platdata(max8998->dev);
> +	struct max8998_platform_data *pdata = max8998->pdata;
>  	struct max8998_rtc_info *info;
>  	int ret;
> 
> diff --git a/include/linux/mfd/max8998-private.h
> b/include/linux/mfd/max8998-private.h index bfb48b6..84844e0 100644
> --- a/include/linux/mfd/max8998-private.h
> +++ b/include/linux/mfd/max8998-private.h
> @@ -137,6 +137,7 @@ struct irq_domain;
>  /**
>   * struct max8998_dev - max8998 master device for sub-drivers
>   * @dev: master device of the chip (can be used to access platform data)
> + * @pdata: platform data for the driver and subdrivers
>   * @i2c: i2c client private data for regulator
>   * @rtc: i2c client private data for rtc
>   * @iolock: mutex for serializing io access
> @@ -150,6 +151,7 @@ struct irq_domain;
>   */
>  struct max8998_dev {
>  	struct device *dev;
> +	struct max8998_platform_data *pdata;
>  	struct i2c_client *i2c;
>  	struct i2c_client *rtc;
>  	struct mutex iolock;
> diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
> index ca56bb0..e3956a6 100644
> --- a/include/linux/mfd/max8998.h
> +++ b/include/linux/mfd/max8998.h
> @@ -58,10 +58,12 @@ enum {
>   * max8998_regulator_data - regulator data
>   * @id: regulator id
>   * @initdata: regulator init data (contraints, supplies, ...)
> + * @reg_node: DT node of regulator (unused on non-DT platforms)
>   */
>  struct max8998_regulator_data {
>  	int				id;
>  	struct regulator_init_data	*initdata;
> +	struct device_node		*reg_node;
>  };
> 
>  /**

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH RESEND v4 3/3] mfd: max8998: Add support for Device Tree
  2013-06-24 14:54   ` Sachin Kamat
  2013-06-25 13:46     ` [PATCH v4 " Tomasz Figa
@ 2013-06-25 14:08     ` Tomasz Figa
  1 sibling, 0 replies; 11+ messages in thread
From: Tomasz Figa @ 2013-06-25 14:08 UTC (permalink / raw)
  To: Sachin Kamat
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Samuel Ortiz, Rasmus Villemoes

This patch adds Device Tree support to max8998 driver.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max8998.txt | 119 ++++++++++++++++++++
 drivers/mfd/max8998.c                             |  67 ++++++++++-
 drivers/regulator/max8998.c                       | 131 +++++++++++++++++++++-
 drivers/rtc/rtc-max8998.c                         |   2 +-
 include/linux/mfd/max8998-private.h               |   2 +
 include/linux/mfd/max8998.h                       |   2 +
 6 files changed, 316 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

Changes since v3:
 - Addressed Sachin's comments:
    - fixed typos in documentation
    - removed redundant error messages
    - and other minor stylistic fixes
 - Reworded parts of the documentation and improved formatting a bit
 - Simplified device tree parsing code even more
 - Replaced most of #ifdef CONFIG_OF with if (IS_ENABLED(CONFIG_OF))
    to move responsibility of dropping unused code to compiler and avoid
    unnecessary function stubs

diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
new file mode 100644
index 0000000..23a3650
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max8998.txt
@@ -0,0 +1,119 @@
+* Maxim MAX8998, National/TI LP3974 multi-function device
+
+The Maxim MAX8998 is a multi-function device which includes voltage/current
+regulators, real time clock, battery charging controller and several
+other sub-blocks. It is interfaced using an I2C interface. Each sub-block
+is addressed by the host system using different i2c slave address.
+
+PMIC sub-block
+--------------
+
+The PMIC sub-block contains a number of voltage and current regulators,
+with controllable parameters and dynamic voltage scaling capability.
+In addition, it includes a real time clock and battery charging controller
+as well. It is accessible at I2C address 0x66.
+
+Required properties:
+- compatible: Should be one of the following:
+    - "maxim,max8998" for Maxim MAX8998
+    - "national,lp3974" or "ti,lp3974" for National/TI LP3974.
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+  the interrupts from MAX8998 are routed to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+  - First interrupt specifier is for main interrupt.
+  - Second interrupt specifier is for power-on/-off interrupt.
+- max8998,pmic-buck1-dvs-gpios: GPIO specifiers for two host gpios used
+  for buck 1 dvs. The format of the gpio specifier depends on the gpio
+  controller.
+- max8998,pmic-buck2-dvs-gpio: GPIO specifier for host gpio used
+  for buck 2 dvs. The format of the gpio specifier depends on the gpio
+  controller.
+- max8998,pmic-buck1-default-dvs-idx: Default voltage setting selected from
+  the possible 4 options selectable by the dvs gpios. The value of this
+  property should be 0, 1, 2 or 3. If not specified or out of range,
+  a default value of 0 is taken.
+- max8998,pmic-buck2-default-dvs-idx: Default voltage setting selected from
+  the possible 2 options selectable by the dvs gpios. The value of this
+  property should be 0 or 1. If not specified or out of range, a default
+  value of 0 is taken.
+- max8998,pmic-buck-voltage-lock: If present, disallows changing of
+  preprogrammed buck dvfs voltages.
+
+Additional properties required if max8998,pmic-buck1-dvs-gpios is defined:
+- max8998,pmic-buck1-dvs-voltage: An array of 4 voltage values in microvolts
+  for buck1 regulator that can be selected using dvs gpio.
+
+Additional properties required if max8998,pmic-buck2-dvs-gpio is defined:
+- max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts
+  for buck2 regulator that can be selected using dvs gpio.
+
+Regulators: All the regulators of MAX8998 to be instantiated shall be
+listed in a child node named 'regulators'. Each regulator is represented
+by a child node of the 'regulators' node.
+
+	regulator-name {
+		/* standard regulator bindings here */
+	};
+
+Following regulators of the MAX8998 PMIC block are supported. Note that
+the 'n' in regulator name, as in LDOn or BUCKn, represents the LDO or BUCK
+number as described in MAX8998 datasheet.
+
+	- LDOn
+		  - valid values for n are 2 to 17
+		  - Example: LDO2, LDO10, LDO17
+	- BUCKn
+		  - valid values for n are 1 to 4.
+		  - Example: BUCK1, BUCK2, BUCK3, BUCK4
+
+	- ENVICHG: Battery Charging Current Monitor Output. This is a fixed
+		   voltage type regulator
+
+	- ESAFEOUT1: (ldo19)
+	- ESAFEOUT2: (ld020)
+
+Standard regulator bindings are used inside regulator subnodes. Check
+  Documentation/devicetree/bindings/regulator/regulator.txt
+for more details.
+
+Example:
+
+	pmic@66 {
+		compatible = "maxim,max8998-pmic";
+		reg = <0x66>;
+		interrupt-parent = <&wakeup_eint>;
+		interrupts = <4 0>, <3 0>;
+
+		/* Buck 1 DVS settings */
+		max8998,pmic-buck1-default-dvs-idx = <0>;
+		max8998,pmic-buck1-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
+					       <&gpx0 1 1 0 0>; /* SET2 */
+		max8998,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
+						 <1000000>, <950000>;
+
+		/* Buck 2 DVS settings */
+		max8998,pmic-buck2-default-dvs-idx = <0>;
+		max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */
+		max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>;
+
+		/* Regulators to instantiate */
+		regulators {
+			ldo2_reg: LDO2 {
+				regulator-name = "VDD_ALIVE_1.1V";
+				regulator-min-microvolt = <1100000>;
+				regulator-max-microvolt = <1100000>;
+				regulator-always-on;
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "VDD_ARM_1.2V";
+				regulator-min-microvolt = <950000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-always-on;
+				regulator-boot-on;
+			};
+		};
+	};
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index d7218cc..21af51a 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -20,12 +20,15 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include <linux/err.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/mutex.h>
 #include <linux/mfd/core.h>
@@ -128,6 +131,56 @@ int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
 }
 EXPORT_SYMBOL(max8998_update_reg);
 
+#ifdef CONFIG_OF
+static struct of_device_id max8998_dt_match[] = {
+	{ .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
+	{ .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
+	{ .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max8998_dt_match);
+#endif
+
+/*
+ * Only the common platform data elements for max8998 are parsed here from the
+ * device tree. Other sub-modules of max8998 such as pmic, rtc and others have
+ * to parse their own platform data elements from device tree.
+ *
+ * The max8998 platform data structure is instantiated here and the drivers for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
+							struct device *dev)
+{
+	struct max8998_platform_data *pd;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return ERR_PTR(-ENOMEM);
+
+	pd->ono = irq_of_parse_and_map(dev->of_node, 1);
+
+	/*
+	 * ToDo: the 'wakeup' member in the platform data is more of a linux
+	 * specfic information. Hence, there is no binding for that yet and
+	 * not parsed here.
+	 */
+	return pd;
+}
+
+static inline int max8998_i2c_get_driver_data(struct i2c_client *i2c,
+						const struct i2c_device_id *id)
+{
+	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(max8998_dt_match, i2c->dev.of_node);
+		return (int)match->data;
+	}
+
+	return (int)id->driver_data;
+}
+
 static int max8998_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
@@ -139,11 +192,20 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 	if (max8998 == NULL)
 		return -ENOMEM;
 
+	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
+		pdata = max8998_i2c_parse_dt_pdata(&i2c->dev);
+		if (IS_ERR(pdata)) {
+			ret = PTR_ERR(pdata);
+			goto err;
+		}
+	}
+
 	i2c_set_clientdata(i2c, max8998);
 	max8998->dev = &i2c->dev;
 	max8998->i2c = i2c;
 	max8998->irq = i2c->irq;
-	max8998->type = id->driver_data;
+	max8998->type = max8998_i2c_get_driver_data(i2c, id);
+	max8998->pdata = pdata;
 	if (pdata) {
 		max8998->ono = pdata->ono;
 		max8998->irq_base = pdata->irq_base;
@@ -158,7 +220,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 
 	pm_runtime_set_active(max8998->dev);
 
-	switch (id->driver_data) {
+	switch (max8998->type) {
 	case TYPE_LP3974:
 		ret = mfd_add_devices(max8998->dev, -1,
 				      lp3974_devs, ARRAY_SIZE(lp3974_devs),
@@ -314,6 +376,7 @@ static struct i2c_driver max8998_i2c_driver = {
 		   .name = "max8998",
 		   .owner = THIS_MODULE,
 		   .pm = &max8998_pm,
+		   .of_match_table = of_match_ptr(max8998_dt_match),
 	},
 	.probe = max8998_i2c_probe,
 	.remove = max8998_i2c_remove,
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 8c45b93..a4c53b2 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -28,8 +28,11 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/mfd/max8998.h>
 #include <linux/mfd/max8998-private.h>
 
@@ -589,13 +592,13 @@ static struct regulator_desc regulators[] = {
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
 	}, {
-		.name		= "EN32KHz AP",
+		.name		= "EN32KHz-AP",
 		.id		= MAX8998_EN32KHZ_AP,
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
 	}, {
-		.name		= "EN32KHz CP",
+		.name		= "EN32KHz-CP",
 		.id		= MAX8998_EN32KHZ_CP,
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
@@ -621,10 +624,122 @@ static struct regulator_desc regulators[] = {
 	}
 };
 
+static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
+			struct max8998_platform_data *pdata,
+			struct device_node *pmic_np)
+{
+	int gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 0);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck1 gpio[0]: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck1_set1 = gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 1);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck1 gpio[1]: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck1_set2 = gpio;
+
+	gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck2-dvs-gpio", 0);
+	if (!gpio_is_valid(gpio)) {
+		dev_err(iodev->dev, "invalid buck 2 gpio: %d\n", gpio);
+		return -EINVAL;
+	}
+	pdata->buck2_set3 = gpio;
+
+	return 0;
+}
+
+static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
+					struct max8998_platform_data *pdata)
+{
+	struct device_node *pmic_np = iodev->dev->of_node;
+	struct device_node *regulators_np, *reg_np;
+	struct max8998_regulator_data *rdata;
+	unsigned int i;
+	int ret;
+
+	regulators_np = of_get_child_by_name(pmic_np, "regulators");
+	if (!regulators_np) {
+		dev_err(iodev->dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+	/* count the number of regulators to be supported in pmic */
+	pdata->num_regulators = of_get_child_count(regulators_np);
+
+	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata)
+		return -ENOMEM;
+
+	pdata->regulators = rdata;
+	for (i = 0; i < ARRAY_SIZE(regulators); ++i) {
+		reg_np = of_get_child_by_name(regulators_np,
+							regulators[i].name);
+		if (!reg_np)
+			continue;
+
+		rdata->id = regulators[i].id;
+		rdata->initdata = of_get_regulator_init_data(
+							iodev->dev, reg_np);
+		rdata->reg_node = reg_np;
+		++rdata;
+	}
+	pdata->num_regulators = rdata - pdata->regulators;
+
+	ret = max8998_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
+	if (ret)
+		return -EINVAL;
+
+	if (of_find_property(pmic_np, "max8998,pmic-buck-voltage-lock", NULL))
+		pdata->buck_voltage_lock = true;
+
+	ret = of_property_read_u32(pmic_np,
+					"max8998,pmic-buck1-default-dvs-idx",
+					&pdata->buck1_default_idx);
+	if (!ret && pdata->buck1_default_idx >= 4) {
+		pdata->buck1_default_idx = 0;
+		dev_warn(iodev->dev, "invalid value for default dvs index, using 0 instead\n");
+	}
+
+	ret = of_property_read_u32(pmic_np,
+					"max8998,pmic-buck2-default-dvs-idx",
+					&pdata->buck2_default_idx);
+	if (!ret && pdata->buck2_default_idx >= 2) {
+		pdata->buck2_default_idx = 0;
+		dev_warn(iodev->dev, "invalid value for default dvs index, using 0 instead\n");
+	}
+
+	ret = of_property_read_u32_array(pmic_np,
+					"max8998,pmic-buck1-dvs-voltage",
+					pdata->buck1_voltage,
+					ARRAY_SIZE(pdata->buck1_voltage));
+	if (ret) {
+		dev_err(iodev->dev, "buck1 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_array(pmic_np,
+					"max8998,pmic-buck2-dvs-voltage",
+					pdata->buck2_voltage,
+					ARRAY_SIZE(pdata->buck2_voltage));
+	if (ret) {
+		dev_err(iodev->dev, "buck2 voltages not specified\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int max8998_pmic_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max8998_platform_data *pdata = iodev->pdata;
 	struct regulator_config config = { };
 	struct regulator_dev **rdev;
 	struct max8998_data *max8998;
@@ -637,6 +752,12 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (IS_ENABLED(CONFIG_OF) && iodev->dev->of_node) {
+		ret = max8998_pmic_dt_parse_pdata(iodev, pdata);
+		if (ret)
+			return ret;
+	}
+
 	max8998 = devm_kzalloc(&pdev->dev, sizeof(struct max8998_data),
 			       GFP_KERNEL);
 	if (!max8998)
@@ -750,13 +871,15 @@ static int max8998_pmic_probe(struct platform_device *pdev)
 		}
 
 		config.dev = max8998->dev;
+		config.of_node = pdata->regulators[i].reg_node;
 		config.init_data = pdata->regulators[i].initdata;
 		config.driver_data = max8998;
 
 		rdev[i] = regulator_register(&regulators[index], &config);
 		if (IS_ERR(rdev[i])) {
 			ret = PTR_ERR(rdev[i]);
-			dev_err(max8998->dev, "regulator init failed\n");
+			dev_err(max8998->dev, "regulator %s init failed (%d)\n",
+						regulators[index].name, ret);
 			rdev[i] = NULL;
 			goto err;
 		}
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index 46f2301..042a873 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -253,7 +253,7 @@ static const struct rtc_class_ops max8998_rtc_ops = {
 static int max8998_rtc_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *max8998 = dev_get_drvdata(pdev->dev.parent);
-	struct max8998_platform_data *pdata = dev_get_platdata(max8998->dev);
+	struct max8998_platform_data *pdata = max8998->pdata;
 	struct max8998_rtc_info *info;
 	int ret;
 
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index bfb48b6..84844e0 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -137,6 +137,7 @@ struct irq_domain;
 /**
  * struct max8998_dev - max8998 master device for sub-drivers
  * @dev: master device of the chip (can be used to access platform data)
+ * @pdata: platform data for the driver and subdrivers
  * @i2c: i2c client private data for regulator
  * @rtc: i2c client private data for rtc
  * @iolock: mutex for serializing io access
@@ -150,6 +151,7 @@ struct irq_domain;
  */
 struct max8998_dev {
 	struct device *dev;
+	struct max8998_platform_data *pdata;
 	struct i2c_client *i2c;
 	struct i2c_client *rtc;
 	struct mutex iolock;
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index ca56bb0..e3956a6 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -58,10 +58,12 @@ enum {
  * max8998_regulator_data - regulator data
  * @id: regulator id
  * @initdata: regulator init data (contraints, supplies, ...)
+ * @reg_node: DT node of regulator (unused on non-DT platforms)
  */
 struct max8998_regulator_data {
 	int				id;
 	struct regulator_init_data	*initdata;
+	struct device_node		*reg_node;
 };
 
 /**
-- 
1.8.2.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/3] regulator: max8998: Add support for Device Tree
  2013-06-24 12:39 [PATCH v3 0/3] regulator: max8998: Add support for Device Tree Tomasz Figa
                   ` (2 preceding siblings ...)
  2013-06-24 12:39 ` [PATCH v3 3/3] mfd: max8998: Add support for Device Tree Tomasz Figa
@ 2013-06-27  8:01 ` Samuel Ortiz
  3 siblings, 0 replies; 11+ messages in thread
From: Samuel Ortiz @ 2013-06-27  8:01 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: linux-kernel, linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Alessandro Zummo, Grant Likely, Jiri Kosina, Liam Girdwood,
	Mark Brown, Masanari Iida, Rob Herring, Rob Landley, rtc-linux,
	Russell King, Rasmus Villemoes

Hi Tomasz,

On Mon, Jun 24, 2013 at 02:39:51PM +0200, Tomasz Figa wrote:
> This series adds Device Tree support to max8998 MFD driver.
> 
> First patch reworks max8998-irq driver to use IRQ domains. Second patch
> prepares platform data structure to ease generating it at runtime from
> data parsed from device tree. Third patch implements Device Tree
> binding and adds necessary documentation.
> 
> Tested on Universal C210 board.
> 
> Changes since v2:
>  - switched to simple domains to retain compatibility with platforms
>    that do not support Device Tree
>  - removed clocks from DT regulator bindings (will be moved to common
>    clock framework in further patch)
>  - beautified the code a bit
> Changes since v1:
>  - rebased to current for-next of regulator tree
> 
> Tomasz Figa (3):
>   mfd: Add irq domain support for max8998 interrupts
>   regulator: max8998: Use arrays for specifying voltages in platform
>     data
>   mfd: max8998: Add support for Device Tree
> 
>  Documentation/devicetree/bindings/mfd/max8998.txt | 108 ++++++++++
>  arch/arm/mach-exynos/mach-universal_c210.c        |   8 +-
>  arch/arm/mach-s5pv210/mach-aquila.c               |   8 +-
>  arch/arm/mach-s5pv210/mach-goni.c                 |   8 +-
>  drivers/mfd/Kconfig                               |   1 +
>  drivers/mfd/max8998-irq.c                         |  65 +++---
>  drivers/mfd/max8998.c                             |  75 ++++++-
>  drivers/regulator/max8998.c                       | 239 +++++++++++++++-------
>  drivers/rtc/rtc-max8998.c                         |  14 +-
>  include/linux/mfd/max8998-private.h               |   7 +-
>  include/linux/mfd/max8998.h                       |  20 +-
>  11 files changed, 421 insertions(+), 132 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt
All 3 patches applied, thanks.
Mark, I added your ACK to patch #2.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-06-27  8:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-24 12:39 [PATCH v3 0/3] regulator: max8998: Add support for Device Tree Tomasz Figa
2013-06-24 12:39 ` [PATCH v3 1/3] mfd: Add irq domain support for max8998 interrupts Tomasz Figa
2013-06-24 12:39 ` [PATCH v3 2/3] regulator: max8998: Use arrays for specifying voltages in platform data Tomasz Figa
2013-06-24 14:54   ` Mark Brown
2013-06-24 12:39 ` [PATCH v3 3/3] mfd: max8998: Add support for Device Tree Tomasz Figa
2013-06-24 14:54   ` Sachin Kamat
2013-06-25 13:46     ` [PATCH v4 " Tomasz Figa
2013-06-25 14:07       ` Tomasz Figa
2013-06-25 14:08     ` [PATCH RESEND " Tomasz Figa
2013-06-24 15:04   ` [PATCH v3 " Mark Brown
2013-06-27  8:01 ` [PATCH v3 0/3] regulator: " Samuel Ortiz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).