[PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Lee Jones]

The STMPE GPIO driver can be used as an IRQ controller by some
related devices. Here we provide it with its very own IRQ Domain
so that IRQs can be issued dynamically. This will stand the
driver in good stead when it is enabled for Device Tree, as this
it a prerequisite.

Cc: Grant Likely <grant.likely@secretlab.ca>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/gpio/gpio-stmpe.c |   67 +++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index dce3472..5a87ed6 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/stmpe.h>
 
@@ -28,6 +29,7 @@ struct stmpe_gpio {
 	struct stmpe *stmpe;
 	struct device *dev;
 	struct mutex irq_lock;
+	struct irq_domain *domain;
 
 	int irq_base;
 	unsigned norequest_mask;
@@ -267,38 +269,55 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
+int stmpe_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+		       irq_hw_number_t hwirq)
 {
-	int base = stmpe_gpio->irq_base;
-	int irq;
+	struct stmpe_gpio *stmpe_gpio = d->host_data;
+
+	if (!stmpe_gpio)
+		return -EINVAL;
 
-	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
-		irq_set_chip_data(irq, stmpe_gpio);
-		irq_set_chip_and_handler(irq, &stmpe_gpio_irq_chip,
-					 handle_simple_irq);
-		irq_set_nested_thread(irq, 1);
+	irq_set_chip_data(hwirq, stmpe_gpio);
+	irq_set_chip_and_handler(hwirq, &stmpe_gpio_irq_chip,
+				 handle_simple_irq);
+	irq_set_nested_thread(hwirq, 1);
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, IRQF_VALID);
+	set_irq_flags(hwirq, IRQF_VALID);
 #else
-		irq_set_noprobe(irq);
+	irq_set_noprobe(hwirq);
 #endif
-	}
 
 	return 0;
 }
 
-static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio)
+void stmpe_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
 {
-	int base = stmpe_gpio->irq_base;
-	int irq;
-
-	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, 0);
+	set_irq_flags(virq, 0);
 #endif
-		irq_set_chip_and_handler(irq, NULL, NULL);
-		irq_set_chip_data(irq, NULL);
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops stmpe_gpio_irq_simple_ops = {
+	.unmap = stmpe_gpio_irq_unmap,
+	.map = stmpe_gpio_irq_map,
+	.xlate = irq_domain_xlate_twocell,
+};
+
+static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
+{
+	int base = stmpe_gpio->irq_base;
+
+	stmpe_gpio->domain = irq_domain_add_simple(NULL,
+				stmpe_gpio->chip.ngpio, base,
+				&stmpe_gpio_irq_simple_ops, stmpe_gpio);
+	if (!stmpe_gpio->domain) {
+		dev_err(stmpe_gpio->dev, "failed to create irqdomain\n");
+		return -ENOSYS;
 	}
+
+	return 0;
 }
 
 static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
@@ -348,7 +367,7 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
 				IRQF_ONESHOT, "stmpe-gpio", stmpe_gpio);
 		if (ret) {
 			dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
-			goto out_removeirq;
+			goto out_disable;
 		}
 	}
 
@@ -368,9 +387,6 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
 out_freeirq:
 	if (irq >= 0)
 		free_irq(irq, stmpe_gpio);
-out_removeirq:
-	if (irq >= 0)
-		stmpe_gpio_irq_remove(stmpe_gpio);
 out_disable:
 	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
 out_free:
@@ -398,10 +414,9 @@ static int __devexit stmpe_gpio_remove(struct platform_device *pdev)
 
 	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
 
-	if (irq >= 0) {
+	if (irq >= 0)
 		free_irq(irq, stmpe_gpio);
-		stmpe_gpio_irq_remove(stmpe_gpio);
-	}
+
 	platform_set_drvdata(pdev, NULL);
 	kfree(stmpe_gpio);
 
-- 
1.7.9.5

Re: [PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Lee Jones]

On Fri, 23 Nov 2012, Lee Jones wrote:

> The STMPE GPIO driver can be used as an IRQ controller by some
> related devices. Here we provide it with its very own IRQ Domain
> so that IRQs can be issued dynamically. This will stand the
> driver in good stead when it is enabled for Device Tree, as this
> it a prerequisite.
> 
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/gpio/gpio-stmpe.c |   67 +++++++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
> index dce3472..5a87ed6 100644
> --- a/drivers/gpio/gpio-stmpe.c
> +++ b/drivers/gpio/gpio-stmpe.c
> @@ -11,6 +11,7 @@
>  #include <linux/slab.h>
>  #include <linux/gpio.h>
>  #include <linux/irq.h>
> +#include <linux/irqdomain.h>
>  #include <linux/interrupt.h>
>  #include <linux/mfd/stmpe.h>
>  
> @@ -28,6 +29,7 @@ struct stmpe_gpio {
>  	struct stmpe *stmpe;
>  	struct device *dev;
>  	struct mutex irq_lock;
> +	struct irq_domain *domain;
>  
>  	int irq_base;
>  	unsigned norequest_mask;
> @@ -267,38 +269,55 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
> +int stmpe_gpio_irq_map(struct irq_domain *d, unsigned int virq,
> +		       irq_hw_number_t hwirq)
>  {
> -	int base = stmpe_gpio->irq_base;
> -	int irq;
> +	struct stmpe_gpio *stmpe_gpio = d->host_data;
> +
> +	if (!stmpe_gpio)
> +		return -EINVAL;
>  
> -	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
> -		irq_set_chip_data(irq, stmpe_gpio);
> -		irq_set_chip_and_handler(irq, &stmpe_gpio_irq_chip,
> -					 handle_simple_irq);
> -		irq_set_nested_thread(irq, 1);
> +	irq_set_chip_data(hwirq, stmpe_gpio);
> +	irq_set_chip_and_handler(hwirq, &stmpe_gpio_irq_chip,
> +				 handle_simple_irq);
> +	irq_set_nested_thread(hwirq, 1);
>  #ifdef CONFIG_ARM
> -		set_irq_flags(irq, IRQF_VALID);
> +	set_irq_flags(hwirq, IRQF_VALID);
>  #else
> -		irq_set_noprobe(irq);
> +	irq_set_noprobe(hwirq);
>  #endif
> -	}
>  
>  	return 0;
>  }
>  
> -static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio)
> +void stmpe_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
>  {
> -	int base = stmpe_gpio->irq_base;
> -	int irq;
> -
> -	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
>  #ifdef CONFIG_ARM
> -		set_irq_flags(irq, 0);
> +	set_irq_flags(virq, 0);
>  #endif
> -		irq_set_chip_and_handler(irq, NULL, NULL);
> -		irq_set_chip_data(irq, NULL);
> +	irq_set_chip_and_handler(virq, NULL, NULL);
> +	irq_set_chip_data(virq, NULL);
> +}
> +
> +static const struct irq_domain_ops stmpe_gpio_irq_simple_ops = {
> +	.unmap = stmpe_gpio_irq_unmap,
> +	.map = stmpe_gpio_irq_map,
> +	.xlate = irq_domain_xlate_twocell,
> +};
> +
> +static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
> +{
> +	int base = stmpe_gpio->irq_base;
> +
> +	stmpe_gpio->domain = irq_domain_add_simple(NULL,
> +				stmpe_gpio->chip.ngpio, base,
> +				&stmpe_gpio_irq_simple_ops, stmpe_gpio);
> +	if (!stmpe_gpio->domain) {
> +		dev_err(stmpe_gpio->dev, "failed to create irqdomain\n");
> +		return -ENOSYS;
>  	}
> +
> +	return 0;
>  }
>  
>  static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
> @@ -348,7 +367,7 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
>  				IRQF_ONESHOT, "stmpe-gpio", stmpe_gpio);
>  		if (ret) {
>  			dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
> -			goto out_removeirq;
> +			goto out_disable;
>  		}
>  	}
>  
> @@ -368,9 +387,6 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
>  out_freeirq:
>  	if (irq >= 0)
>  		free_irq(irq, stmpe_gpio);
> -out_removeirq:
> -	if (irq >= 0)
> -		stmpe_gpio_irq_remove(stmpe_gpio);
>  out_disable:
>  	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
>  out_free:
> @@ -398,10 +414,9 @@ static int __devexit stmpe_gpio_remove(struct platform_device *pdev)
>  
>  	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
>  
> -	if (irq >= 0) {
> +	if (irq >= 0)
>  		free_irq(irq, stmpe_gpio);
> -		stmpe_gpio_irq_remove(stmpe_gpio);
> -	}
> +
>  	platform_set_drvdata(pdev, NULL);
>  	kfree(stmpe_gpio);
>  
> -- 
> 1.7.9.5

Poke.

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

Re: [PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Linus Walleij]

On Fri, Nov 23, 2012 at 4:19 PM, Lee Jones <lee.jones@linaro.org> wrote:

> The STMPE GPIO driver can be used as an IRQ controller by some
> related devices. Here we provide it with its very own IRQ Domain
> so that IRQs can be issued dynamically. This will stand the
> driver in good stead when it is enabled for Device Tree, as this
> it a prerequisite.
>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

The patch looks bogus since there is no code exercising the added
irqdomain code in this patch, it's basically a no-op in the irq
path, which is wrong for an irqdomain.

It is lacking a irq_create_mapping() in stmpe_gpio_to_irq(),
removal of the irq_base member and usage of
irq_find_mapping() in stmpe_gpio_irq().

Only then will the irqdomain actually be used for what it's
intended for.

So I get the impression that this is one of these "let's
just throw a dummy irqdomain in there so we can use
it with devicetree" kind of things...

Yours,
Linus Walleij

Re: [PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Lee Jones]

On Sat, 01 Dec 2012, Linus Walleij wrote:

> On Fri, Nov 23, 2012 at 4:19 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > The STMPE GPIO driver can be used as an IRQ controller by some
> > related devices. Here we provide it with its very own IRQ Domain
> > so that IRQs can be issued dynamically. This will stand the
> > driver in good stead when it is enabled for Device Tree, as this
> > it a prerequisite.
> >
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> The patch looks bogus since there is no code exercising the added
> irqdomain code in this patch, it's basically a no-op in the irq
> path, which is wrong for an irqdomain.
> 
> It is lacking a irq_create_mapping() in stmpe_gpio_to_irq(),
> removal of the irq_base member and usage of
> irq_find_mapping() in stmpe_gpio_irq().
> 
> Only then will the irqdomain actually be used for what it's
> intended for.
> 
> So I get the impression that this is one of these "let's
> just throw a dummy irqdomain in there so we can use
> it with devicetree" kind of things...

It's not meant to be bogus, as it will be used.

Albeit, we are not uses of the GPIO functionality AFAICT.

I'll look at your comments in a bit and fixup.

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

[PATCH 1/1 v2] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Lee Jones]

Author: Lee Jones <lee.jones@linaro.org>
Date:   Fri Nov 23 12:36:23 2012 +0000

    gpio: Provide the STMPE GPIO driver with its own IRQ Domain
    
    The STMPE GPIO driver can be used as an IRQ controller by some
    related devices. Here we provide it with its very own IRQ Domain
    so that IRQs can be issued dynamically. This will stand the
    driver in good stead when it is enabled for Device Tree, as this
    it a prerequisite.
    
    Cc: Grant Likely <grant.likely@secretlab.ca>
    Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index dce3472..6365cd0 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/stmpe.h>
 
@@ -28,6 +29,7 @@ struct stmpe_gpio {
 	struct stmpe *stmpe;
 	struct device *dev;
 	struct mutex irq_lock;
+	struct irq_domain *domain;
 
 	int irq_base;
 	unsigned norequest_mask;
@@ -103,7 +105,7 @@ static int stmpe_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	struct stmpe_gpio *stmpe_gpio = to_stmpe_gpio(chip);
 
-	return stmpe_gpio->irq_base + offset;
+	return irq_create_mapping(stmpe_gpio->domain, offset);
 }
 
 static int stmpe_gpio_request(struct gpio_chip *chip, unsigned offset)
@@ -132,7 +134,7 @@ static struct gpio_chip template_chip = {
 static int stmpe_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct stmpe_gpio *stmpe_gpio = irq_data_get_irq_chip_data(d);
-	int offset = d->irq - stmpe_gpio->irq_base;
+	int offset = d->hwirq;
 	int regoffset = offset / 8;
 	int mask = 1 << (offset % 8);
 
@@ -199,7 +201,7 @@ static void stmpe_gpio_irq_sync_unlock(struct irq_data *d)
 static void stmpe_gpio_irq_mask(struct irq_data *d)
 {
 	struct stmpe_gpio *stmpe_gpio = irq_data_get_irq_chip_data(d);
-	int offset = d->irq - stmpe_gpio->irq_base;
+	int offset = d->hwirq;
 	int regoffset = offset / 8;
 	int mask = 1 << (offset % 8);
 
@@ -209,7 +211,7 @@ static void stmpe_gpio_irq_mask(struct irq_data *d)
 static void stmpe_gpio_irq_unmask(struct irq_data *d)
 {
 	struct stmpe_gpio *stmpe_gpio = irq_data_get_irq_chip_data(d);
-	int offset = d->irq - stmpe_gpio->irq_base;
+	int offset = d->hwirq;
 	int regoffset = offset / 8;
 	int mask = 1 << (offset % 8);
 
@@ -251,8 +253,9 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 		while (stat) {
 			int bit = __ffs(stat);
 			int line = bank * 8 + bit;
+			int virq = irq_find_mapping(stmpe_gpio->domain, line);
 
-			handle_nested_irq(stmpe_gpio->irq_base + line);
+			handle_nested_irq(virq);
 			stat &= ~(1 << bit);
 		}
 
@@ -267,38 +270,55 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
+int stmpe_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+		       irq_hw_number_t hwirq)
 {
-	int base = stmpe_gpio->irq_base;
-	int irq;
+	struct stmpe_gpio *stmpe_gpio = d->host_data;
+
+	if (!stmpe_gpio)
+		return -EINVAL;
 
-	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
-		irq_set_chip_data(irq, stmpe_gpio);
-		irq_set_chip_and_handler(irq, &stmpe_gpio_irq_chip,
-					 handle_simple_irq);
-		irq_set_nested_thread(irq, 1);
+	irq_set_chip_data(hwirq, stmpe_gpio);
+	irq_set_chip_and_handler(hwirq, &stmpe_gpio_irq_chip,
+				 handle_simple_irq);
+	irq_set_nested_thread(hwirq, 1);
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, IRQF_VALID);
+	set_irq_flags(hwirq, IRQF_VALID);
 #else
-		irq_set_noprobe(irq);
+	irq_set_noprobe(hwirq);
 #endif
-	}
 
 	return 0;
 }
 
-static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio)
+void stmpe_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
 {
-	int base = stmpe_gpio->irq_base;
-	int irq;
-
-	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, 0);
+	set_irq_flags(virq, 0);
 #endif
-		irq_set_chip_and_handler(irq, NULL, NULL);
-		irq_set_chip_data(irq, NULL);
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops stmpe_gpio_irq_simple_ops = {
+	.unmap = stmpe_gpio_irq_unmap,
+	.map = stmpe_gpio_irq_map,
+	.xlate = irq_domain_xlate_twocell,
+};
+
+static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
+{
+	int base = stmpe_gpio->irq_base;
+
+	stmpe_gpio->domain = irq_domain_add_simple(NULL,
+				stmpe_gpio->chip.ngpio, base,
+				&stmpe_gpio_irq_simple_ops, stmpe_gpio);
+	if (!stmpe_gpio->domain) {
+		dev_err(stmpe_gpio->dev, "failed to create irqdomain\n");
+		return -ENOSYS;
 	}
+
+	return 0;
 }
 
 static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
@@ -348,7 +368,7 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
 				IRQF_ONESHOT, "stmpe-gpio", stmpe_gpio);
 		if (ret) {
 			dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
-			goto out_removeirq;
+			goto out_disable;
 		}
 	}
 
@@ -368,9 +388,6 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
 out_freeirq:
 	if (irq >= 0)
 		free_irq(irq, stmpe_gpio);
-out_removeirq:
-	if (irq >= 0)
-		stmpe_gpio_irq_remove(stmpe_gpio);
 out_disable:
 	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
 out_free:
@@ -398,10 +415,9 @@ static int __devexit stmpe_gpio_remove(struct platform_device *pdev)
 
 	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
 
-	if (irq >= 0) {
+	if (irq >= 0)
 		free_irq(irq, stmpe_gpio);
-		stmpe_gpio_irq_remove(stmpe_gpio);
-	}
+
 	platform_set_drvdata(pdev, NULL);
 	kfree(stmpe_gpio);

Re: [PATCH 1/1 v2] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Linus Walleij]

On Mon, Dec 10, 2012 at 11:07 AM, Lee Jones <lee.jones@linaro.org> wrote:

> Author: Lee Jones <lee.jones@linaro.org>
> Date:   Fri Nov 23 12:36:23 2012 +0000
>
>     gpio: Provide the STMPE GPIO driver with its own IRQ Domain
>
>     The STMPE GPIO driver can be used as an IRQ controller by some

What is this funny citing and indentation business here... I stripped
all the whitespace by hand, no big deal, just curious.

Looks like you e.g. copy/pasted from a "git log" command log rather
than from an actual patch.

>     related devices. Here we provide it with its very own IRQ Domain
>     so that IRQs can be issued dynamically. This will stand the
>     driver in good stead when it is enabled for Device Tree, as this
>     it a prerequisite.
>
>     Cc: Grant Likely <grant.likely@secretlab.ca>
>     Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
>     Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied.

I apply this late because not using an irqdomain when we can is
an abomination so I'm taking the risk.

Yours,
Linus Walleij

Re: [PATCH 1/1 v2] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Lee Jones]

On Mon, 10 Dec 2012, Linus Walleij wrote:

> On Mon, Dec 10, 2012 at 11:07 AM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Author: Lee Jones <lee.jones@linaro.org>
> > Date:   Fri Nov 23 12:36:23 2012 +0000
> >
> >     gpio: Provide the STMPE GPIO driver with its own IRQ Domain
> >
> >     The STMPE GPIO driver can be used as an IRQ controller by some
> 
> What is this funny citing and indentation business here... I stripped
> all the whitespace by hand, no big deal, just curious.
> 
> Looks like you e.g. copy/pasted from a "git log" command log rather
> than from an actual patch.

Yeah, for v2 review, I usually use `git show` and copy from "Author:".

> >     related devices. Here we provide it with its very own IRQ Domain
> >     so that IRQs can be issued dynamically. This will stand the
> >     driver in good stead when it is enabled for Device Tree, as this
> >     it a prerequisite.
> >
> >     Cc: Grant Likely <grant.likely@secretlab.ca>
> >     Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> >     Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Applied.

Thanks dude.

> I apply this late because not using an irqdomain when we can is
> an abomination so I'm taking the risk.

NP.

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

Re: [PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Viresh Kumar]

On 23 November 2012 18:14, Lee Jones <lee.jones@linaro.org> wrote:
> The STMPE GPIO driver can be used as an IRQ controller by some
> related devices. Here we provide it with its very own IRQ Domain
> so that IRQs can be issued dynamically. This will stand the
> driver in good stead when it is enabled for Device Tree, as this
> it a prerequisite.
>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c

> +const struct irq_domain_ops stmpe_gpio_irq_simple_ops = {

static?

> +       .unmap = stmpe_gpio_irq_unmap,
> +       .map = stmpe_gpio_irq_map,
> +       .xlate = irq_domain_xlate_twocell,
> +};


Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

[PATCH 1/1] gpio: Provide the STMPE GPIO driver with its own IRQ Domain

[Lee Jones]

The STMPE GPIO driver can be used as an IRQ controller by some
related devices. Here we provide it with its very own IRQ Domain
so that IRQs can be issued dynamically. This will stand the
driver in good stead when it is enabled for Device Tree, as this
it a prerequisite.

Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/gpio/gpio-stmpe.c |   67 +++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index dce3472..0e0471b 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/stmpe.h>
 
@@ -28,6 +29,7 @@ struct stmpe_gpio {
 	struct stmpe *stmpe;
 	struct device *dev;
 	struct mutex irq_lock;
+	struct irq_domain *domain;
 
 	int irq_base;
 	unsigned norequest_mask;
@@ -267,38 +269,55 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
+int stmpe_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+		       irq_hw_number_t hwirq)
 {
-	int base = stmpe_gpio->irq_base;
-	int irq;
+	struct stmpe_gpio *stmpe_gpio = d->host_data;
+
+	if (!stmpe_gpio)
+		return -EINVAL;
 
-	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
-		irq_set_chip_data(irq, stmpe_gpio);
-		irq_set_chip_and_handler(irq, &stmpe_gpio_irq_chip,
-					 handle_simple_irq);
-		irq_set_nested_thread(irq, 1);
+	irq_set_chip_data(hwirq, stmpe_gpio);
+	irq_set_chip_and_handler(hwirq, &stmpe_gpio_irq_chip,
+				 handle_simple_irq);
+	irq_set_nested_thread(hwirq, 1);
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, IRQF_VALID);
+	set_irq_flags(hwirq, IRQF_VALID);
 #else
-		irq_set_noprobe(irq);
+	irq_set_noprobe(hwirq);
 #endif
-	}
 
 	return 0;
 }
 
-static void stmpe_gpio_irq_remove(struct stmpe_gpio *stmpe_gpio)
+void stmpe_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
 {
-	int base = stmpe_gpio->irq_base;
-	int irq;
-
-	for (irq = base; irq < base + stmpe_gpio->chip.ngpio; irq++) {
 #ifdef CONFIG_ARM
-		set_irq_flags(irq, 0);
+	set_irq_flags(virq, 0);
 #endif
-		irq_set_chip_and_handler(irq, NULL, NULL);
-		irq_set_chip_data(irq, NULL);
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+const struct irq_domain_ops stmpe_gpio_irq_simple_ops = {
+	.unmap = stmpe_gpio_irq_unmap,
+	.map = stmpe_gpio_irq_map,
+	.xlate = irq_domain_xlate_twocell,
+};
+
+static int __devinit stmpe_gpio_irq_init(struct stmpe_gpio *stmpe_gpio)
+{
+	int base = stmpe_gpio->irq_base;
+
+	stmpe_gpio->domain = irq_domain_add_simple(NULL,
+				stmpe_gpio->chip.ngpio, base,
+				&stmpe_gpio_irq_simple_ops, stmpe_gpio);
+	if (!stmpe_gpio->domain) {
+		dev_err(stmpe_gpio->dev, "failed to create irqdomain\n");
+		return -ENOSYS;
 	}
+
+	return 0;
 }
 
 static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
@@ -348,7 +367,7 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
 				IRQF_ONESHOT, "stmpe-gpio", stmpe_gpio);
 		if (ret) {
 			dev_err(&pdev->dev, "unable to get irq: %d\n", ret);
-			goto out_removeirq;
+			goto out_disable;
 		}
 	}
 
@@ -368,9 +387,6 @@ static int __devinit stmpe_gpio_probe(struct platform_device *pdev)
 out_freeirq:
 	if (irq >= 0)
 		free_irq(irq, stmpe_gpio);
-out_removeirq:
-	if (irq >= 0)
-		stmpe_gpio_irq_remove(stmpe_gpio);
 out_disable:
 	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
 out_free:
@@ -398,10 +414,9 @@ static int __devexit stmpe_gpio_remove(struct platform_device *pdev)
 
 	stmpe_disable(stmpe, STMPE_BLOCK_GPIO);
 
-	if (irq >= 0) {
+	if (irq >= 0)
 		free_irq(irq, stmpe_gpio);
-		stmpe_gpio_irq_remove(stmpe_gpio);
-	}
+
 	platform_set_drvdata(pdev, NULL);
 	kfree(stmpe_gpio);
 
-- 
1.7.9.5