2023-07-19 08:37:36

by Waqar Hameed

[permalink] [raw]
Subject: [PATCH v4 0/3] Add driver for Murata IRS-D200

Murata IRS-D200 is a PIR sensor for human detection. In this series we
add devicetree bindings and an IIO driver with support for triggered
buffer and events. Link to the datasheet should be added to the
devicetree bindings, when that is available.

Changes in v4:

[iio]
* Use `iio_trigger_poll_nested()` instead of `iio_trigger_poll_chained()`.
(Remnant from old tree... Sorry!)

Link to v3: https://lore.kernel.org/lkml/[email protected]/

Changes in v3:

[dt-bindings]
* Add "Reviewed-by" footer in commit message.

[iio]
* Use `__le16` for buffers in `regmap_bulk_read/write()`.
* Use `8` instead of `BIT(3)` in `irsd200_write_nr_count()`.
* Rename `val` to `tmp` in `irsd200_write_event_config()`.
* Return `0` on success in `irsd200_write_event_config()`.
* Return `IRQ_HANDLED` instead of `IRQ_NONE` in error paths.
* Return `IRQ_NONE` when not clearing anything in
`irsd200_irq_thread()`.
* Use `devm_regulator_get_enable()` in probe.

Link to v2: https://lore.kernel.org/lkml/[email protected]/

Changes in v2:

[dt-bindings]
* Remove "bindings for" in commit subject.
* Remove superfluous yaml block style indicator ('|') for
`description:`.
* Change node name in example from `pir` to `proximity`.
* Add required `vdd-supply` property.

[iio]
* Add event enums for running period and count.
* Use `set_trigger_state` callback instead of `iio_buffer_setup_ops`'s
`predisable` and `postenable`.
* Use `regmap_bulk_read()` in `irsd200_read_data()` and
`irsd200_read_timer()`.
* Use `regmap_bulk_write()` in `irsd200_write_timer()`.
* Remove comment for macro `IRS_UPPER_COUNT()`.
* Move `IIO_EV_INFO_LOW/HIGH_PASS_FILTER_3DB` from `iio_event_spec` to
`iio_chan_spec`.
* Ignore timer (`IRS_INTR_TIMER`) interrupts.
* Clarify comment on `ssleep(3)` in `irsd200_write_data_rate()`.
* Only check for non-zero return values from `regmap` functions (as
opposed to `ret < 0`).
* Add macro defines for operation states.
* Remove fix size in static const array declarations.
* Remove unnecessary `ret` variable in `irsd200_write_raw()`.
* Remove comments in `irsd200_event_spec[]`.
* Remove unnecessary call to `i2c_set_clientdata()` in probe.
* Use `dev_err_probe()` everywhere in probe.
* Remove unnecessary braces around if statement in probe.
* Get and enable regulator in probe.

Link to v1: https://lore.kernel.org/lkml/[email protected]/

Waqar Hameed (3):
dt-bindings: iio: proximity: Add Murata IRS-D200
iio: Add event enums for running period and count
iio: Add driver for Murata IRS-D200

Documentation/ABI/testing/sysfs-bus-iio | 16 +
.../iio/proximity/murata,irsd200.yaml | 60 ++
drivers/iio/industrialio-event.c | 2 +
drivers/iio/proximity/Kconfig | 12 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/irsd200.c | 962 ++++++++++++++++++
include/linux/iio/types.h | 2 +
7 files changed, 1055 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/murata,irsd200.yaml
create mode 100644 drivers/iio/proximity/irsd200.c


base-commit: 3f01e9fed8454dcd89727016c3e5b2fbb8f8e50c
--
2.30.2



2023-07-19 08:39:54

by Waqar Hameed

[permalink] [raw]
Subject: [PATCH v4 2/3] iio: Add event enums for running period and count

There are devices (such as Murata IRS-D200 PIR proximity sensor) that
check the data signal with a running period. I.e. for a specified time,
they count the number of conditions that have occurred, and then signal
if that is more than a specified amount.

`IIO_EV_INFO_PERIOD` resets when the condition no longer is true and is
therefore not suitable for these devices. Add a new `iio_event_info`
`IIO_EV_INFO_RUNNING_PERIOD` that can be used as a running period. Also
add a new `IIO_EV_INFO_RUNNING_COUNT` that can be used to specify the
number of conditions that must occur during this running period.

Signed-off-by: Waqar Hameed <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-iio | 16 ++++++++++++++++
drivers/iio/industrialio-event.c | 2 ++
include/linux/iio/types.h | 2 ++
3 files changed, 20 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 7140e8e7313f..a2854dc9a839 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2163,3 +2163,19 @@ Contact: [email protected]
Description:
An example format is 16-bytes, 2-digits-per-byte, HEX-string
representing the sensor unique ID number.
+
+What: /sys/.../events/in_proximity_thresh_either_runningperiod
+KernelVersion: 6.6
+Contact: [email protected]
+Description:
+ A running period of time (in seconds) for which
+ in_proximity_thresh_either_runningcount amount of conditions
+ must occur before an event is generated. If direction is not
+ specified then this period applies to both directions.
+
+What: /sys/.../events/in_proximity_thresh_either_runningcount
+KernelVersion: 6.6
+Contact: [email protected]
+Description:
+ Number of conditions that must occur, during a running
+ period, before an event is generated.
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index f77ce49d4c36..19f7a91157ee 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -252,6 +252,8 @@ static const char * const iio_ev_info_text[] = {
[IIO_EV_INFO_TIMEOUT] = "timeout",
[IIO_EV_INFO_RESET_TIMEOUT] = "reset_timeout",
[IIO_EV_INFO_TAP2_MIN_DELAY] = "tap2_min_delay",
+ [IIO_EV_INFO_RUNNING_PERIOD] = "runningperiod",
+ [IIO_EV_INFO_RUNNING_COUNT] = "runningcount",
};

static enum iio_event_direction iio_ev_attr_dir(struct iio_dev_attr *attr)
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 82faa98c719a..117bde7d6ad7 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -19,6 +19,8 @@ enum iio_event_info {
IIO_EV_INFO_TIMEOUT,
IIO_EV_INFO_RESET_TIMEOUT,
IIO_EV_INFO_TAP2_MIN_DELAY,
+ IIO_EV_INFO_RUNNING_PERIOD,
+ IIO_EV_INFO_RUNNING_COUNT,
};

#define IIO_VAL_INT 1
--
2.30.2


2023-07-19 08:49:16

by Waqar Hameed

[permalink] [raw]
Subject: [PATCH v4 3/3] iio: Add driver for Murata IRS-D200

Murata IRS-D200 is a PIR sensor for human detection. It has support for
raw data measurements and detection event notification.

Add a driver with support for triggered buffer and events. Map the
various settings to the `iio` framework, e.g. threshold values, sampling
frequency, filter frequencies etc.

Signed-off-by: Waqar Hameed <[email protected]>
---
drivers/iio/proximity/Kconfig | 12 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/irsd200.c | 962 ++++++++++++++++++++++++++++++++
3 files changed, 975 insertions(+)
create mode 100644 drivers/iio/proximity/irsd200.c

diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index 0e5c17530b8b..2ca3b0bc5eba 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -32,6 +32,18 @@ config CROS_EC_MKBP_PROXIMITY
To compile this driver as a module, choose M here: the
module will be called cros_ec_mkbp_proximity.

+config IRSD200
+ tristate "Murata IRS-D200 PIR sensor"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ select REGMAP_I2C
+ depends on I2C
+ help
+ Say Y here to build a driver for the Murata IRS-D200 PIR sensor.
+
+ To compile this driver as a module, choose M here: the module will be
+ called irsd200.
+
config ISL29501
tristate "Intersil ISL29501 Time Of Flight sensor"
depends on I2C
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index cc838bb5408a..f36598380446 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -6,6 +6,7 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AS3935) += as3935.o
obj-$(CONFIG_CROS_EC_MKBP_PROXIMITY) += cros_ec_mkbp_proximity.o
+obj-$(CONFIG_IRSD200) += irsd200.o
obj-$(CONFIG_ISL29501) += isl29501.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
obj-$(CONFIG_MB1232) += mb1232.o
diff --git a/drivers/iio/proximity/irsd200.c b/drivers/iio/proximity/irsd200.c
new file mode 100644
index 000000000000..19f7eb2a3ded
--- /dev/null
+++ b/drivers/iio/proximity/irsd200.c
@@ -0,0 +1,962 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Murata IRS-D200 PIR sensor.
+ *
+ * Copyright (C) 2023 Axis Communications AB
+ */
+
+#include <asm/unaligned.h>
+#include <linux/bitfield.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/types.h>
+
+#define IRS_DRV_NAME "irsd200"
+
+/* Registers. */
+#define IRS_REG_OP 0x00 /* Operation mode. */
+#define IRS_REG_DATA_LO 0x02 /* Sensor data LSB. */
+#define IRS_REG_DATA_HI 0x03 /* Sensor data MSB. */
+#define IRS_REG_STATUS 0x04 /* Interrupt status. */
+#define IRS_REG_COUNT 0x05 /* Count of exceeding threshold. */
+#define IRS_REG_DATA_RATE 0x06 /* Output data rate. */
+#define IRS_REG_FILTER 0x07 /* High-pass and low-pass filter. */
+#define IRS_REG_INTR 0x09 /* Interrupt mode. */
+#define IRS_REG_NR_COUNT 0x0a /* Number of counts before interrupt. */
+#define IRS_REG_THR_HI 0x0b /* Upper threshold. */
+#define IRS_REG_THR_LO 0x0c /* Lower threshold. */
+#define IRS_REG_TIMER_LO 0x0d /* Timer setting LSB. */
+#define IRS_REG_TIMER_HI 0x0e /* Timer setting MSB. */
+
+/* Interrupt status bits. */
+#define IRS_INTR_DATA 0 /* Data update. */
+#define IRS_INTR_TIMER 1 /* Timer expiration. */
+#define IRS_INTR_COUNT_THR_AND 2 /* Count "AND" threshold. */
+#define IRS_INTR_COUNT_THR_OR 3 /* Count "OR" threshold. */
+
+/* Operation states. */
+#define IRS_OP_ACTIVE 0x00
+#define IRS_OP_SLEEP 0x01
+
+/*
+ * Quantization scale value for threshold. Used for conversion from/to register
+ * value.
+ */
+#define IRS_THR_QUANT_SCALE 128
+
+#define IRS_UPPER_COUNT(count) FIELD_GET(GENMASK(7, 4), count)
+#define IRS_LOWER_COUNT(count) FIELD_GET(GENMASK(3, 0), count)
+
+/* Index corresponds to the value of IRS_REG_DATA_RATE register. */
+static const int irsd200_data_rates[] = {
+ 50,
+ 100,
+};
+
+/* Index corresponds to the (field) value of IRS_REG_FILTER register. */
+static const unsigned int irsd200_lp_filter_freq[] = {
+ 10,
+ 7,
+};
+
+/*
+ * Index corresponds to the (field) value of IRS_REG_FILTER register. Note that
+ * this represents a fractional value (e.g the first value corresponds to 3 / 10
+ * = 0.3 Hz).
+ */
+static const unsigned int irsd200_hp_filter_freq[][2] = {
+ {3, 10},
+ {5, 10},
+};
+
+/* Register fields. */
+enum irsd200_regfield {
+ /* Data interrupt. */
+ IRS_REGF_INTR_DATA,
+ /* Timer interrupt. */
+ IRS_REGF_INTR_TIMER,
+ /* AND count threshold interrupt. */
+ IRS_REGF_INTR_COUNT_THR_AND,
+ /* OR count threshold interrupt. */
+ IRS_REGF_INTR_COUNT_THR_OR,
+
+ /* Low-pass filter frequency. */
+ IRS_REGF_LP_FILTER,
+ /* High-pass filter frequency. */
+ IRS_REGF_HP_FILTER,
+
+ /* Sentinel value. */
+ IRS_REGF_MAX
+};
+
+static const struct reg_field irsd200_regfields[] = {
+ [IRS_REGF_INTR_DATA] =
+ REG_FIELD(IRS_REG_INTR, IRS_INTR_DATA, IRS_INTR_DATA),
+ [IRS_REGF_INTR_TIMER] =
+ REG_FIELD(IRS_REG_INTR, IRS_INTR_TIMER, IRS_INTR_TIMER),
+ [IRS_REGF_INTR_COUNT_THR_AND] = REG_FIELD(
+ IRS_REG_INTR, IRS_INTR_COUNT_THR_AND, IRS_INTR_COUNT_THR_AND),
+ [IRS_REGF_INTR_COUNT_THR_OR] = REG_FIELD(
+ IRS_REG_INTR, IRS_INTR_COUNT_THR_OR, IRS_INTR_COUNT_THR_OR),
+
+ [IRS_REGF_LP_FILTER] = REG_FIELD(IRS_REG_FILTER, 1, 1),
+ [IRS_REGF_HP_FILTER] = REG_FIELD(IRS_REG_FILTER, 0, 0),
+};
+
+static const struct regmap_config irsd200_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = IRS_REG_TIMER_HI,
+};
+
+struct irsd200_data {
+ struct regmap *regmap;
+ struct regmap_field *regfields[IRS_REGF_MAX];
+ struct device *dev;
+};
+
+static int irsd200_setup(struct irsd200_data *data)
+{
+ unsigned int val;
+ int ret;
+
+ /* Disable all interrupt sources. */
+ ret = regmap_write(data->regmap, IRS_REG_INTR, 0);
+ if (ret) {
+ dev_err(data->dev, "Could not set interrupt sources (%d)\n",
+ ret);
+ return ret;
+ }
+
+ /* Set operation to active. */
+ ret = regmap_write(data->regmap, IRS_REG_OP, IRS_OP_ACTIVE);
+ if (ret) {
+ dev_err(data->dev, "Could not set operation mode (%d)\n", ret);
+ return ret;
+ }
+
+ /* Clear threshold count. */
+ ret = regmap_read(data->regmap, IRS_REG_COUNT, &val);
+ if (ret) {
+ dev_err(data->dev, "Could not clear threshold count (%d)\n",
+ ret);
+ return ret;
+ }
+
+ /* Clear status. */
+ ret = regmap_write(data->regmap, IRS_REG_STATUS, 0x0f);
+ if (ret) {
+ dev_err(data->dev, "Could not clear status (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int irsd200_read_threshold(struct irsd200_data *data,
+ enum iio_event_direction dir, int *val)
+{
+ unsigned int regval;
+ unsigned int reg;
+ int scale;
+ int ret;
+
+ /* Set quantization scale. */
+ if (dir == IIO_EV_DIR_RISING) {
+ scale = IRS_THR_QUANT_SCALE;
+ reg = IRS_REG_THR_HI;
+ } else if (dir == IIO_EV_DIR_FALLING) {
+ scale = -IRS_THR_QUANT_SCALE;
+ reg = IRS_REG_THR_LO;
+ } else {
+ return -EINVAL;
+ }
+
+ ret = regmap_read(data->regmap, reg, &regval);
+ if (ret) {
+ dev_err(data->dev, "Could not read threshold (%d)\n", ret);
+ return ret;
+ }
+
+ *val = ((int)regval) * scale;
+
+ return 0;
+}
+
+static int irsd200_write_threshold(struct irsd200_data *data,
+ enum iio_event_direction dir, int val)
+{
+ unsigned int regval;
+ unsigned int reg;
+ int scale;
+ int ret;
+
+ /* Set quantization scale. */
+ if (dir == IIO_EV_DIR_RISING) {
+ if (val < 0)
+ return -ERANGE;
+
+ scale = IRS_THR_QUANT_SCALE;
+ reg = IRS_REG_THR_HI;
+ } else if (dir == IIO_EV_DIR_FALLING) {
+ if (val > 0)
+ return -ERANGE;
+
+ scale = -IRS_THR_QUANT_SCALE;
+ reg = IRS_REG_THR_LO;
+ } else {
+ return -EINVAL;
+ }
+
+ regval = val / scale;
+
+ if (regval >= BIT(8))
+ return -ERANGE;
+
+ ret = regmap_write(data->regmap, reg, regval);
+ if (ret) {
+ dev_err(data->dev, "Could not write threshold (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int irsd200_read_data(struct irsd200_data *data, s16 *val)
+{
+ __le16 buf;
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, IRS_REG_DATA_LO, &buf,
+ sizeof(buf));
+ if (ret) {
+ dev_err(data->dev, "Could not bulk read data (%d)\n", ret);
+ return ret;
+ }
+
+ *val = le16_to_cpu(buf);
+
+ return 0;
+}
+
+static int irsd200_read_data_rate(struct irsd200_data *data, int *val)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(data->regmap, IRS_REG_DATA_RATE, &regval);
+ if (ret) {
+ dev_err(data->dev, "Could not read data rate (%d)\n", ret);
+ return ret;
+ }
+
+ if (regval >= ARRAY_SIZE(irsd200_data_rates))
+ return -ERANGE;
+
+ *val = irsd200_data_rates[regval];
+
+ return 0;
+}
+
+static int irsd200_write_data_rate(struct irsd200_data *data, int val)
+{
+ size_t idx;
+ int ret;
+
+ for (idx = 0; idx < ARRAY_SIZE(irsd200_data_rates); ++idx) {
+ if (irsd200_data_rates[idx] == val)
+ break;
+ }
+
+ if (idx == ARRAY_SIZE(irsd200_data_rates))
+ return -ERANGE;
+
+ ret = regmap_write(data->regmap, IRS_REG_DATA_RATE, idx);
+ if (ret) {
+ dev_err(data->dev, "Could not write data rate (%d)\n", ret);
+ return ret;
+ }
+
+ /*
+ * Data sheet says the device needs 3 seconds of settling time. The
+ * device operates normally during this period though. This is more of a
+ * "guarantee" than trying to prevent other user space reads/writes.
+ */
+ ssleep(3);
+
+ return 0;
+}
+
+static int irsd200_read_timer(struct irsd200_data *data, int *val, int *val2)
+{
+ __le16 buf;
+ int ret;
+
+ ret = regmap_bulk_read(data->regmap, IRS_REG_TIMER_LO, &buf,
+ sizeof(buf));
+ if (ret) {
+ dev_err(data->dev, "Could not bulk read timer (%d)\n", ret);
+ return ret;
+ }
+
+ ret = irsd200_read_data_rate(data, val2);
+ if (ret)
+ return ret;
+
+ *val = le16_to_cpu(buf);
+
+ return 0;
+}
+
+static int irsd200_write_timer(struct irsd200_data *data, int val, int val2)
+{
+ unsigned int regval;
+ int data_rate;
+ __le16 buf;
+ int ret;
+
+ if (val < 0 || val2 < 0)
+ return -ERANGE;
+
+ ret = irsd200_read_data_rate(data, &data_rate);
+ if (ret)
+ return ret;
+
+ /* Quantize from seconds. */
+ regval = val * data_rate + (val2 * data_rate) / 1000000;
+
+ /* Value is 10 bits. */
+ if (regval >= BIT(10))
+ return -ERANGE;
+
+ buf = cpu_to_le16((u16)regval);
+
+ ret = regmap_bulk_write(data->regmap, IRS_REG_TIMER_LO, &buf,
+ sizeof(buf));
+ if (ret) {
+ dev_err(data->dev, "Could not bulk write timer (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int irsd200_read_nr_count(struct irsd200_data *data, int *val)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(data->regmap, IRS_REG_NR_COUNT, &regval);
+ if (ret) {
+ dev_err(data->dev, "Could not read nr count (%d)\n", ret);
+ return ret;
+ }
+
+ *val = regval;
+
+ return 0;
+}
+
+static int irsd200_write_nr_count(struct irsd200_data *data, int val)
+{
+ unsigned int regval;
+ int ret;
+
+ /* A value of zero means that IRS_REG_STATUS is never set. */
+ if (val <= 0 || val >= 8)
+ return -ERANGE;
+
+ regval = val;
+
+ if (regval >= 2) {
+ /*
+ * According to the data sheet, timer must be also set in this
+ * case (i.e. be non-zero). Check and enforce that.
+ */
+ ret = irsd200_read_timer(data, &val, &val);
+ if (ret)
+ return ret;
+
+ if (val == 0) {
+ dev_err(data->dev,
+ "Timer must be non-zero when nr count is %u\n",
+ regval);
+ return -EPERM;
+ }
+ }
+
+ ret = regmap_write(data->regmap, IRS_REG_NR_COUNT, regval);
+ if (ret) {
+ dev_err(data->dev, "Could not write nr count (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int irsd200_read_lp_filter(struct irsd200_data *data, int *val)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_field_read(data->regfields[IRS_REGF_LP_FILTER], &regval);
+ if (ret) {
+ dev_err(data->dev, "Could not read lp filter frequency (%d)\n",
+ ret);
+ return ret;
+ }
+
+ *val = irsd200_lp_filter_freq[regval];
+
+ return 0;
+}
+
+static int irsd200_write_lp_filter(struct irsd200_data *data, int val)
+{
+ size_t idx;
+ int ret;
+
+ for (idx = 0; idx < ARRAY_SIZE(irsd200_lp_filter_freq); ++idx) {
+ if (irsd200_lp_filter_freq[idx] == val)
+ break;
+ }
+
+ if (idx == ARRAY_SIZE(irsd200_lp_filter_freq))
+ return -ERANGE;
+
+ ret = regmap_field_write(data->regfields[IRS_REGF_LP_FILTER], idx);
+ if (ret) {
+ dev_err(data->dev, "Could not write lp filter frequency (%d)\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int irsd200_read_hp_filter(struct irsd200_data *data, int *val,
+ int *val2)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_field_read(data->regfields[IRS_REGF_HP_FILTER], &regval);
+ if (ret) {
+ dev_err(data->dev, "Could not read hp filter frequency (%d)\n",
+ ret);
+ return ret;
+ }
+
+ *val = irsd200_hp_filter_freq[regval][0];
+ *val2 = irsd200_hp_filter_freq[regval][1];
+
+ return 0;
+}
+
+static int irsd200_write_hp_filter(struct irsd200_data *data, int val, int val2)
+{
+ size_t idx;
+ int ret;
+
+ /* Truncate fractional part to one digit. */
+ val2 /= 100000;
+
+ for (idx = 0; idx < ARRAY_SIZE(irsd200_hp_filter_freq); ++idx) {
+ if (irsd200_hp_filter_freq[idx][0] == val2)
+ break;
+ }
+
+ if (idx == ARRAY_SIZE(irsd200_hp_filter_freq) || val != 0)
+ return -ERANGE;
+
+ ret = regmap_field_write(data->regfields[IRS_REGF_HP_FILTER], idx);
+ if (ret) {
+ dev_err(data->dev, "Could not write hp filter frequency (%d)\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int irsd200_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ struct irsd200_data *data = iio_priv(indio_dev);
+ int ret;
+ s16 buf;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = irsd200_read_data(data, &buf);
+ if (ret)
+ return ret;
+
+ *val = buf;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ ret = irsd200_read_data_rate(data, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ ret = irsd200_read_lp_filter(data, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+ ret = irsd200_read_hp_filter(data, val, val2);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_FRACTIONAL;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int irsd200_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *vals = irsd200_data_rates;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(irsd200_data_rates);
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ *vals = irsd200_lp_filter_freq;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(irsd200_lp_filter_freq);
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+ *vals = (int *)irsd200_hp_filter_freq;
+ *type = IIO_VAL_FRACTIONAL;
+ *length = 2 * ARRAY_SIZE(irsd200_hp_filter_freq);
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int irsd200_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ struct irsd200_data *data = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return irsd200_write_data_rate(data, val);
+ case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+ return irsd200_write_lp_filter(data, val);
+ case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+ return irsd200_write_hp_filter(data, val, val2);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int irsd200_read_event(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int *val, int *val2)
+{
+ struct irsd200_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ ret = irsd200_read_threshold(data, dir, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ case IIO_EV_INFO_RUNNING_PERIOD:
+ ret = irsd200_read_timer(data, val, val2);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_FRACTIONAL;
+ case IIO_EV_INFO_RUNNING_COUNT:
+ ret = irsd200_read_nr_count(data, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int irsd200_write_event(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int val, int val2)
+{
+ struct irsd200_data *data = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ return irsd200_write_threshold(data, dir, val);
+ case IIO_EV_INFO_RUNNING_PERIOD:
+ return irsd200_write_timer(data, val, val2);
+ case IIO_EV_INFO_RUNNING_COUNT:
+ return irsd200_write_nr_count(data, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int irsd200_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct irsd200_data *data = iio_priv(indio_dev);
+ unsigned int val;
+ int ret;
+
+ switch (type) {
+ case IIO_EV_TYPE_THRESH:
+ ret = regmap_field_read(
+ data->regfields[IRS_REGF_INTR_COUNT_THR_OR], &val);
+ if (ret)
+ return ret;
+
+ return val;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int irsd200_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir, int state)
+{
+ struct irsd200_data *data = iio_priv(indio_dev);
+ unsigned int tmp;
+ int ret;
+
+ switch (type) {
+ case IIO_EV_TYPE_THRESH:
+ /* Clear the count register (by reading from it). */
+ ret = regmap_read(data->regmap, IRS_REG_COUNT, &tmp);
+ if (ret)
+ return ret;
+
+ tmp = !!state;
+ ret = regmap_field_write(
+ data->regfields[IRS_REGF_INTR_COUNT_THR_OR], tmp);
+ if (ret)
+ return ret;
+
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static irqreturn_t irsd200_irq_thread(int irq, void *dev_id)
+{
+ struct iio_dev *indio_dev = dev_id;
+ struct irsd200_data *data = iio_priv(indio_dev);
+ enum iio_event_direction dir;
+ unsigned int lower_count;
+ unsigned int upper_count;
+ unsigned int status = 0;
+ unsigned int source = 0;
+ unsigned int clear = 0;
+ unsigned int count = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, IRS_REG_INTR, &source);
+ if (ret) {
+ dev_err(data->dev, "Could not read interrupt source (%d)\n",
+ ret);
+ return IRQ_HANDLED;
+ }
+
+ ret = regmap_read(data->regmap, IRS_REG_STATUS, &status);
+ if (ret) {
+ dev_err(data->dev, "Could not acknowledge interrupt (%d)\n",
+ ret);
+ return IRQ_HANDLED;
+ }
+
+ if (status & BIT(IRS_INTR_DATA) && iio_buffer_enabled(indio_dev)) {
+ iio_trigger_poll_nested(indio_dev->trig);
+ clear |= BIT(IRS_INTR_DATA);
+ }
+
+ if (status & BIT(IRS_INTR_COUNT_THR_OR) &&
+ source & BIT(IRS_INTR_COUNT_THR_OR)) {
+ /*
+ * The register value resets to zero after reading. We therefore
+ * need to read once and manually extract the lower and upper
+ * count register fields.
+ */
+ ret = regmap_read(data->regmap, IRS_REG_COUNT, &count);
+ if (ret)
+ dev_err(data->dev, "Could not read count (%d)\n", ret);
+
+ upper_count = IRS_UPPER_COUNT(count);
+ lower_count = IRS_LOWER_COUNT(count);
+
+ /*
+ * We only check the OR mode to be able to push events for
+ * rising and falling thresholds. AND mode is covered when both
+ * upper and lower count is non-zero, and is signaled with
+ * IIO_EV_DIR_EITHER.
+ */
+ if (upper_count && !lower_count)
+ dir = IIO_EV_DIR_RISING;
+ else if (!upper_count && lower_count)
+ dir = IIO_EV_DIR_FALLING;
+ else
+ dir = IIO_EV_DIR_EITHER;
+
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
+ IIO_EV_TYPE_THRESH, dir),
+ iio_get_time_ns(indio_dev));
+
+ /*
+ * The OR mode will always trigger when the AND mode does, but
+ * not vice versa. However, it seems like the AND bit needs to
+ * be cleared if data capture _and_ threshold count interrupts
+ * are desirable, even though it hasn't explicitly been selected
+ * (with IRS_REG_INTR). Either way, it doesn't hurt...
+ */
+ clear |= BIT(IRS_INTR_COUNT_THR_OR) |
+ BIT(IRS_INTR_COUNT_THR_AND);
+ }
+
+ if (clear) {
+ ret = regmap_write(data->regmap, IRS_REG_STATUS, clear);
+ if (ret)
+ dev_err(data->dev,
+ "Could not clear interrupt status (%d)\n", ret);
+ }
+
+ return clear ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static irqreturn_t irsd200_trigger_handler(int irq, void *pollf)
+{
+ struct iio_dev *indio_dev = ((struct iio_poll_func *)pollf)->indio_dev;
+ struct irsd200_data *data = iio_priv(indio_dev);
+ s16 buf = 0;
+ int ret;
+
+ ret = irsd200_read_data(data, &buf);
+ if (ret)
+ goto end;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &buf,
+ iio_get_time_ns(indio_dev));
+
+end:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int irsd200_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+ struct irsd200_data *data = iio_trigger_get_drvdata(trig);
+ int ret;
+
+ ret = regmap_field_write(data->regfields[IRS_REGF_INTR_DATA], state);
+ if (ret) {
+ dev_err(data->dev, "Could not %s data interrupt source (%d)\n",
+ state ? "enable" : "disable", ret);
+ }
+
+ return ret;
+}
+
+static const struct iio_info irsd200_info = {
+ .read_raw = irsd200_read_raw,
+ .read_avail = irsd200_read_avail,
+ .write_raw = irsd200_write_raw,
+ .read_event_value = irsd200_read_event,
+ .write_event_value = irsd200_write_event,
+ .read_event_config = irsd200_read_event_config,
+ .write_event_config = irsd200_write_event_config,
+};
+
+static const struct iio_trigger_ops irsd200_trigger_ops = {
+ .set_trigger_state = irsd200_set_trigger_state,
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static const struct iio_event_spec irsd200_event_spec[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate =
+ BIT(IIO_EV_INFO_RUNNING_PERIOD) |
+ BIT(IIO_EV_INFO_RUNNING_COUNT) |
+ BIT(IIO_EV_INFO_ENABLE),
+ },
+};
+
+static const struct iio_chan_spec irsd200_channels[] = {
+ {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |
+ BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),
+ .info_mask_separate_available =
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |
+ BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),
+ .event_spec = irsd200_event_spec,
+ .num_event_specs = ARRAY_SIZE(irsd200_event_spec),
+ .scan_type = {
+ .sign = 's',
+ .realbits = 16,
+ .storagebits = 16,
+ .endianness = IIO_CPU,
+ },
+ },
+};
+
+static int irsd200_probe(struct i2c_client *client)
+{
+ struct iio_trigger *trigger;
+ struct irsd200_data *data;
+ struct iio_dev *indio_dev;
+ size_t i;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return dev_err_probe(&client->dev, -ENOMEM,
+ "Could not allocate iio device\n");
+
+ data = iio_priv(indio_dev);
+ data->dev = &client->dev;
+
+ data->regmap = devm_regmap_init_i2c(client, &irsd200_regmap_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(data->dev, PTR_ERR(data->regmap),
+ "Could not initialize regmap\n");
+
+ for (i = 0; i < IRS_REGF_MAX; ++i) {
+ data->regfields[i] = devm_regmap_field_alloc(
+ data->dev, data->regmap, irsd200_regfields[i]);
+ if (IS_ERR(data->regfields[i]))
+ return dev_err_probe(
+ data->dev, PTR_ERR(data->regfields[i]),
+ "Could not allocate register field %zu\n", i);
+ }
+
+ ret = devm_regulator_get_enable(data->dev, "vdd");
+ if (ret)
+ return dev_err_probe(
+ data->dev, ret,
+ "Could not get and enable regulator (%d)\n", ret);
+
+ ret = irsd200_setup(data);
+ if (ret)
+ return ret;
+
+ indio_dev->info = &irsd200_info;
+ indio_dev->name = IRS_DRV_NAME;
+ indio_dev->channels = irsd200_channels;
+ indio_dev->num_channels = ARRAY_SIZE(irsd200_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ if (!client->irq)
+ return dev_err_probe(data->dev, -ENXIO, "No irq available\n");
+
+ ret = devm_iio_triggered_buffer_setup(data->dev, indio_dev, NULL,
+ irsd200_trigger_handler, NULL);
+ if (ret)
+ return dev_err_probe(
+ data->dev, ret,
+ "Could not setup iio triggered buffer (%d)\n", ret);
+
+ ret = devm_request_threaded_irq(data->dev, client->irq, NULL,
+ irsd200_irq_thread,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ NULL, indio_dev);
+ if (ret)
+ return dev_err_probe(data->dev, ret,
+ "Could not request irq (%d)\n", ret);
+
+ trigger = devm_iio_trigger_alloc(data->dev, "%s-dev%d", indio_dev->name,
+ iio_device_id(indio_dev));
+ if (!trigger)
+ return dev_err_probe(data->dev, -ENOMEM,
+ "Could not allocate iio trigger\n");
+
+ trigger->ops = &irsd200_trigger_ops;
+ iio_trigger_set_drvdata(trigger, data);
+
+ ret = devm_iio_trigger_register(data->dev, trigger);
+ if (ret)
+ return dev_err_probe(data->dev, ret,
+ "Could not register iio trigger (%d)\n",
+ ret);
+
+ ret = devm_iio_device_register(data->dev, indio_dev);
+ if (ret)
+ return dev_err_probe(data->dev, ret,
+ "Could not register iio device (%d)\n",
+ ret);
+
+ return 0;
+}
+
+static const struct of_device_id irsd200_of_match[] = {
+ {
+ .compatible = "murata,irsd200",
+ },
+ {}
+};
+MODULE_DEVICE_TABLE(of, irsd200_of_match);
+
+static struct i2c_driver irsd200_driver = {
+ .driver = {
+ .name = IRS_DRV_NAME,
+ .of_match_table = irsd200_of_match,
+ },
+ .probe = irsd200_probe,
+};
+module_i2c_driver(irsd200_driver);
+
+MODULE_AUTHOR("Waqar Hameed <[email protected]>");
+MODULE_DESCRIPTION("Murata IRS-D200 PIR sensor driver");
+MODULE_LICENSE("GPL");
--
2.30.2


2023-07-23 12:27:34

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] iio: Add driver for Murata IRS-D200

On Wed, 19 Jul 2023 09:51:17 +0200
Waqar Hameed <[email protected]> wrote:

> Murata IRS-D200 is a PIR sensor for human detection. It has support for
> raw data measurements and detection event notification.
>
> Add a driver with support for triggered buffer and events. Map the
> various settings to the `iio` framework, e.g. threshold values, sampling
> frequency, filter frequencies etc.
>
> Signed-off-by: Waqar Hameed <[email protected]>

Hi Waqar.

I had some more feedback (see inline) but as it was all very minor, I've
made the changes whilst applying. Let me know if I messed anything up!

Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to see if it can find anything we missed.

Thanks,

Jonathan


> +
> +/*
> + * Index corresponds to the (field) value of IRS_REG_FILTER register. Note that
> + * this represents a fractional value (e.g the first value corresponds to 3 / 10
> + * = 0.3 Hz).
> + */
> +static const unsigned int irsd200_hp_filter_freq[][2] = {
> + {3, 10},
> + {5, 10},
> +};

Prefer space after / before {}

...

> +static int irsd200_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir, int state)
> +{
> + struct irsd200_data *data = iio_priv(indio_dev);
> + unsigned int tmp;
> + int ret;
> +
> + switch (type) {
> + case IIO_EV_TYPE_THRESH:
> + /* Clear the count register (by reading from it). */
> + ret = regmap_read(data->regmap, IRS_REG_COUNT, &tmp);
> + if (ret)
> + return ret;
> +
> + tmp = !!state;

Don't use tmp again here as it suggests some use of previous value.
I'd just put !!state in where the parameter is used.

> + ret = regmap_field_write(
> + data->regfields[IRS_REGF_INTR_COUNT_THR_OR], tmp);
> + if (ret)
> + return ret;
> +
> + return 0;
return regmap_field_write(
data->regfields[IRS_REGF_INTR_COUNT_THR_OR], !!state);

> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static irqreturn_t irsd200_irq_thread(int irq, void *dev_id)
> +{
> + struct iio_dev *indio_dev = dev_id;
> + struct irsd200_data *data = iio_priv(indio_dev);
> + enum iio_event_direction dir;
> + unsigned int lower_count;
> + unsigned int upper_count;
> + unsigned int status = 0;
> + unsigned int source = 0;
> + unsigned int clear = 0;
> + unsigned int count = 0;
> + int ret;
> +
> + ret = regmap_read(data->regmap, IRS_REG_INTR, &source);
> + if (ret) {
> + dev_err(data->dev, "Could not read interrupt source (%d)\n",
> + ret);
> + return IRQ_HANDLED;
> + }
> +
> + ret = regmap_read(data->regmap, IRS_REG_STATUS, &status);
> + if (ret) {
> + dev_err(data->dev, "Could not acknowledge interrupt (%d)\n",
> + ret);
> + return IRQ_HANDLED;
> + }
> +
> + if (status & BIT(IRS_INTR_DATA) && iio_buffer_enabled(indio_dev)) {
> + iio_trigger_poll_nested(indio_dev->trig);
> + clear |= BIT(IRS_INTR_DATA);
> + }
> +
> + if (status & BIT(IRS_INTR_COUNT_THR_OR) &&
> + source & BIT(IRS_INTR_COUNT_THR_OR)) {
> + /*
> + * The register value resets to zero after reading. We therefore
> + * need to read once and manually extract the lower and upper
> + * count register fields.
> + */
> + ret = regmap_read(data->regmap, IRS_REG_COUNT, &count);
> + if (ret)
> + dev_err(data->dev, "Could not read count (%d)\n", ret);
> +
> + upper_count = IRS_UPPER_COUNT(count);
> + lower_count = IRS_LOWER_COUNT(count);
> +
> + /*
> + * We only check the OR mode to be able to push events for
> + * rising and falling thresholds. AND mode is covered when both
> + * upper and lower count is non-zero, and is signaled with
> + * IIO_EV_DIR_EITHER.
> + */
> + if (upper_count && !lower_count)
> + dir = IIO_EV_DIR_RISING;
> + else if (!upper_count && lower_count)
> + dir = IIO_EV_DIR_FALLING;
> + else
> + dir = IIO_EV_DIR_EITHER;
> +
> + iio_push_event(indio_dev,
> + IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
> + IIO_EV_TYPE_THRESH, dir),
> + iio_get_time_ns(indio_dev));
> +
> + /*
> + * The OR mode will always trigger when the AND mode does, but
> + * not vice versa. However, it seems like the AND bit needs to
> + * be cleared if data capture _and_ threshold count interrupts
> + * are desirable, even though it hasn't explicitly been selected
> + * (with IRS_REG_INTR). Either way, it doesn't hurt...
> + */
> + clear |= BIT(IRS_INTR_COUNT_THR_OR) |
> + BIT(IRS_INTR_COUNT_THR_AND);
> + }
> +
> + if (clear) {
> + ret = regmap_write(data->regmap, IRS_REG_STATUS, clear);
> + if (ret)
> + dev_err(data->dev,
> + "Could not clear interrupt status (%d)\n", ret);
> + }
> +
> + return clear ? IRQ_HANDLED : IRQ_NONE;

if (!clear)
return IRQ_NONE

ret = regmap_write(data->regmap ...
if (ret)
...
return IRQ_HANDLED;

Avoids complexity of checking clear multiple times and keeps the 'error' paths as
the ones that are indented. (here it might just be a shared interrupt rather than
an error).



> +}


> +
> +static const struct iio_chan_spec irsd200_channels[] = {
> + {
> + .type = IIO_PROXIMITY,
> + .info_mask_separate =
> + BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |
> + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),
> + .info_mask_separate_available =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |
> + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY),
> + .event_spec = irsd200_event_spec,
> + .num_event_specs = ARRAY_SIZE(irsd200_event_spec),
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_CPU,

As a general rule, we don't adjust the byte ordering of data going out via
the buffered interface, instead leaving that work for userspace.
Given the handling here is very simple, I don't feel that strongly about
keeping to that, but thought I'd mention it.

Jonathan

> + },
> + },
> +};


2023-07-25 09:23:01

by Waqar Hameed

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] iio: Add driver for Murata IRS-D200

On Sun, Jul 23, 2023 at 13:22 +0100 Jonathan Cameron <[email protected]> wrote:

[...]

> I had some more feedback (see inline) but as it was all very minor, I've
> made the changes whilst applying. Let me know if I messed anything up!
>
> Applied to the togreg branch of iio.git and pushed out as testing for 0-day
> to see if it can find anything we missed.

Looks good to me! Thank you very much Jonathan!