[RFC 0/4] iio-input-bridge so that accelerometers which only have an iio driver can still present evdev input events

[RFC 0/4] iio-input-bridge so that accelerometers which only have an iio driver can still present evdev input events

[H. Nikolaus Schaller]

Some user spaces (e.g. some Android) use /dev/input/event* for handling the 3D
position of the device with respect to the center of gravity (earth). This can
be used for gaming input, rotation of screens etc.

This should be the standard because this interface is an abstraction of how
this data is acquired from sensor chips. Sensor chips may be connected through
different interfaces and in different positions. They may also have different
parameters. And, if a chip is replaced by a different one, the values reported
by the device position interface should remain the same.

But nowadays, new accelerometer chips usually just get iio drivers and rarely
an evdev input driver.

Therefore we need something like a protocol stack: input device vs. raw data.
It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
input events vs. raw gpio or raw USB access.

This patch set bridges the gap between raw iio data and the input device abstraction
so that accelerometer measurements can also be presented as X/Y/Z accelerometer
channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.


H. Nikolaus Schaller (4):
  iio: input-bridge: optionally bridge iio acceleometers to create a
    /dev/input
  iio: input-bridge: add iio-input-bridge to Makefile
  iio: input-bridge: add IIO_INPUT_BRIDGE kernel config option
  iio: input-bridge: make the iio-input-bridge driver called by iio-core

 drivers/iio/Kconfig                    |   7 +
 drivers/iio/Makefile                   |   1 +
 drivers/iio/industrialio-core.c        |  12 +
 drivers/iio/industrialio-inputbridge.c | 420 +++++++++++++++++++++++++
 drivers/iio/industrialio-inputbridge.h |  28 ++
 5 files changed, 468 insertions(+)
 create mode 100644 drivers/iio/industrialio-inputbridge.c
 create mode 100644 drivers/iio/industrialio-inputbridge.h

-- 
2.19.1

[RFC 1/4] iio: input-bridge: optionally bridge iio acceleometers to create a /dev/input

[H. Nikolaus Schaller]

Some user spaces (e.g. some Android devices) use /dev/input/event* for handling
the 3D position of the device with respect to the center of gravity (earth).
This can be used for gaming input, auto-rotation of screens etc.

This interface should be the standard because it is an abstraction of how
orientation data is acquired from sensor chips. Sensor chips may be connected
through different interfaces and in different positions. They may also have different
parameters. And, if a chip is replaced by a different one, the values reported
by the device position interface should remain the same, provided the device
tree reflects the changed chip.

But nowadays, new accelerometer chips mostly get iio drivers and rarely input drivers.
Therefore we need something like a protocol stack: input device vs. raw data.
It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
input events vs. raw gpio or raw USB access.

This patch bridges the gap between raw iio data and the input device abstraction
so that accelerometer measurements can also be presented as X/Y/Z accelerometer
channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.

There are no special requirements or changes needed for an iio driver.

This driver simply collects the first 3 accelerometer channels as X, Y and Z.
If only 1 or 2 channels are available, they are used for X and Y only. Additional
channels are ignored.

Scaling is done automatically so that 1g is represented by value 256 and
range is assumed to be -511 .. +511.

If a mount-matrix is provided by the iio driver, it is also taken into account
so that the input event automatically gets the correct orientation with respect
to the device.

If this extension is not configured into the kernel it takes no resources (except
source code).

If it is configured, but there is no accelerometer, there is only a tiny penalty
for scanning for accelerometer channels once during probe of each iio device.

If it runs, the driver polls the device(s) once every 100 ms. A mode where the
iio device defines the update rate is not implemented and for further study.

The driver is capable of supporting up to 5 iio accelerometers and they are
presented by unique /dev/input/event* files. The iio chip name is used to define
the input device name so that it can be identified (e.g. by udev rules or evtest).

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/iio/industrialio-inputbridge.c | 420 +++++++++++++++++++++++++
 drivers/iio/industrialio-inputbridge.h |  28 ++
 2 files changed, 448 insertions(+)
 create mode 100644 drivers/iio/industrialio-inputbridge.c
 create mode 100644 drivers/iio/industrialio-inputbridge.h

diff --git a/drivers/iio/industrialio-inputbridge.c b/drivers/iio/industrialio-inputbridge.c
new file mode 100644
index 000000000000..0288ba2ef69f
--- /dev/null
+++ b/drivers/iio/industrialio-inputbridge.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * The Industrial I/O core, bridge to input devices
+ *
+ * Copyright (c) 2016-2019 Golden Delicious Computers GmbH&Co. KG
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "industrialio-inputbridge.h"
+
+/* currently, only polling is implemented */
+#define POLLING_MSEC	100
+
+/* handle up to this number of input devices */
+#define MAX_INPUT_DEVICES	5
+
+#define FIXED_POINT_UNIT	1000
+
+static struct iio_input_map {
+	struct iio_dev *indio_dev;	/* the iio device */
+	struct input_dev *input;	/* the input device */
+	struct iio_channel channels[3];	/* x, y, z channels */
+	int values[3];		/* values while processing */
+	struct matrix {
+		int mxx, myx, mzx;	/* translated and scaled mount-matrix */
+		int mxy, myy, mzy;
+		int mxz, myz, mzz;
+	} matrix;
+} devices[MAX_INPUT_DEVICES];
+
+static inline struct iio_input_map *to_iio_input_map(
+		struct iio_channel *channel)
+{
+	return (struct iio_input_map *) channel->data;
+}
+
+/* we must protect against races in channel allocation */
+
+static DEFINE_MUTEX(inputbridge_device_mutex);
+
+static struct delayed_work input_work;
+
+/* we can start/stop the worker by open("/dev/input/event") */
+static int open_count;
+
+/* we must protect the open counter */
+static DEFINE_MUTEX(inputbridge_open_mutex);
+
+/* minimum and maximum range we want to report */
+#define ABSMAX_ACC_VAL		(512 - 1)
+#define ABSMIN_ACC_VAL		-(ABSMAX_ACC_VAL)
+
+/* scale processed iio values so that 1g maps to ABSMAX_ACC_VAL / 2 */
+#define SCALE			((100 * ABSMAX_ACC_VAL) / (2 * 981))
+
+/*
+ * convert float string to scaled fixed point format, e.g.
+ *   1.23	-> 1230
+ *   0.1234	->  123
+ *   .01234	->   12
+ */
+
+static int32_t atofix(const char *str)
+{
+	int32_t mant = 0;
+	bool sign = false;
+	bool decimal = false;
+	int32_t divisor = 1;
+
+	if (*str == '-')
+		sign = true, str++;
+	while (*str && divisor < FIXED_POINT_UNIT) {
+		if (*str >= '0' && *str <= '9') {
+			mant = 10 * mant + (*str - '0');
+			if (decimal)
+				divisor *= 10;
+		} else if (*str == '.')
+			decimal = true;
+		else
+			return 0;	/* error */
+		str++;
+	}
+
+	mant = (FIXED_POINT_UNIT * mant) / divisor;
+	if (sign)
+		mant = -mant;
+
+	return mant;
+}
+
+static void iio_apply_matrix(struct matrix *m, int *in, int *out)
+{
+	/* apply mount matrix */
+	out[0] = (m->mxx * in[0] + m->myx * in[1] + m->mzx * in[2])
+			/ FIXED_POINT_UNIT;
+	out[1] = (m->mxy * in[0] + m->myy * in[1] + m->mzy * in[2])
+			/ FIXED_POINT_UNIT;
+	out[2] = (m->mxz * in[0] + m->myz * in[1] + m->mzz * in[2])
+			/ FIXED_POINT_UNIT;
+}
+
+static void iio_accel_report_channels(void)
+{
+	int dindex;
+
+	for (dindex = 0; dindex < ARRAY_SIZE(devices); dindex++) {
+		struct iio_input_map *map = &devices[dindex];
+
+		/* device might become closed while we are still processing */
+		mutex_lock(&inputbridge_device_mutex);
+
+		if (map->input) {
+			int aligned_values[3];
+			int cindex = 0;
+
+			while (cindex < ARRAY_SIZE(map->channels)) {
+				struct iio_channel *channel =
+					&map->channels[cindex];
+				int val;
+				int ret;
+
+				if (!channel->indio_dev)
+					continue;
+
+				ret = iio_read_channel_raw(channel, &val);
+
+				if (ret < 0) {
+					pr_err("%s(): channel read error %d\n",
+						__func__, cindex);
+					return;
+				}
+
+				ret = iio_convert_raw_to_processed(channel, val,
+						 &map->values[cindex], SCALE);
+
+				if (ret < 0) {
+					pr_err("%s(): channel processing error\n",
+						__func__);
+					return;
+				}
+
+				cindex++;
+			}
+
+			iio_apply_matrix(&map->matrix, map->values,
+					 aligned_values);
+
+			input_report_abs(map->input, ABS_X, aligned_values[0]);
+			input_report_abs(map->input, ABS_Y, aligned_values[1]);
+			input_report_abs(map->input, ABS_Z, aligned_values[2]);
+			input_sync(map->input);
+		}
+
+		mutex_unlock(&inputbridge_device_mutex);
+	}
+}
+
+static void iio_inputbridge_work(struct work_struct *work)
+{
+	struct delayed_work *delayed_work;
+
+	delayed_work = to_delayed_work(work);
+
+	mutex_lock(&inputbridge_open_mutex);
+
+	iio_accel_report_channels();
+
+	schedule_delayed_work(delayed_work,
+		msecs_to_jiffies(POLLING_MSEC));
+
+	mutex_unlock(&inputbridge_open_mutex);
+}
+
+static int iio_accel_open(struct input_dev *input)
+{
+	struct iio_dev *iiodev = input_get_drvdata(input);
+
+	mutex_lock(&inputbridge_open_mutex);
+
+	/* start on first open */
+	if (open_count++ == 0)
+		schedule_delayed_work(&input_work,
+			msecs_to_jiffies(0));
+
+	mutex_unlock(&inputbridge_open_mutex);
+
+	return 0;
+}
+
+static void iio_accel_close(struct input_dev *input)
+{
+	struct iio_dev *iiodev = input_get_drvdata(input);
+
+	mutex_lock(&inputbridge_open_mutex);
+
+	/* stop after last close */
+	if (--open_count == 0)
+		cancel_delayed_work(&input_work);
+
+	mutex_unlock(&inputbridge_open_mutex);
+}
+
+static int iio_input_register_accel_channel(struct iio_dev *indio_dev,
+		 const struct iio_chan_spec *chan)
+{ /* we found some accelerometer channel */
+	int ret;
+
+	int dindex, cindex;
+	struct iio_input_map *map;
+
+	mutex_lock(&inputbridge_device_mutex);
+
+	/* look for existing input device for this iio device */
+
+	for (dindex = 0; dindex < ARRAY_SIZE(devices); dindex++) {
+		if (devices[dindex].indio_dev == indio_dev)
+			break;
+	}
+
+	if (dindex == ARRAY_SIZE(devices)) {
+		struct input_dev *input;
+		const struct iio_chan_spec_ext_info *ext_info;
+
+		/* not found, look for a free slot for a new input device */
+
+		for (dindex = 0; dindex < ARRAY_SIZE(devices); dindex++) {
+			if (!devices[dindex].input)
+				break;
+		}
+
+		if (dindex == ARRAY_SIZE(devices)) {
+			mutex_unlock(&inputbridge_device_mutex);
+			return -ENOMEM;
+		}
+
+		input = input_allocate_device();
+
+		if (!input) {
+			mutex_unlock(&inputbridge_device_mutex);
+			return -ENOMEM;
+		}
+
+		input->name = kasprintf(GFP_KERNEL, "iio-bridge: %s",
+						    indio_dev->name);
+		input->phys = kasprintf(GFP_KERNEL, "accel/input%d",
+						    dindex);
+
+//		input->id.bustype = BUS_I2C;
+//		input->dev.parent = &indio_dev->client->dev;
+
+		set_bit(INPUT_PROP_ACCELEROMETER, input->propbit);
+
+		input->evbit[0] = BIT_MASK(EV_ABS);
+		input->open = iio_accel_open;
+		input->close = iio_accel_close;
+
+// FIXME: what happens if we unregister the first device?
+// and then register another one?
+
+		if (dindex == 0) // first input
+			INIT_DELAYED_WORK(&input_work, iio_inputbridge_work);
+
+		input_set_drvdata(input, indio_dev);
+
+		input_alloc_absinfo(input);
+		ret = input_register_device(input);
+
+		if (ret < 0) {
+			kfree(input->name);
+			kfree(input->phys);
+			input_free_device(input);
+			mutex_unlock(&inputbridge_device_mutex);
+			return ret;
+		}
+
+		map = &devices[dindex];
+
+		map->input = input;
+		map->indio_dev = indio_dev;
+
+		/* assume all channels of a device share the same matrix */
+
+		ext_info = chan->ext_info;
+		for (; ext_info && ext_info->name; ext_info++) {
+			if (strcmp(ext_info->name, "mount_matrix") == 0)
+				break;
+		}
+
+		if (ext_info && ext_info->name) {
+			/* matrix found */
+			uintptr_t priv = ext_info->private;
+			const struct iio_mount_matrix *mtx;
+
+			mtx = ((iio_get_mount_matrix_t *) priv)(indio_dev,
+								chan);
+
+			map->matrix.mxx = atofix(mtx->rotation[0]);
+			map->matrix.myx = atofix(mtx->rotation[1]);
+			map->matrix.mzx = atofix(mtx->rotation[2]);
+			map->matrix.mxy = atofix(mtx->rotation[3]);
+			map->matrix.myy = atofix(mtx->rotation[4]);
+			map->matrix.mzy = atofix(mtx->rotation[5]);
+			map->matrix.mxz = atofix(mtx->rotation[6]);
+			map->matrix.myz = atofix(mtx->rotation[7]);
+			map->matrix.mzz = atofix(mtx->rotation[8]);
+		} else {
+			map->matrix.mxx = FIXED_POINT_UNIT;
+			map->matrix.myx = 0;
+			map->matrix.mzx = 0;
+			map->matrix.mxy = 0;
+			map->matrix.myy = FIXED_POINT_UNIT;
+			map->matrix.mzy = 0;
+			map->matrix.mxz = 0;
+			map->matrix.myz = 0;
+			map->matrix.mzz = FIXED_POINT_UNIT;
+		}
+	}
+
+	/* find free channel within this device block */
+	map = &devices[dindex];
+
+	for (cindex = 0; cindex < ARRAY_SIZE(map->channels); cindex++) {
+		if (!map->channels[cindex].indio_dev)
+			break;
+	}
+
+	/* check if we already have collected enough channels */
+	if (cindex == ARRAY_SIZE(map->channels)) {
+		mutex_unlock(&inputbridge_device_mutex);
+		return 0;	/* silently ignore */
+	}
+
+	map->channels[cindex].indio_dev = indio_dev;
+	map->channels[cindex].channel = chan;
+	map->channels[cindex].data = (void *) &devices[dindex];
+
+	switch (cindex) {
+	case 0:
+		input_set_abs_params(map->input, ABS_X, ABSMIN_ACC_VAL,
+					ABSMAX_ACC_VAL, 0, 0);
+		break;
+	case 1:
+		input_set_abs_params(map->input, ABS_Y, ABSMIN_ACC_VAL,
+					ABSMAX_ACC_VAL, 0, 0);
+		break;
+	case 2:
+		input_set_abs_params(map->input, ABS_Z, ABSMIN_ACC_VAL,
+					ABSMAX_ACC_VAL, 0, 0);
+		break;
+	}
+	mutex_unlock(&inputbridge_device_mutex);
+
+	return 0;
+}
+
+int iio_device_register_inputbridge(struct iio_dev *indio_dev)
+{
+	int i;
+
+	for (i = 0; i < indio_dev->num_channels; i++) {
+		const struct iio_chan_spec *chan =
+				&indio_dev->channels[i];
+
+		if (chan->type == IIO_ACCEL) {
+			int r = iio_input_register_accel_channel(indio_dev,
+								 chan);
+
+			if (r < 0)
+				return r;
+		}
+	}
+
+	return 0;
+}
+
+void iio_device_unregister_inputbridge(struct iio_dev *indio_dev)
+{
+	struct input_dev *input = NULL;
+
+	int dindex = 0;
+
+	mutex_lock(&inputbridge_device_mutex);
+
+	for (; dindex < ARRAY_SIZE(devices); dindex++) {
+		int cindex = 0;
+
+		while (cindex < ARRAY_SIZE(devices[dindex].channels)) {
+			struct iio_channel *channel =
+				&devices[dindex].channels[cindex];
+
+			/* mark slot as empty */
+			if (channel->indio_dev == indio_dev)
+				channel->indio_dev = NULL;
+			cindex++;
+		}
+		input_unregister_device(input);
+		kfree(input->name);
+		kfree(input->phys);
+		input_free_device(input);
+
+		devices[dindex].indio_dev = NULL;
+		devices[dindex].input = NULL;
+	}
+
+	mutex_unlock(&inputbridge_device_mutex);
+}
+
+MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
+MODULE_DESCRIPTION("Bridge to present Industrial I/O accelerometers as properly oriented Input devices");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/industrialio-inputbridge.h b/drivers/iio/industrialio-inputbridge.h
new file mode 100644
index 000000000000..1363b10ab3f7
--- /dev/null
+++ b/drivers/iio/industrialio-inputbridge.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * The Industrial I/O core, bridge to input devices
+ *
+ * Copyright (c) 2016-2019 Golden Delicious Computers GmbH&Co. KG
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#if defined(CONFIG_IIO_INPUT_BRIDGE)
+
+extern int iio_device_register_inputbridge(struct iio_dev *indio_dev);
+extern void iio_device_unregister_inputbridge(struct iio_dev *indio_dev);
+
+#else
+
+static inline int iio_device_register_inputbridge(struct iio_dev *indio_dev)
+{
+	return 0;
+}
+
+static inline void iio_device_unregister_inputbridge(struct iio_dev *indio_dev)
+{
+}
+
+#endif
-- 
2.19.1

[RFC 2/4] iio: input-bridge: add iio-input-bridge to Makefile

[H. Nikolaus Schaller]

Add the iio-input-bridge to the Makefile.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/iio/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index cb5993251381..d695e5a27da5 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_IIO) += industrialio.o
 industrialio-y := industrialio-core.o industrialio-event.o inkern.o
 industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
+industrialio-$(CONFIG_IIO_INPUT_BRIDGE) += industrialio-inputbridge.o
 
 obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
 obj-$(CONFIG_IIO_SW_DEVICE) += industrialio-sw-device.o
-- 
2.19.1

[RFC 3/4] iio: input-bridge: add IIO_INPUT_BRIDGE kernel config option

[H. Nikolaus Schaller]

Add the iio-input-bridge to the Kconfig.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/iio/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index d08aeb41cd07..d85afe002613 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -68,6 +68,13 @@ config IIO_TRIGGERED_EVENT
 	help
 	  Provides helper functions for setting up triggered events.
 
+config IIO_INPUT_BRIDGE
+	bool "Enable accelerometer bridge to input driver"
+	help
+	  Provides a /dev/input/event* device for accelerometers
+	  to use as a 3D input device, e.g. for gaming or auto-rotation
+	  of screen contents.
+
 source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/afe/Kconfig"
-- 
2.19.1

[RFC 4/4] iio: input-bridge: make the iio-input-bridge driver called by iio-core

[H. Nikolaus Schaller]

This adds the iio-input-bridge to iio device (un)registering so
that accelerometers are located and made present an input event
device.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/iio/industrialio-core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 4700fd5d8c90..81f412b41a78 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -29,6 +29,7 @@
 #include <linux/iio/iio.h>
 #include "iio_core.h"
 #include "iio_core_trigger.h"
+#include "industrialio-inputbridge.h"
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
 #include <linux/iio/buffer.h>
@@ -1723,6 +1724,15 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 	if (ret < 0)
 		goto error_unreg_eventset;
 
+	ret = iio_device_register_inputbridge(indio_dev);
+	if (ret) {
+		dev_err(indio_dev->dev.parent,
+			"Failed to register as input driver\n");
+		device_del(&indio_dev->dev);
+
+		return ret;
+	}
+
 	return 0;
 
 error_unreg_eventset:
@@ -1745,6 +1755,8 @@ void iio_device_unregister(struct iio_dev *indio_dev)
 {
 	mutex_lock(&indio_dev->info_exist_lock);
 
+	iio_device_unregister_inputbridge(indio_dev);
+
 	cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
 
 	iio_device_unregister_debugfs(indio_dev);
-- 
2.19.1

Re: [RFC 0/4] iio-input-bridge so that accelerometers which only have an iio driver can still present evdev input events

[Jonathan Cameron]

On Mon, 18 Mar 2019 21:39:30 +0100
"H. Nikolaus Schaller" <hns@goldelico.com> wrote:

	
> Some user spaces (e.g. some Android) use /dev/input/event* for handling the 3D
> position of the device with respect to the center of gravity (earth). This can
> be used for gaming input, rotation of screens etc.
> 
> This should be the standard because this interface is an abstraction of how
> this data is acquired from sensor chips. Sensor chips may be connected through
> different interfaces and in different positions. They may also have different
> parameters. And, if a chip is replaced by a different one, the values reported
> by the device position interface should remain the same.
> 
> But nowadays, new accelerometer chips usually just get iio drivers and rarely
> an evdev input driver.
> 
> Therefore we need something like a protocol stack: input device vs. raw data.
> It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
> input events vs. raw gpio or raw USB access.
> 
> This patch set bridges the gap between raw iio data and the input device abstraction
> so that accelerometer measurements can also be presented as X/Y/Z accelerometer
> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
> 
Hi,

I've kind of run out of time today and want to give this a detailed look.

In the meantime a few initial comments.

1. Resend the whole series cc'ing the linux-input list and maintainer.
2. In the vast majority of devices the interrupt pin is connected for an
   accelerometer. and they support data ready signals.  My gut feeling is
   that should be the preferred mode.  It was for that use case that we originally
   put all the demux and multiple buffer code in IIO. The original series for
   that actually had an input bridge.
https://lore.kernel.org/linux-iio/1351679431-7963-5-git-send-email-jic23@kernel.org/

3. It's going to be a hard sell to have it 'always' map an accelerometer in IIO
   to an input device when configured.  There are lots of other accelerometer use
   cases where this would be crazy.  In the repost, give some comentary perhaps on
   the following options:
   a) Explicit binding - like the iio-hwmon bridge.
   b) A userspace bridge (I even wrote one of those years ago using uevent)
   c) Some sort of userspace triggered way of creating these on demand.

4. Input has polled modes. Don't reinvent them.

5. The patch break up is very very random.  Just have one patch :)

Anyhow, I'll take a look in detail but may be a little while.

Thanks,

Jonathan


> 
> H. Nikolaus Schaller (4):
>   iio: input-bridge: optionally bridge iio acceleometers to create a
>     /dev/input
>   iio: input-bridge: add iio-input-bridge to Makefile
>   iio: input-bridge: add IIO_INPUT_BRIDGE kernel config option
>   iio: input-bridge: make the iio-input-bridge driver called by iio-core
> 
>  drivers/iio/Kconfig                    |   7 +
>  drivers/iio/Makefile                   |   1 +
>  drivers/iio/industrialio-core.c        |  12 +
>  drivers/iio/industrialio-inputbridge.c | 420 +++++++++++++++++++++++++
>  drivers/iio/industrialio-inputbridge.h |  28 ++
>  5 files changed, 468 insertions(+)
>  create mode 100644 drivers/iio/industrialio-inputbridge.c
>  create mode 100644 drivers/iio/industrialio-inputbridge.h
> 

Re: [RFC 0/4] iio-input-bridge so that accelerometers which only have an iio driver can still present evdev input events

[H. Nikolaus Schaller]

Hi Jonathan,

> Am 24.03.2019 um 19:29 schrieb Jonathan Cameron <jic23@kernel.org>:
> 
> On Mon, 18 Mar 2019 21:39:30 +0100
> "H. Nikolaus Schaller" <hns@goldelico.com> wrote:
> 
> 	
>> Some user spaces (e.g. some Android) use /dev/input/event* for handling the 3D
>> position of the device with respect to the center of gravity (earth). This can
>> be used for gaming input, rotation of screens etc.
>> 
>> This should be the standard because this interface is an abstraction of how
>> this data is acquired from sensor chips. Sensor chips may be connected through
>> different interfaces and in different positions. They may also have different
>> parameters. And, if a chip is replaced by a different one, the values reported
>> by the device position interface should remain the same.
>> 
>> But nowadays, new accelerometer chips usually just get iio drivers and rarely
>> an evdev input driver.
>> 
>> Therefore we need something like a protocol stack: input device vs. raw data.
>> It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
>> input events vs. raw gpio or raw USB access.
>> 
>> This patch set bridges the gap between raw iio data and the input device abstraction
>> so that accelerometer measurements can also be presented as X/Y/Z accelerometer
>> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
>> 
> Hi,
> 
> I've kind of run out of time today and want to give this a detailed look.

No problem, please take the time it needs.

> 
> In the meantime a few initial comments.
> 
> 1. Resend the whole series cc'ing the linux-input list and maintainer.

Ok!

> 2. In the vast majority of devices the interrupt pin is connected for an
>   accelerometer. and they support data ready signals.  My gut feeling is
>   that should be the preferred mode.

Mine too, and the commit message 1/4 describes this by 

"If it runs, the driver polls the device(s) once every 100 ms. A mode where the
iio device defines the update rate is not implemented and for further study."

But I didn't manage to get this correctly set up for in-kernel iio on demand
(start/stop when a process does an open/close - it was simpler to start/stop
polling).

Here I am lacking some understanding of details of the subsystems and their
capabilities.

Therefore I focussed on the polling case to leave the interrupt driven case
for later improvements. Also, since 100ms doesn't seem to be a big resource
burden.

I am also not sure if we really want to process every fidget detected by the
raw sensor to the input subsystem. This could require more resources than
polling with 100ms distance.

>  It was for that use case that we originally
>   put all the demux and multiple buffer code in IIO. The original series for
>   that actually had an input bridge.
> https://lore.kernel.org/linux-iio/1351679431-7963-5-git-send-email-jic23@kernel.org/

Oh, nice! I wasn't aware of that. If I had known, we might have based our
code on it.

It seems to be a good blueprint to understand how to set up the callbacks.

What I do not understand is where the "iio map" comes from in your approach.

From device tree? That would IMHO be against the rule that DTS should describe
hardware but bridging data to a different user-space interface is not related
to hardware description.

So our approach does not need a specific mapping.

> 
> 3. It's going to be a hard sell to have it 'always' map an accelerometer in IIO
>   to an input device when configured.  There are lots of other accelerometer use
>   cases where this would be crazy.  In the repost, give some comentary perhaps on
>   the following options:
>   a) Explicit binding - like the iio-hwmon bridge.
>   b) A userspace bridge (I even wrote one of those years ago using uevent)
>   c) Some sort of userspace triggered way of creating these on demand.

Basically, if you have one of these many other use cases, one can just keep this
bridge unconfigured. Then, it does not eat up memory or code space or processor
cycles.

Its only use case is to map iio accelerometers to input devices which are
really used as input devices, i.e. gaming, device orientation etc. Other use
cases (e.g. industrial sensor grid data aggregator) should not consider this as
an alternative interface and use e.g. libiio of course.

Those (handheld) devices in mind usually have only a single accelerometer chip, but
there are exceptions like the GTA04 and the Pyra which can have two of them (with
different precision, mounting orientation and functions). Maybe there are some
devices with more than two. So these iio devices should be reported separately
but the data (X,Y,Z) should mainly agree on input device level.

In the two-sensor case, our proposed driver simply creates two distinct /dev/input/event
files which can be given fixed file names by udev instead of inventing a new trigger
to create these on demand. IMHO it should not be handled differently from plugging in
multiple mice or joysticks or other gaming devices to USB.

And, this driver converts "raw" accelerometer data into a higher abstraction level
input event in a way that user-space becomes independent of the specific chip set
and its orientation. This is like hiding different flash hardware interfaces
by MTD or file systems. All the hardware details are hidden and shielded. Or it
is like mapping gpios or alternatively an i2c scanner chio into standardized key codes.

Finally, there is no polling (in this polling setup) if no process opens the input device.
So the only waste of 'always' mapping 'all' accelerometers but not using them are some
data structure allocations in the kernel and duration of probe().

> 4. Input has polled modes. Don't reinvent them.

Haven't heard of, but that is what a RFC is good for.

Anyways, if we use iio interrupts we don't need it, because the information flow
should go from the chip to the iio driver to the input bridge to the input subsystem
and only then notify user space (wake up the process running a select on the event
device file).

Now, there is something I have learned from studying your comment and struct input_dev.

Although I did not find polled modes, I can simplify our approach significantly
since we do not have to count open/close ourselves and define a mutex for that.

This is already done by the input_dev. So I should improve this part of our patches
before resending to input list. Thanks for inpsiring this.

> 5. The patch break up is very very random.  Just have one patch :)

Well, it tries to have small pieces in a specific order that you can bisect and
do not run into compile problems (e.g. adding Makefile before code).

But I can squash it. Seems to make sense since it is not really useful to know
which of these pieces breaks other things. It can only be the driver
and bisect will still pin-point it.

> 
> Anyhow, I'll take a look in detail but may be a little while.

No reason to hurry. We can wait.

> 
> Thanks,
> 
> Jonathan

Thanks and BR,
Nikolaus

> 
> 
>> 
>> H. Nikolaus Schaller (4):
>>  iio: input-bridge: optionally bridge iio acceleometers to create a
>>    /dev/input
>>  iio: input-bridge: add iio-input-bridge to Makefile
>>  iio: input-bridge: add IIO_INPUT_BRIDGE kernel config option
>>  iio: input-bridge: make the iio-input-bridge driver called by iio-core
>> 
>> drivers/iio/Kconfig                    |   7 +
>> drivers/iio/Makefile                   |   1 +
>> drivers/iio/industrialio-core.c        |  12 +
>> drivers/iio/industrialio-inputbridge.c | 420 +++++++++++++++++++++++++
>> drivers/iio/industrialio-inputbridge.h |  28 ++
>> 5 files changed, 468 insertions(+)
>> create mode 100644 drivers/iio/industrialio-inputbridge.c
>> create mode 100644 drivers/iio/industrialio-inputbridge.h
>> 
> 

Re: [RFC 0/4] iio-input-bridge so that accelerometers which only have an iio driver can still present evdev input events

[H. Nikolaus Schaller]

Hi all,
I'll send a single v2 patch in some minutes.

Changes compared to this v1:
* add linux-input list and maintainers for discussion
* use input-polldev
* allocate one mapping record for each iio device instead of trying to manage that in some array

> Am 28.03.2019 um 18:42 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Hi Jonathan,
> 
>> Am 24.03.2019 um 19:29 schrieb Jonathan Cameron <jic23@kernel.org>:
>> 
>> On Mon, 18 Mar 2019 21:39:30 +0100
>> "H. Nikolaus Schaller" <hns@goldelico.com> wrote:
>> 
>> 	
>>> Some user spaces (e.g. some Android) use /dev/input/event* for handling the 3D
>>> position of the device with respect to the center of gravity (earth). This can
>>> be used for gaming input, rotation of screens etc.
>>> 
>>> This should be the standard because this interface is an abstraction of how
>>> this data is acquired from sensor chips. Sensor chips may be connected through
>>> different interfaces and in different positions. They may also have different
>>> parameters. And, if a chip is replaced by a different one, the values reported
>>> by the device position interface should remain the same.
>>> 
>>> But nowadays, new accelerometer chips usually just get iio drivers and rarely
>>> an evdev input driver.
>>> 
>>> Therefore we need something like a protocol stack: input device vs. raw data.
>>> It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
>>> input events vs. raw gpio or raw USB access.
>>> 
>>> This patch set bridges the gap between raw iio data and the input device abstraction
>>> so that accelerometer measurements can also be presented as X/Y/Z accelerometer
>>> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
>>> 
>> Hi,
>> 
>> I've kind of run out of time today and want to give this a detailed look.
> 
> No problem, please take the time it needs.
> 
>> 
>> In the meantime a few initial comments.
>> 
>> 1. Resend the whole series cc'ing the linux-input list and maintainer.
> 
> Ok!

Done.

> 
>> 2. In the vast majority of devices the interrupt pin is connected for an
>>  accelerometer. and they support data ready signals.  My gut feeling is
>>  that should be the preferred mode.
> 
> Mine too, and the commit message 1/4 describes this by 
> 
> "If it runs, the driver polls the device(s) once every 100 ms. A mode where the
> iio device defines the update rate is not implemented and for further study."
> 
> But I didn't manage to get this correctly set up for in-kernel iio on demand
> (start/stop when a process does an open/close - it was simpler to start/stop
> polling).
> 
> Here I am lacking some understanding of details of the subsystems and their
> capabilities.
> 
> Therefore I focussed on the polling case to leave the interrupt driven case
> for later improvements. Also, since 100ms doesn't seem to be a big resource
> burden.
> 
> I am also not sure if we really want to process every fidget detected by the
> raw sensor to the input subsystem. This could require more resources than
> polling with 100ms distance.

Because this is a quite fundamental design decision (do we call input_register_device
or input_register_polled_device?) and the above expressed concern about noise
triggering more input events than user-space may want to see, I have stayed
with low-speed polling for the time being.

> 
>> It was for that use case that we originally
>>  put all the demux and multiple buffer code in IIO. The original series for
>>  that actually had an input bridge.
>> https://lore.kernel.org/linux-iio/1351679431-7963-5-git-send-email-jic23@kernel.org/
> 
> Oh, nice! I wasn't aware of that. If I had known, we might have based our
> code on it.
> 
> It seems to be a good blueprint to understand how to set up the callbacks.
> 
> What I do not understand is where the "iio map" comes from in your approach.
> 
> From device tree? That would IMHO be against the rule that DTS should describe
> hardware but bridging data to a different user-space interface is not related
> to hardware description.
> 
> So our approach does not need a specific mapping.
> 
>> 
>> 3. It's going to be a hard sell to have it 'always' map an accelerometer in IIO
>>  to an input device when configured.  There are lots of other accelerometer use
>>  cases where this would be crazy.  In the repost, give some comentary perhaps on
>>  the following options:
>>  a) Explicit binding - like the iio-hwmon bridge.
>>  b) A userspace bridge (I even wrote one of those years ago using uevent)
>>  c) Some sort of userspace triggered way of creating these on demand.
> 
> Basically, if you have one of these many other use cases, one can just keep this
> bridge unconfigured. Then, it does not eat up memory or code space or processor
> cycles.
> 
> Its only use case is to map iio accelerometers to input devices which are
> really used as input devices, i.e. gaming, device orientation etc. Other use
> cases (e.g. industrial sensor grid data aggregator) should not consider this as
> an alternative interface and use e.g. libiio of course.
> 
> Those (handheld) devices in mind usually have only a single accelerometer chip, but
> there are exceptions like the GTA04 and the Pyra which can have two of them (with
> different precision, mounting orientation and functions). Maybe there are some
> devices with more than two. So these iio devices should be reported separately
> but the data (X,Y,Z) should mainly agree on input device level.
> 
> In the two-sensor case, our proposed driver simply creates two distinct /dev/input/event
> files which can be given fixed file names by udev instead of inventing a new trigger
> to create these on demand. IMHO it should not be handled differently from plugging in
> multiple mice or joysticks or other gaming devices to USB.
> 
> And, this driver converts "raw" accelerometer data into a higher abstraction level
> input event in a way that user-space becomes independent of the specific chip set
> and its orientation. This is like hiding different flash hardware interfaces
> by MTD or file systems. All the hardware details are hidden and shielded. Or it
> is like mapping gpios or alternatively an i2c scanner chio into standardized key codes.
> 
> Finally, there is no polling (in this polling setup) if no process opens the input device.
> So the only waste of 'always' mapping 'all' accelerometers but not using them are some
> data structure allocations in the kernel and duration of probe().
> 
>> 4. Input has polled modes. Don't reinvent them.
> 
> Haven't heard of, but that is what a RFC is good for.
> 
> Anyways, if we use iio interrupts we don't need it, because the information flow
> should go from the chip to the iio driver to the input bridge to the input subsystem
> and only then notify user space (wake up the process running a select on the event
> device file).
> 
> Now, there is something I have learned from studying your comment and struct input_dev.
> 
> Although I did not find polled modes, I can simplify our approach significantly
> since we do not have to count open/close ourselves and define a mutex for that.
> 
> This is already done by the input_dev. So I should improve this part of our patches
> before resending to input list. Thanks for inpsiring this.

I finally found it. It is an input-polldev wrapper.

I have reworked the driver in polling mode to use that now. It has simplified the
code a lot.

Thanks again!

Nikolaus

> 
>> 5. The patch break up is very very random.  Just have one patch :)
> 
> Well, it tries to have small pieces in a specific order that you can bisect and
> do not run into compile problems (e.g. adding Makefile before code).
> 
> But I can squash it. Seems to make sense since it is not really useful to know
> which of these pieces breaks other things. It can only be the driver
> and bisect will still pin-point it.
> 
>> 
>> Anyhow, I'll take a look in detail but may be a little while.
> 
> No reason to hurry. We can wait.

> 
>> 
>> Thanks,
>> 
>> Jonathan
> 
> Thanks and BR,
> Nikolaus
> 
>> 
>> 
>>> 
>>> H. Nikolaus Schaller (4):
>>> iio: input-bridge: optionally bridge iio acceleometers to create a
>>>   /dev/input
>>> iio: input-bridge: add iio-input-bridge to Makefile
>>> iio: input-bridge: add IIO_INPUT_BRIDGE kernel config option
>>> iio: input-bridge: make the iio-input-bridge driver called by iio-core
>>> 
>>> drivers/iio/Kconfig                    |   7 +
>>> drivers/iio/Makefile                   |   1 +
>>> drivers/iio/industrialio-core.c        |  12 +
>>> drivers/iio/industrialio-inputbridge.c | 420 +++++++++++++++++++++++++
>>> drivers/iio/industrialio-inputbridge.h |  28 ++
>>> 5 files changed, 468 insertions(+)
>>> create mode 100644 drivers/iio/industrialio-inputbridge.c
>>> create mode 100644 drivers/iio/industrialio-inputbridge.h
>>> 
>> 
> 

Re: [RFC 0/4] iio-input-bridge so that accelerometers which only have an iio driver can still present evdev input events

[Jonathan Cameron]

On Thu, 28 Mar 2019 18:42:44 +0100
"H. Nikolaus Schaller" <hns@goldelico.com> wrote:

> Hi Jonathan,
> 
> > Am 24.03.2019 um 19:29 schrieb Jonathan Cameron <jic23@kernel.org>:
> > 
> > On Mon, 18 Mar 2019 21:39:30 +0100
> > "H. Nikolaus Schaller" <hns@goldelico.com> wrote:
> > 
> > 	  
> >> Some user spaces (e.g. some Android) use /dev/input/event* for handling the 3D
> >> position of the device with respect to the center of gravity (earth). This can
> >> be used for gaming input, rotation of screens etc.
> >> 
> >> This should be the standard because this interface is an abstraction of how
> >> this data is acquired from sensor chips. Sensor chips may be connected through
> >> different interfaces and in different positions. They may also have different
> >> parameters. And, if a chip is replaced by a different one, the values reported
> >> by the device position interface should remain the same.
> >> 
> >> But nowadays, new accelerometer chips usually just get iio drivers and rarely
> >> an evdev input driver.
> >> 
> >> Therefore we need something like a protocol stack: input device vs. raw data.
> >> It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
> >> input events vs. raw gpio or raw USB access.
> >> 
> >> This patch set bridges the gap between raw iio data and the input device abstraction
> >> so that accelerometer measurements can also be presented as X/Y/Z accelerometer
> >> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
> >>   
> > Hi,
> > 
> > I've kind of run out of time today and want to give this a detailed look.  
> 
> No problem, please take the time it needs.
> 
> > 
> > In the meantime a few initial comments.
> > 
> > 1. Resend the whole series cc'ing the linux-input list and maintainer.  
> 
> Ok!
> 
> > 2. In the vast majority of devices the interrupt pin is connected for an
> >   accelerometer. and they support data ready signals.  My gut feeling is
> >   that should be the preferred mode.  
> 
> Mine too, and the commit message 1/4 describes this by 
> 
> "If it runs, the driver polls the device(s) once every 100 ms. A mode where the
> iio device defines the update rate is not implemented and for further study."

Absolutely, I saw that, just wanted to encourage this avenue ;) It's particularly
good if we have a controllable frequency device and can have it self clock
at a rather that makes sense for human input.  Also lets the gamers run at
2kHz if they really want to ;)

> 
> But I didn't manage to get this correctly set up for in-kernel iio on demand
> (start/stop when a process does an open/close - it was simpler to start/stop
> polling).
> 
> Here I am lacking some understanding of details of the subsystems and their
> capabilities.
> 
> Therefore I focussed on the polling case to leave the interrupt driven case
> for later improvements. Also, since 100ms doesn't seem to be a big resource
> burden.

One thing the IIO consumer interfaces have always lacked is nice fallback
between the buffered / interrupt mode and polled read_raw modes.  In theory
if should be possible to elegantly assume that we have a self clocking sensor
but if it doesn't actually support that, fall back to polling and still have
it come out of the buffered interface.  The snag has always been that we
currently end up adding, non-trivial, code to the IIO drivers to support
buffered modes and we don't want to put that burden on all drivers.

> 
> I am also not sure if we really want to process every fidget detected by the
> raw sensor to the input subsystem. This could require more resources than
> polling with 100ms distance.

Agreed. Thankfully most such sensors also have frequency controls so we
can normally get to a sensible level.  Input gets to do all the filtering
etc beyond that.

> 
> >  It was for that use case that we originally
> >   put all the demux and multiple buffer code in IIO. The original series for
> >   that actually had an input bridge.
> > https://lore.kernel.org/linux-iio/1351679431-7963-5-git-send-email-jic23@kernel.org/  
> 
> Oh, nice! I wasn't aware of that. If I had known, we might have based our
> code on it.

You probably noticed in the comments, it was never finished!  The problem is that
all my boards are headless so I never cared ;)  Good to have someone looking
at this problem who does.

> 
> It seems to be a good blueprint to understand how to set up the callbacks.
> 
> What I do not understand is where the "iio map" comes from in your approach.
> 
> From device tree? That would IMHO be against the rule that DTS should describe
> hardware but bridging data to a different user-space interface is not related
> to hardware description.

This has long been an 'argument' on what exactly makes up hardware description.
The problem is that usecases really are hardware description.  There has to
be some means fo knowing what the thing is for, to dictate what software
support we supply.  Whether that is in userspace or kernel or as you have
done is assumed "if plausible" is an open question.

Back when this was originally proposed, there weren't clear rules on this in
DT.  It's hard to argue this is any different from hwmon, though Mark in
particular really dislikes the iio-hwmon interface.  It has the problem that
no one has ever come up with a better one.  Only potential option is to create
abstract devices to represent the fact a given ADC channel is used for
measuring a 'board voltage' or something like that.  They provide no useful
meaning that is actually different from putting them in the iio-hwmon bindings.

The same is kind of true here with the reality being we need to know what
the 'driver' of the acceleration is.  

Could have a binding for 'human' vs 'bridge' vs 'car' :)

> 
> So our approach does not need a specific mapping.
> 
> > 
> > 3. It's going to be a hard sell to have it 'always' map an accelerometer in IIO
> >   to an input device when configured.  There are lots of other accelerometer use
> >   cases where this would be crazy.  In the repost, give some comentary perhaps on
> >   the following options:
> >   a) Explicit binding - like the iio-hwmon bridge.
> >   b) A userspace bridge (I even wrote one of those years ago using uevent)
> >   c) Some sort of userspace triggered way of creating these on demand.  
> 
> Basically, if you have one of these many other use cases, one can just keep this
> bridge unconfigured. Then, it does not eat up memory or code space or processor
> cycles.

That's a non starter.   Lots of people aren't going to tune their kernel to
'unconfigure' some support because they want to turn a feature off.  Take
a raspberry pi for instance. If it has an accelerometer, the question of whether
anyone wants to use it is down to whether there is a screen attached or not.

> 
> Its only use case is to map iio accelerometers to input devices which are
> really used as input devices, i.e. gaming, device orientation etc. Other use
> cases (e.g. industrial sensor grid data aggregator) should not consider this as
> an alternative interface and use e.g. libiio of course.
> 
> Those (handheld) devices in mind usually have only a single accelerometer chip, but
> there are exceptions like the GTA04 and the Pyra which can have two of them (with
> different precision, mounting orientation and functions). Maybe there are some
> devices with more than two. So these iio devices should be reported separately
> but the data (X,Y,Z) should mainly agree on input device level.

There are lots of two sensor cases where the sensors don't agree, mainly because
they are connected by a hinge (screen and keyboard on laptops for example).
Doesn't matter really though.

> 
> In the two-sensor case, our proposed driver simply creates two distinct /dev/input/event
> files which can be given fixed file names by udev instead of inventing a new trigger
> to create these on demand. IMHO it should not be handled differently from plugging in
> multiple mice or joysticks or other gaming devices to USB.

Alternative UDEV can pick up on a new IIO device and poke an interface to register
an input device from it.  We've talked about allowing iio_maps to be instantiated
via configfs and it may be that is the modern choice for how to do this.

> 
> And, this driver converts "raw" accelerometer data into a higher abstraction level
> input event in a way that user-space becomes independent of the specific chip set
> and its orientation. This is like hiding different flash hardware interfaces
> by MTD or file systems. All the hardware details are hidden and shielded. Or it
> is like mapping gpios or alternatively an i2c scanner chio into standardized key codes.
> 
> Finally, there is no polling (in this polling setup) if no process opens the input device.
> So the only waste of 'always' mapping 'all' accelerometers but not using them are some
> data structure allocations in the kernel and duration of probe().
Sure, it's not a heavy burden - but it is conceptually dubious to register an
interface because 'it might' be wanted.  I'd like Dmitry and the input lists
view on this (plus others in IIO!)

> 
> > 4. Input has polled modes. Don't reinvent them.  
> 
> Haven't heard of, but that is what a RFC is good for.
> 
> Anyways, if we use iio interrupts we don't need it, because the information flow
> should go from the chip to the iio driver to the input bridge to the input subsystem
> and only then notify user space (wake up the process running a select on the event
> device file).
> 
> Now, there is something I have learned from studying your comment and struct input_dev.
> 
> Although I did not find polled modes, I can simplify our approach significantly
> since we do not have to count open/close ourselves and define a mutex for that.
> 
> This is already done by the input_dev. So I should improve this part of our patches
> before resending to input list. Thanks for inpsiring this.
> 
I see from the next mail you found the polled code.  Dmitry would have kicked back
on that if you hadn't (we have had this a few times before ;)

> > 5. The patch break up is very very random.  Just have one patch :)  
> 
> Well, it tries to have small pieces in a specific order that you can bisect and
> do not run into compile problems (e.g. adding Makefile before code).
> 
> But I can squash it. Seems to make sense since it is not really useful to know
> which of these pieces breaks other things. It can only be the driver
> and bisect will still pin-point it.

Quite. It's really one new function so keeping it in one patch is fine.

Patch break up of large new changes is always an interesting one.  Sometimes
it's much easier to review a 2000 line patch than 5 smaller ones, but
conversely a 2000 line patch doesn't always fit into lunch time ;)
> 
> > 
> > Anyhow, I'll take a look in detail but may be a little while.  
> 
> No reason to hurry. We can wait.
> 
> > 
> > Thanks,
> > 
> > Jonathan  
> 
> Thanks and BR,
> Nikolaus
Great to have this 'hole' filled at last. I've been moaning about it for
years but never actually got interested enough to do it myself!

Jonathan

> 
> > 
> >   
> >> 
> >> H. Nikolaus Schaller (4):
> >>  iio: input-bridge: optionally bridge iio acceleometers to create a
> >>    /dev/input
> >>  iio: input-bridge: add iio-input-bridge to Makefile
> >>  iio: input-bridge: add IIO_INPUT_BRIDGE kernel config option
> >>  iio: input-bridge: make the iio-input-bridge driver called by iio-core
> >> 
> >> drivers/iio/Kconfig                    |   7 +
> >> drivers/iio/Makefile                   |   1 +
> >> drivers/iio/industrialio-core.c        |  12 +
> >> drivers/iio/industrialio-inputbridge.c | 420 +++++++++++++++++++++++++
> >> drivers/iio/industrialio-inputbridge.h |  28 ++
> >> 5 files changed, 468 insertions(+)
> >> create mode 100644 drivers/iio/industrialio-inputbridge.c
> >> create mode 100644 drivers/iio/industrialio-inputbridge.h
> >>   
> >   
>