Re: [PATCH v2] iio: hid-sensors: Use software trigger

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 23 Mar 2026 17:10:58 +0200
Hanne-Lotta Mäenpää <hannelotta@gmail.com> wrote:

> On 2/21/26 12:45 AM, Srinivas Pandruvada wrote:
> > Recent changes linux mainline resulted in warning:
> > "genirq: Warn about using IRQF_ONESHOT without a threaded handler"
> > when HID sensor hub is used.
> > 
> > When INDIO_BUFFER_TRIGGERED is used, the core attaches a poll function
> > when enabling the buffer. This poll function uses request_threaded_irq()
> > with both bottom half and top half handlers. But when using HID
> > sensor hub, bottom half (thread handler) is not registered.
> > 
> > In HID sensors, once a sensor is powered on, the hub collects samples
> > and pushes data to the host when programmed thresholds are met. When
> > this data is received for a sensor, it is pushed using
> > iio_push_to_buffers_with_ts().
> > 
> > The sensor is powered ON or OFF based on the trigger callback
> > set_trigger_state() when the poll function is attached. During the call
> > to iio_triggered_buffer_setup_ext(), the HID sensor specifies only a
> > handler function but provides no thread handler, as there is no data
> > to read from the hub in thread context. Internally, this results in
> > calling request_threaded_irq(). Recent kernel changes now warn when
> > request_threaded_irq() is called without a thread handler.
> > 
> > To address this issue, fundamental changes are required to avoid using
> > iio_triggered_buffer_setup_ext(). HID sensors can use
> > INDIO_BUFFER_SOFTWARE instead of INDIO_BUFFER_TRIGGERED, as this can
> > work in trigger-less mode.
> > 
> > In this approach, when user space opens the buffer, the sensor is powered
> > on, and when the buffer is closed, the sensor is powered off using
> > iio_buffer_setup_ops callbacks.
> > 
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > ---
> > Changes:
> > v2:
> > changed
> > indio_dev->modes |= INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
> > to
> > indio_dev->modes = INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
> > 
> >  From RFC
> > - The trigger alloc and register is still kept as old code with
> > additional
> > indio_dev->modes |= INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
> > 
> >   .../common/hid-sensors/hid-sensor-trigger.c   | 48 ++++++++++++-------
> >   1 file changed, 30 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > index 5540e2d28f4a..417c4ab8c1b2 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > @@ -14,6 +14,7 @@
> >   #include <linux/iio/triggered_buffer.h>
> >   #include <linux/iio/trigger_consumer.h>
> >   #include <linux/iio/sysfs.h>
> > +#include <linux/iio/kfifo_buf.h>
> >   #include "hid-sensor-trigger.h"
> >   
> >   static ssize_t _hid_sensor_set_report_latency(struct device *dev,
> > @@ -202,12 +203,21 @@ static void hid_sensor_set_power_work(struct work_struct *work)
> >   		_hid_sensor_power_state(attrb, true);
> >   }
> >   
> > -static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
> > -						bool state)
> > +static int buffer_postenable(struct iio_dev *indio_dev)
> >   {
> > -	return hid_sensor_power_state(iio_trigger_get_drvdata(trig), state);
> > +	return hid_sensor_power_state(iio_device_get_drvdata(indio_dev), 1);
> >   }
> >   
> > +static int buffer_predisable(struct iio_dev *indio_dev)
> > +{
> > +	return hid_sensor_power_state(iio_device_get_drvdata(indio_dev), 0);
> > +}
> > +
> > +static const struct iio_buffer_setup_ops hid_sensor_buffer_ops = {
> > +	.postenable = buffer_postenable,
> > +	.predisable = buffer_predisable,
> > +};
> > +
> >   void hid_sensor_remove_trigger(struct iio_dev *indio_dev,
> >   			       struct hid_sensor_common *attrb)
> >   {
> > @@ -219,14 +229,9 @@ void hid_sensor_remove_trigger(struct iio_dev *indio_dev,
> >   	cancel_work_sync(&attrb->work);
> >   	iio_trigger_unregister(attrb->trigger);
> >   	iio_trigger_free(attrb->trigger);
> > -	iio_triggered_buffer_cleanup(indio_dev);
> >   }
> >   EXPORT_SYMBOL_NS(hid_sensor_remove_trigger, "IIO_HID");
> >   
> > -static const struct iio_trigger_ops hid_sensor_trigger_ops = {
> > -	.set_trigger_state = &hid_sensor_data_rdy_trigger_set_state,
> > -};
> > -
> >   int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name,
> >   				struct hid_sensor_common *attrb)
> >   {
> > @@ -239,25 +244,34 @@ int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name,
> >   	else
> >   		fifo_attrs = NULL;
> >   
> > -	ret = iio_triggered_buffer_setup_ext(indio_dev,
> > -					     &iio_pollfunc_store_time, NULL,
> > -					     IIO_BUFFER_DIRECTION_IN,
> > -					     NULL, fifo_attrs);
> > +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
> > +
> > +	ret = devm_iio_kfifo_buffer_setup_ext(&indio_dev->dev, indio_dev,
> > +					      &hid_sensor_buffer_ops,
> > +					      fifo_attrs);
> >   	if (ret) {
> > -		dev_err(&indio_dev->dev, "Triggered Buffer Setup Failed\n");
> > +		dev_err(&indio_dev->dev, "Kfifo Buffer Setup Failed\n");
> >   		return ret;
> >   	}
> >   
> > +	/*
> > +	 * The current user space in distro "iio-sensor-proxy" is not working in
> > +	 * trigerless mode and it expects
> > +	 * /sys/bus/iio/devices/iio:device0/trigger/current_trigger.
> > +	 * The change replacing iio_triggered_buffer_setup_ext() with
> > +	 * devm_iio_kfifo_buffer_setup_ext() will not create attribute without
> > +	 * registering a trigger with INDIO_HARDWARE_TRIGGERED.
> > +	 * So the below code fragment is still required.
> > +	 */
> > +
> >   	trig = iio_trigger_alloc(indio_dev->dev.parent,
> >   				 "%s-dev%d", name, iio_device_id(indio_dev));
> >   	if (trig == NULL) {
> >   		dev_err(&indio_dev->dev, "Trigger Allocate Failed\n");
> > -		ret = -ENOMEM;
> > -		goto error_triggered_buffer_cleanup;
> > +		return -ENOMEM;
> >   	}
> >   
> >   	iio_trigger_set_drvdata(trig, attrb);
> > -	trig->ops = &hid_sensor_trigger_ops;
> >   	ret = iio_trigger_register(trig);
> >   
> >   	if (ret) {
> > @@ -284,8 +298,6 @@ int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name,
> >   	iio_trigger_unregister(trig);
> >   error_free_trig:
> >   	iio_trigger_free(trig);
> > -error_triggered_buffer_cleanup:
> > -	iio_triggered_buffer_cleanup(indio_dev);
> >   	return ret;
> >   }
> >   EXPORT_SYMBOL_NS(hid_sensor_setup_trigger, "IIO_HID");  
> 
> 
> Hi,
> 
> This patch clears the warning "WARNING: kernel/irq/manage.c:1502 at 
> __setup_irq" on Debian forky.
> 
> Tested-by: Hanne-Lotta Mäenpää
> 
> The referenced commit ID could be included in the commit message, like this:
> 
> Commit aef30c8d569c0f317154 ("genirq: Warn about using IRQF_ONESHOT 
> without a threaded handler")
> 
> With that included, this patch will be easier to find if someone bisects 
> the issue.
> 
> Also, a Closes tag could be added:
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221256
The patch was merged a while ago and in the discussions in the related
threads we decide it was too invasive to handle as a fix for what is a
warning print, not a failure.

The closes tag without a fixes would trigger various bot warnings.
It could have been there as a Link however.  

Also, the small temporal issue of the fix predating the bugzilla
entry made it trickier!  I could have added it when merging but
we didn't have confirmation that the two were definitely the same
issue at that time and I chose not to connect up the dots.

Your reply may well help anyone looking to find the issue though
so thanks for that.

Jonathan

> 
> Best regards,
> 
> Hanne-Lotta Mäenpää