[PATCH] gpio: unmap gpio irqs properly

[PATCH] gpio: unmap gpio irqs properly

[Linus Walleij]

When using the irqchip helper inside the gpiolib, make sure
the IRQs are unmapped/disposed before the irqdomain is removed
as part of removing the gpiochip.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c      | 33 +++++++++++++++++++++++++++++----
 include/linux/gpio/driver.h |  1 +
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f41cb4f3d715..761013f8b82f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1399,8 +1399,18 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
 	return 0;
 }
 
+static void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq)
+{
+#ifdef CONFIG_ARM
+	set_irq_flags(irq, 0);
+#endif
+	irq_set_chip_and_handler(irq, NULL, NULL);
+	irq_set_chip_data(irq, NULL);
+}
+
 static const struct irq_domain_ops gpiochip_domain_ops = {
 	.map	= gpiochip_irq_map,
+	.unmap	= gpiochip_irq_unmap,
 	/* Virtually all GPIO irqchips are twocell:ed */
 	.xlate	= irq_domain_xlate_twocell,
 };
@@ -1438,8 +1448,14 @@ static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
  */
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
 {
-	if (gpiochip->irqdomain)
+	unsigned int offset;
+
+	/* Remove all IRQ mappings and delete the domain */
+	if (gpiochip->irqdomain) {
+		for (offset = 0; offset < gpiochip->ngpio; offset++)
+			irq_dispose_mapping(gpiochip->irq_base + offset);
 		irq_domain_remove(gpiochip->irqdomain);
+	}
 
 	if (gpiochip->irqchip) {
 		gpiochip->irqchip->irq_request_resources = NULL;
@@ -1467,7 +1483,8 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
  * translation. The gpiochip will need to be initialized and registered
  * before calling this function.
  *
- * This function will handle two cell:ed simple IRQs. Everything else
+ * This function will handle two cell:ed simple IRQs and assumes all
+ * the pins on the gpiochip can generate a unique IRQ. Everything else
  * need to be open coded.
  */
 int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
@@ -1478,6 +1495,7 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
 {
 	struct device_node *of_node;
 	unsigned int offset;
+	unsigned irq_base = 0;
 
 	if (!gpiochip || !irqchip)
 		return -EINVAL;
@@ -1514,8 +1532,15 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
 	 * any gpiochip calls. If the first_irq was zero, this is
 	 * necessary to allocate descriptors for all IRQs.
 	 */
-	for (offset = 0; offset < gpiochip->ngpio; offset++)
-		irq_create_mapping(gpiochip->irqdomain, offset);
+	for (offset = 0; offset < gpiochip->ngpio; offset++) {
+		irq_base = irq_create_mapping(gpiochip->irqdomain, offset);
+		if (offset == 0)
+			/*
+			 * Store the base into the gpiochip to be used when
+			 * unmapping the irqs.
+			 */
+			gpiochip->irq_base = irq_base;
+	}
 
 	return 0;
 }
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index c1c5c2368fc8..1827b43966d9 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -107,6 +107,7 @@ struct gpio_chip {
 	 */
 	struct irq_chip		*irqchip;
 	struct irq_domain	*irqdomain;
+	unsigned int		irq_base;
 	irq_flow_handler_t	irq_handler;
 	unsigned int		irq_default_type;
 #endif
-- 
1.8.5.3

Re: [PATCH] gpio: unmap gpio irqs properly

[Alexandre Courbot]

On Sat, Mar 29, 2014 at 4:44 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> When using the irqchip helper inside the gpiolib, make sure
> the IRQs are unmapped/disposed before the irqdomain is removed
> as part of removing the gpiochip.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpio/gpiolib.c      | 33 +++++++++++++++++++++++++++++----
>  include/linux/gpio/driver.h |  1 +
>  2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index f41cb4f3d715..761013f8b82f 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1399,8 +1399,18 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
>         return 0;
>  }
>
> +static void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq)
> +{
> +#ifdef CONFIG_ARM
> +       set_irq_flags(irq, 0);
> +#endif

Just curious here, why is a special case needed for ARM? I would not
expect architecture-specific code on gpiolib, but I'm sure you have
your reasons for that.

Re: [PATCH] gpio: unmap gpio irqs properly

[Linus Walleij]

On Sat, Mar 29, 2014 at 5:27 PM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Sat, Mar 29, 2014 at 4:44 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> +static void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq)
>> +{
>> +#ifdef CONFIG_ARM
>> +       set_irq_flags(irq, 0);
>> +#endif
>
> Just curious here, why is a special case needed for ARM? I would not
> expect architecture-specific code on gpiolib, but I'm sure you have
> your reasons for that.

This is open-coded like this in all ARM-applicable drivers so I'm
just refactoring really.

I don't really know why but I was under the impression that this
ARM-specific function exists because the IRQ implementations in
the different archs have never really converged.

Yours,
Linus Walleij