request_irq() usage in wm8350_register_irq().

request_irq() usage in wm8350_register_irq().

[Sebastian Andrzej Siewior]

Hi,

I've been staring wm8350_register_irq(). It does

| request_threaded_irq(irq + wm8350->irq_base, NULL,
| 		handler, flags, name, data);

and every single user passes 0 as flags. This means it asks for a
threaded IRQ and does not pass IRQF_ONESHOT. 

So either this is not working because it triggers the warnings
	Threaded irq requested with handler=NULL and !ONESHOT

followed by -EINVAL _or_ every single user of this driver sits in
system where the irqchip is IRQCHIP_ONESHOT_SAFE marked.
Which is it?

Sebastian

Re: request_irq() usage in wm8350_register_irq().

[Richard Fitzgerald]

On 21/01/2026 11:16 am, Sebastian Andrzej Siewior wrote:
> Hi,
> 
> I've been staring wm8350_register_irq(). It does
> 
> | request_threaded_irq(irq + wm8350->irq_base, NULL,
> | 		handler, flags, name, data);
> 
> and every single user passes 0 as flags. This means it asks for a
> threaded IRQ and does not pass IRQF_ONESHOT.
> 
> So either this is not working because it triggers the warnings
> 	Threaded irq requested with handler=NULL and !ONESHOT
> 
> followed by -EINVAL _or_ every single user of this driver sits in
> system where the irqchip is IRQCHIP_ONESHOT_SAFE marked.
> Which is it?
> 
> Sebastian

Or because request_threaded_irq() supplies a default handler, so
handler!=NULL when it calls __setup_irq().


int request_threaded_irq(unsigned int irq, irq_handler_t handler,
			 irq_handler_t thread_fn, unsigned long irqflags,
			 const char *devname, void *dev_id)
{

...

	if (!handler) {
		if (!thread_fn)
			return -EINVAL;
		handler = irq_default_primary_handler;
	}

...


	retval = __setup_irq(irq, desc, action);

...
}

Re: request_irq() usage in wm8350_register_irq().

[Sebastian Andrzej Siewior]

On 2026-01-21 11:22:32 [+0000], Richard Fitzgerald wrote:
> Or because request_threaded_irq() supplies a default handler, so
> handler!=NULL when it calls __setup_irq().
> 
> 
> int request_threaded_irq(unsigned int irq, irq_handler_t handler,
> 			 irq_handler_t thread_fn, unsigned long irqflags,
> 			 const char *devname, void *dev_id)
> {
> 
> ...
> 
> 	if (!handler) {
> 		if (!thread_fn)
> 			return -EINVAL;
> 		handler = irq_default_primary_handler;
> 	}
> 
> ...
> 
> 
> 	retval = __setup_irq(irq, desc, action);
> 
> ...
> }

->

|         } else if (new->handler == irq_default_primary_handler &&
|                    !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)) {
…
|                 pr_err("Threaded irq requested with handler=NULL and !ONESHOT for %s (irq %d)\n",
|                        new->name, irq);
|                 ret = -EINVAL;

Sebastian

Re: request_irq() usage in wm8350_register_irq().

[Richard Fitzgerald]

On 21/01/2026 11:36 am, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 11:22:32 [+0000], Richard Fitzgerald wrote:
>> Or because request_threaded_irq() supplies a default handler, so
>> handler!=NULL when it calls __setup_irq().
>>
>>
>> int request_threaded_irq(unsigned int irq, irq_handler_t handler,
>> 			 irq_handler_t thread_fn, unsigned long irqflags,
>> 			 const char *devname, void *dev_id)
>> {
>>
>> ...
>>
>> 	if (!handler) {
>> 		if (!thread_fn)
>> 			return -EINVAL;
>> 		handler = irq_default_primary_handler;
>> 	}
>>
>> ...
>>
>>
>> 	retval = __setup_irq(irq, desc, action);
>>
>> ...
>> }
> 
> ->
> 
> |         } else if (new->handler == irq_default_primary_handler &&
> |                    !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)) {
> …
> |                 pr_err("Threaded irq requested with handler=NULL and !ONESHOT for %s (irq %d)\n",
> |                        new->name, irq);
> |                 ret = -EINVAL;
> 
> Sebastian

Ah. I didn't notice that. Confusing error message. It says
"handler=NULL" but handler != NULL. More like "handler=default".

Re: request_irq() usage in wm8350_register_irq().

[Sebastian Andrzej Siewior]

On 2026-01-21 11:53:28 [+0000], Richard Fitzgerald wrote:
> Ah. I didn't notice that. Confusing error message. It says
> "handler=NULL" but handler != NULL. More like "handler=default".

The supplied argument was NULL.
Anyway. Do you happen to have an answer to my original question.

Sebastian

Re: request_irq() usage in wm8350_register_irq().

[Andy Shevchenko]

On Wed, Jan 21, 2026 at 12:57:23PM +0100, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 11:53:28 [+0000], Richard Fitzgerald wrote:
> > Ah. I didn't notice that. Confusing error message. It says
> > "handler=NULL" but handler != NULL. More like "handler=default".
> 
> The supplied argument was NULL.
> Anyway. Do you happen to have an answer to my original question.

Why not asking Mark? I think he might know more about this HW.

Btw, Mark, I noticed that your old addresses appear in Git history,
are they working? Or do you want to add them into .mailmap?

-- 
With Best Regards,
Andy Shevchenko

Re: request_irq() usage in wm8350_register_irq().

[Mark Brown]

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

On Wed, Jan 21, 2026 at 02:43:43PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 21, 2026 at 12:57:23PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2026-01-21 11:53:28 [+0000], Richard Fitzgerald wrote:
> > > Ah. I didn't notice that. Confusing error message. It says
> > > "handler=NULL" but handler != NULL. More like "handler=default".

> > The supplied argument was NULL.
> > Anyway. Do you happen to have an answer to my original question.

> Why not asking Mark? I think he might know more about this HW.

What was the question?

> Btw, Mark, I noticed that your old addresses appear in Git history,
> are they working? Or do you want to add them into .mailmap?

They'll be broken but I'm not sure I care.

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

Re: request_irq() usage in wm8350_register_irq().

[Sebastian Andrzej Siewior]

On 2026-01-21 13:12:36 [+0000], Mark Brown wrote:
> What was the question?

to quote myself:

I've been staring wm8350_register_irq(). It does

| request_threaded_irq(irq + wm8350->irq_base, NULL,
| 		handler, flags, name, data);

and every single user passes 0 as flags. This means it asks for a
threaded IRQ and does not pass IRQF_ONESHOT.

So either this is not working because it triggers the warnings
	Threaded irq requested with handler=NULL and !ONESHOT

followed by -EINVAL _or_ every single user of this driver sits in
system where the irqchip is IRQCHIP_ONESHOT_SAFE marked.
Which is it?

Sebastian

Re: request_irq() usage in wm8350_register_irq().

[Mark Brown]

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

On Wed, Jan 21, 2026 at 02:18:11PM +0100, Sebastian Andrzej Siewior wrote:

> I've been staring wm8350_register_irq(). It does

> | request_threaded_irq(irq + wm8350->irq_base, NULL,
> | 		handler, flags, name, data);

> and every single user passes 0 as flags. This means it asks for a
> threaded IRQ and does not pass IRQF_ONESHOT.

> So either this is not working because it triggers the warnings
> 	Threaded irq requested with handler=NULL and !ONESHOT

> followed by -EINVAL _or_ every single user of this driver sits in
> system where the irqchip is IRQCHIP_ONESHOT_SAFE marked.
> Which is it?

The chip is only accessable via I2C or SPI so the primary interrupt
handler is itself always running and reporting child interrupts in
threaded context.  They can't really deliver a hardirq context
interrupt.

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

Re: request_irq() usage in wm8350_register_irq().

[Sebastian Andrzej Siewior]

On 2026-01-21 13:37:09 [+0000], Mark Brown wrote:
> On Wed, Jan 21, 2026 at 02:18:11PM +0100, Sebastian Andrzej Siewior wrote:
> 
> > I've been staring wm8350_register_irq(). It does
> 
> > | request_threaded_irq(irq + wm8350->irq_base, NULL,
> > | 		handler, flags, name, data);
> 
> > and every single user passes 0 as flags. This means it asks for a
> > threaded IRQ and does not pass IRQF_ONESHOT.
> 
> > So either this is not working because it triggers the warnings
> > 	Threaded irq requested with handler=NULL and !ONESHOT
> 
> > followed by -EINVAL _or_ every single user of this driver sits in
> > system where the irqchip is IRQCHIP_ONESHOT_SAFE marked.
> > Which is it?
> 
> The chip is only accessable via I2C or SPI so the primary interrupt
> handler is itself always running and reporting child interrupts in
> threaded context.  They can't really deliver a hardirq context
> interrupt.

In that case it lacks the needed IRQF_ONEHOST flag and nobody noticed
that request_irq() fails for a few years now. I add a patch to my
stack…

Thanks Mark.

Sebastian

Re: request_irq() usage in wm8350_register_irq().

[Mark Brown]

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

On Wed, Jan 21, 2026 at 02:45:08PM +0100, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 13:37:09 [+0000], Mark Brown wrote:

> > The chip is only accessable via I2C or SPI so the primary interrupt
> > handler is itself always running and reporting child interrupts in
> > threaded context.  They can't really deliver a hardirq context
> > interrupt.

> In that case it lacks the needed IRQF_ONEHOST flag and nobody noticed
> that request_irq() fails for a few years now. I add a patch to my
> stack…

Thanks.

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

Re: request_irq() usage in wm8350_register_irq().

[Andy Shevchenko]

On Wed, Jan 21, 2026 at 01:12:36PM +0000, Mark Brown wrote:
> On Wed, Jan 21, 2026 at 02:43:43PM +0200, Andy Shevchenko wrote:
> > On Wed, Jan 21, 2026 at 12:57:23PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2026-01-21 11:53:28 [+0000], Richard Fitzgerald wrote:

...

> > > > Ah. I didn't notice that. Confusing error message. It says
> > > > "handler=NULL" but handler != NULL. More like "handler=default".

> > > The supplied argument was NULL.
> > > Anyway. Do you happen to have an answer to my original question.
> 
> > Why not asking Mark? I think he might know more about this HW.
> 
> What was the question?

How does the IRQ handling work in this multifunctional device for its
children? @bigeasy is puzzled here and he started this thread.

> > Btw, Mark, I noticed that your old addresses appear in Git history,
> > are they working? Or do you want to add them into .mailmap?
> 
> They'll be broken but I'm not sure I care.

Hmm... I would have sent this email to your wolfsonmicro address :-)

-- 
With Best Regards,
Andy Shevchenko

Re: request_irq() usage in wm8350_register_irq().

[Richard Fitzgerald]

On 21/01/2026 11:57 am, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 11:53:28 [+0000], Richard Fitzgerald wrote:
>> Ah. I didn't notice that. Confusing error message. It says
>> "handler=NULL" but handler != NULL. More like "handler=default".
> 
> The supplied argument was NULL.
> Anyway. Do you happen to have an answer to my original question.
> 
> Sebastian

Afraid I'm not familiar with the WM8350.

But I note that call is not registering against a real interrupt pin.
It's registering against a virtual irqchip that is itself a threaded
IRQ handler. Maybe that makes a difference. That top-level virtual IRQ
handler is ONESHOT.

Re: request_irq() usage in wm8350_register_irq().

[Charles Keepax]

On Wed, Jan 21, 2026 at 12:49:11PM +0000, Richard Fitzgerald wrote:
> On 21/01/2026 11:57 am, Sebastian Andrzej Siewior wrote:
> > On 2026-01-21 11:53:28 [+0000], Richard Fitzgerald wrote:
> > > Ah. I didn't notice that. Confusing error message. It says
> > > "handler=NULL" but handler != NULL. More like "handler=default".
> > 
> > The supplied argument was NULL.
> > Anyway. Do you happen to have an answer to my original question.
> > 
> > Sebastian
> 
> Afraid I'm not familiar with the WM8350.
> 
> But I note that call is not registering against a real interrupt pin.
> It's registering against a virtual irqchip that is itself a threaded
> IRQ handler. Maybe that makes a difference. That top-level virtual IRQ
> handler is ONESHOT.

Yeah I suspect there is some trickery like this which means this
doesn't actually cause a problem in practice, since these are
internal IRQs to the chip, and the IRQ coming out of the chip is
marked as one shot. So the alluded to IRQ storm probably can't
happen as the chip level IRQ is still masked.

I also note the driver predates the check by a good few years, so
it wouldn't have generated any errors whilst being developed. To
be fair it has also never been the most heavily used driver, so
quite plausible no one has noticed the error message since. I
would wager the correct thing to do here is just to add the
ONESHOT flag in.

Thanks,
Charles