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

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

[Ahmad Fatoum]

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.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
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?
---
 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

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Thomas Gleixner]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> 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.

handle_nested_irq() documentation clearly says: "Handle a nested irq
from a irq thread". As a consequence the caller of this function must
run in an interrupt thread. This is an optimization to spare tons of
interrupt threads and context switches.

So the solution for this driver is either to make the dispatch handler
threaded or use the hard interrupt variant of dispatching the
demultiplexed GPIO interrupts.

Thanks,

        tglx

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

[Linus Walleij]

On Thu, Aug 6, 2020 at 12:20 PM Thomas Gleixner <tglx@linutronix.de> wrote:

> So the solution for this driver is either to make the dispatch handler
> threaded or use the hard interrupt variant of dispatching the
> demultiplexed GPIO interrupts.

The struct gpio_irq_chip .threaded bool that the patch
sets just instructs the gpio core to issue
irq_set_nested_thread(irq, 1) on the child IRQ.

This is a driver of type "struct siox_driver" handling the
IRQ through the special .get_data callback supplied in the
driver struct and it calls handle_nested_irq(irq) so with
this fix it percolated up to the parent as intended.

So far so good. So I think the patch should be applied.

But what is behind this .get_data() callback for siox drivers?

The siox driver framework in drivers/siox dispatches calls
to .get_data() from a polling thread which is just some ordinary
kthread. It looks like this because the SIOX (I think) needs
to do polled I/O. (drivers/siox/siox-core.c)

So this is a thread but it is not an irq thread from the irq core,
however it is treated like such by the driver, and in a way what
happens is events, just polled by a thread.

So when we call handle_nested_irq() ... we are not really
calling that from an irq handler.

I am just very confused :D

But Uwe must have designed this thread to mimic IRQs
specifically? (Uwe?)

I don't know if the IRQ core even sees a difference between which
thread it gets interfaced with. I suppose it does? :/

Yours,
Linus Walleij

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

[Thomas Gleixner]

Linus Walleij <linus.walleij@linaro.org> writes:
> On Thu, Aug 6, 2020 at 12:20 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
>> So the solution for this driver is either to make the dispatch handler
>> threaded or use the hard interrupt variant of dispatching the
>> demultiplexed GPIO interrupts.
>
> The struct gpio_irq_chip .threaded bool that the patch
> sets just instructs the gpio core to issue
> irq_set_nested_thread(irq, 1) on the child IRQ.
>
> This is a driver of type "struct siox_driver" handling the
> IRQ through the special .get_data callback supplied in the
> driver struct and it calls handle_nested_irq(irq) so with
> this fix it percolated up to the parent as intended.
>
> So far so good. So I think the patch should be applied.
>
> But what is behind this .get_data() callback for siox drivers?
>
> The siox driver framework in drivers/siox dispatches calls
> to .get_data() from a polling thread which is just some ordinary
> kthread. It looks like this because the SIOX (I think) needs
> to do polled I/O. (drivers/siox/siox-core.c)
>
> So this is a thread but it is not an irq thread from the irq core,
> however it is treated like such by the driver, and in a way what
> happens is events, just polled by a thread.

As Uwe just explained.

> So when we call handle_nested_irq() ... we are not really
> calling that from an irq handler.
>
> I don't know if the IRQ core even sees a difference between which
> thread it gets interfaced with. I suppose it does? :/

handle_nested_irq() does not care. It cares about thread context,
external reentrancy protection for the same nested interrupt and that
the nested interrupt has a thread handler.

The latter is what goes belly up because w/o that threaded bit set the
GPIO core fails to set nested thread. So if a consumer requests an
interrupt with request_any_context_irq() then that fails to select
thread mode which means the threaded handler is not set causing
handle_nested_irq() to fail.

The polling kthread is a slight but clever abomination, but it just
works because it provides thread context and cannot run concurrently.

So Ahmad's patch is correct, just the changelog needs polishing.

Thanks,

        tglx

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3533 bytes --]

On Thu, Aug 06, 2020 at 08:50:45PM +0200, Thomas Gleixner wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
> > On Thu, Aug 6, 2020 at 12:20 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> >> So the solution for this driver is either to make the dispatch handler
> >> threaded or use the hard interrupt variant of dispatching the
> >> demultiplexed GPIO interrupts.
> >
> > The struct gpio_irq_chip .threaded bool that the patch
> > sets just instructs the gpio core to issue
> > irq_set_nested_thread(irq, 1) on the child IRQ.
> >
> > This is a driver of type "struct siox_driver" handling the
> > IRQ through the special .get_data callback supplied in the
> > driver struct and it calls handle_nested_irq(irq) so with
> > this fix it percolated up to the parent as intended.
> >
> > So far so good. So I think the patch should be applied.
> >
> > But what is behind this .get_data() callback for siox drivers?
> >
> > The siox driver framework in drivers/siox dispatches calls
> > to .get_data() from a polling thread which is just some ordinary
> > kthread. It looks like this because the SIOX (I think) needs
> > to do polled I/O. (drivers/siox/siox-core.c)
> >
> > So this is a thread but it is not an irq thread from the irq core,
> > however it is treated like such by the driver, and in a way what
> > happens is events, just polled by a thread.
> 
> As Uwe just explained.
> 
> > So when we call handle_nested_irq() ... we are not really
> > calling that from an irq handler.
> >
> > I don't know if the IRQ core even sees a difference between which
> > thread it gets interfaced with. I suppose it does? :/
> 
> handle_nested_irq() does not care. It cares about thread context,
> external reentrancy protection for the same nested interrupt and that
> the nested interrupt has a thread handler.
> 
> The latter is what goes belly up because w/o that threaded bit set the
> GPIO core fails to set nested thread. So if a consumer requests an
> interrupt with request_any_context_irq() then that fails to select
> thread mode which means the threaded handler is not set causing
> handle_nested_irq() to fail.

For a caller of request_threaded_irq() that passes a relevant hardirq
handler the hardirq handler is never called but request_threaded_irq()
doesn't fail. The handler is just replaced by irq_nested_primary_handler
in __setup_irq(). Is that a bug? (I didn't test, just read the code, so I
might have missed something.) 

> The polling kthread is a slight but clever abomination, but it just
> works because it provides thread context and cannot run concurrently.

I think this is the first time you called any of my code "clever" :-)
 
> So Ahmad's patch is correct, just the changelog needs polishing.

Trying to be constructive, here is my suggested changelog:

	gpio: siox: explicitly only support threaded irqs

	The gpio-siox driver uses handle_nested_irq() to implement its
	interrupt support. This is only capable to handle threaded irq
	actions. For a hardirq action it triggers a NULL pointer oops.
	(It calls action->thread_fn which is NULL then.)

	So prevent registration of a hardirq action by setting
	gpio_irq_chip::threaded to true.

Does this address all your concerns?

Is this bad enough to justify sending this patch to stable?

Best regards
Uwe

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Thomas Gleixner]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> On Thu, Aug 06, 2020 at 08:50:45PM +0200, Thomas Gleixner wrote:
>> handle_nested_irq() does not care. It cares about thread context,
>> external reentrancy protection for the same nested interrupt and that
>> the nested interrupt has a thread handler.
>> 
>> The latter is what goes belly up because w/o that threaded bit set the
>> GPIO core fails to set nested thread. So if a consumer requests an
>> interrupt with request_any_context_irq() then that fails to select
>> thread mode which means the threaded handler is not set causing
>> handle_nested_irq() to fail.
>
> For a caller of request_threaded_irq() that passes a relevant hardirq
> handler the hardirq handler is never called but request_threaded_irq()
> doesn't fail. The handler is just replaced by irq_nested_primary_handler
> in __setup_irq(). Is that a bug? (I didn't test, just read the code, so I
> might have missed something.)

Depends on what the threaded handler expects what the primary handler
has done. It might just work or not :)

> Trying to be constructive, here is my suggested changelog:
>
> 	gpio: siox: explicitly only support threaded irqs
>
> 	The gpio-siox driver uses handle_nested_irq() to implement its
> 	interrupt support. This is only capable to handle threaded irq
> 	actions. For a hardirq action it triggers a NULL pointer oops.
> 	(It calls action->thread_fn which is NULL then.)
>
> 	So prevent registration of a hardirq action by setting
> 	gpio_irq_chip::threaded to true.
>
> Does this address all your concerns?

LGTM

> Is this bad enough to justify sending this patch to stable?

Yes, a Cc: stable and a Fixes: tag is justified.

Thanks,

        tglx

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2678 bytes --]

Hello Thomas,

On Thu, Aug 06, 2020 at 10:33:06PM +0200, Thomas Gleixner wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > On Thu, Aug 06, 2020 at 08:50:45PM +0200, Thomas Gleixner wrote:
> >> handle_nested_irq() does not care. It cares about thread context,
> >> external reentrancy protection for the same nested interrupt and that
> >> the nested interrupt has a thread handler.
> >> 
> >> The latter is what goes belly up because w/o that threaded bit set the
> >> GPIO core fails to set nested thread. So if a consumer requests an
> >> interrupt with request_any_context_irq() then that fails to select
> >> thread mode which means the threaded handler is not set causing
> >> handle_nested_irq() to fail.
> >
> > For a caller of request_threaded_irq() that passes a relevant hardirq
> > handler the hardirq handler is never called but request_threaded_irq()
> > doesn't fail. The handler is just replaced by irq_nested_primary_handler
> > in __setup_irq(). Is that a bug? (I didn't test, just read the code, so I
> > might have missed something.)
> 
> Depends on what the threaded handler expects what the primary handler
> has done. It might just work or not :)

So we need something like:

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 48c38e09c673..31777a0b79df 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1393,12 +1393,18 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 			ret = -EINVAL;
 			goto out_mput;
 		}
-		/*
-		 * Replace the primary handler which was provided from
-		 * the driver for non nested interrupt handling by the
-		 * dummy function which warns when called.
-		 */
-		new->handler = irq_nested_primary_handler;
+
+		if (new->handler == NULL) {
+			/* Scream loud if the primary handler gets called */
+			new->handler = irq_nested_primary_handler;
+		} else {
+			/*
+			 * The handler won't be called as the requestor expects,
+			 * so refuse to install the handler
+			 */
+			ret = -EINVAL;
+			goto out_mput;
+		}
 	} else {
 		if (irq_settings_can_thread(desc)) {
 			ret = irq_setup_forced_threading(new);

? Do we need to care for other allowed values of new->handler? Maybe
irq_default_primary_handler?

> > Is this bad enough to justify sending this patch to stable?
> 
> Yes, a Cc: stable and a Fixes: tag is justified.

That would be

Fixes: be8c8facc707 ("gpio: new driver to work with a 8x12 siox")

Best regards
Uwe

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3358 bytes --]

On Thu, Aug 06, 2020 at 11:07:09PM +0200, Uwe Kleine-König wrote:
> Hello Thomas,
> 
> On Thu, Aug 06, 2020 at 10:33:06PM +0200, Thomas Gleixner wrote:
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > > On Thu, Aug 06, 2020 at 08:50:45PM +0200, Thomas Gleixner wrote:
> > >> handle_nested_irq() does not care. It cares about thread context,
> > >> external reentrancy protection for the same nested interrupt and that
> > >> the nested interrupt has a thread handler.
> > >> 
> > >> The latter is what goes belly up because w/o that threaded bit set the
> > >> GPIO core fails to set nested thread. So if a consumer requests an
> > >> interrupt with request_any_context_irq() then that fails to select
> > >> thread mode which means the threaded handler is not set causing
> > >> handle_nested_irq() to fail.
> > >
> > > For a caller of request_threaded_irq() that passes a relevant hardirq
> > > handler the hardirq handler is never called but request_threaded_irq()
> > > doesn't fail. The handler is just replaced by irq_nested_primary_handler
> > > in __setup_irq(). Is that a bug? (I didn't test, just read the code, so I
> > > might have missed something.)
> > 
> > Depends on what the threaded handler expects what the primary handler
> > has done. It might just work or not :)
> 
> So we need something like:
> 
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 48c38e09c673..31777a0b79df 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1393,12 +1393,18 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
>  			ret = -EINVAL;
>  			goto out_mput;
>  		}
> -		/*
> -		 * Replace the primary handler which was provided from
> -		 * the driver for non nested interrupt handling by the
> -		 * dummy function which warns when called.
> -		 */
> -		new->handler = irq_nested_primary_handler;
> +
> +		if (new->handler == NULL) {
> +			/* Scream loud if the primary handler gets called */
> +			new->handler = irq_nested_primary_handler;
> +		} else {
> +			/*
> +			 * The handler won't be called as the requestor expects,
> +			 * so refuse to install the handler
> +			 */
> +			ret = -EINVAL;
> +			goto out_mput;
> +		}
>  	} else {
>  		if (irq_settings_can_thread(desc)) {
>  			ret = irq_setup_forced_threading(new);
> 

The siox stuff is used at Eckelmann (i.e. probably the only siox user)
via /dev/gpiochip%d. The code providing this device uses
request_threaded_irq(), so that's why we didn't run into the oops. That
the primary handler might not run was noticed already and cared for in
commit 1033be58992f ("gpiolib: fix line event timestamps for nested
irqs").

I grepped around a bit and I think most other drivers depend on their
primary handler being called. (Some primary handlers disable and/or mask
the irq[1], this is wrong, isn't it?)

So I really think request_threaded_irq should not silently rop the
primary handler on the floor.

Best regards
Uwe

[1] I saw:
 - arch/mips/alchemy/devboards/db1200.c
 - drivers/crypto/hisilicon/sec/sec_drv.c
 - drivers/crypto/stm32/stm32-hash.c
 - drivers/dma/idxd/init.c



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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]

On Thu, Aug 06, 2020 at 12:20:52PM +0200, Thomas Gleixner wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > 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.
> 
> handle_nested_irq() documentation clearly says: "Handle a nested irq
> from a irq thread". As a consequence the caller of this function must
> run in an interrupt thread. This is an optimization to spare tons of
> interrupt threads and context switches.
> 
> So the solution for this driver is either to make the dispatch handler
> threaded or use the hard interrupt variant of dispatching the
> demultiplexed GPIO interrupts.

The action item isn't entirely clear for me. There is no "parent" irq, I
have for siox a kthread that does some IO and looks that the resulting
data which effectively reports the current state of the GPIO line. And
if this GPIO is configured to trigger an irq and the matching transition
(or state) is seen, I want to trigger the irq action. So the caller has
neither hard nor threaded irq context.

Best regards
Uwe

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Linus Walleij]

Hi Ahmad,

On Tue, Aug 4, 2020 at 11:18 AM Ahmad Fatoum <a.fatoum@pengutronix.de> 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.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

I think we concluded we want to apply this patch, but with
a fixed commit text, can you send a V2? (Or ask Uwe if he wants
to pick it up and write the text.)

Yours,
Linus Walleij

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

[Ahmad Fatoum]

Hello Linus,

On 8/28/20 12:55 AM, Linus Walleij wrote:
> Hi Ahmad,
> 
> On Tue, Aug 4, 2020 at 11:18 AM Ahmad Fatoum <a.fatoum@pengutronix.de> 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.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> I think we concluded we want to apply this patch, but with
> a fixed commit text, can you send a V2? (Or ask Uwe if he wants
> to pick it up and write the text.)

Thanks for the reminder. This slipped through.
I just sent out v2 (<20200907153135.3307-1-a.fatoum@pengutronix.de>)

Cheers,
Ahmad

> 
> Yours,
> Linus Walleij
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |