linux-kernel - Re: [PATCH 18/21] iio: Replace IRQF_ONESHOT with IRQF_NO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXOKVqxyhQ10l9V4@smile.fi.intel.com>
Date: Fri, 23 Jan 2026 16:48:54 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev,
	Thomas Gleixner <tglx@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Michael Hennerich <Michael.Hennerich@...log.com>,
	Puranjay Mohan <puranjay@...nel.org>,
	Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>,
	Marcelo Schmitt <marcelo.schmitt@...log.com>,
	Marcus Folkesson <marcus.folkesson@...il.com>,
	Kent Gustavsson <kent@...oris.se>,
	Gustavo Silva <gustavograzs@...il.com>,
	Nishant Malpani <nish.malpani25@...il.com>,
	linux-iio@...r.kernel.org
Subject: Re: [PATCH 18/21] iio: Replace IRQF_ONESHOT with IRQF_NO_THREAD

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@...el.com>

There is a few nit-picks regarding style below.

...

> Cc: Lars-Peter Clausen <lars@...afoo.de>
> Cc: Michael Hennerich <Michael.Hennerich@...log.com>
> Cc: Puranjay Mohan <puranjay@...nel.org>
> Cc: Jonathan Cameron <jic23@...nel.org>
> Cc: David Lechner <dlechner@...libre.com>
> Cc: "Nuno Sá" <nuno.sa@...log.com>
> Cc: Andy Shevchenko <andy@...nel.org>
> Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>
> Cc: Marcus Folkesson <marcus.folkesson@...il.com>
> Cc: Kent Gustavsson <kent@...oris.se>
> Cc: Gustavo Silva <gustavograzs@...il.com>
> Cc: Nishant Malpani <nish.malpani25@...il.com>
> Cc: linux-iio@...r.kernel.org

Move these...

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.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.

...

> --- 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.

...

> --- 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.

...

> +		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?

-- 
With Best Regards,
Andy Shevchenko