Re: [PATCH] gpio: set parent irq on chained handlers

[Grygorii Strashko <grygorii.strashko@xxxxxx>]

On 09/26/2014 02:52 PM, Linus Walleij wrote:
> If the IRQ from the parent is nested the IRQ may need to be
> resent under certain conditions. Currently the chained IRQ
> handler in gpiolib does not handle connecting nested IRQs

Seems there is still some misunderstanding ( - chained vs nested IRQs

> but it is conceptually correct to indicate the actual parent
> IRQ.
> 
> Reported-by: Grygorii Strashko <grygorii.strashko@xxxxxx>

+ Reported-by: Lothar Waßmann <LW@xxxxxxxxxxxxxxxxxxx>

> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
>   drivers/gpio/gpiolib.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 550e575c6ffb..9362b5b817af 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -398,17 +398,30 @@ void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip,
>   				  int parent_irq,
>   				  irq_flow_handler_t parent_handler)
>   {
> +	unsigned int offset;
> +
>   	if (gpiochip->can_sleep) {
>   		chip_err(gpiochip, "you cannot have chained interrupts on a chip that may sleep\n");
>   		return;
>   	}


This function can't be called by gpio-pca953x.c driver  (

> +	if (!gpiochip->irqdomain) {
> +		chip_err(gpiochip, "called %s before setting up irqchip\n",
> +			 __func__);
> +		return;
> +	}
>   
>   	irq_set_chained_handler(parent_irq, parent_handler);
> +
>   	/*
>   	 * The parent irqchip is already using the chip_data for this
>   	 * irqchip, so our callbacks simply use the handler_data.
>   	 */
>   	irq_set_handler_data(parent_irq, gpiochip);
> +
> +	/* Set the parent IRQ for all affected IRQs */
> +	for (offset = 0; offset < gpiochip->ngpio; offset++)
> +		irq_set_parent(irq_find_mapping(gpiochip->irqdomain, offset),
> +			       parent_irq);
>   }
>   EXPORT_SYMBOL_GPL(gpiochip_set_chained_irqchip);
>   
> 

May be some simple helper can be added instead:
 void gpiochip_set_nested_irqchip(struct gpio_chip *gpiochip,
   				  int parent_irq)
  {
 	unsigned int offset;
 
   	if (!gpiochip->can_sleep && !chip->irq_not_threaded) {
   		chip_err(gpiochip, "you cannot have nested interrupts on a chip that can't sleep\n");
   		return;
   	}

	if (!gpiochip->irqdomain) {
		chip_err(gpiochip, "called %s before setting up irqchip\n",
			 __func__);
		return;
	}
 

	/* Set the parent IRQ for all affected IRQs */
	for (offset = 0; offset < gpiochip->ngpio; offset++)
		irq_set_parent(irq_find_mapping(gpiochip->irqdomain, offset),
			       parent_irq);
 }
 EXPORT_SYMBOL_GPL(gpiochip_set_nested_irqchip);
 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html