[PATCH] iio: iio_triggered_buffer_setup_ext(): request IRQF_ONESHOT only if threaded handler is present

Marc Kleine-Budde posted 1 patch 4 weeks ago
drivers/iio/buffer/industrialio-triggered-buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] iio: iio_triggered_buffer_setup_ext(): request IRQF_ONESHOT only if threaded handler is present
Posted by Marc Kleine-Budde 4 weeks ago
Since commit aef30c8d569c ("genirq: Warn about using IRQF_ONESHOT without a
threaded handler") the kernel warns when requesting an IRQ using
IRQF_ONESHOT without a threaded handler.

iio_triggered_buffer_setup_ext() always passes IRQF_ONESHOT to
iio_alloc_pollfunc(), even if no threaded handler is given by the caller,
which results in a kernel warning.

Fix this warning by only passing IRQF_ONESHOT if a threaded handler is
given.

Fixes: 23f2d735a932 ("iio: Add helper function for initializing triggered buffers")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hello,

I'm not sure if this is the correct fix, or if the
stm32-adc.c/stm32-dfsdm-adc.c driver needs fixing.

For iio_triggered_buffer_setup(), they both use iio_pollfunc_store_time()
as the hard IRQ handler, but the stm32-adc.c in PIO mode and the
stm32-dfsdm-adc.c always no threaded IRQ handler.

As iio_triggered_buffer_setup_ext() always uses IRQF_ONESHOT this triggers
a warning since ommit aef30c8d569c ("genirq: Warn about using IRQF_ONESHOT
without a threaded handler").

Another thing I noticed is that iio_pollfunc_store_time() returns
IRQ_WAKE_THREAD, which should trigger yet another warning ("IRQ %d device
%s returned IRQ_WAKE_THREAD but no thread function available.") if used
without a IRQ thread. But I'm not using this subsystem right now, just
stumbled over the new warning.
---
 drivers/iio/buffer/industrialio-triggered-buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
index 9bf75dee7ff8..40eea3a44724 100644
--- a/drivers/iio/buffer/industrialio-triggered-buffer.c
+++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
@@ -64,7 +64,7 @@ int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
 
 	indio_dev->pollfunc = iio_alloc_pollfunc(h,
 						 thread,
-						 IRQF_ONESHOT,
+						 thread ? IRQF_ONESHOT : 0,
 						 indio_dev,
 						 "%s_consumer%d",
 						 indio_dev->name,

---
base-commit: 86138b484d6367a57312f69af4ec958806c2673c
change-id: 20260514-iio-irq-fixes-1485db263ac3

Best regards,
--  
Marc Kleine-Budde <mkl@pengutronix.de>
Re: [PATCH] iio: iio_triggered_buffer_setup_ext(): request IRQF_ONESHOT only if threaded handler is present
Posted by Jonathan Cameron 4 weeks ago
On Thu, 14 May 2026 16:43:41 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> Since commit aef30c8d569c ("genirq: Warn about using IRQF_ONESHOT without a
> threaded handler") the kernel warns when requesting an IRQ using
> IRQF_ONESHOT without a threaded handler.
> 
> iio_triggered_buffer_setup_ext() always passes IRQF_ONESHOT to
> iio_alloc_pollfunc(), even if no threaded handler is given by the caller,
> which results in a kernel warning.
> 
> Fix this warning by only passing IRQF_ONESHOT if a threaded handler is
> given.
> 
> Fixes: 23f2d735a932 ("iio: Add helper function for initializing triggered buffers")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Hello,
Hi Marc,

Thanks for the report + patch.
> 
> I'm not sure if this is the correct fix, or if the
> stm32-adc.c/stm32-dfsdm-adc.c driver needs fixing.

Sounds like another case of what we had to fix for hid-sensors (and a few others
IIRC).

They should never have been using triggers in the first place for this path -
but now we have ABI we can't touch.  Note there are even comments saying that
the infrastructure registered is not used!

So we have to have do a small dance to ensure the sysfs files are still created
but without the need for the pollfunc setup etc.

https://lore.kernel.org/all/20260220224514.471348-1-srinivas.pandruvada@linux.intel.com/

It is in theory possible to have a case that needs the fix you have below
but that would not be using iio_pollfunc_store_time().
That being used without a thread is pretty much pointless as no one reads
the timestamp.

I took a look just at stm32_adc and things are more complex because in some
cases we should be registering a triggered buffer (with both top and bottom
half set) and in some cases we should not - but should be doing what we
did in the hid sensors.

Given I believe this is a harmless warning I'm not in a huge rush to fix it.
We need someone who has the time + hardware to test solutions as they
can be a bit fiddly.

Whilst the code you have here papers over the problem it's changing from
having an entirely correct warning to hiding that the code is wrong so
I'd rather not merge that.  I've rejected it as a proposed solution a couple
of times so far - and thought we were getting to the end of drivers that abused
this stuff :(  Seems not quite yet.

Jonathan

> 
> For iio_triggered_buffer_setup(), they both use iio_pollfunc_store_time()
> as the hard IRQ handler, but the stm32-adc.c in PIO mode and the
> stm32-dfsdm-adc.c always no threaded IRQ handler.
> 
> As iio_triggered_buffer_setup_ext() always uses IRQF_ONESHOT this triggers
> a warning since ommit aef30c8d569c ("genirq: Warn about using IRQF_ONESHOT
> without a threaded handler").
> 
> Another thing I noticed is that iio_pollfunc_store_time() returns
> IRQ_WAKE_THREAD, which should trigger yet another warning ("IRQ %d device
> %s returned IRQ_WAKE_THREAD but no thread function available.") if used
> without a IRQ thread. But I'm not using this subsystem right now, just
> stumbled over the new warning.
> ---
>  drivers/iio/buffer/industrialio-triggered-buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
> index 9bf75dee7ff8..40eea3a44724 100644
> --- a/drivers/iio/buffer/industrialio-triggered-buffer.c
> +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
> @@ -64,7 +64,7 @@ int iio_triggered_buffer_setup_ext(struct iio_dev *indio_dev,
>  
>  	indio_dev->pollfunc = iio_alloc_pollfunc(h,
>  						 thread,
> -						 IRQF_ONESHOT,
> +						 thread ? IRQF_ONESHOT : 0,
>  						 indio_dev,
>  						 "%s_consumer%d",
>  						 indio_dev->name,
> 
> ---
> base-commit: 86138b484d6367a57312f69af4ec958806c2673c
> change-id: 20260514-iio-irq-fixes-1485db263ac3
> 
> Best regards,
> --  
> Marc Kleine-Budde <mkl@pengutronix.de>
>
Re: [PATCH] iio: iio_triggered_buffer_setup_ext(): request IRQF_ONESHOT only if threaded handler is present
Posted by Marc Kleine-Budde 4 weeks ago
On 14.05.2026 16:41:32, Jonathan Cameron wrote:
> Thanks for the report + patch.
> >
> > I'm not sure if this is the correct fix, or if the
> > stm32-adc.c/stm32-dfsdm-adc.c driver needs fixing.
>
> Sounds like another case of what we had to fix for hid-sensors (and a few others
> IIRC).
>
> They should never have been using triggers in the first place for this path -
> but now we have ABI we can't touch.  Note there are even comments saying that
> the infrastructure registered is not used!
>
> So we have to have do a small dance to ensure the sysfs files are still created
> but without the need for the pollfunc setup etc.
>
> https://lore.kernel.org/all/20260220224514.471348-1-srinivas.pandruvada@linux.intel.com/
>
> It is in theory possible to have a case that needs the fix you have below
> but that would not be using iio_pollfunc_store_time().
> That being used without a thread is pretty much pointless as no one reads
> the timestamp.
>
> I took a look just at stm32_adc and things are more complex because in some
> cases we should be registering a triggered buffer (with both top and bottom
> half set) and in some cases we should not - but should be doing what we
> did in the hid sensors.
>
> Given I believe this is a harmless warning I'm not in a huge rush to fix it.

It's a warning including a stack trace:

| ------------[ cut here ]------------
| WARNING: kernel/irq/manage.c:1502 at __setup_irq+0x104/0x81c, CPU#1: adc-stm32/370
| Modules linked in:
| CPU: 1 UID: 0 PID: 370 Comm: adc-stm32 Not tainted 7.1.0-rc3+ #2 PREEMPT  dba7a20aa885d5558721d75378dc67d9c58ca68c
| Hardware name: STM32 (Device Tree Support)
| Call trace:
|  unwind_backtrace from show_stack+0x18/0x1c
|  show_stack from dump_stack_lvl+0x50/0x64
|  dump_stack_lvl from __warn+0x98/0x178
|  __warn from warn_slowpath_fmt+0x1a4/0x1ac
|  warn_slowpath_fmt from __setup_irq+0x104/0x81c
|  __setup_irq from request_threaded_irq+0xb8/0x14c
|  request_threaded_irq from iio_trigger_attach_poll_func+0xa8/0x1a4
|  iio_trigger_attach_poll_func from __iio_update_buffers+0xab8/0xb6c
|  __iio_update_buffers from enable_store+0x84/0xd4
|  enable_store from kernfs_fop_write_iter+0x154/0x220
|  kernfs_fop_write_iter from vfs_write+0x27c/0x4fc
|  vfs_write from ksys_write+0x68/0xf0
|  ksys_write from ret_fast_syscall+0x0/0x54
| Exception stack(0xe12f9fa8 to 0xe12f9ff0)
| 9fa0:                   00000000 00000000 00000009 01ad2358 00000002 00000000
| 9fc0: 00000000 00000000 0f387d24 00000004 01accc68 01ad13b8 01accc68 b5ffe44c
| 9fe0: b5ffdf08 b5ffdef4 b6d34659 b6d3d038
| ---[ end trace 0000000000000000 ]---

which looks quite scary :)

> We need someone who has the time + hardware to test solutions as they
> can be a bit fiddly.

We have the hardware, we're happy to test patches.

> Whilst the code you have here papers over the problem it's changing from
> having an entirely correct warning to hiding that the code is wrong so
> I'd rather not merge that.  I've rejected it as a proposed solution a couple
> of times so far - and thought we were getting to the end of drivers that abused
> this stuff :(  Seems not quite yet.

regards,
Marc

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |