Re: [PATCH 18/21] iio: Replace IRQF_ONESHOT with IRQF_NO_THREAD

Sebastian Andrzej Siewior
2026-01-26T08:15:12Z

On 2026-01-23 16:48:54 [+0200], Andy Shevchenko wrote:
> On Fri, Jan 23, 2026 at 12:37:04PM +0100, Sebastian Andrzej Siewior wrote:
> > Passing IRQF_ONESHOT ensures that the interrupt source is masked until
> > the secondary (threaded) handler is done. If only a primary handler is
> > used then the flag makes no sense because the interrupt can not fire
> > (again) while its handler is running.
> > The flag also disallows force-threading of the primary handler and the
> > irq-core will warn about this.
> >
> > The intention here was probably not allowing forced-threading for
> > handlers such as iio_trigger_generic_data_rdy_poll() will intends to
> > invoke hard-interrupt handlers.
> >
> > Replace IRQF_ONESHOT with IRQF_NO_THREAD.
>
> Code wise LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> There is a few nit-picks regarding style below.
>
> ...
>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>
> > Cc: Michael Hennerich <Michael.Hennerich@analog.com>
> > Cc: Puranjay Mohan <puranjay@kernel.org>
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Cc: David Lechner <dlechner@baylibre.com>
> > Cc: "Nuno Sá" <nuno.sa@analog.com>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Cc: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > Cc: Marcus Folkesson <marcus.folkesson@gmail.com>
> > Cc: Kent Gustavsson <kent@minoris.se>
> > Cc: Gustavo Silva <gustavograzs@gmail.com>
> > Cc: Nishant Malpani <nish.malpani25@gmail.com>
> > Cc: linux-iio@vger.kernel.org
>
> Move these...
>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
>
> ...to be here. This will drastically reduce the unneeded noise in the commit
> message. The email will have (if using git format-patch and git send-email)
> them.

Yeah but if I have to send v2 then they are gone, aren't they? So I have
to move them for each submission. Or is there something I am not aware
of?

> ...
>
> > --- a/drivers/iio/accel/adxl355_core.c
> > +++ b/drivers/iio/accel/adxl355_core.c
> > @@ -770,7 +770,8 @@ static int adxl355_probe_trigger(struct iio_dev *indio_dev, int irq)
> >
> > ret = devm_request_irq(data->dev, irq,
> > &iio_trigger_generic_data_rdy_poll,
> > - IRQF_ONESHOT, "adxl355_irq", data->dready_trig);
> > + IRQF_NO_THREAD,
> > + "adxl355_irq", data->dready_trig);
>
> Leave it on a single line. It's only 81 characters and it's fine to be like
> this.
>
> > if (ret)
> > return dev_err_probe(data->dev, ret, "request irq %d failed\n",
> > irq);
>
> ...
>
> > +++ b/drivers/iio/accel/adxl372.c
>
> > + ret = devm_request_irq(dev, st->irq,
> > + iio_trigger_generic_data_rdy_poll,
> > + IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> > + indio_dev->name, st->dready_trig);
> > if (ret < 0)
> > return ret;
>
> Interestingly that this driver ignores the flags from firmware... Seems to me
> like a bug (not in your patch, obviously). Ditto for other drivers doing similar
> things.

If the irq-chip is level or unknown mode on boot up/ default and the
device can only operate as an edge-rising then I don't see why this
should be a bug.

> ...
>
> > --- a/drivers/iio/pressure/dlhl60d.c
> > +++ b/drivers/iio/pressure/dlhl60d.c
> > @@ -306,9 +306,9 @@ static int dlh_probe(struct i2c_client *client)
> > indio_dev->num_channels = ARRAY_SIZE(dlh_channels);
> >
> > if (client->irq > 0) {
> > - ret = devm_request_threaded_irq(&client->dev, client->irq,
> > - dlh_interrupt, NULL,
> > - IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > + ret = devm_request_irq(&client->dev, client->irq,
> > + dlh_interrupt,
>
> Joined to the previous line it gives exactly 80 characters, which is allowed.
>
> > + IRQF_TRIGGER_RISING | IRQF_NO_THREAD,
> > st->info->name, indio_dev);
>
> Forgot to indent.

Okay, let me fix this up.

>
> ...
>
> > + ret = devm_request_irq(&client->dev, client->irq,
> > + iio_trigger_generic_data_rdy_poll,
> > + IRQF_NO_THREAD, "tmp006_irq",
> > + data->drdy_trig);
> > if (ret < 0)
> > return ret;
>
> Jonathan et al., this seems to me like a duplication from one to another
> driver. Can't we do like a helper for it, so we don't change it each time
> the IRQ core or other things got changed?
>

Sebastian