* [PATCH v2] iio: hid-sensors: Use software trigger
@ 2026-02-20 22:45 Srinivas Pandruvada
2026-03-01 12:42 ` Jonathan Cameron
2026-03-23 15:10 ` Hanne-Lotta Mäenpää
0 siblings, 2 replies; 4+ messages in thread
From: Srinivas Pandruvada @ 2026-02-20 22:45 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, bigeasy, spasswolf, linux-kernel, Srinivas Pandruvada
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");
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] iio: hid-sensors: Use software trigger
2026-02-20 22:45 [PATCH v2] iio: hid-sensors: Use software trigger Srinivas Pandruvada
@ 2026-03-01 12:42 ` Jonathan Cameron
2026-03-23 15:10 ` Hanne-Lotta Mäenpää
1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-03-01 12:42 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: linux-iio, bigeasy, spasswolf, linux-kernel
On Fri, 20 Feb 2026 14:45:14 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> 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>
I think this is fine, but it almost looks like coincidence that where we
introduce a new devm_ call is at the point where the driver moves
from devm to non devm - the only place we can safely swap one for
the other like this.
It might we be worth exploring if the driver can easily make more
use of devm to reduce what we have to manually cleanup.
Anyhow, applied this to the fixes-togreg branch of iio.git.
I haven't given it a fixes tag though as there wasn't really a bug here
just some misuse of interfaces.
> ---
> 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");
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] iio: hid-sensors: Use software trigger
2026-02-20 22:45 [PATCH v2] iio: hid-sensors: Use software trigger Srinivas Pandruvada
2026-03-01 12:42 ` Jonathan Cameron
@ 2026-03-23 15:10 ` Hanne-Lotta Mäenpää
2026-04-12 13:38 ` Jonathan Cameron
1 sibling, 1 reply; 4+ messages in thread
From: Hanne-Lotta Mäenpää @ 2026-03-23 15:10 UTC (permalink / raw)
To: Srinivas Pandruvada, jic23; +Cc: linux-iio, bigeasy, spasswolf, linux-kernel
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
Best regards,
Hanne-Lotta Mäenpää
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] iio: hid-sensors: Use software trigger
2026-03-23 15:10 ` Hanne-Lotta Mäenpää
@ 2026-04-12 13:38 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-04-12 13:38 UTC (permalink / raw)
To: Hanne-Lotta Mäenpää
Cc: Srinivas Pandruvada, linux-iio, bigeasy, spasswolf, linux-kernel
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ää
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-12 13:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-20 22:45 [PATCH v2] iio: hid-sensors: Use software trigger Srinivas Pandruvada
2026-03-01 12:42 ` Jonathan Cameron
2026-03-23 15:10 ` Hanne-Lotta Mäenpää
2026-04-12 13:38 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox