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

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

[Thomas Abraham]

Add irq domain support for max8997 interrupts. The reverse mapping method
used is linear mapping since the sub-drivers of max8997 such as regulator
and charger drivers can use the max8997 irq_domain to get the linux irq
number for max8997 interrupts. All uses of irq_base in platform data and
max8997 driver private data are removed.

Cc: Grant Likely <grant.likely@secretlab.ca>
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           |   60 ++++++++++++++++++++--------------
 drivers/mfd/max8997.c               |    1 -
 include/linux/mfd/max8997-private.h |    4 ++-
 include/linux/mfd/max8997.h         |    1 -
 6 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
index a6b9162..1e9ba12 100644
--- a/arch/arm/mach-exynos/mach-nuri.c
+++ b/arch/arm/mach-exynos/mach-nuri.c
@@ -1078,12 +1078,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 02c242e..328dadb 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -424,7 +424,6 @@ static 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..3d8c9d1 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)
@@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 	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) {
@@ -269,8 +270,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 
 	/* Report */
 	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
-			handle_nested_irq(max8997->irq_base + i);
+		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
+			cur_irq = irq_find_mapping(max8997->irq_domain, i);
+			handle_nested_irq(cur_irq);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -278,26 +281,40 @@ 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)
+		max8997_irq_thread(0, max8997);
+	return 0;
+}
+
+static int max8997_irq_domain_map(struct irq_domain *d, unsigned int irq,
+					irq_hw_number_t hw)
+{
+	struct max8997_dev *max8997 = d->host_data;
+
+	irq_set_chip_data(irq, max8997);
+	irq_set_chip_and_handler(irq, &max8997_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 max8997_irq_domain_ops = {
+	.map = max8997_irq_domain_map,
+};
+
 int max8997_irq_init(struct max8997_dev *max8997)
 {
+	struct irq_domain *domain;
 	int i;
-	int cur_irq;
 	int ret;
 	u8 val;
 
 	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,18 +344,11 @@ int max8997_irq_init(struct max8997_dev *max8997)
 					true : false;
 	}
 
-	/* Register with genirq */
-	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		cur_irq = i + max8997->irq_base;
-		irq_set_chip_data(cur_irq, max8997);
-		irq_set_chip_and_handler(cur_irq, &max8997_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_linear(NULL, MAX8997_IRQ_NR,
+					&max8997_irq_domain_ops, &max8997);
+	if (!domain) {
+		dev_err(max8997->dev, "could not create irq domain\n");
+		return -ENODEV;
 	}
 
 	ret = request_threaded_irq(max8997->irq, NULL, max8997_irq_thread,
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index cb83a7a..20ecad3 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -143,7 +143,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..830152c 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 fff5905..818486c 100644
--- a/include/linux/mfd/max8997.h
+++ b/include/linux/mfd/max8997.h
@@ -155,7 +155,6 @@ struct max8997_led_platform_data {
 
 struct max8997_platform_data {
 	/* IRQ */
-	int irq_base;
 	int ono;
 	int wakeup;
 
-- 
1.6.6.rc2

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

[Thomas Abraham]

Add irq domain support for max8997 interrupts. The reverse mapping method
used is linear mapping since the sub-drivers of max8997 such as regulator
and charger drivers can use the max8997 irq_domain to get the linux irq
number for max8997 interrupts. All uses of irq_base in platform data and
max8997 driver private data are removed.

Cc: Grant Likely <grant.likely@secretlab.ca>
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           |   60 ++++++++++++++++++++--------------
 drivers/mfd/max8997.c               |    1 -
 include/linux/mfd/max8997-private.h |    4 ++-
 include/linux/mfd/max8997.h         |    1 -
 6 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
index a6b9162..1e9ba12 100644
--- a/arch/arm/mach-exynos/mach-nuri.c
+++ b/arch/arm/mach-exynos/mach-nuri.c
@@ -1078,12 +1078,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 02c242e..328dadb 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -424,7 +424,6 @@ static 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..3d8c9d1 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)
@@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 	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) {
@@ -269,8 +270,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 
 	/* Report */
 	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
-			handle_nested_irq(max8997->irq_base + i);
+		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
+			cur_irq = irq_find_mapping(max8997->irq_domain, i);
+			handle_nested_irq(cur_irq);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -278,26 +281,40 @@ 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)
+		max8997_irq_thread(0, max8997);
+	return 0;
+}
+
+static int max8997_irq_domain_map(struct irq_domain *d, unsigned int irq,
+					irq_hw_number_t hw)
+{
+	struct max8997_dev *max8997 = d->host_data;
+
+	irq_set_chip_data(irq, max8997);
+	irq_set_chip_and_handler(irq, &max8997_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 max8997_irq_domain_ops = {
+	.map = max8997_irq_domain_map,
+};
+
 int max8997_irq_init(struct max8997_dev *max8997)
 {
+	struct irq_domain *domain;
 	int i;
-	int cur_irq;
 	int ret;
 	u8 val;
 
 	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,18 +344,11 @@ int max8997_irq_init(struct max8997_dev *max8997)
 					true : false;
 	}
 
-	/* Register with genirq */
-	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		cur_irq = i + max8997->irq_base;
-		irq_set_chip_data(cur_irq, max8997);
-		irq_set_chip_and_handler(cur_irq, &max8997_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_linear(NULL, MAX8997_IRQ_NR,
+					&max8997_irq_domain_ops, &max8997);
+	if (!domain) {
+		dev_err(max8997->dev, "could not create irq domain\n");
+		return -ENODEV;
 	}
 
 	ret = request_threaded_irq(max8997->irq, NULL, max8997_irq_thread,
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index cb83a7a..20ecad3 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -143,7 +143,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..830152c 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 fff5905..818486c 100644
--- a/include/linux/mfd/max8997.h
+++ b/include/linux/mfd/max8997.h
@@ -155,7 +155,6 @@ struct max8997_led_platform_data {
 
 struct max8997_platform_data {
 	/* IRQ */
-	int irq_base;
 	int ono;
 	int wakeup;
 
-- 
1.6.6.rc2

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

[Grant Likely]

On Thu, 23 Feb 2012 18:08:07 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> Add irq domain support for max8997 interrupts. The reverse mapping method
> used is linear mapping since the sub-drivers of max8997 such as regulator
> and charger drivers can use the max8997 irq_domain to get the linux irq
> number for max8997 interrupts. All uses of irq_base in platform data and
> max8997 driver private data are removed.
> 
> Cc: Grant Likely <grant.likely@secretlab.ca>
> 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           |   60 ++++++++++++++++++++--------------
>  drivers/mfd/max8997.c               |    1 -
>  include/linux/mfd/max8997-private.h |    4 ++-
>  include/linux/mfd/max8997.h         |    1 -
>  6 files changed, 38 insertions(+), 33 deletions(-)

Nice patch.  Nice diffstat too.  Some comments below, but I'm happy with where
this is at.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
> index a6b9162..1e9ba12 100644
> --- a/arch/arm/mach-exynos/mach-nuri.c
> +++ b/arch/arm/mach-exynos/mach-nuri.c
> @@ -1078,12 +1078,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 02c242e..328dadb 100644
> --- a/arch/arm/mach-exynos/mach-origen.c
> +++ b/arch/arm/mach-exynos/mach-origen.c
> @@ -424,7 +424,6 @@ static 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..3d8c9d1 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];

Hahaha.  The max8997_irqs table is kind of overdone when the group and mask values
can be arithmetically derived from the hwirq number..

This change is fine, but the driver could use some refactoring.  :-)

>  }
>  
>  static void max8997_irq_mask(struct irq_data *data)
> @@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>  	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) {
> @@ -269,8 +270,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>  
>  	/* Report */
>  	for (i = 0; i < MAX8997_IRQ_NR; i++) {
> -		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
> -			handle_nested_irq(max8997->irq_base + i);
> +		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
> +			cur_irq = irq_find_mapping(max8997->irq_domain, i);
> +			handle_nested_irq(cur_irq);
> +		}

Well, if you're using the linear mapping, then only actually allocated irqs will
have a mapping set up.  So the driver could simply do:

	for (i = 0; i < MAX8997_IRQ_NR; i++) {
		cur_irq = irq_find_mapping(max8997->irq_domain, i);
		if (cur_irq)
			handle_nested_irq(cur_irq);
	}

>  	}
>  
>  	return IRQ_HANDLED;
> @@ -278,26 +281,40 @@ 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)
> +		max8997_irq_thread(0, max8997);
> +	return 0;
> +}
> +
> +static int max8997_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +					irq_hw_number_t hw)
> +{
> +	struct max8997_dev *max8997 = d->host_data;
> +
> +	irq_set_chip_data(irq, max8997);
> +	irq_set_chip_and_handler(irq, &max8997_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 max8997_irq_domain_ops = {
> +	.map = max8997_irq_domain_map,
> +};
> +
>  int max8997_irq_init(struct max8997_dev *max8997)
>  {
> +	struct irq_domain *domain;
>  	int i;
> -	int cur_irq;
>  	int ret;
>  	u8 val;
>  
>  	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,18 +344,11 @@ int max8997_irq_init(struct max8997_dev *max8997)
>  					true : false;
>  	}
>  
> -	/* Register with genirq */
> -	for (i = 0; i < MAX8997_IRQ_NR; i++) {
> -		cur_irq = i + max8997->irq_base;
> -		irq_set_chip_data(cur_irq, max8997);
> -		irq_set_chip_and_handler(cur_irq, &max8997_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_linear(NULL, MAX8997_IRQ_NR,
> +					&max8997_irq_domain_ops, &max8997);
> +	if (!domain) {
> +		dev_err(max8997->dev, "could not create irq domain\n");
> +		return -ENODEV;
>  	}
>  
>  	ret = request_threaded_irq(max8997->irq, NULL, max8997_irq_thread,
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index cb83a7a..20ecad3 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -143,7 +143,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..830152c 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 fff5905..818486c 100644
> --- a/include/linux/mfd/max8997.h
> +++ b/include/linux/mfd/max8997.h
> @@ -155,7 +155,6 @@ struct max8997_led_platform_data {
>  
>  struct max8997_platform_data {
>  	/* IRQ */
> -	int irq_base;
>  	int ono;
>  	int wakeup;
>  
> -- 
> 1.6.6.rc2
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

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

[Grant Likely]

On Thu, 23 Feb 2012 18:08:07 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> Add irq domain support for max8997 interrupts. The reverse mapping method
> used is linear mapping since the sub-drivers of max8997 such as regulator
> and charger drivers can use the max8997 irq_domain to get the linux irq
> number for max8997 interrupts. All uses of irq_base in platform data and
> max8997 driver private data are removed.
> 
> Cc: Grant Likely <grant.likely@secretlab.ca>
> 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           |   60 ++++++++++++++++++++--------------
>  drivers/mfd/max8997.c               |    1 -
>  include/linux/mfd/max8997-private.h |    4 ++-
>  include/linux/mfd/max8997.h         |    1 -
>  6 files changed, 38 insertions(+), 33 deletions(-)

Nice patch.  Nice diffstat too.  Some comments below, but I'm happy with where
this is at.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> 
> diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
> index a6b9162..1e9ba12 100644
> --- a/arch/arm/mach-exynos/mach-nuri.c
> +++ b/arch/arm/mach-exynos/mach-nuri.c
> @@ -1078,12 +1078,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 02c242e..328dadb 100644
> --- a/arch/arm/mach-exynos/mach-origen.c
> +++ b/arch/arm/mach-exynos/mach-origen.c
> @@ -424,7 +424,6 @@ static 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..3d8c9d1 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];

Hahaha.  The max8997_irqs table is kind of overdone when the group and mask values
can be arithmetically derived from the hwirq number..

This change is fine, but the driver could use some refactoring.  :-)

>  }
>  
>  static void max8997_irq_mask(struct irq_data *data)
> @@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>  	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) {
> @@ -269,8 +270,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>  
>  	/* Report */
>  	for (i = 0; i < MAX8997_IRQ_NR; i++) {
> -		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
> -			handle_nested_irq(max8997->irq_base + i);
> +		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
> +			cur_irq = irq_find_mapping(max8997->irq_domain, i);
> +			handle_nested_irq(cur_irq);
> +		}

Well, if you're using the linear mapping, then only actually allocated irqs will
have a mapping set up.  So the driver could simply do:

	for (i = 0; i < MAX8997_IRQ_NR; i++) {
		cur_irq = irq_find_mapping(max8997->irq_domain, i);
		if (cur_irq)
			handle_nested_irq(cur_irq);
	}

>  	}
>  
>  	return IRQ_HANDLED;
> @@ -278,26 +281,40 @@ 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)
> +		max8997_irq_thread(0, max8997);
> +	return 0;
> +}
> +
> +static int max8997_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +					irq_hw_number_t hw)
> +{
> +	struct max8997_dev *max8997 = d->host_data;
> +
> +	irq_set_chip_data(irq, max8997);
> +	irq_set_chip_and_handler(irq, &max8997_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 max8997_irq_domain_ops = {
> +	.map = max8997_irq_domain_map,
> +};
> +
>  int max8997_irq_init(struct max8997_dev *max8997)
>  {
> +	struct irq_domain *domain;
>  	int i;
> -	int cur_irq;
>  	int ret;
>  	u8 val;
>  
>  	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,18 +344,11 @@ int max8997_irq_init(struct max8997_dev *max8997)
>  					true : false;
>  	}
>  
> -	/* Register with genirq */
> -	for (i = 0; i < MAX8997_IRQ_NR; i++) {
> -		cur_irq = i + max8997->irq_base;
> -		irq_set_chip_data(cur_irq, max8997);
> -		irq_set_chip_and_handler(cur_irq, &max8997_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_linear(NULL, MAX8997_IRQ_NR,
> +					&max8997_irq_domain_ops, &max8997);
> +	if (!domain) {
> +		dev_err(max8997->dev, "could not create irq domain\n");
> +		return -ENODEV;
>  	}
>  
>  	ret = request_threaded_irq(max8997->irq, NULL, max8997_irq_thread,
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index cb83a7a..20ecad3 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -143,7 +143,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..830152c 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 fff5905..818486c 100644
> --- a/include/linux/mfd/max8997.h
> +++ b/include/linux/mfd/max8997.h
> @@ -155,7 +155,6 @@ struct max8997_led_platform_data {
>  
>  struct max8997_platform_data {
>  	/* IRQ */
> -	int irq_base;
>  	int ono;
>  	int wakeup;
>  
> -- 
> 1.6.6.rc2
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

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

[Thomas Abraham]

On 13 March 2012 08:58, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, 23 Feb 2012 18:08:07 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>> Add irq domain support for max8997 interrupts. The reverse mapping method
>> used is linear mapping since the sub-drivers of max8997 such as regulator
>> and charger drivers can use the max8997 irq_domain to get the linux irq
>> number for max8997 interrupts. All uses of irq_base in platform data and
>> max8997 driver private data are removed.
>>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> 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           |   60 ++++++++++++++++++++--------------
>>  drivers/mfd/max8997.c               |    1 -
>>  include/linux/mfd/max8997-private.h |    4 ++-
>>  include/linux/mfd/max8997.h         |    1 -
>>  6 files changed, 38 insertions(+), 33 deletions(-)
>
> Nice patch.  Nice diffstat too.  Some comments below, but I'm happy with where
> this is at.
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Thank you Grant for reviewing this patch.

>
>>
>> diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
>> index a6b9162..1e9ba12 100644
>> --- a/arch/arm/mach-exynos/mach-nuri.c
>> +++ b/arch/arm/mach-exynos/mach-nuri.c
>> @@ -1078,12 +1078,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 02c242e..328dadb 100644
>> --- a/arch/arm/mach-exynos/mach-origen.c
>> +++ b/arch/arm/mach-exynos/mach-origen.c
>> @@ -424,7 +424,6 @@ static 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..3d8c9d1 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];
>
> Hahaha.  The max8997_irqs table is kind of overdone when the group and mask values
> can be arithmetically derived from the hwirq number..

Yes, I agree. Each group could have maximum of 8 interrupts. Then,
data->hwirq % 8 will be the group number and data->hwirq & 7 would be
the mask. The enum max8997_irq can then be adjusted to accommodate
unused irq numbers.

>
> This change is fine, but the driver could use some refactoring.  :-)

Ok.

>
>>  }
>>
>>  static void max8997_irq_mask(struct irq_data *data)
>> @@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>>       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) {
>> @@ -269,8 +270,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>>
>>       /* Report */
>>       for (i = 0; i < MAX8997_IRQ_NR; i++) {
>> -             if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
>> -                     handle_nested_irq(max8997->irq_base + i);
>> +             if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
>> +                     cur_irq = irq_find_mapping(max8997->irq_domain, i);
>> +                     handle_nested_irq(cur_irq);
>> +             }
>
> Well, if you're using the linear mapping, then only actually allocated irqs will
> have a mapping set up.  So the driver could simply do:
>
>        for (i = 0; i < MAX8997_IRQ_NR; i++) {
>                cur_irq = irq_find_mapping(max8997->irq_domain, i);
>                if (cur_irq)
>                        handle_nested_irq(cur_irq);
>        }

Thanks for the suggestion. I will include this change and repost this patch.

Regards,
Thomas.

[...]

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

[Thomas Abraham]

On 13 March 2012 08:58, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, 23 Feb 2012 18:08:07 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>> Add irq domain support for max8997 interrupts. The reverse mapping method
>> used is linear mapping since the sub-drivers of max8997 such as regulator
>> and charger drivers can use the max8997 irq_domain to get the linux irq
>> number for max8997 interrupts. All uses of irq_base in platform data and
>> max8997 driver private data are removed.
>>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> 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 ? ? ? ? ? | ? 60 ++++++++++++++++++++--------------
>> ?drivers/mfd/max8997.c ? ? ? ? ? ? ? | ? ?1 -
>> ?include/linux/mfd/max8997-private.h | ? ?4 ++-
>> ?include/linux/mfd/max8997.h ? ? ? ? | ? ?1 -
>> ?6 files changed, 38 insertions(+), 33 deletions(-)
>
> Nice patch. ?Nice diffstat too. ?Some comments below, but I'm happy with where
> this is at.
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Thank you Grant for reviewing this patch.

>
>>
>> diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
>> index a6b9162..1e9ba12 100644
>> --- a/arch/arm/mach-exynos/mach-nuri.c
>> +++ b/arch/arm/mach-exynos/mach-nuri.c
>> @@ -1078,12 +1078,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 02c242e..328dadb 100644
>> --- a/arch/arm/mach-exynos/mach-origen.c
>> +++ b/arch/arm/mach-exynos/mach-origen.c
>> @@ -424,7 +424,6 @@ static 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..3d8c9d1 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];
>
> Hahaha. ?The max8997_irqs table is kind of overdone when the group and mask values
> can be arithmetically derived from the hwirq number..

Yes, I agree. Each group could have maximum of 8 interrupts. Then,
data->hwirq % 8 will be the group number and data->hwirq & 7 would be
the mask. The enum max8997_irq can then be adjusted to accommodate
unused irq numbers.

>
> This change is fine, but the driver could use some refactoring. ?:-)

Ok.

>
>> ?}
>>
>> ?static void max8997_irq_mask(struct irq_data *data)
>> @@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>> ? ? ? 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) {
>> @@ -269,8 +270,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
>>
>> ? ? ? /* Report */
>> ? ? ? for (i = 0; i < MAX8997_IRQ_NR; i++) {
>> - ? ? ? ? ? ? if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
>> - ? ? ? ? ? ? ? ? ? ? handle_nested_irq(max8997->irq_base + i);
>> + ? ? ? ? ? ? if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
>> + ? ? ? ? ? ? ? ? ? ? cur_irq = irq_find_mapping(max8997->irq_domain, i);
>> + ? ? ? ? ? ? ? ? ? ? handle_nested_irq(cur_irq);
>> + ? ? ? ? ? ? }
>
> Well, if you're using the linear mapping, then only actually allocated irqs will
> have a mapping set up. ?So the driver could simply do:
>
> ? ? ? ?for (i = 0; i < MAX8997_IRQ_NR; i++) {
> ? ? ? ? ? ? ? ?cur_irq = irq_find_mapping(max8997->irq_domain, i);
> ? ? ? ? ? ? ? ?if (cur_irq)
> ? ? ? ? ? ? ? ? ? ? ? ?handle_nested_irq(cur_irq);
> ? ? ? ?}

Thanks for the suggestion. I will include this change and repost this patch.

Regards,
Thomas.

[...]

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

[Thomas Abraham]

On 13 March 2012 11:35, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> On 13 March 2012 08:58, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Thu, 23 Feb 2012 18:08:07 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>>> Add irq domain support for max8997 interrupts. The reverse mapping method
>>> used is linear mapping since the sub-drivers of max8997 such as regulator
>>> and charger drivers can use the max8997 irq_domain to get the linux irq
>>> number for max8997 interrupts. All uses of irq_base in platform data and
>>> max8997 driver private data are removed.
>>>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> 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           |   60 ++++++++++++++++++++--------------
>>>  drivers/mfd/max8997.c               |    1 -
>>>  include/linux/mfd/max8997-private.h |    4 ++-
>>>  include/linux/mfd/max8997.h         |    1 -
>>>  6 files changed, 38 insertions(+), 33 deletions(-)

[...]

>>
>> Hahaha.  The max8997_irqs table is kind of overdone when the group and mask values
>> can be arithmetically derived from the hwirq number..
>
> Yes, I agree. Each group could have maximum of 8 interrupts. Then,
> data->hwirq % 8 will be the group number and data->hwirq & 7 would be

Typo here. group number = data->hwirq >> 3 and mask = data->hwirq & 7.

Thanks,
Thomas.

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

[Thomas Abraham]

On 13 March 2012 11:35, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> On 13 March 2012 08:58, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Thu, 23 Feb 2012 18:08:07 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>>> Add irq domain support for max8997 interrupts. The reverse mapping method
>>> used is linear mapping since the sub-drivers of max8997 such as regulator
>>> and charger drivers can use the max8997 irq_domain to get the linux irq
>>> number for max8997 interrupts. All uses of irq_base in platform data and
>>> max8997 driver private data are removed.
>>>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> 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 ? ? ? ? ? | ? 60 ++++++++++++++++++++--------------
>>> ?drivers/mfd/max8997.c ? ? ? ? ? ? ? | ? ?1 -
>>> ?include/linux/mfd/max8997-private.h | ? ?4 ++-
>>> ?include/linux/mfd/max8997.h ? ? ? ? | ? ?1 -
>>> ?6 files changed, 38 insertions(+), 33 deletions(-)

[...]

>>
>> Hahaha. ?The max8997_irqs table is kind of overdone when the group and mask values
>> can be arithmetically derived from the hwirq number..
>
> Yes, I agree. Each group could have maximum of 8 interrupts. Then,
> data->hwirq % 8 will be the group number and data->hwirq & 7 would be

Typo here. group number = data->hwirq >> 3 and mask = data->hwirq & 7.

Thanks,
Thomas.

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

[Chanwoo Choi]

From: Thomas Abraham <thomas.abraham@linaro.org>

Add irq domain support for max8997 interrupts. The reverse mapping method
used is linear mapping since the sub-drivers of max8997 such as regulator
and charger drivers can use the max8997 irq_domain to get the linux irq
number for max8997 interrupts. All uses of irq_base in platform data and
max8997 driver private data are removed.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
	[Fix two bug which set max8997->irq_domain and correct wrong parameter]
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/mach-nuri.c    |    4 --
 arch/arm/mach-exynos/mach-origen.c  |    1 -
 drivers/mfd/max8997-irq.c           |   62 +++++++++++++++++++++--------------
 drivers/mfd/max8997.c               |    1 -
 include/linux/mfd/max8997-private.h |    4 ++-
 include/linux/mfd/max8997.h         |    1 -
 6 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
index 23d2254..0a53849 100644
--- a/arch/arm/mach-exynos/mach-nuri.c
+++ b/arch/arm/mach-exynos/mach-nuri.c
@@ -1064,12 +1064,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 827cb99..d3b2e9d 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -424,7 +424,6 @@ static 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 87cdfbd..3454855 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)
@@ -182,7 +183,7 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 	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) {
@@ -269,8 +270,11 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
 
 	/* Report */
 	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask)
-			handle_nested_irq(max8997->irq_base + i);
+		if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
+			cur_irq = irq_find_mapping(max8997->irq_domain, i);
+			if (cur_irq)
+				handle_nested_irq(cur_irq);
+		}
 	}
 
 	return IRQ_HANDLED;
@@ -278,26 +282,40 @@ 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)
+		max8997_irq_thread(0, max8997);
+	return 0;
+}
+
+static int max8997_irq_domain_map(struct irq_domain *d, unsigned int irq,
+					irq_hw_number_t hw)
+{
+	struct max8997_dev *max8997 = d->host_data;
+
+	irq_set_chip_data(irq, max8997);
+	irq_set_chip_and_handler(irq, &max8997_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 max8997_irq_domain_ops = {
+	.map = max8997_irq_domain_map,
+};
+
 int max8997_irq_init(struct max8997_dev *max8997)
 {
+	struct irq_domain *domain;
 	int i;
-	int cur_irq;
 	int ret;
 	u8 val;
 
 	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,19 +345,13 @@ int max8997_irq_init(struct max8997_dev *max8997)
 					true : false;
 	}
 
-	/* Register with genirq */
-	for (i = 0; i < MAX8997_IRQ_NR; i++) {
-		cur_irq = i + max8997->irq_base;
-		irq_set_chip_data(cur_irq, max8997);
-		irq_set_chip_and_handler(cur_irq, &max8997_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_linear(NULL, MAX8997_IRQ_NR,
+					&max8997_irq_domain_ops, max8997);
+	if (!domain) {
+		dev_err(max8997->dev, "could not create irq domain\n");
+		return -ENODEV;
 	}
+	max8997->irq_domain = domain;
 
 	ret = request_threaded_irq(max8997->irq, NULL, max8997_irq_thread,
 			IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index cb83a7a..20ecad3 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -143,7 +143,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..830152c 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 b40c08c..328d8e2 100644
--- a/include/linux/mfd/max8997.h
+++ b/include/linux/mfd/max8997.h
@@ -181,7 +181,6 @@ struct max8997_led_platform_data {
 
 struct max8997_platform_data {
 	/* IRQ */
-	int irq_base;
 	int ono;
 	int wakeup;
 
-- 
1.7.0.4

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

[Greg KH]

On Wed, May 16, 2012 at 04:41:44PM +0900, Chanwoo Choi wrote:
> From: Thomas Abraham <thomas.abraham@linaro.org>
> 
> Add irq domain support for max8997 interrupts. The reverse mapping method
> used is linear mapping since the sub-drivers of max8997 such as regulator
> and charger drivers can use the max8997 irq_domain to get the linux irq
> number for max8997 interrupts. All uses of irq_base in platform data and
> max8997 driver private data are removed.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 	[Fix two bug which set max8997->irq_domain and correct wrong parameter]
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Same problem as before, I apply this and it breaks the build with the
following errors:

drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_irq_work’:
drivers/extcon/extcon-max8997.c:350:36: error: ‘struct max8997_dev’ has no member named ‘irq_base’
drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_probe’:
drivers/extcon/extcon-max8997.c:452:35: error: ‘struct max8997_platform_data’ has no member named ‘irq_base’
drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_remove’:
drivers/extcon/extcon-max8997.c:514:19: error: ‘struct max8997_dev’ has no member named ‘irq_base’

Please fix.

greg k-h

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

[Chanwoo Choi]

Hi Greg,

On 05/19/2012 08:59 AM, Greg KH wrote:

> On Wed, May 16, 2012 at 04:41:44PM +0900, Chanwoo Choi wrote:
>> From: Thomas Abraham <thomas.abraham@linaro.org>
>>
>> Add irq domain support for max8997 interrupts. The reverse mapping method
>> used is linear mapping since the sub-drivers of max8997 such as regulator
>> and charger drivers can use the max8997 irq_domain to get the linux irq
>> number for max8997 interrupts. All uses of irq_base in platform data and
>> max8997 driver private data are removed.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>> Acked-by: Samuel Ortiz <sameo@linux.intel.com>
>> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> 	[Fix two bug which set max8997->irq_domain and correct wrong parameter]
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> 
> Same problem as before, I apply this and it breaks the build with the
> following errors:
> 
> drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_irq_work’:
> drivers/extcon/extcon-max8997.c:350:36: error: ‘struct max8997_dev’ has no member named ‘irq_base’
> drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_probe’:
> drivers/extcon/extcon-max8997.c:452:35: error: ‘struct max8997_platform_data’ has no member named ‘irq_base’
> drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_remove’:
> drivers/extcon/extcon-max8997.c:514:19: error: ‘struct max8997_dev’ has no member named ‘irq_base’
> 
> Please fix.

I posted patchset including below two patch. The second patch fix that
extcon-max8997 driver use irq_domain instead of irq_base field in
'struct max8997_dev'.

Please apply second patch when test this patchset.
If you want to consolidate this patchset, I will resend it.

[PATCH v3 1/2] mfd: add irq domain support for max8997 interrupts
[PATCH v3 2/2] Extcon: MAX8997: Add support irq domain for MAX8997 muic

Best Regards,
Chanwoo Choi

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

[Greg KH]

On Mon, May 21, 2012 at 08:28:34AM +0900, Chanwoo Choi wrote:
> Hi Greg,
> 
> On 05/19/2012 08:59 AM, Greg KH wrote:
> 
> > On Wed, May 16, 2012 at 04:41:44PM +0900, Chanwoo Choi wrote:
> >> From: Thomas Abraham <thomas.abraham@linaro.org>
> >>
> >> Add irq domain support for max8997 interrupts. The reverse mapping method
> >> used is linear mapping since the sub-drivers of max8997 such as regulator
> >> and charger drivers can use the max8997 irq_domain to get the linux irq
> >> number for max8997 interrupts. All uses of irq_base in platform data and
> >> max8997 driver private data are removed.
> >>
> >> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> >> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> >> Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> >> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> >> 	[Fix two bug which set max8997->irq_domain and correct wrong parameter]
> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > 
> > Same problem as before, I apply this and it breaks the build with the
> > following errors:
> > 
> > drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_irq_work’:
> > drivers/extcon/extcon-max8997.c:350:36: error: ‘struct max8997_dev’ has no member named ‘irq_base’
> > drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_probe’:
> > drivers/extcon/extcon-max8997.c:452:35: error: ‘struct max8997_platform_data’ has no member named ‘irq_base’
> > drivers/extcon/extcon-max8997.c: In function ‘max8997_muic_remove’:
> > drivers/extcon/extcon-max8997.c:514:19: error: ‘struct max8997_dev’ has no member named ‘irq_base’
> > 
> > Please fix.
> 
> I posted patchset including below two patch. The second patch fix that
> extcon-max8997 driver use irq_domain instead of irq_base field in
> 'struct max8997_dev'.
> 
> Please apply second patch when test this patchset.

No, you can not break the build at any point in the patch process,
sorry.

Please resend these so the build does not break, but note, you have
missed the merge window for 3.5, as 3.4 is now out (well, will be in a
few hours once kernel.org comes back from the dead...)

So you will have to wait for 3.5-rc1 to be released before I can apply
it.

thanks,

greg k-h