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

[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@xxxxxxxxxxxxxxx>
---
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