[PATCH 0/2] Add device tree support for MAX8997

[PATCH 0/2] Add device tree support for MAX8997

[Thomas Abraham]

MAX8997 is a multi-function device which includes support for regulators, rtc,
battery charger and other sub-blocks. This patchset adds device tree support
for the pmic (regulators) sub-block.

The first patch adds irq domain support for the interrupts supported by max8997
mainly for removing the need to pass a irq_base from the platform code. The
irq_base could not anyway be passed in case of device tree based instantiation.

The second patch adds device tree support for max8997. This patch modifies both
mfd and regulator portions of the max8997 code. So does this patch have to be
split into two, one for mfd and one for regulator?

This patchset is based on the following tree.
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git  for-next

and depends on the following patchs.
[1] All the regulator device tree related patches from
    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git topic/dt
    (from commit 8f446e6fa1d506be2cb80f91c214f1705327c7f9).
[2] Device tree support patches for wakeup interrupt sources on Exynos4.
[3] [PATCH] irqdomain: export irq_domain._simple_op.s for !CONFIG_OF

This patchset has been tested on Origen board.

Thomas Abraham (2):
  mfd: add irq domain support for max8997 interrupts
  regulator: add device tree support for max8997

 .../devicetree/bindings/regulator/max8997-pmic.txt |  122 +++++++++++++++++
 arch/arm/mach-exynos/mach-nuri.c                   |    4 -
 arch/arm/mach-exynos/mach-origen.c                 |    1 -
 drivers/mfd/max8997-irq.c                          |   33 +++--
 drivers/mfd/max8997.c                              |   71 ++++++++++-
 drivers/regulator/max8997.c                        |  137 ++++++++++++++++++++
 include/linux/mfd/max8997-private.h                |    4 +-
 include/linux/mfd/max8997.h                        |    1 -
 8 files changed, 350 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/max8997-pmic.txt

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Thomas Abraham]

Add irq domain support for max8997 interrupts. All uses of irq_base in platform
data and max8997 driver private data are removed.

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 arch/arm/mach-exynos/mach-nuri.c    |    4 ----
 arch/arm/mach-exynos/mach-origen.c  |    1 -
 drivers/mfd/max8997-irq.c           |   33 +++++++++++++++++++--------------
 drivers/mfd/max8997.c               |    1 -
 include/linux/mfd/max8997-private.h |    4 +++-
 include/linux/mfd/max8997.h         |    1 -
 6 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
index 236bbe1..ae333e5 100644
--- a/arch/arm/mach-exynos/mach-nuri.c
+++ b/arch/arm/mach-exynos/mach-nuri.c
@@ -1077,12 +1077,8 @@ static struct platform_device nuri_max8903_device = {
 static void __init nuri_power_init(void)
 {
 	int gpio;
-	int irq_base = IRQ_GPIO_END + 1;
 	int ta_en = 0;
 
-	nuri_max8997_pdata.irq_base = irq_base;
-	irq_base += MAX8997_IRQ_NR;
-
 	gpio = EXYNOS4_GPX0(7);
 	gpio_request(gpio, "AP_PMIC_IRQ");
 	s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
index f56d027..588b0a8 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -421,7 +421,6 @@ struct max8997_platform_data __initdata origen_max8997_pdata = {
 	.buck1_gpiodvs	= false,
 	.buck2_gpiodvs	= false,
 	.buck5_gpiodvs	= false,
-	.irq_base	= IRQ_GPIO_END + 1,
 
 	.ignore_gpiodvs_side_effect = true,
 	.buck125_default_idx = 0x0,
diff --git a/drivers/mfd/max8997-irq.c b/drivers/mfd/max8997-irq.c
index 09274cf..eb9cad5 100644
--- a/drivers/mfd/max8997-irq.c
+++ b/drivers/mfd/max8997-irq.c
@@ -142,7 +142,8 @@ static void max8997_irq_sync_unlock(struct irq_data *data)
 static const inline struct max8997_irq_data *
 irq_to_max8997_irq(struct max8997_dev *max8997, int irq)
 {
-	return &max8997_irqs[irq - max8997->irq_base];
+	struct irq_data *data = irq_get_irq_data(irq);
+	return &max8997_irqs[data->hwirq];
 }
 
 static void max8997_irq_mask(struct irq_data *data)
@@ -179,10 +180,11 @@ static struct irq_chip max8997_irq_chip = {
 static irqreturn_t max8997_irq_thread(int irq, void *data)
 {
 	struct max8997_dev *max8997 = data;
+	struct irq_domain *domain = &max8997->irq_domain;
 	u8 irq_reg[MAX8997_IRQ_GROUP_NR] = {};
 	u8 irq_src;
 	int ret;
-	int i;
+	int i, cur_irq;
 
 	ret = max8997_read_reg(max8997->i2c, MAX8997_REG_INTSRC, &irq_src);
 	if (ret < 0) {
@@ -268,9 +270,9 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 		irq_reg[i] &= ~max8997->irq_masks_cur[i];
 
 	/* Report */
-	for (i = 0; i < MAX8997_IRQ_NR; i++) {
+	irq_domain_for_each_irq(domain, i, cur_irq) {
 		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
-			handle_nested_irq(max8997->irq_base + i);
+			handle_nested_irq(cur_irq);
 	}
 
 	return IRQ_HANDLED;
@@ -278,13 +280,14 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 
 int max8997_irq_resume(struct max8997_dev *max8997)
 {
-	if (max8997->irq && max8997->irq_base)
-		max8997_irq_thread(max8997->irq_base, max8997);
+	if (max8997->irq && max8997->irq_domain.irq_base)
+		max8997_irq_thread(max8997->irq_domain.irq_base, max8997);
 	return 0;
 }
 
 int max8997_irq_init(struct max8997_dev *max8997)
 {
+	struct irq_domain *domain = &max8997->irq_domain;
 	int i;
 	int cur_irq;
 	int ret;
@@ -292,12 +295,6 @@ int max8997_irq_init(struct max8997_dev *max8997)
 
 	if (!max8997->irq) {
 		dev_warn(max8997->dev, "No interrupt specified.\n");
-		max8997->irq_base = 0;
-		return 0;
-	}
-
-	if (!max8997->irq_base) {
-		dev_err(max8997->dev, "No interrupt base specified.\n");
 		return 0;
 	}
 
@@ -327,9 +324,17 @@ int max8997_irq_init(struct max8997_dev *max8997)
 					true : false;
 	}
 
+	domain->irq_base = irq_alloc_descs(-1, 0, MAX8997_IRQ_NR, 0);
+	if (domain->irq_base < 0) {
+		dev_err(max8997->dev, "failed to alloc irq descs\n");
+		return 0;
+	}
+	domain->nr_irq = MAX8997_IRQ_NR;
+	domain->ops = &irq_domain_simple_ops;
+	irq_domain_add(domain);
+
 	/* Register with genirq */
-	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		cur_irq = i + max8997->irq_base;
+	irq_domain_for_each_irq(domain, i, cur_irq) {
 		irq_set_chip_data(cur_irq, max8997);
 		irq_set_chip_and_handler(cur_irq, &max8997_irq_chip,
 				handle_edge_irq);
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 5be53ae..b996c6e 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -142,7 +142,6 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
 	if (!pdata)
 		goto err;
 
-	max8997->irq_base = pdata->irq_base;
 	max8997->ono = pdata->ono;
 
 	mutex_init(&max8997->iolock);
diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h
index 3f4deb6..04c5779 100644
--- a/include/linux/mfd/max8997-private.h
+++ b/include/linux/mfd/max8997-private.h
@@ -23,6 +23,8 @@
 #define __LINUX_MFD_MAX8997_PRIV_H
 
 #include <linux/i2c.h>
+#include <linux/export.h>
+#include <linux/irqdomain.h>
 
 #define MAX8997_REG_INVALID	(0xff)
 
@@ -325,7 +327,7 @@ struct max8997_dev {
 
 	int irq;
 	int ono;
-	int irq_base;
+	struct irq_domain irq_domain;
 	struct mutex irqlock;
 	int irq_masks_cur[MAX8997_IRQ_GROUP_NR];
 	int irq_masks_cache[MAX8997_IRQ_GROUP_NR];
diff --git a/include/linux/mfd/max8997.h b/include/linux/mfd/max8997.h
index 0bbd13d..1e0b9cc 100644
--- a/include/linux/mfd/max8997.h
+++ b/include/linux/mfd/max8997.h
@@ -79,7 +79,6 @@ struct max8997_regulator_data {
 
 struct max8997_platform_data {
 	/* IRQ */
-	int irq_base;
 	int ono;
 	int wakeup;
 
-- 
1.6.6.rc2

[PATCH 2/2] regulator: add device tree support for max8997

[Thomas Abraham]

Add device tree based discovery support for max8997.

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 .../devicetree/bindings/regulator/max8997-pmic.txt |  122 +++++++++++++++++
 drivers/mfd/max8997.c                              |   70 ++++++++++-
 drivers/regulator/max8997.c                        |  137 ++++++++++++++++++++
 3 files changed, 328 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/max8997-pmic.txt

diff --git a/Documentation/devicetree/bindings/regulator/max8997-pmic.txt b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
new file mode 100644
index 0000000..1d5db76
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
@@ -0,0 +1,122 @@
+* Maxim MAX8997 Voltage and Current Regulator
+
+The Maxim MAX8997 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 max8997.
+
+Required properties:
+- compatible: Should be "maxim,max8997-pmic".
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+Required properties, if interrupts are delivered to host system:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+  the interrupts from max8997 are delivered to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+  - First interrupt specifier is for 'irq1' interrupt.
+  - Second interrupt specifier is for 'alert' interrupt.
+
+Optional properties:
+- max8997,pmic-buck1-uses-gpio-dvs: 'buck1' can be controlled by gpio dvs.
+- max8997,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
+- max8997,pmic-buck5-uses-gpio-dvs: 'buck5' can be controlled by gpio dvs.
+
+Additional properties required if either of the optional properties are used:
+- max8997,pmic-ignore-gpiodvs-side-effect: When GPIO-DVS mode is used for
+  multiple bucks, changing the voltage value of one of the bucks may affect
+  that of another buck, which is the side effect of the change (set_voltage).
+  Use this property to ignore such side effects and change the voltage.
+
+- max8997,pmic-buck125-default-dvs-idx: Default voltage setting selected from
+  the possible 8 options selectable by the dvs gpios. The value of this
+  property should be between 0 and 7. If not specified or if out of range, the
+  default value of this property is set to 0.
+
+- max8997,pmic-buck125-dvs-gpios: GPIO specifiers for three host gpio's used
+  for dvs. The format of the gpio specifier depends in the gpio controller.
+
+- max8997,pmic-buck1-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+  units for buck1 when changing voltage using gpio dvs.
+
+- max8997,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+  units for buck2 when changing voltage using gpio dvs.
+
+- max8997,pmic-buck5-dvs-voltage: A set of 8 voltage values in micro-volt (uV)
+  units for buck5 when changing voltage using gpio dvs.
+
+Regulators: The regulators of max8997 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 below. Note: The 'n' in LDOn and BUCKn
+represents the LDO or BUCK number as per the datasheet of max8997.
+
+    For LDO's:
+		LDOn {
+			standard regulator bindings here
+		};
+
+    For BUCK's
+		BUCKn {
+			standard regulator bindings here
+		};
+
+The bindings inside the regulator nodes use the standard regulator bindings
+which are documented elsewhere.
+
+Example:
+
+	max8997_pmic at 66 {
+		compatible = "maxim,max8997-pmic";
+		interrupt-parent = <&wakeup_eint>;
+		reg = <0x66>;
+		interrupts = <4 0>, <3 0>;
+
+		max8997,pmic-buck1-uses-gpio-dvs;
+		max8997,pmic-buck2-uses-gpio-dvs;
+		max8997,pmic-buck5-uses-gpio-dvs;
+
+		max8997,pmic-ignore-gpiodvs-side-effect;
+		max8997,pmic-buck125-default-dvs-idx = <0>;
+
+		max8997,pmic-buck125-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
+						 <&gpx0 1 1 0 0>, /* SET2 */
+						 <&gpx0 2 1 0 0>; /* SET3 */
+
+		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
+						 <1250000>, <1200000>,
+						 <1150000>, <1100000>,
+						 <1000000>, <950000>;
+
+		max8997,pmic-buck2-dvs-voltage = <1100000>, <1100000>,
+				    		 <1100000>, <1100000>,
+						 <1000000>, <1000000>,
+						 <1000000>, <1000000>;
+
+		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
+						 <1200000>, <1200000>,
+						 <1200000>, <1200000>,
+						 <1200000>, <1200000>;
+
+		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "VDD_ABB_3.3V";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+			};
+
+			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/max8997.c b/drivers/mfd/max8997.c
index b996c6e..1749cf9 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -23,6 +23,7 @@
 
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/of_irq.h>
 #include <linux/interrupt.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
@@ -122,6 +123,60 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
 }
 EXPORT_SYMBOL_GPL(max8997_update_reg);
 
+#ifdef CONFIG_OF
+/*
+ * Only the common platform data elements for max8997 are parsed here from the
+ * device tree. Other sub-modules of max8997 such as pmic, rtc and others have
+ * to parse their own platform data elements from device tree.
+ *
+ * The max8997 platform data structure is instantiated here and the drivers for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static int max8997_i2c_parse_dt_pdata(struct device *dev,
+					struct max8997_platform_data **pdata)
+{
+	struct max8997_platform_data *pd;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		dev_err(dev, "could not allocate memory for pdata\n");
+		return -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.
+	 */
+
+	*pdata = pd;
+	return 0;
+}
+#else
+static int *max8997_i2c_parse_dt_pdata(struct device *dev
+					struct max8997_platform_data **pdata)
+{
+	return 0;
+}
+#endif
+
+static struct of_device_id __devinitdata max8997_pmic_dt_match[];
+static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
+						const struct i2c_device_id *id)
+{
+#ifdef CONFIG_OF
+	if (i2c->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(max8997_pmic_dt_match, i2c->dev.of_node);
+		return (int)match->data;
+	}
+#endif
+	return (int)id->driver_data;
+}
+
 static int max8997_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
@@ -136,9 +191,16 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
 	i2c_set_clientdata(i2c, max8997);
 	max8997->dev = &i2c->dev;
 	max8997->i2c = i2c;
-	max8997->type = id->driver_data;
+	max8997->type = max8997_i2c_get_driver_data(i2c, id);
 	max8997->irq = i2c->irq;
 
+	if (max8997->dev->of_node) {
+		ret = max8997_i2c_parse_dt_pdata(max8997->dev, &pdata);
+		if (ret)
+			goto err;
+		i2c->dev.platform_data = pdata;
+	}
+
 	if (!pdata)
 		goto err;
 
@@ -428,11 +490,17 @@ const struct dev_pm_ops max8997_pm = {
 	.restore = max8997_restore,
 };
 
+static struct of_device_id __devinitdata max8997_pmic_dt_match[] = {
+	{ .compatible = "maxim,max8997-pmic", .data = TYPE_MAX8997 },
+	{},
+};
+
 static struct i2c_driver max8997_i2c_driver = {
 	.driver = {
 		   .name = "max8997",
 		   .owner = THIS_MODULE,
 		   .pm = &max8997_pm,
+		   .of_match_table = of_match_ptr(max8997_pmic_dt_match),
 	},
 	.probe = max8997_i2c_probe,
 	.remove = max8997_i2c_remove,
diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
index d26e864..a78695c 100644
--- a/drivers/regulator/max8997.c
+++ b/drivers/regulator/max8997.c
@@ -25,6 +25,7 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -32,6 +33,7 @@
 #include <linux/regulator/machine.h>
 #include <linux/mfd/max8997.h>
 #include <linux/mfd/max8997-private.h>
+#include <linux/regulator/of_regulator.h>
 
 struct max8997_data {
 	struct device *dev;
@@ -958,6 +960,137 @@ static struct regulator_desc regulators[] = {
 	},
 };
 
+#if CONFIG_OF
+static int max8997_pmic_dt_parse_dvs_gpio(struct max8997_dev *iodev,
+			struct max8997_platform_data *pdata,
+			struct device_node *pmic_np)
+{
+	int i, gpio;
+
+	for (i = 0; i < 3; i++) {
+		gpio = of_get_named_gpio(pmic_np,
+					"max8997,pmic-buck125-dvs-gpios", i);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(iodev->dev, "invalid gpio[%d]: %d\n", i, gpio);
+			return -EINVAL;
+		}
+		pdata->buck125_gpios[i] = gpio;
+	}
+	return 0;
+}
+
+static int max8997_pmic_dt_parse_pdata(struct max8997_dev *iodev,
+					struct max8997_platform_data *pdata)
+{
+	struct device_node *pmic_np, *regulators_np, *reg_np;
+	struct max8997_regulator_data *rdata;
+	unsigned int i, ret;
+
+	pmic_np = iodev->dev->of_node;
+	if (!pmic_np) {
+		dev_err(iodev->dev, "could not find pmic sub-node\n");
+		return -ENODEV;
+	}
+
+	regulators_np = of_find_node_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 = 0;
+	for_each_child_of_node(regulators_np, reg_np)
+		pdata->num_regulators++;
+
+	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata) {
+		dev_err(iodev->dev, "could not allocate memory for "
+						"regulator data\n");
+		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;
+		rdata->id = i;
+		rdata->initdata = of_get_regulator_init_data(
+						iodev->dev, reg_np);
+		rdata++;
+	}
+
+	if (of_get_property(pmic_np, "max8997,pmic-buck1-uses-gpio-dvs", NULL))
+		pdata->buck1_gpiodvs = true;
+
+	if (of_get_property(pmic_np, "max8997,pmic-buck2-uses-gpio-dvs", NULL))
+		pdata->buck2_gpiodvs = true;
+
+	if (of_get_property(pmic_np, "max8997,pmic-buck5-uses-gpio-dvs", NULL))
+		pdata->buck5_gpiodvs = true;
+
+	if (pdata->buck1_gpiodvs) {
+		if (of_property_read_u32_array(pmic_np,
+				"max8997,pmic-buck1-dvs-voltage",
+				pdata->buck1_voltage, 8)) {
+			dev_err(iodev->dev, "buck1 voltages not specified\n");
+			return -EINVAL;
+		}
+	}
+
+	if (pdata->buck2_gpiodvs) {
+		if (of_property_read_u32_array(pmic_np,
+				"max8997,pmic-buck2-dvs-voltage",
+				pdata->buck2_voltage, 8)) {
+			dev_err(iodev->dev, "buck2 voltages not specified\n");
+			return -EINVAL;
+		}
+	}
+
+	if (pdata->buck5_gpiodvs) {
+		if (of_property_read_u32_array(pmic_np,
+				"max8997,pmic-buck5-dvs-voltage",
+				pdata->buck5_voltage, 8)) {
+			dev_err(iodev->dev, "buck5 voltages not specified\n");
+			return -EINVAL;
+		}
+	}
+
+	if (pdata->buck1_gpiodvs || pdata->buck2_gpiodvs ||
+						pdata->buck5_gpiodvs) {
+		ret = max8997_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
+		if (ret)
+			return -EINVAL;
+
+		if (of_property_read_u32(pmic_np,
+				"max8997,pmic-buck125-default-dvs-idx",
+				&pdata->buck125_default_idx)) {
+			pdata->buck125_default_idx = 0;
+		} else {
+			if (pdata->buck125_default_idx >= 8) {
+				pdata->buck125_default_idx = 0;
+				dev_info(iodev->dev, "invalid value for "
+				"default dvs index, using 0 instead\n");
+			}
+		}
+
+		if (of_get_property(pmic_np,
+			"max8997,pmic-ignore-gpiodvs-side-effect", NULL))
+			pdata->ignore_gpiodvs_side_effect = true;
+	}
+
+	return 0;
+}
+#else
+static int max8997_pmic_dt_parse_pdata(struct max8997_dev *iodev,
+					struct max8997_platform_data *pdata)
+{
+	return 0;
+}
+#endif /* CONFIG_OF */
+
 static __devinit int max8997_pmic_probe(struct platform_device *pdev)
 {
 	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
@@ -973,6 +1106,10 @@ static __devinit int max8997_pmic_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	ret = max8997_pmic_dt_parse_pdata(iodev, pdata);
+	if (ret)
+		return ret;
+
 	max8997 = kzalloc(sizeof(struct max8997_data), GFP_KERNEL);
 	if (!max8997)
 		return -ENOMEM;
-- 
1.6.6.rc2

[PATCH 2/2] regulator: add device tree support for max8997

[Kyungmin Park]

On 12/9/11, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> Add device tree based discovery support for max8997.
>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  .../devicetree/bindings/regulator/max8997-pmic.txt |  122 +++++++++++++++++
>  drivers/mfd/max8997.c                              |   70 ++++++++++-
>  drivers/regulator/max8997.c                        |  137
> ++++++++++++++++++++
>  3 files changed, 328 insertions(+), 1 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/regulator/max8997-pmic.txt
>
> diff --git a/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
> b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
> new file mode 100644
> index 0000000..1d5db76
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/max8997-pmic.txt
> @@ -0,0 +1,122 @@
> +* Maxim MAX8997 Voltage and Current Regulator
> +
> +The Maxim MAX8997 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 max8997.
> +
> +Required properties:
> +- compatible: Should be "maxim,max8997-pmic".
> +- reg: Specifies the i2c slave address of the pmic block. It should be
> 0x66.
> +
> +Required properties, if interrupts are delivered to host system:
> +- interrupt-parent: Specifies the phandle of the interrupt controller to
> which
> +  the interrupts from max8997 are delivered to.
> +- interrupts: Interrupt specifiers for two interrupt sources.
> +  - First interrupt specifier is for 'irq1' interrupt.
> +  - Second interrupt specifier is for 'alert' interrupt.
> +
> +Optional properties:
> +- max8997,pmic-buck1-uses-gpio-dvs: 'buck1' can be controlled by gpio dvs.
> +- max8997,pmic-buck2-uses-gpio-dvs: 'buck2' can be controlled by gpio dvs.
> +- max8997,pmic-buck5-uses-gpio-dvs: 'buck5' can be controlled by gpio dvs.
> +
> +Additional properties required if either of the optional properties are
> used:
> +- max8997,pmic-ignore-gpiodvs-side-effect: When GPIO-DVS mode is used for
> +  multiple bucks, changing the voltage value of one of the bucks may affect
> +  that of another buck, which is the side effect of the change
> (set_voltage).
> +  Use this property to ignore such side effects and change the voltage.
> +
> +- max8997,pmic-buck125-default-dvs-idx: Default voltage setting selected
> from
> +  the possible 8 options selectable by the dvs gpios. The value of this
> +  property should be between 0 and 7. If not specified or if out of range,
> the
> +  default value of this property is set to 0.
> +
> +- max8997,pmic-buck125-dvs-gpios: GPIO specifiers for three host gpio's
> used
> +  for dvs. The format of the gpio specifier depends in the gpio controller.
> +
> +- max8997,pmic-buck1-dvs-voltage: A set of 8 voltage values in micro-volt
> (uV)
> +  units for buck1 when changing voltage using gpio dvs.
> +
> +- max8997,pmic-buck2-dvs-voltage: A set of 8 voltage values in micro-volt
> (uV)
> +  units for buck2 when changing voltage using gpio dvs.
> +
> +- max8997,pmic-buck5-dvs-voltage: A set of 8 voltage values in micro-volt
> (uV)
> +  units for buck5 when changing voltage using gpio dvs.
> +
> +Regulators: The regulators of max8997 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 below. Note: The 'n' in LDOn and BUCKn
> +represents the LDO or BUCK number as per the datasheet of max8997.
> +
> +    For LDO's:
> +		LDOn {
> +			standard regulator bindings here
> +		};
> +
> +    For BUCK's
> +		BUCKn {
> +			standard regulator bindings here
> +		};
> +
> +The bindings inside the regulator nodes use the standard regulator bindings
> +which are documented elsewhere.
> +
> +Example:
> +
> +	max8997_pmic at 66 {
> +		compatible = "maxim,max8997-pmic";
> +		interrupt-parent = <&wakeup_eint>;
> +		reg = <0x66>;
> +		interrupts = <4 0>, <3 0>;
> +
> +		max8997,pmic-buck1-uses-gpio-dvs;
> +		max8997,pmic-buck2-uses-gpio-dvs;
> +		max8997,pmic-buck5-uses-gpio-dvs;
> +
> +		max8997,pmic-ignore-gpiodvs-side-effect;
> +		max8997,pmic-buck125-default-dvs-idx = <0>;
> +
> +		max8997,pmic-buck125-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
> +						 <&gpx0 1 1 0 0>, /* SET2 */
> +						 <&gpx0 2 1 0 0>; /* SET3 */
> +
> +		max8997,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
> +						 <1250000>, <1200000>,
> +						 <1150000>, <1100000>,
> +						 <1000000>, <950000>;
> +
> +		max8997,pmic-buck2-dvs-voltage = <1100000>, <1100000>,
> +				    		 <1100000>, <1100000>,
> +						 <1000000>, <1000000>,
> +						 <1000000>, <1000000>;
> +
> +		max8997,pmic-buck5-dvs-voltage = <1200000>, <1200000>,
> +						 <1200000>, <1200000>,
> +						 <1200000>, <1200000>,
> +						 <1200000>, <1200000>;
> +
> +		regulators {
> +			ldo1_reg: LDO1 {
> +				regulator-name = "VDD_ABB_3.3V";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +
> +			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/max8997.c b/drivers/mfd/max8997.c
> index b996c6e..1749cf9 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -23,6 +23,7 @@
>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
> +#include <linux/of_irq.h>
>  #include <linux/interrupt.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/module.h>
> @@ -122,6 +123,60 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg,
> u8 val, u8 mask)
>  }
>  EXPORT_SYMBOL_GPL(max8997_update_reg);
>
> +#ifdef CONFIG_OF
> +/*
> + * Only the common platform data elements for max8997 are parsed here from
> the
> + * device tree. Other sub-modules of max8997 such as pmic, rtc and others
> have
> + * to parse their own platform data elements from device tree.
> + *
> + * The max8997 platform data structure is instantiated here and the drivers
> for
> + * the sub-modules need not instantiate another instance while parsing
> their
> + * platform data.
> + */
> +static int max8997_i2c_parse_dt_pdata(struct device *dev,
> +					struct max8997_platform_data **pdata)
> +{
> +	struct max8997_platform_data *pd;
> +
> +	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> +	if (!pd) {
> +		dev_err(dev, "could not allocate memory for pdata\n");
> +		return -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.
> +	 */
> +
> +	*pdata = pd;
> +	return 0;
> +}
> +#else
> +static int *max8997_i2c_parse_dt_pdata(struct device *dev
> +					struct max8997_platform_data **pdata)
It should be 'int' instead of 'int *'

It's helpful to compile without CONFIG_OF.

Thank you,
Kyungmin Park
> +{
> +	return 0;
> +}
> +#endif
> +
> +static struct of_device_id __devinitdata max8997_pmic_dt_match[];
> +static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
> +						const struct i2c_device_id *id)
> +{
> +#ifdef CONFIG_OF
> +	if (i2c->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_node(max8997_pmic_dt_match, i2c->dev.of_node);
> +		return (int)match->data;
> +	}
> +#endif
> +	return (int)id->driver_data;
> +}
> +
>  static int max8997_i2c_probe(struct i2c_client *i2c,
>  			    const struct i2c_device_id *id)
>  {
> @@ -136,9 +191,16 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
>  	i2c_set_clientdata(i2c, max8997);
>  	max8997->dev = &i2c->dev;
>  	max8997->i2c = i2c;
> -	max8997->type = id->driver_data;
> +	max8997->type = max8997_i2c_get_driver_data(i2c, id);
>  	max8997->irq = i2c->irq;
>
> +	if (max8997->dev->of_node) {
> +		ret = max8997_i2c_parse_dt_pdata(max8997->dev, &pdata);
> +		if (ret)
> +			goto err;
> +		i2c->dev.platform_data = pdata;
> +	}
> +
>  	if (!pdata)
>  		goto err;
>
> @@ -428,11 +490,17 @@ const struct dev_pm_ops max8997_pm = {
>  	.restore = max8997_restore,
>  };
>
> +static struct of_device_id __devinitdata max8997_pmic_dt_match[] = {
> +	{ .compatible = "maxim,max8997-pmic", .data = TYPE_MAX8997 },
> +	{},
> +};
> +
>  static struct i2c_driver max8997_i2c_driver = {
>  	.driver = {
>  		   .name = "max8997",
>  		   .owner = THIS_MODULE,
>  		   .pm = &max8997_pm,
> +		   .of_match_table = of_match_ptr(max8997_pmic_dt_match),
>  	},
>  	.probe = max8997_i2c_probe,
>  	.remove = max8997_i2c_remove,
> diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
> index d26e864..a78695c 100644
> --- a/drivers/regulator/max8997.c
> +++ b/drivers/regulator/max8997.c
> @@ -25,6 +25,7 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> @@ -32,6 +33,7 @@
>  #include <linux/regulator/machine.h>
>  #include <linux/mfd/max8997.h>
>  #include <linux/mfd/max8997-private.h>
> +#include <linux/regulator/of_regulator.h>
>
>  struct max8997_data {
>  	struct device *dev;
> @@ -958,6 +960,137 @@ static struct regulator_desc regulators[] = {
>  	},
>  };
>
> +#if CONFIG_OF
> +static int max8997_pmic_dt_parse_dvs_gpio(struct max8997_dev *iodev,
> +			struct max8997_platform_data *pdata,
> +			struct device_node *pmic_np)
> +{
> +	int i, gpio;
> +
> +	for (i = 0; i < 3; i++) {
> +		gpio = of_get_named_gpio(pmic_np,
> +					"max8997,pmic-buck125-dvs-gpios", i);
> +		if (!gpio_is_valid(gpio)) {
> +			dev_err(iodev->dev, "invalid gpio[%d]: %d\n", i, gpio);
> +			return -EINVAL;
> +		}
> +		pdata->buck125_gpios[i] = gpio;
> +	}
> +	return 0;
> +}
> +
> +static int max8997_pmic_dt_parse_pdata(struct max8997_dev *iodev,
> +					struct max8997_platform_data *pdata)
> +{
> +	struct device_node *pmic_np, *regulators_np, *reg_np;
> +	struct max8997_regulator_data *rdata;
> +	unsigned int i, ret;
> +
> +	pmic_np = iodev->dev->of_node;
> +	if (!pmic_np) {
> +		dev_err(iodev->dev, "could not find pmic sub-node\n");
> +		return -ENODEV;
> +	}
> +
> +	regulators_np = of_find_node_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 = 0;
> +	for_each_child_of_node(regulators_np, reg_np)
> +		pdata->num_regulators++;
> +
> +	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
> +				pdata->num_regulators, GFP_KERNEL);
> +	if (!rdata) {
> +		dev_err(iodev->dev, "could not allocate memory for "
> +						"regulator data\n");
> +		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;
> +		rdata->id = i;
> +		rdata->initdata = of_get_regulator_init_data(
> +						iodev->dev, reg_np);
> +		rdata++;
> +	}
> +
> +	if (of_get_property(pmic_np, "max8997,pmic-buck1-uses-gpio-dvs", NULL))
> +		pdata->buck1_gpiodvs = true;
> +
> +	if (of_get_property(pmic_np, "max8997,pmic-buck2-uses-gpio-dvs", NULL))
> +		pdata->buck2_gpiodvs = true;
> +
> +	if (of_get_property(pmic_np, "max8997,pmic-buck5-uses-gpio-dvs", NULL))
> +		pdata->buck5_gpiodvs = true;
> +
> +	if (pdata->buck1_gpiodvs) {
> +		if (of_property_read_u32_array(pmic_np,
> +				"max8997,pmic-buck1-dvs-voltage",
> +				pdata->buck1_voltage, 8)) {
> +			dev_err(iodev->dev, "buck1 voltages not specified\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (pdata->buck2_gpiodvs) {
> +		if (of_property_read_u32_array(pmic_np,
> +				"max8997,pmic-buck2-dvs-voltage",
> +				pdata->buck2_voltage, 8)) {
> +			dev_err(iodev->dev, "buck2 voltages not specified\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (pdata->buck5_gpiodvs) {
> +		if (of_property_read_u32_array(pmic_np,
> +				"max8997,pmic-buck5-dvs-voltage",
> +				pdata->buck5_voltage, 8)) {
> +			dev_err(iodev->dev, "buck5 voltages not specified\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (pdata->buck1_gpiodvs || pdata->buck2_gpiodvs ||
> +						pdata->buck5_gpiodvs) {
> +		ret = max8997_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
> +		if (ret)
> +			return -EINVAL;
> +
> +		if (of_property_read_u32(pmic_np,
> +				"max8997,pmic-buck125-default-dvs-idx",
> +				&pdata->buck125_default_idx)) {
> +			pdata->buck125_default_idx = 0;
> +		} else {
> +			if (pdata->buck125_default_idx >= 8) {
> +				pdata->buck125_default_idx = 0;
> +				dev_info(iodev->dev, "invalid value for "
> +				"default dvs index, using 0 instead\n");
> +			}
> +		}
> +
> +		if (of_get_property(pmic_np,
> +			"max8997,pmic-ignore-gpiodvs-side-effect", NULL))
> +			pdata->ignore_gpiodvs_side_effect = true;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static int max8997_pmic_dt_parse_pdata(struct max8997_dev *iodev,
> +					struct max8997_platform_data *pdata)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_OF */
> +
>  static __devinit int max8997_pmic_probe(struct platform_device *pdev)
>  {
>  	struct max8997_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> @@ -973,6 +1106,10 @@ static __devinit int max8997_pmic_probe(struct
> platform_device *pdev)
>  		return -ENODEV;
>  	}
>
> +	ret = max8997_pmic_dt_parse_pdata(iodev, pdata);
> +	if (ret)
> +		return ret;
> +
>  	max8997 = kzalloc(sizeof(struct max8997_data), GFP_KERNEL);
>  	if (!max8997)
>  		return -ENOMEM;
> --
> 1.6.6.rc2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH 2/2] regulator: add device tree support for max8997

[Thomas Abraham]

Dear Mr. Park,

On 9 December 2011 08:00, Kyungmin Park <kyungmin.park@samsung.com> wrote:
> On 12/9/11, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>> Add device tree based discovery support for max8997.
>>
>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Cc: Rajendra Nayak <rnayak@ti.com>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>

[...]

>> +#else
>> +static int *max8997_i2c_parse_dt_pdata(struct device *dev
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct max8997_platform_data **pdata)
> It should be 'int' instead of 'int *'
>
> It's helpful to compile without CONFIG_OF.

I apologize for that. I compiled for exynos-dt first and then compiled
for origen board (which does not select CONFIG_OF). When compiling for
non-dt origen, I missed to notice that CONFIG_OF selection was left
over from the previous exynos-dt compile.

I have checked again without CONFIG_OF and found this issue as well as
two other errors/warnings.

Thanks. This was very helpful.

Regards,
Thomas.

[...]

[PATCH 2/2] regulator: add device tree support for max8997

[Mark Brown]

On Thu, Dec 08, 2011 at 09:57:39PM +0530, Thomas Abraham wrote:

> +Required properties, if interrupts are delivered to host system:

These are just optional properties, really.

> +- interrupt-parent: Specifies the phandle of the interrupt controller to which
> +  the interrupts from max8997 are delivered to.
> +- interrupts: Interrupt specifiers for two interrupt sources.
> +  - First interrupt specifier is for 'irq1' interrupt.
> +  - Second interrupt specifier is for 'alert' interrupt.

What if the two are connected to different interrupt controllers?

> +    For LDO's:

Extra '.

[PATCH 2/2] regulator: add device tree support for max8997

[Thomas Abraham]

On 9 December 2011 11:47, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Dec 08, 2011 at 09:57:39PM +0530, Thomas Abraham wrote:
>
>> +Required properties, if interrupts are delivered to host system:
>
> These are just optional properties, really.

Ok. Yeah, these can be classified as optional.

>
>> +- interrupt-parent: Specifies the phandle of the interrupt controller to which
>> + ?the interrupts from max8997 are delivered to.
>> +- interrupts: Interrupt specifiers for two interrupt sources.
>> + ?- First interrupt specifier is for 'irq1' interrupt.
>> + ?- Second interrupt specifier is for 'alert' interrupt.
>
> What if the two are connected to different interrupt controllers?

I have not thought of that. I will try to find out how to write a
device tree node for a device that delivers interrupts to separate
interrupt controllers.

Thanks,
Thomas.

>
>> + ? ?For LDO's:
>
> Extra '.

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[MyungJoo Ham]

On Fri, Dec 9, 2011 at 1:27 AM, Thomas Abraham
<thomas.abraham@linaro.org> wrote:
> Add irq domain support for max8997 interrupts. All uses of irq_base in platform
> data and max8997 driver private data are removed.
>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>

Looks good to me.

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>

> ---
> ?arch/arm/mach-exynos/mach-nuri.c ? ?| ? ?4 ----
> ?arch/arm/mach-exynos/mach-origen.c ?| ? ?1 -
> ?drivers/mfd/max8997-irq.c ? ? ? ? ? | ? 33 +++++++++++++++++++--------------
> ?drivers/mfd/max8997.c ? ? ? ? ? ? ? | ? ?1 -
> ?include/linux/mfd/max8997-private.h | ? ?4 +++-
> ?include/linux/mfd/max8997.h ? ? ? ? | ? ?1 -
> ?6 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
> index 236bbe1..ae333e5 100644
> --- a/arch/arm/mach-exynos/mach-nuri.c
> +++ b/arch/arm/mach-exynos/mach-nuri.c
> @@ -1077,12 +1077,8 @@ static struct platform_device nuri_max8903_device = {
> ?static void __init nuri_power_init(void)
> ?{
> ? ? ? ?int gpio;
> - ? ? ? int irq_base = IRQ_GPIO_END + 1;
> ? ? ? ?int ta_en = 0;
>
> - ? ? ? nuri_max8997_pdata.irq_base = irq_base;
> - ? ? ? irq_base += MAX8997_IRQ_NR;
> -
> ? ? ? ?gpio = EXYNOS4_GPX0(7);
> ? ? ? ?gpio_request(gpio, "AP_PMIC_IRQ");
> ? ? ? ?s3c_gpio_cfgpin(gpio, S3C_GPIO_SFN(0xf));
> diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
> index f56d027..588b0a8 100644
> --- a/arch/arm/mach-exynos/mach-origen.c
> +++ b/arch/arm/mach-exynos/mach-origen.c
> @@ -421,7 +421,6 @@ struct max8997_platform_data __initdata origen_max8997_pdata = {
> ? ? ? ?.buck1_gpiodvs ?= false,
> ? ? ? ?.buck2_gpiodvs ?= false,
> ? ? ? ?.buck5_gpiodvs ?= false,
> - ? ? ? .irq_base ? ? ? = IRQ_GPIO_END + 1,
>
> ? ? ? ?.ignore_gpiodvs_side_effect = true,
> ? ? ? ?.buck125_default_idx = 0x0,
> diff --git a/drivers/mfd/max8997-irq.c b/drivers/mfd/max8997-irq.c
> index 09274cf..eb9cad5 100644
> --- a/drivers/mfd/max8997-irq.c
> +++ b/drivers/mfd/max8997-irq.c
> @@ -142,7 +142,8 @@ static void max8997_irq_sync_unlock(struct irq_data *data)
> ?static const inline struct max8997_irq_data *
> ?irq_to_max8997_irq(struct max8997_dev *max8997, int irq)
> ?{
> - ? ? ? return &max8997_irqs[irq - max8997->irq_base];
> + ? ? ? struct irq_data *data = irq_get_irq_data(irq);
> + ? ? ? return &max8997_irqs[data->hwirq];
> ?}
>
> ?static void max8997_irq_mask(struct irq_data *data)
> @@ -179,10 +180,11 @@ static struct irq_chip max8997_irq_chip = {
> ?static irqreturn_t max8997_irq_thread(int irq, void *data)
> ?{
> ? ? ? ?struct max8997_dev *max8997 = data;
> + ? ? ? struct irq_domain *domain = &max8997->irq_domain;
> ? ? ? ?u8 irq_reg[MAX8997_IRQ_GROUP_NR] = {};
> ? ? ? ?u8 irq_src;
> ? ? ? ?int ret;
> - ? ? ? int i;
> + ? ? ? int i, cur_irq;
>
> ? ? ? ?ret = max8997_read_reg(max8997->i2c, MAX8997_REG_INTSRC, &irq_src);
> ? ? ? ?if (ret < 0) {
> @@ -268,9 +270,9 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
> ? ? ? ? ? ? ? ?irq_reg[i] &= ~max8997->irq_masks_cur[i];
>
> ? ? ? ?/* Report */
> - ? ? ? for (i = 0; i < MAX8997_IRQ_NR; i++) {
> + ? ? ? irq_domain_for_each_irq(domain, i, cur_irq) {
> ? ? ? ? ? ? ? ?if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
> - ? ? ? ? ? ? ? ? ? ? ? handle_nested_irq(max8997->irq_base + i);
> + ? ? ? ? ? ? ? ? ? ? ? handle_nested_irq(cur_irq);
> ? ? ? ?}
>
> ? ? ? ?return IRQ_HANDLED;
> @@ -278,13 +280,14 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>
> ?int max8997_irq_resume(struct max8997_dev *max8997)
> ?{
> - ? ? ? if (max8997->irq && max8997->irq_base)
> - ? ? ? ? ? ? ? max8997_irq_thread(max8997->irq_base, max8997);
> + ? ? ? if (max8997->irq && max8997->irq_domain.irq_base)
> + ? ? ? ? ? ? ? max8997_irq_thread(max8997->irq_domain.irq_base, max8997);
> ? ? ? ?return 0;
> ?}
>
> ?int max8997_irq_init(struct max8997_dev *max8997)
> ?{
> + ? ? ? struct irq_domain *domain = &max8997->irq_domain;
> ? ? ? ?int i;
> ? ? ? ?int cur_irq;
> ? ? ? ?int ret;
> @@ -292,12 +295,6 @@ int max8997_irq_init(struct max8997_dev *max8997)
>
> ? ? ? ?if (!max8997->irq) {
> ? ? ? ? ? ? ? ?dev_warn(max8997->dev, "No interrupt specified.\n");
> - ? ? ? ? ? ? ? max8997->irq_base = 0;
> - ? ? ? ? ? ? ? return 0;
> - ? ? ? }
> -
> - ? ? ? if (!max8997->irq_base) {
> - ? ? ? ? ? ? ? dev_err(max8997->dev, "No interrupt base specified.\n");
> ? ? ? ? ? ? ? ?return 0;
> ? ? ? ?}
>
> @@ -327,9 +324,17 @@ int max8997_irq_init(struct max8997_dev *max8997)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?true : false;
> ? ? ? ?}
>
> + ? ? ? domain->irq_base = irq_alloc_descs(-1, 0, MAX8997_IRQ_NR, 0);
> + ? ? ? if (domain->irq_base < 0) {
> + ? ? ? ? ? ? ? dev_err(max8997->dev, "failed to alloc irq descs\n");
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? }
> + ? ? ? domain->nr_irq = MAX8997_IRQ_NR;
> + ? ? ? domain->ops = &irq_domain_simple_ops;
> + ? ? ? irq_domain_add(domain);
> +
> ? ? ? ?/* Register with genirq */
> - ? ? ? for (i = 0; i < MAX8997_IRQ_NR; i++) {
> - ? ? ? ? ? ? ? cur_irq = i + max8997->irq_base;
> + ? ? ? irq_domain_for_each_irq(domain, i, cur_irq) {
> ? ? ? ? ? ? ? ?irq_set_chip_data(cur_irq, max8997);
> ? ? ? ? ? ? ? ?irq_set_chip_and_handler(cur_irq, &max8997_irq_chip,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?handle_edge_irq);
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index 5be53ae..b996c6e 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -142,7 +142,6 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
> ? ? ? ?if (!pdata)
> ? ? ? ? ? ? ? ?goto err;
>
> - ? ? ? max8997->irq_base = pdata->irq_base;
> ? ? ? ?max8997->ono = pdata->ono;
>
> ? ? ? ?mutex_init(&max8997->iolock);
> diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h
> index 3f4deb6..04c5779 100644
> --- a/include/linux/mfd/max8997-private.h
> +++ b/include/linux/mfd/max8997-private.h
> @@ -23,6 +23,8 @@
> ?#define __LINUX_MFD_MAX8997_PRIV_H
>
> ?#include <linux/i2c.h>
> +#include <linux/export.h>
> +#include <linux/irqdomain.h>
>
> ?#define MAX8997_REG_INVALID ? ?(0xff)
>
> @@ -325,7 +327,7 @@ struct max8997_dev {
>
> ? ? ? ?int irq;
> ? ? ? ?int ono;
> - ? ? ? int irq_base;
> + ? ? ? struct irq_domain irq_domain;
> ? ? ? ?struct mutex irqlock;
> ? ? ? ?int irq_masks_cur[MAX8997_IRQ_GROUP_NR];
> ? ? ? ?int irq_masks_cache[MAX8997_IRQ_GROUP_NR];
> diff --git a/include/linux/mfd/max8997.h b/include/linux/mfd/max8997.h
> index 0bbd13d..1e0b9cc 100644
> --- a/include/linux/mfd/max8997.h
> +++ b/include/linux/mfd/max8997.h
> @@ -79,7 +79,6 @@ struct max8997_regulator_data {
>
> ?struct max8997_platform_data {
> ? ? ? ?/* IRQ */
> - ? ? ? int irq_base;
> ? ? ? ?int ono;
> ? ? ? ?int wakeup;
>
> --
> 1.6.6.rc2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Mark Brown]

On Thu, Dec 08, 2011 at 09:57:38PM +0530, Thomas Abraham wrote:

You should be sending this to Samuel for review as it's a MFD patch.

> Add irq domain support for max8997 interrupts. All uses of irq_base in platform
> data and max8997 driver private data are removed.

Removing irq_base from platform data isn't going to be helpful for
anyone using the driver on non-DT platforms as it's going to make it
impossible to find the interrupts.

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Thomas Abraham]

Hi Mark,

On 9 December 2011 10:11, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Dec 08, 2011 at 09:57:38PM +0530, Thomas Abraham wrote:
>
> You should be sending this to Samuel for review as it's a MFD patch.

Ok. I will Cc Samuel for the v2 of this patchset.

>
>> Add irq domain support for max8997 interrupts. All uses of irq_base in platform
>> data and max8997 driver private data are removed.
>
> Removing irq_base from platform data isn't going to be helpful for
> anyone using the driver on non-DT platforms as it's going to make it
> impossible to find the interrupts.

All the consumers of the max8997 interrupts are the drivers for the
sub-blocks of max8997. These drivers have access to 'irq_domain'
member of 'struct max8997_dev', From irq_domain, the sub-block driver
can get the irq_base and add one of the 'enum max8997_irq' offset when
registering for interrupt notifications.

So irq_base from platform data and all uses of it was removed. All
max8997 sub-block drivers should use the irq_base of the irq_domain.

Thanks for your review.

Regards,
Thomas.

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Mark Brown]

On Fri, Dec 09, 2011 at 10:55:10AM +0530, Thomas Abraham wrote:
> On 9 December 2011 10:11, Mark Brown

> > Removing irq_base from platform data isn't going to be helpful for
> > anyone using the driver on non-DT platforms as it's going to make it
> > impossible to find the interrupts.

> All the consumers of the max8997 interrupts are the drivers for the
> sub-blocks of max8997. These drivers have access to 'irq_domain'
> member of 'struct max8997_dev', From irq_domain, the sub-block driver
> can get the irq_base and add one of the 'enum max8997_irq' offset when
> registering for interrupt notifications.

There's no GPIOs with interrupt support (even if they're not implemented
yet)?

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Thomas Abraham]

On 9 December 2011 11:32, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Dec 09, 2011 at 10:55:10AM +0530, Thomas Abraham wrote:
>> On 9 December 2011 10:11, Mark Brown
>
>> > Removing irq_base from platform data isn't going to be helpful for
>> > anyone using the driver on non-DT platforms as it's going to make it
>> > impossible to find the interrupts.
>
>> All the consumers of the max8997 interrupts are the drivers for the
>> sub-blocks of max8997. These drivers have access to 'irq_domain'
>> member of 'struct max8997_dev', From irq_domain, the sub-block driver
>> can get the irq_base and add one of the 'enum max8997_irq' offset when
>> registering for interrupt notifications.
>
> There's no GPIOs with interrupt support (even if they're not implemented
> yet)?

Ok. There does seem to be 12 gpios supported by max8997, but not used
on the origen board.

If these gpio's are connected to the host system, and host system sets
up gpio interrupt notification for these gpio lines, the linux irq
number for the gpio interrupt would belong to the irq domain of the
host interrupt controller (gic irq domain in case of exynos).
Consumers of this interrupt would use the gpio interrupt independent
of the max8997 irq domain. So irq_base can be maintained private to
the max8997 mfd driver and its sub-block drivers.

Thanks,
Thomas.

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Mark Brown]

On Fri, Dec 09, 2011 at 12:00:07PM +0530, Thomas Abraham wrote:

> If these gpio's are connected to the host system, and host system sets
> up gpio interrupt notification for these gpio lines, the linux irq
> number for the gpio interrupt would belong to the irq domain of the
> host interrupt controller (gic irq domain in case of exynos).
> Consumers of this interrupt would use the gpio interrupt independent
> of the max8997 irq domain. So irq_base can be maintained private to
> the max8997 mfd driver and its sub-block drivers.

How would you provide this interrupt to the device using it in a non-DT
systemm without passing the irq_base into the device as platform data?

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Thomas Abraham]

On 9 December 2011 12:09, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Dec 09, 2011 at 12:00:07PM +0530, Thomas Abraham wrote:
>
>> If these gpio's are connected to the host system, and host system sets
>> up gpio interrupt notification for these gpio lines, the linux irq
>> number for the gpio interrupt would belong to the irq domain of the
>> host interrupt controller (gic irq domain in case of exynos).
>> Consumers of this interrupt would use the gpio interrupt independent
>> of the max8997 irq domain. So irq_base can be maintained private to
>> the max8997 mfd driver and its sub-block drivers.
>
> How would you provide this interrupt to the device using it in a non-DT
> systemm without passing the irq_base into the device as platform data?

In non-DT system, the linux irq number that is used to register the
gpio interrupt notifier would belong to the irq domain of the host
interrupt controller (gic). For Exynos, the linux irq numbers that
belong to the gic irq domain are statically assigned to specific
interrupt notifiers and known to the entire system.

For instance, the linux virq number for gpio interrupt for the gpio
connected from max8997 to Exynos GPIO (hypothetical case) would be
statically defined as GPIO_EINT_MAX8997_GPIO0. The device that
requires to be notified of this interrupt would request_irq or
setup_irq using this macro and hence needs no knowledge of the
irq_base of the max8997 irq domain.

If the system uses sparse irqs, or if static mapping of linux virq's
are not possible in a system, that is a problem that this patch does
not address. irq_domain of max8997 is self contained to provide all
max8997 linux irq numbers required by the sub-block drivers of
max8997.

Thanks,
Thomas.

[PATCH 1/2] mfd: add irq domain support for max8997 interrupts

[Mark Brown]

On Fri, Dec 09, 2011 at 12:49:49PM +0530, Thomas Abraham wrote:
> On 9 December 2011 12:09, Mark Brown

> > How would you provide this interrupt to the device using it in a non-DT
> > systemm without passing the irq_base into the device as platform data?

> In non-DT system, the linux irq number that is used to register the
> gpio interrupt notifier would belong to the irq domain of the host
> interrupt controller (gic). For Exynos, the linux irq numbers that
> belong to the gic irq domain are statically assigned to specific
> interrupt notifiers and known to the entire system.

This seems like it's going to be a bit of a usability issue at best as
users will have to figure out the relevant domain and the offset we end
up with within it, and you appear to be making assumptions about the
system you're running on here.  The way this has been handled for other
drivers is by using irq_alloc_descs with platform data to provide a
default, preserving the old behaviour on non-OF systems.