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: <20260126081510.jr8Rp6R7@linutronix.de>
Date: Mon, 26 Jan 2026 09:15:10 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
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 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@...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.

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