Re: [PATCH] gpio: siox: indicate exclusive support of threaded IRQs

[Uwe Kleine-König]

Hello,

[adding tglx for irq expertise to Cc]

On Tue, Aug 04, 2020 at 11:16:03AM +0200, Ahmad Fatoum wrote:
> Generic GPIO consumers like gpio-keys use request_any_context_irq()
> to request a threaded handler if irq_settings_is_nested_thread() ==
> true or a hardirq handler otherwise.
> 
> Drivers using handle_nested_irq() must be sure that the nested
> IRQs were requested with threaded handlers, because the IRQ
> is handled by calling action->thread_fn().
> 
> The gpio-siox driver dispatches IRQs via handle_nested_irq,
> but has irq_settings_is_nested_thread() == false.
> 
> Set gpio_irq_chip::threaded to remedy this.

Sounds reasonable, but I have to keep this for others to judge if this
is indeed how the irq stuff works.

> ---
> I am writing a driver similar to gpio-siox and I ran into a null pointer
> dereference, because ->threaded wasn't set. I didn't test this on actual
> SIOX hardware.
> 
> This patch doesn't fix the case were are driver explicitly calls
> request_irq and is combined with a driver that does handle_nested_irq.
> 
> Is there a flag, such drivers should additionally set or should we
> check action->thread_fn before calling it inside handle_nested_irq?

If gpio_irq_chip::threaded being set means that this problem happens
IMHO the driver is fine and something in the generic gpio code should do
the right thing here.

Best regards and thanks for caring,
Uwe

> ---
>  drivers/gpio/gpio-siox.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-siox.c b/drivers/gpio/gpio-siox.c
> index 26e1fe092304..f8c5e9fc4bac 100644
> --- a/drivers/gpio/gpio-siox.c
> +++ b/drivers/gpio/gpio-siox.c
> @@ -245,6 +245,7 @@ static int gpio_siox_probe(struct siox_device *sdevice)
>  	girq->chip = &ddata->ichip;
>  	girq->default_type = IRQ_TYPE_NONE;
>  	girq->handler = handle_level_irq;
> +	girq->threaded = true;
>  
>  	ret = devm_gpiochip_add_data(dev, &ddata->gchip, NULL);
>  	if (ret)
> -- 
> 2.28.0
> 
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature