[PATCH 0/6] iio: accel: bma220: add events

[PATCH 0/6] iio: accel: bma220: add events

[Petre Rodan]

Series of patches that add tap/double-tap, high-g, low-g and slope
event detection to the bma220 driver.

The first two patches also do a bit of cleanup after the previous series.

The latch functionality is skipped for now since I'm unsure at this time
on how to implement a new API for it within iio.

To be applied to the iio testing branch.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
Petre Rodan (6):
      iio: accel: bma220: white space cleanup
      iio: accel: bma220: remove useless include
      iio: accel: bma220: add tap detection
      iio: accel: bma220: add low-g event detection
      iio: accel: bma220: add high-g event detection
      iio: accel: bma220: add any-motion detection

 drivers/iio/accel/bma220_core.c | 647 +++++++++++++++++++++++++++++++++++++++-
 drivers/iio/accel/bma220_i2c.c  |   1 -
 drivers/iio/accel/bma220_spi.c  |   4 +-
 3 files changed, 647 insertions(+), 5 deletions(-)
---
base-commit: 4b17a60d1e1c2d9d2ccbd58642f6f4ac2fa364ba
change-id: 20251014-bma220_events-6c36acb4a7c1

Best regards,
-- 
Petre Rodan <petre.rodan@subdimension.ro>

[PATCH 1/6] iio: accel: bma220: white space cleanup

[Petre Rodan]

Clean up white space inconsistencies from the last patch series as
requested by Jonathan.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 2 +-
 drivers/iio/accel/bma220_spi.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 2531d6a54ff0..871342d21456 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -128,7 +128,7 @@ enum bma220_axis {
 };
 
 static const int bma220_scale_table[][2] = {
-	{0, 623000}, {1, 248000}, {2, 491000}, {4, 983000},
+	{ 0, 623000 }, { 1, 248000 }, { 2, 491000 }, { 4, 983000 },
 };
 
 struct bma220_data {
diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c
index 7aced4017373..383ee8a135ee 100644
--- a/drivers/iio/accel/bma220_spi.c
+++ b/drivers/iio/accel/bma220_spi.c
@@ -26,12 +26,12 @@ static int bma220_spi_probe(struct spi_device *spi)
 }
 
 static const struct spi_device_id bma220_spi_id[] = {
-	{"bma220", 0},
+	{ "bma220", 0 },
 	{ }
 };
 
 static const struct acpi_device_id bma220_acpi_id[] = {
-	{"BMA0220", 0},
+	{ "BMA0220", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, bma220_spi_id);

-- 
2.49.1

[PATCH 2/6] iio: accel: bma220: remove useless include

[Petre Rodan]

Remove errno.h include from bma220_i2c.c since error codes are generated
within bma220_core.c instead.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_i2c.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_i2c.c b/drivers/iio/accel/bma220_i2c.c
index 5dc7c38f53b3..2b85d4921768 100644
--- a/drivers/iio/accel/bma220_i2c.c
+++ b/drivers/iio/accel/bma220_i2c.c
@@ -8,7 +8,6 @@
  * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
  */
 
-#include <linux/errno.h>
 #include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>

-- 
2.49.1

[PATCH 3/6] iio: accel: bma220: add tap detection

[Petre Rodan]

Add support for tap events.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
Please advise on where should I should be using the mutex guard()
(single regmap reads vs single writes vs more complex read/write combo)
---
 drivers/iio/accel/bma220_core.c | 296 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 295 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index 871342d21456..c4bebf3e5548 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -21,6 +21,7 @@
 #include <linux/types.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/trigger.h>
@@ -101,6 +102,25 @@
 #define BMA220_COF_64Hz				0x4
 #define BMA220_COF_32Hz				0x5
 
+#define BMA220_TAP_TYPE_DOUBLE			0x0
+#define BMA220_TAP_TYPE_SINGLE			0x1
+
+static const struct iio_event_spec bma220_events[] = {
+	{
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_SINGLETAP,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE)
+	},
+	{
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_DOUBLETAP,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD),
+	},
+};
+
 #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
 	.type = IIO_ACCEL,						\
 	.address = reg,							\
@@ -112,6 +132,8 @@
 	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE) |\
 	    BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),		\
 	.scan_index = index,						\
+	.event_spec = bma220_events,					\
+	.num_event_specs = ARRAY_SIZE(bma220_events),			\
 	.scan_type = {							\
 		.sign = 's',						\
 		.realbits = 6,						\
@@ -136,6 +158,7 @@ struct bma220_data {
 	struct mutex lock;
 	u8 lpf_3dB_freq_idx;
 	u8 range_idx;
+	u8 tap_type;
 	struct iio_trigger *trig;
 	struct {
 		s8 chans[3];
@@ -231,6 +254,13 @@ static const struct iio_trigger_ops bma220_trigger_ops = {
 	.validate_device = &iio_trigger_validate_own_device,
 };
 
+static int bma220_reset_int(struct bma220_data *data)
+{
+	return regmap_update_bits(data->regmap, BMA220_REG_IE1,
+				  BMA220_INT_RST_MSK,
+				  FIELD_PREP(BMA220_INT_RST_MSK, 1));
+}
+
 static irqreturn_t bma220_trigger_handler(int irq, void *p)
 {
 	int ret;
@@ -376,6 +406,241 @@ static int bma220_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static int bma220_is_tap_en(struct bma220_data *data,
+			    enum iio_modifier axis,
+			    int type,
+			    bool *en)
+{
+	unsigned int reg_val, val;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMA220_REG_IE0, &reg_val);
+	if (ret)
+		return ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		*en = FIELD_GET(BMA220_INT_EN_TAP_X_MSK, reg_val);
+		break;
+	case IIO_MOD_Y:
+		*en = FIELD_GET(BMA220_INT_EN_TAP_Y_MSK, reg_val);
+		break;
+	case IIO_MOD_Z:
+		*en = FIELD_GET(BMA220_INT_EN_TAP_Z_MSK, reg_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (*en) {
+		ret = regmap_read(data->regmap, BMA220_REG_CONF5, &reg_val);
+		if (ret)
+			return ret;
+		val = FIELD_GET(BMA220_TIP_EN_MSK, reg_val);
+		data->tap_type = val;
+/*
+ * the tip_en reg_flag  - if '1' it enables SingleTap, '0' DoubleTap
+ * truth table for the logic below, *en has to be switched to false in two cases:
+ *   ST  DT  reg_flag  *en
+ *   1   0   0 (DT)    false
+ *   1   0   1 (ST)    true
+ *   0   1   0 (DT)    true
+ *   0   1   1 (ST)    false
+ */
+		if ((type == IIO_EV_DIR_DOUBLETAP) && val)
+			*en = false;
+		else if ((type == IIO_EV_DIR_SINGLETAP) && !val)
+			*en = false;
+	}
+
+	return 0;
+}
+
+static int bma220_set_tap_en(struct bma220_data *data,
+			     enum iio_modifier axis,
+			     int type,
+			     bool en)
+{
+	unsigned int flags = BMA220_TAP_TYPE_DOUBLE;
+	int ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_TAP_X_MSK,
+					 FIELD_PREP(BMA220_INT_EN_TAP_X_MSK, en));
+		break;
+	case IIO_MOD_Y:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_TAP_Y_MSK,
+					 FIELD_PREP(BMA220_INT_EN_TAP_Y_MSK, en));
+		break;
+	case IIO_MOD_Z:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_TAP_Z_MSK,
+					 FIELD_PREP(BMA220_INT_EN_TAP_Z_MSK, en));
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (ret)
+		return ret;
+
+	/* tip_en must be 1 to select singletap detection */
+	if (type == IIO_EV_DIR_SINGLETAP)
+		flags = BMA220_TAP_TYPE_SINGLE;
+
+	ret = regmap_update_bits(data->regmap, BMA220_REG_CONF5,
+				 BMA220_TIP_EN_MSK,
+				 FIELD_PREP(BMA220_TIP_EN_MSK, flags));
+	if (ret)
+		return ret;
+
+	data->tap_type = flags;
+
+	return 0;
+}
+
+static int bma220_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 bma220_data *data = iio_priv(indio_dev);
+	bool int_en;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (dir) {
+		case IIO_EV_DIR_SINGLETAP:
+			ret = bma220_is_tap_en(data, chan->channel2,
+					       IIO_EV_DIR_SINGLETAP, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		case IIO_EV_DIR_DOUBLETAP:
+			ret = bma220_is_tap_en(data, chan->channel2,
+					       IIO_EV_DIR_DOUBLETAP, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bma220_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     bool state)
+{
+	struct bma220_data *data = iio_priv(indio_dev);
+	int ret = 0;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (dir) {
+		case IIO_EV_DIR_SINGLETAP:
+			ret = bma220_set_tap_en(data, chan->channel2,
+						IIO_EV_DIR_SINGLETAP, state);
+			break;
+		case IIO_EV_DIR_DOUBLETAP:
+			ret = bma220_set_tap_en(data, chan->channel2,
+						IIO_EV_DIR_DOUBLETAP, state);
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ret)
+		return ret;
+
+	return bma220_reset_int(data);
+}
+
+static int bma220_read_event_value(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 bma220_data *data = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
+			if (ret)
+				return ret;
+			*val = FIELD_GET(BMA220_TT_TH_MSK, reg_val);
+			return IIO_VAL_INT;
+		case IIO_EV_INFO_PERIOD:
+			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
+			if (ret)
+				return ret;
+			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bma220_write_event_value(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 bma220_data *data = iio_priv(indio_dev);
+
+	guard(mutex)(&data->lock);
+
+	switch (type) {
+	case IIO_EV_TYPE_GESTURE:
+		switch (info) {
+		case IIO_EV_INFO_VALUE:
+			if (!FIELD_FIT(BMA220_TT_TH_MSK, val))
+				return -EINVAL;
+			return regmap_update_bits(data->regmap, BMA220_REG_CONF3,
+						  BMA220_TT_TH_MSK,
+						  FIELD_PREP(BMA220_TT_TH_MSK, val));
+		case IIO_EV_INFO_PERIOD:
+			if (!FIELD_FIT(BMA220_TT_DUR_MSK, val))
+				return -EINVAL;
+			return regmap_update_bits(data->regmap, BMA220_REG_CONF3,
+						  BMA220_TT_DUR_MSK,
+						  FIELD_PREP(BMA220_TT_DUR_MSK, val));
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bma220_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 			     unsigned int writeval, unsigned int *readval)
 {
@@ -390,6 +655,10 @@ static const struct iio_info bma220_info = {
 	.read_raw		= bma220_read_raw,
 	.write_raw		= bma220_write_raw,
 	.read_avail		= bma220_read_avail,
+	.read_event_config	= bma220_read_event_config,
+	.write_event_config	= bma220_write_event_config,
+	.read_event_value	= bma220_read_event_value,
+	.write_event_value	= bma220_write_event_value,
 	.debugfs_reg_access	= &bma220_reg_access,
 };
 
@@ -484,6 +753,8 @@ static int bma220_init(struct device *dev, struct bma220_data *data)
 					     "Failed to set i2c watchdog\n");
 	}
 
+	data->tap_type = BMA220_TAP_TYPE_DOUBLE;
+
 	return 0;
 }
 
@@ -506,13 +777,36 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 	struct bma220_data *data = iio_priv(indio_dev);
 	int ret;
 	unsigned int bma220_reg_if1;
+	s64 timestamp = iio_get_time_ns(indio_dev);
+
+	guard(mutex)(&data->lock);
 
 	ret = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
 	if (ret)
 		return IRQ_NONE;
 
-	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1))
+	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
 		iio_trigger_poll_nested(data->trig);
+		return IRQ_HANDLED;
+	}
+
+	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
+
+		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
+			iio_push_event(indio_dev,
+				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							  IIO_MOD_X_OR_Y_OR_Z,
+							  IIO_EV_TYPE_GESTURE,
+							  IIO_EV_DIR_SINGLETAP),
+				       timestamp);
+		else
+			iio_push_event(indio_dev,
+				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							  IIO_MOD_X_OR_Y_OR_Z,
+							  IIO_EV_TYPE_GESTURE,
+							  IIO_EV_DIR_DOUBLETAP),
+				       timestamp);
+	}
 
 	return IRQ_HANDLED;
 }

-- 
2.49.1

[PATCH 4/6] iio: accel: bma220: add low-g event detection

[Petre Rodan]

Add support for low-g detection.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 101 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index c4bebf3e5548..daff22ec1f2d 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -119,6 +119,14 @@ static const struct iio_event_spec bma220_events[] = {
 		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
 				       BIT(IIO_EV_INFO_PERIOD),
 	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
+				       BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD) |
+				       BIT(IIO_EV_INFO_HYSTERESIS),
+	},
 };
 
 #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
@@ -509,6 +517,7 @@ static int bma220_read_event_config(struct iio_dev *indio_dev,
 	struct bma220_data *data = iio_priv(indio_dev);
 	bool int_en;
 	int ret;
+	unsigned int reg_val, val;
 
 	guard(mutex)(&data->lock);
 
@@ -530,6 +539,18 @@ static int bma220_read_event_config(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_FALLING:
+			ret = regmap_read(data->regmap, BMA220_REG_IE1,
+					  &reg_val);
+			if (ret)
+				return ret;
+			val = FIELD_GET(BMA220_INT_EN_LOW_MSK, reg_val);
+			return val;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -561,6 +582,17 @@ static int bma220_write_event_config(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 		break;
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_FALLING:
+			ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+						 BMA220_INT_EN_LOW_MSK,
+						 FIELD_PREP(BMA220_INT_EN_LOW_MSK, state));
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -602,6 +634,37 @@ static int bma220_read_event_value(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_FALLING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_LOW_TH_MSK, reg_val);
+				return IIO_VAL_INT;
+			case IIO_EV_INFO_PERIOD:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_LOW_DUR_MSK, reg_val);
+				return IIO_VAL_INT;
+			case IIO_EV_INFO_HYSTERESIS:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_LOW_HY_MSK, reg_val);
+				return IIO_VAL_INT;
+			default:
+				return -EINVAL;
+			}
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -636,6 +699,37 @@ static int bma220_write_event_value(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_EV_TYPE_THRESH:
+		switch (dir) {
+		case IIO_EV_DIR_FALLING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				if (!FIELD_FIT(BMA220_LOW_TH_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF1,
+							  BMA220_LOW_TH_MSK,
+							  FIELD_PREP(BMA220_LOW_TH_MSK, val));
+			case IIO_EV_INFO_PERIOD:
+				if (!FIELD_FIT(BMA220_LOW_DUR_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF2,
+							  BMA220_LOW_DUR_MSK,
+							  FIELD_PREP(BMA220_LOW_DUR_MSK, val));
+			case IIO_EV_INFO_HYSTERESIS:
+				if (!FIELD_FIT(BMA220_LOW_HY_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF2,
+							  BMA220_LOW_HY_MSK,
+							  FIELD_PREP(BMA220_LOW_HY_MSK, val));
+			default:
+				return -EINVAL;
+			}
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -790,6 +884,13 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 		return IRQ_HANDLED;
 	}
 
+	if (FIELD_GET(BMA220_IF_LOW, bma220_reg_if1))
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_FALLING),
+			       timestamp);
 	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
 
 		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)

-- 
2.49.1

[PATCH 5/6] iio: accel: bma220: add high-g event detection

[Petre Rodan]

Add support for high-g detection.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/accel/bma220_core.c | 132 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index daff22ec1f2d..f1d4646b495d 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -127,6 +127,14 @@ static const struct iio_event_spec bma220_events[] = {
 				       BIT(IIO_EV_INFO_PERIOD) |
 				       BIT(IIO_EV_INFO_HYSTERESIS),
 	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD) |
+				       BIT(IIO_EV_INFO_HYSTERESIS),
+	},
 };
 
 #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
@@ -464,6 +472,34 @@ static int bma220_is_tap_en(struct bma220_data *data,
 	return 0;
 }
 
+static int bma220_is_high_en(struct bma220_data *data,
+			      enum iio_modifier axis,
+			      bool *en)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMA220_REG_IE1, &reg_val);
+	if (ret)
+		return ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		*en = FIELD_GET(BMA220_INT_EN_HIGH_X_MSK, reg_val);
+		break;
+	case IIO_MOD_Y:
+		*en = FIELD_GET(BMA220_INT_EN_HIGH_Y_MSK, reg_val);
+		break;
+	case IIO_MOD_Z:
+		*en = FIELD_GET(BMA220_INT_EN_HIGH_Z_MSK, reg_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int bma220_set_tap_en(struct bma220_data *data,
 			     enum iio_modifier axis,
 			     int type,
@@ -509,6 +545,35 @@ static int bma220_set_tap_en(struct bma220_data *data,
 	return 0;
 }
 
+static int bma220_set_high_en(struct bma220_data *data,
+			      enum iio_modifier axis,
+			      bool en)
+{
+	int ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_EN_HIGH_X_MSK,
+					 FIELD_PREP(BMA220_INT_EN_HIGH_X_MSK, en));
+		break;
+	case IIO_MOD_Y:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_EN_HIGH_Y_MSK,
+					 FIELD_PREP(BMA220_INT_EN_HIGH_Y_MSK, en));
+		break;
+	case IIO_MOD_Z:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE1,
+					 BMA220_INT_EN_HIGH_Z_MSK,
+					 FIELD_PREP(BMA220_INT_EN_HIGH_Z_MSK, en));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 static int bma220_read_event_config(struct iio_dev *indio_dev,
 				    const struct iio_chan_spec *chan,
 				    enum iio_event_type type,
@@ -548,6 +613,11 @@ static int bma220_read_event_config(struct iio_dev *indio_dev,
 				return ret;
 			val = FIELD_GET(BMA220_INT_EN_LOW_MSK, reg_val);
 			return val;
+		case IIO_EV_DIR_RISING:
+			ret = bma220_is_high_en(data, chan->channel2, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
 		default:
 			return -EINVAL;
 		}
@@ -589,6 +659,9 @@ static int bma220_write_event_config(struct iio_dev *indio_dev,
 						 BMA220_INT_EN_LOW_MSK,
 						 FIELD_PREP(BMA220_INT_EN_LOW_MSK, state));
 			break;
+		case IIO_EV_DIR_RISING:
+			ret = bma220_set_high_en(data, chan->channel2, state);
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -662,6 +735,32 @@ static int bma220_read_event_value(struct iio_dev *indio_dev,
 			default:
 				return -EINVAL;
 			}
+		case IIO_EV_DIR_RISING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_HIGH_TH_MSK, reg_val);
+				return IIO_VAL_INT;
+			case IIO_EV_INFO_PERIOD:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF0,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_HIGH_DUR_MSK, reg_val);
+				return IIO_VAL_INT;
+			case IIO_EV_INFO_HYSTERESIS:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF0,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_HIGH_HY_MSK, reg_val);
+				return IIO_VAL_INT;
+			default:
+				return -EINVAL;
+			}
 		default:
 			return -EINVAL;
 		}
@@ -727,6 +826,32 @@ static int bma220_write_event_value(struct iio_dev *indio_dev,
 			default:
 				return -EINVAL;
 			}
+		case IIO_EV_DIR_RISING:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				if (!FIELD_FIT(BMA220_HIGH_TH_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF1,
+							  BMA220_HIGH_TH_MSK,
+							  FIELD_PREP(BMA220_HIGH_TH_MSK, val));
+			case IIO_EV_INFO_PERIOD:
+				if (!FIELD_FIT(BMA220_HIGH_DUR_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF0,
+							  BMA220_HIGH_DUR_MSK,
+							  FIELD_PREP(BMA220_HIGH_DUR_MSK, val));
+			case IIO_EV_INFO_HYSTERESIS:
+				if (!FIELD_FIT(BMA220_HIGH_HY_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF0,
+							  BMA220_HIGH_HY_MSK,
+							  FIELD_PREP(BMA220_HIGH_HY_MSK, val));
+			default:
+				return -EINVAL;
+			}
 		default:
 			return -EINVAL;
 		}
@@ -891,6 +1016,13 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 						  IIO_EV_TYPE_THRESH,
 						  IIO_EV_DIR_FALLING),
 			       timestamp);
+	if (FIELD_GET(BMA220_IF_HIGH, bma220_reg_if1))
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_RISING),
+			       timestamp);
 	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
 
 		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)

-- 
2.49.1

[PATCH 6/6] iio: accel: bma220: add any-motion detection

[Petre Rodan]

Add support for what Bosch calls "any-motion (slope)" detection.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
I think Jonathan remarked that this might be improper use of the iio API?
please advise.
---
 drivers/iio/accel/bma220_core.c | 116 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
index f1d4646b495d..f79901ae8786 100644
--- a/drivers/iio/accel/bma220_core.c
+++ b/drivers/iio/accel/bma220_core.c
@@ -135,6 +135,13 @@ static const struct iio_event_spec bma220_events[] = {
 				       BIT(IIO_EV_INFO_PERIOD) |
 				       BIT(IIO_EV_INFO_HYSTERESIS),
 	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD),
+	},
 };
 
 #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
@@ -500,6 +507,34 @@ static int bma220_is_high_en(struct bma220_data *data,
 	return 0;
 }
 
+static int bma220_is_slope_en(struct bma220_data *data,
+			      enum iio_modifier axis,
+			      bool *en)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMA220_REG_IE0, &reg_val);
+	if (ret)
+		return ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		*en = FIELD_GET(BMA220_INT_EN_SLOPE_X_MSK, reg_val);
+		break;
+	case IIO_MOD_Y:
+		*en = FIELD_GET(BMA220_INT_EN_SLOPE_Y_MSK, reg_val);
+		break;
+	case IIO_MOD_Z:
+		*en = FIELD_GET(BMA220_INT_EN_SLOPE_Z_MSK, reg_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int bma220_set_tap_en(struct bma220_data *data,
 			     enum iio_modifier axis,
 			     int type,
@@ -574,6 +609,34 @@ static int bma220_set_high_en(struct bma220_data *data,
 	return ret;
 }
 
+static int bma220_set_slope_en(struct bma220_data *data, enum iio_modifier axis,
+			       bool en)
+{
+	int ret;
+
+	switch (axis) {
+	case IIO_MOD_X:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_SLOPE_X_MSK,
+					 FIELD_PREP(BMA220_INT_EN_SLOPE_X_MSK, en));
+		break;
+	case IIO_MOD_Y:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_SLOPE_Y_MSK,
+					 FIELD_PREP(BMA220_INT_EN_SLOPE_Y_MSK, en));
+		break;
+	case IIO_MOD_Z:
+		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
+					 BMA220_INT_EN_SLOPE_Z_MSK,
+					 FIELD_PREP(BMA220_INT_EN_SLOPE_Z_MSK, en));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 static int bma220_read_event_config(struct iio_dev *indio_dev,
 				    const struct iio_chan_spec *chan,
 				    enum iio_event_type type,
@@ -618,6 +681,11 @@ static int bma220_read_event_config(struct iio_dev *indio_dev,
 			if (ret)
 				return ret;
 			return int_en;
+		case IIO_EV_DIR_EITHER:
+			ret = bma220_is_slope_en(data, chan->channel2, &int_en);
+			if (ret)
+				return ret;
+			return int_en;
 		default:
 			return -EINVAL;
 		}
@@ -662,6 +730,9 @@ static int bma220_write_event_config(struct iio_dev *indio_dev,
 		case IIO_EV_DIR_RISING:
 			ret = bma220_set_high_en(data, chan->channel2, state);
 			break;
+		case IIO_EV_DIR_EITHER:
+			ret = bma220_set_slope_en(data, chan->channel2, state);
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -761,6 +832,25 @@ static int bma220_read_event_value(struct iio_dev *indio_dev,
 			default:
 				return -EINVAL;
 			}
+		case IIO_EV_DIR_EITHER:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF4,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_SLOPE_TH_MSK, reg_val);
+				return IIO_VAL_INT;
+			case IIO_EV_INFO_PERIOD:
+				ret = regmap_read(data->regmap, BMA220_REG_CONF4,
+						  &reg_val);
+				if (ret)
+					return ret;
+				*val = FIELD_GET(BMA220_SLOPE_DUR_MSK, reg_val);
+				return IIO_VAL_INT;
+			default:
+				return -EINVAL;
+			}
 		default:
 			return -EINVAL;
 		}
@@ -852,6 +942,25 @@ static int bma220_write_event_value(struct iio_dev *indio_dev,
 			default:
 				return -EINVAL;
 			}
+		case IIO_EV_DIR_EITHER:
+			switch (info) {
+			case IIO_EV_INFO_VALUE:
+				if (!FIELD_FIT(BMA220_SLOPE_TH_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF4,
+							  BMA220_SLOPE_TH_MSK,
+							  FIELD_PREP(BMA220_SLOPE_TH_MSK, val));
+			case IIO_EV_INFO_PERIOD:
+				if (!FIELD_FIT(BMA220_SLOPE_DUR_MSK, val))
+					return -EINVAL;
+				return regmap_update_bits(data->regmap,
+							  BMA220_REG_CONF4,
+							  BMA220_SLOPE_DUR_MSK,
+							  FIELD_PREP(BMA220_SLOPE_DUR_MSK, val));
+			default:
+				return -EINVAL;
+			}
 		default:
 			return -EINVAL;
 		}
@@ -1023,6 +1132,13 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
 						  IIO_EV_TYPE_THRESH,
 						  IIO_EV_DIR_RISING),
 			       timestamp);
+	if (FIELD_GET(BMA220_IF_SLOPE, bma220_reg_if1))
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_EITHER),
+			       timestamp);
 	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
 
 		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)

-- 
2.49.1

Re: [PATCH 2/6] iio: accel: bma220: remove useless include

[Jonathan Cameron]

On Tue, 14 Oct 2025 19:42:58 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Remove errno.h include from bma220_i2c.c since error codes are generated
> within bma220_core.c instead.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Applied.
> ---
>  drivers/iio/accel/bma220_i2c.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma220_i2c.c b/drivers/iio/accel/bma220_i2c.c
> index 5dc7c38f53b3..2b85d4921768 100644
> --- a/drivers/iio/accel/bma220_i2c.c
> +++ b/drivers/iio/accel/bma220_i2c.c
> @@ -8,7 +8,6 @@
>   * I2C address is either 0x0b or 0x0a depending on CSB (pin 10)
>   */
>  
> -#include <linux/errno.h>
>  #include <linux/i2c.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> 

Re: [PATCH 1/6] iio: accel: bma220: white space cleanup

[Jonathan Cameron]

On Tue, 14 Oct 2025 19:42:57 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Clean up white space inconsistencies from the last patch series as
> requested by Jonathan.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Applied. thanks.

Re: [PATCH 3/6] iio: accel: bma220: add tap detection

[Jonathan Cameron]

On Tue, 14 Oct 2025 19:42:59 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add support for tap events.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> Please advise on where should I should be using the mutex guard()
> (single regmap reads vs single writes vs more complex read/write combo)

It usually depends not so much on what is being accessed directly under the lock
but more whether we need to prevent interruption of sequences that might
be running from other paths.  That or protecting a buffer that might be
hit from another path.

The regmap calls themselves are effectively atomic due to internal locking
so we shouldn't need to add to that if there are no issues with more complex
sequences that need protecting.

In general tap detection is one of my least favourite things to enable because
every device seems to have a different set of control parameters.
I've +CC Jagath who looked at this a lot a while back and defined much of
the existing ABI.


> ---
>  drivers/iio/accel/bma220_core.c | 296 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 295 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index 871342d21456..c4bebf3e5548 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -21,6 +21,7 @@

> @@ -231,6 +254,13 @@ static const struct iio_trigger_ops bma220_trigger_ops = {
>  	.validate_device = &iio_trigger_validate_own_device,
>  };
>  
> +static int bma220_reset_int(struct bma220_data *data)
> +{
> +	return regmap_update_bits(data->regmap, BMA220_REG_IE1,
> +				  BMA220_INT_RST_MSK,
> +				  FIELD_PREP(BMA220_INT_RST_MSK, 1));

regmap_set_bits()

> +}

> +static int bma220_set_tap_en(struct bma220_data *data,
> +			     enum iio_modifier axis,
> +			     int type,
> +			     bool en)
> +{
> +	unsigned int flags = BMA220_TAP_TYPE_DOUBLE;
> +	int ret;
> +
> +	switch (axis) {
> +	case IIO_MOD_X:
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
> +					 BMA220_INT_EN_TAP_X_MSK,
> +					 FIELD_PREP(BMA220_INT_EN_TAP_X_MSK, en));

regmap_assign_bits() comes in handy when you are setting just one bit.

> +		break;
> +	case IIO_MOD_Y:
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
> +					 BMA220_INT_EN_TAP_Y_MSK,
> +					 FIELD_PREP(BMA220_INT_EN_TAP_Y_MSK, en));
> +		break;
> +	case IIO_MOD_Z:
> +		ret = regmap_update_bits(data->regmap, BMA220_REG_IE0,
> +					 BMA220_INT_EN_TAP_Z_MSK,
> +					 FIELD_PREP(BMA220_INT_EN_TAP_Z_MSK, en));
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	if (ret)
> +		return ret;
> +
> +	/* tip_en must be 1 to select singletap detection */
> +	if (type == IIO_EV_DIR_SINGLETAP)
> +		flags = BMA220_TAP_TYPE_SINGLE;
> +
> +	ret = regmap_update_bits(data->regmap, BMA220_REG_CONF5,
> +				 BMA220_TIP_EN_MSK,
> +				 FIELD_PREP(BMA220_TIP_EN_MSK, flags));
> +	if (ret)
> +		return ret;
> +
> +	data->tap_type = flags;
> +
> +	return 0;
> +}

> +
> +static int bma220_read_event_value(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 bma220_data *data = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	guard(mutex)(&data->lock);
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_GESTURE:
> +		switch (info) {
> +		case IIO_EV_INFO_VALUE:
> +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> +			if (ret)
> +				return ret;
> +			*val = FIELD_GET(BMA220_TT_TH_MSK, reg_val);
> +			return IIO_VAL_INT;
> +		case IIO_EV_INFO_PERIOD:
Period for a tap detector, particularly one that can do double tap is unusual.
Period is normally a case of how long a condition must be true before it causes
an interrupt.

I haven't figured out if this maps to one of the rather rich set of double tap
attributes in the ABI. See Documentation/ABI/testing/sysfs-bus-iio *doubletap*

Related stuff are
in_accel_gesture_doubletap_tap2_min_delay - I think that is tt_quiet in this case.
in_accel_gesture_doubletap_reset_timeout - I think this is at least approximately
tt_quiet + tt_dur.  It gives the time at which we give up on this being a potential
double tap. It's not quite right though as the actual reset is a bit after that.

+CC Jagath who defined most of the tap ABI and might be able to remember how this
all works better than me.

We may have a gap here that needs yet more ABI.


> +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> +			if (ret)
> +				return ret;
> +			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);

This needs to be in second if you are using duration. Is the register really in seconds?

> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}


>  
> @@ -506,13 +777,36 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
>  	struct bma220_data *data = iio_priv(indio_dev);
>  	int ret;
>  	unsigned int bma220_reg_if1;
> +	s64 timestamp = iio_get_time_ns(indio_dev);
> +
> +	guard(mutex)(&data->lock);
>  
>  	ret = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
>  	if (ret)
>  		return IRQ_NONE;
>  
> -	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1))
> +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {

Is it an either / or case? I.e. we can only have buffered reads with
the data ready interrupt or events?   That does happen in some devices
but is fairly unusual.

>  		iio_trigger_poll_nested(data->trig);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
> +
> +		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
> +			iio_push_event(indio_dev,
> +				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +							  IIO_MOD_X_OR_Y_OR_Z,
> +							  IIO_EV_TYPE_GESTURE,
> +							  IIO_EV_DIR_SINGLETAP),
> +				       timestamp);
> +		else
> +			iio_push_event(indio_dev,
> +				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +							  IIO_MOD_X_OR_Y_OR_Z,
> +							  IIO_EV_TYPE_GESTURE,
> +							  IIO_EV_DIR_DOUBLETAP),
> +				       timestamp);
> +	}
>  
>  	return IRQ_HANDLED;
>  }
> 

Re: [PATCH 4/6] iio: accel: bma220: add low-g event detection

[Jonathan Cameron]

On Tue, 14 Oct 2025 19:43:00 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add support for low-g detection.
> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
Just a follow on related units of _period attributes.

> ---
>  drivers/iio/accel/bma220_core.c | 101 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
> index c4bebf3e5548..daff22ec1f2d 100644
> --- a/drivers/iio/accel/bma220_core.c
> +++ b/drivers/iio/accel/bma220_core.c
> @@ -119,6 +119,14 @@ static const struct iio_event_spec bma220_events[] = {
>  		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>  				       BIT(IIO_EV_INFO_PERIOD),
>  	},
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) |
> +				       BIT(IIO_EV_INFO_VALUE) |
> +				       BIT(IIO_EV_INFO_PERIOD) |
> +				       BIT(IIO_EV_INFO_HYSTERESIS),
> +	},
>  };
>  
>  #define BMA220_ACCEL_CHANNEL(index, reg, axis) {			\
> @@ -509,6 +517,7 @@ static int bma220_read_event_config(struct iio_dev *indio_dev,

> @@ -602,6 +634,37 @@ static int bma220_read_event_value(struct iio_dev *indio_dev,
>  		default:
>  			return -EINVAL;
>  		}
> +	case IIO_EV_TYPE_THRESH:
> +		switch (dir) {
> +		case IIO_EV_DIR_FALLING:
> +			switch (info) {
> +			case IIO_EV_INFO_VALUE:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF1,
> +						  &reg_val);
> +				if (ret)
> +					return ret;
> +				*val = FIELD_GET(BMA220_LOW_TH_MSK, reg_val);
> +				return IIO_VAL_INT;
> +			case IIO_EV_INFO_PERIOD:
> +				ret = regmap_read(data->regmap, BMA220_REG_CONF2,
> +						  &reg_val);
> +				if (ret)
> +					return ret;
> +				*val = FIELD_GET(BMA220_LOW_DUR_MSK, reg_val);
As in previous. Seems very unlikely period is in second here, given that would be far too long
to make sense!  See the ABI docs.  Looks to be samples which means
you need to deal with variable sampling frequencies as part of this
calculation.

> +				return IIO_VAL_INT;
>

Re: [PATCH 4/6] iio: accel: bma220: add low-g event detection

[Jonathan Cameron]

> @@ -790,6 +884,13 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
>  		return IRQ_HANDLED;
>  	}
>  
> +	if (FIELD_GET(BMA220_IF_LOW, bma220_reg_if1))
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +						  IIO_MOD_X_OR_Y_OR_Z,
I only noticed this when looking at next patch.  
Would be unusual to have reporting on 'any' acceleration dropping below
a threshold. Tends to be all drop (which is free fall detection).

I checked the datasheet quickly and looks like this should be
IIO_MOD_X_AND_Y_AND_Z  which is what we normally see for an inactivity / freefall
threshold detector.

> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_FALLING),
> +			       timestamp);
>  	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
>  
>  		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
> 

Re: [PATCH 6/6] iio: accel: bma220: add any-motion detection

[Jonathan Cameron]

On Tue, 14 Oct 2025 19:43:02 +0300
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Add support for what Bosch calls "any-motion (slope)" detection.

Slope is normally rate of change in IIO ABI terms. See the _roc_
event types.

The slight wrinkle here is that we have it based on the magnitude
and, whilst it has been a while since most of the sensors using
_roc_ were added I think they were typically signed. So it was
acceleration is getting higher at x rate, rather than what I think
we have here which is that the acceleration is either growing or shrinking
at x rate.  (kind of _rocmag_, similar to _mag_ relationship to _thresh_
events)

What this has to do with 'any motion' I have no idea - I'm just
looking at the graphs.  If I read them right we need something new.
Or it's entirely possible I've forgotten some part of existing ABI
that can handle this.

Jonathan


> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> I think Jonathan remarked that this might be improper use of the iio API?
> please advise.

Re: [PATCH 3/6] iio: accel: bma220: add tap detection

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 2954 bytes --]


Hello Jonathan,

thank you for the review.

On Sat, Oct 18, 2025 at 06:16:32PM +0100, Jonathan Cameron wrote:
> > +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> > +			if (ret)
> > +				return ret;
> > +			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);
> 
> This needs to be in second if you are using duration. Is the register really in seconds?

this IC has a very small number of bits that configure duration/hysteresis/threshold levels. it's between 2 and 6 for each of them. in the case of high and low G events the duration is not even directly defined as a time interval, but as a count of samples that are over a threshold value.

I was hoping that simply passing along a unitless value between 0 and parameter_max would be enough to customize all the event parameters. this does mean that the driver makes the assumption that the user is familiar with the device datasheet and knows the number of bits every parameter has been allocated. should the driver provide a conversion table for tt_duration just like for _scale_table and _lpf_3dB_freq_Hz_table?

> > @@ -506,13 +777,36 @@ static irqreturn_t bma220_irq_handler(int irq, void *private)
> >  	struct bma220_data *data = iio_priv(indio_dev);
> >  	int ret;
> >  	unsigned int bma220_reg_if1;
> > +	s64 timestamp = iio_get_time_ns(indio_dev);
> > +
> > +	guard(mutex)(&data->lock);
> >  
> >  	ret = regmap_read(data->regmap, BMA220_REG_IF1, &bma220_reg_if1);
> >  	if (ret)
> >  		return IRQ_NONE;
> >  
> > -	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1))
> > +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {
> 
> Is it an either / or case? I.e. we can only have buffered reads with
> the data ready interrupt or events?   That does happen in some devices
> but is fairly unusual.

the driver got an interrupt, so it checks the source - it's either a data ready when the sensor is used to sample the environment or it's an event in which case it just sets the event.
now that you mention it I think I would miss events if both happened before the kernel executes the _irq_handler(), so I will rewrite this bit. if you ment something else please tell me.

best regards,
peter

> 
> >  		iio_trigger_poll_nested(data->trig);
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
> > +
> > +		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
> > +			iio_push_event(indio_dev,
> > +				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> > +							  IIO_MOD_X_OR_Y_OR_Z,
> > +							  IIO_EV_TYPE_GESTURE,
> > +							  IIO_EV_DIR_SINGLETAP),
> > +				       timestamp);
> > +		else
> > +			iio_push_event(indio_dev,
> > +				       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> > +							  IIO_MOD_X_OR_Y_OR_Z,
> > +							  IIO_EV_TYPE_GESTURE,
> > +							  IIO_EV_DIR_DOUBLETAP),
> > +				       timestamp);
> > +	}
> >  
> >  	return IRQ_HANDLED;
> >  }
> > 
> 

-- 
petre rodan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/6] iio: accel: bma220: add tap detection

[Jonathan Cameron]

On Sat, 1 Nov 2025 09:56:42 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> Hello Jonathan,
> 
> thank you for the review.
> 
> On Sat, Oct 18, 2025 at 06:16:32PM +0100, Jonathan Cameron wrote:
> > > +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> > > +			if (ret)
> > > +				return ret;
> > > +			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);  
> > 
> > This needs to be in second if you are using duration. Is the register really in seconds?  
> 
> this IC has a very small number of bits that configure
> duration/hysteresis/threshold levels. it's between 2 and 6 for each
> of them. in the case of high and low G events the duration is not
> even directly defined as a time interval, but as a count of samples
> that are over a threshold value.

The ABI is in seconds, so you have to deal with scaling wrt to the sampling
frequency at the time.  I know it can be a pain to do, but consistent userspace
is the aim and so we need to match the ABI.

> 
> I was hoping that simply passing along a unitless value between 0 and
> parameter_max would be enough to customize all the event parameters.
> this does mean that the driver makes the assumption that the user is
> familiar with the device datasheet and knows the number of bits every
> parameter has been allocated. should the driver provide a conversion
> table for tt_duration just like for _scale_table and
> _lpf_3dB_freq_Hz_table?

Exactly.

> 
> > > @@ -506,13 +777,36 @@ static irqreturn_t bma220_irq_handler(int
> > > irq, void *private) struct bma220_data *data =
> > > iio_priv(indio_dev); int ret;
> > >  	unsigned int bma220_reg_if1;
> > > +	s64 timestamp = iio_get_time_ns(indio_dev);
> > > +
> > > +	guard(mutex)(&data->lock);
> > >  
> > >  	ret = regmap_read(data->regmap, BMA220_REG_IF1,
> > > &bma220_reg_if1); if (ret)
> > >  		return IRQ_NONE;
> > >  
> > > -	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1))
> > > +	if (FIELD_GET(BMA220_IF_DRDY, bma220_reg_if1)) {  
> > 
> > Is it an either / or case? I.e. we can only have buffered reads with
> > the data ready interrupt or events?   That does happen in some
> > devices but is fairly unusual.  
> 
> the driver got an interrupt, so it checks the source - it's either a
> data ready when the sensor is used to sample the environment or it's
> an event in which case it just sets the event. now that you mention
> it I think I would miss events if both happened before the kernel
> executes the _irq_handler(), so I will rewrite this bit. if you ment
> something else please tell me.

That was exactly what I was getting at.  Given both occur on a particular
sample, it is guaranteed this will sometimes happen.

Jonathan

> 
> best regards,
> peter
> 
> >   
> > >  		iio_trigger_poll_nested(data->trig);
> > > +		return IRQ_HANDLED;
> > > +	}
> > > +
> > > +	if (FIELD_GET(BMA220_IF_TT, bma220_reg_if1)) {
> > > +
> > > +		if (data->tap_type == BMA220_TAP_TYPE_SINGLE)
> > > +			iio_push_event(indio_dev,
> > > +
> > > IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> > > +
> > > IIO_MOD_X_OR_Y_OR_Z,
> > > +
> > > IIO_EV_TYPE_GESTURE,
> > > +
> > > IIO_EV_DIR_SINGLETAP),
> > > +				       timestamp);
> > > +		else
> > > +			iio_push_event(indio_dev,
> > > +
> > > IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> > > +
> > > IIO_MOD_X_OR_Y_OR_Z,
> > > +
> > > IIO_EV_TYPE_GESTURE,
> > > +
> > > IIO_EV_DIR_DOUBLETAP),
> > > +				       timestamp);
> > > +	}
> > >  
> > >  	return IRQ_HANDLED;
> > >  }
> > >   
> >   
> 

Re: [PATCH 3/6] iio: accel: bma220: add tap detection

[Petre Rodan]

[-- Attachment #1: Type: text/plain, Size: 2922 bytes --]

On Sun, Nov 02, 2025 at 12:20:53PM +0000, Jonathan Cameron wrote:
> On Sat, 1 Nov 2025 09:56:42 +0200
> Petre Rodan <petre.rodan@subdimension.ro> wrote:
> 
> > Hello Jonathan,
> > 
> > thank you for the review.
> > 
> > On Sat, Oct 18, 2025 at 06:16:32PM +0100, Jonathan Cameron wrote:
> > > > +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> > > > +			if (ret)
> > > > +				return ret;
> > > > +			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);  
> > > 
> > > This needs to be in second if you are using duration. Is the register really in seconds?  
> > 
> > this IC has a very small number of bits that configure
> > duration/hysteresis/threshold levels. it's between 2 and 6 for each
> > of them. in the case of high and low G events the duration is not
> > even directly defined as a time interval, but as a count of samples
> > that are over a threshold value.
> 
> The ABI is in seconds, so you have to deal with scaling wrt to the sampling
> frequency at the time.  I know it can be a pain to do, but consistent userspace
> is the aim and so we need to match the ABI.

on this bma220 chip, when someone modifies the cut off frequency of the filter then the ic automatically adjusts the sampling rate. and this sample rate is not exposed on any of the registers.
since duration parameters are defined as a count of samples and the sample rate looks to be unknown I don't see how I could adapt to an API that is based on a unit of seconds.

> > I was hoping that simply passing along a unitless value between 0 and
> > parameter_max would be enough to customize all the event parameters.
> > this does mean that the driver makes the assumption that the user is
> > familiar with the device datasheet and knows the number of bits every
> > parameter has been allocated. should the driver provide a conversion
> > table for tt_duration just like for _scale_table and
> > _lpf_3dB_freq_Hz_table?
> 
> Exactly.

I was thinking today of a more analog-feeling API, one in which a variable that can take values linearly between min and max can be set to a percentage of it's scale. think of stereo systems - most of us don't want to set a precise amount of decibels of attenuation when operating the volume knob, we just want to set it lower or higher until a condition matches. in this API the primary unit of measurement would not be dBs but notches or ticks - calculated based on min, max and the native resolution of the control (how many bits are allocated for it in the ic's memory map). this has also the benefit of translating nicely when the control is rendered as a widget in a GUI. think about a 0 to 11 volume knob.
is there anything like this already implemented? is there any merit to this idea?

 I will shelve the event part of this driver for another time, just got some Honeywell pressure sensors that need a new driver.

best regards,
peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/6] iio: accel: bma220: add tap detection

[Jonathan Cameron]

On Fri, 7 Nov 2025 02:30:33 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> On Sun, Nov 02, 2025 at 12:20:53PM +0000, Jonathan Cameron wrote:
> > On Sat, 1 Nov 2025 09:56:42 +0200
> > Petre Rodan <petre.rodan@subdimension.ro> wrote:
> >   
> > > Hello Jonathan,
> > > 
> > > thank you for the review.
> > > 
> > > On Sat, Oct 18, 2025 at 06:16:32PM +0100, Jonathan Cameron wrote:  
> > > > > +			ret = regmap_read(data->regmap, BMA220_REG_CONF3, &reg_val);
> > > > > +			if (ret)
> > > > > +				return ret;
> > > > > +			*val = FIELD_GET(BMA220_TT_DUR_MSK, reg_val);    
> > > > 
> > > > This needs to be in second if you are using duration. Is the register really in seconds?    
> > > 
> > > this IC has a very small number of bits that configure
> > > duration/hysteresis/threshold levels. it's between 2 and 6 for each
> > > of them. in the case of high and low G events the duration is not
> > > even directly defined as a time interval, but as a count of samples
> > > that are over a threshold value.  
> > 
> > The ABI is in seconds, so you have to deal with scaling wrt to the sampling
> > frequency at the time.  I know it can be a pain to do, but consistent userspace
> > is the aim and so we need to match the ABI.  
> 
> on this bma220 chip, when someone modifies the cut off frequency of the
> filter then the ic automatically adjusts the sampling rate. and this 
> sample rate is not exposed on any of the registers.
> since duration parameters are defined as a count of samples 
> and the sample rate looks to be unknown I don't see how I could
> adapt to an API that is based on a unit of seconds.

Does the datasheet document that relationship? Table 4 on the datasheet I looked
at to seems to give settling time but I think is only relevant to low power
modes that might not apply here.

Would be very odd if there is no way to establish the sampling frequency from
the settings configured.

> 
> > > I was hoping that simply passing along a unitless value between 0 and
> > > parameter_max would be enough to customize all the event parameters.
> > > this does mean that the driver makes the assumption that the user is
> > > familiar with the device datasheet and knows the number of bits every
> > > parameter has been allocated. should the driver provide a conversion
> > > table for tt_duration just like for _scale_table and
> > > _lpf_3dB_freq_Hz_table?  
> > 
> > Exactly.  
> 
> I was thinking today of a more analog-feeling API, one in which a variable that can take values linearly between min and max can be set to a percentage of it's scale. think of stereo systems - most of us don't want to set a precise amount of decibels of attenuation when operating the volume knob, we just want to set it lower or higher until a condition matches. in this API the primary unit of measurement would not be dBs but notches or ticks - calculated based on min, max and the native resolution of the control (how many bits are allocated for it in the ic's memory map). this has also the benefit of translating nicely when the control is rendered as a widget in a GUI. think about a 0 to 11 volume knob.
> is there anything like this already implemented? is there any merit to this idea?

ABI is fixed the day it is defined and I'm not keen to have what sounds like
a duplicate ABI.  We also need to if even vaguely possible have consistency across sensors.
If you do it on basis of range set, and one sensor oddly provides twice the range
of another, then a 5 on one is a 10 on that other.

Hence the only things where we can do this sort of percentage thing are where
the thing being controlled is a unit less ratio and hence a percentage type
control is the only possible units.

Thanks,

Jonathan


> 
>  I will shelve the event part of this driver for another time, just got some Honeywell pressure sensors that need a new driver.
> 
> best regards,
> peter