[PATCH v5 3/4] iio: imu: bmi270: add support for motion events

Gustavo Silva posted 4 patches 1 month ago
Only 2 patches received!
There is a newer version of this series
[PATCH v5 3/4] iio: imu: bmi270: add support for motion events
Posted by Gustavo Silva 1 month ago
Any-motion event can be enabled on a per-axis basis and triggers a
combined event when motion is detected on any axis.

No-motion event is triggered if the rate of change on all axes falls
below a specified threshold for a configurable duration. A fake channel
is used to report this event.

Threshold and duration can be configured from userspace.

Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 384 +++++++++++++++++++++++++--
 1 file changed, 364 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 519f1c9d466d..714754160c2b 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -31,6 +31,8 @@
 
 #define BMI270_INT_STATUS_0_REG				0x1c
 #define BMI270_INT_STATUS_0_STEP_CNT_MSK		BIT(1)
+#define BMI270_INT_STATUS_0_NOMOTION_MSK		BIT(5)
+#define BMI270_INT_STATUS_0_MOTION_MSK			BIT(6)
 
 #define BMI270_INT_STATUS_1_REG				0x1d
 #define BMI270_INT_STATUS_1_ACC_GYR_DRDY_MSK		GENMASK(7, 6)
@@ -81,6 +83,8 @@
 #define BMI270_INT1_MAP_FEAT_REG			0x56
 #define BMI270_INT2_MAP_FEAT_REG			0x57
 #define BMI270_INT_MAP_FEAT_STEP_CNT_WTRMRK_MSK		BIT(1)
+#define BMI270_INT_MAP_FEAT_NOMOTION_MSK		BIT(5)
+#define BMI270_INT_MAP_FEAT_ANYMOTION_MSK		BIT(6)
 
 #define BMI270_INT_MAP_DATA_REG				0x58
 #define BMI270_INT_MAP_DATA_DRDY_INT1_MSK		BIT(2)
@@ -106,6 +110,25 @@
 #define BMI270_STEP_SC26_RST_CNT_MSK			BIT(10)
 #define BMI270_STEP_SC26_EN_CNT_MSK			BIT(12)
 
+#define BMI270_FEAT_MOTION_DURATION_MSK			GENMASK(12, 0)
+#define BMI270_FEAT_MOTION_X_EN_MSK			BIT(13)
+#define BMI270_FEAT_MOTION_Y_EN_MSK			BIT(14)
+#define BMI270_FEAT_MOTION_Z_EN_MSK			BIT(15)
+#define BMI270_FEAT_MOTION_XYZ_EN_MSK			GENMASK(15, 13)
+#define BMI270_FEAT_MOTION_THRESHOLD_MSK		GENMASK(10, 0)
+#define BMI270_FEAT_MOTION_OUT_CONF_MSK			GENMASK(14, 11)
+#define BMI270_FEAT_MOTION_ENABLE_MSK			BIT(15)
+
+#define BMI270_MOTION_XYZ_MSK				GENMASK(2, 0)
+
+/* See pages 92 and 93 of the datasheet */
+#define BMI270_MOTION_THRES_FULL_SCALE			GENMASK(10, 0)
+#define BMI270_MOTION_DURAT_SCALE			50
+#define BMI270_MOTION_DURAT_MAX				162
+
+/* 9.81 * 1000000 m/s^2 */
+#define BMI270_G_MICRO_M_S_2				9810000
+
 /* See datasheet section 4.6.14, Temperature Sensor */
 #define BMI270_TEMP_OFFSET				11776
 #define BMI270_TEMP_SCALE				1953125
@@ -114,6 +137,11 @@
 #define BMI270_STEP_COUNTER_FACTOR			20
 #define BMI270_STEP_COUNTER_MAX				20460
 
+#define BMI270_INT_MICRO_TO_RAW(val, val2, scale) \
+	((val) * (scale) + ((val2) * (scale)) / MEGA)
+#define BMI270_RAW_TO_MICRO(raw, scale) \
+	((((raw) % (scale)) * MEGA) / scale)
+
 #define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
 #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
 
@@ -309,6 +337,13 @@ static const struct  bmi270_odr_item bmi270_odr_table[] = {
 };
 
 enum bmi270_feature_reg_id {
+	/* Page 1 registers */
+	BMI270_ANYMO1_REG,
+	BMI270_ANYMO2_REG,
+	/* Page 2 registers */
+	BMI270_NOMO1_REG,
+	BMI270_NOMO2_REG,
+	/* Page 6 registers */
 	BMI270_SC_26_REG,
 };
 
@@ -318,6 +353,22 @@ struct bmi270_feature_reg {
 };
 
 static const struct bmi270_feature_reg bmi270_feature_regs[] = {
+	[BMI270_ANYMO1_REG] = {
+		.page = 1,
+		.addr = 0x3c,
+	},
+	[BMI270_ANYMO2_REG] = {
+		.page = 1,
+		.addr = 0x3e,
+	},
+	[BMI270_NOMO1_REG] = {
+		.page = 2,
+		.addr = 0x30,
+	},
+	[BMI270_NOMO2_REG] = {
+		.page = 2,
+		.addr = 0x32,
+	},
 	[BMI270_SC_26_REG] = {
 		.page = 6,
 		.addr = 0x32,
@@ -439,6 +490,121 @@ static int bmi270_step_wtrmrk_en(struct bmi270_data *data, bool state)
 					     state));
 }
 
+static int bmi270_motion_reg(enum iio_event_type type, enum iio_event_info info)
+{
+	switch (info) {
+	case IIO_EV_INFO_PERIOD:
+		switch (type) {
+		case IIO_EV_TYPE_MAG_ADAPTIVE:
+			return BMI270_ANYMO1_REG;
+		case IIO_EV_TYPE_ROC:
+			return BMI270_NOMO1_REG;
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_VALUE:
+		switch (type) {
+		case IIO_EV_TYPE_MAG_ADAPTIVE:
+			return BMI270_ANYMO2_REG;
+		case IIO_EV_TYPE_ROC:
+			return BMI270_NOMO2_REG;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bmi270_anymotion_event_en(struct bmi270_data *data,
+				     struct iio_chan_spec const *chan,
+				     bool state)
+{
+	u16 axis_msk, axis_field_val, regval;
+	int ret, irq_reg;
+	bool axis_en;
+
+	irq_reg = bmi270_int_map_reg(data->irq_pin);
+	if (irq_reg < 0)
+		return irq_reg;
+
+	guard(mutex)(&data->mutex);
+
+	ret = bmi270_read_feature_reg(data, BMI270_ANYMO1_REG, &regval);
+	if (ret)
+		return ret;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		axis_msk = BMI270_FEAT_MOTION_X_EN_MSK;
+		axis_field_val = FIELD_PREP(BMI270_FEAT_MOTION_X_EN_MSK, state);
+		axis_en = FIELD_GET(BMI270_FEAT_MOTION_Y_EN_MSK, regval) |
+			  FIELD_GET(BMI270_FEAT_MOTION_Z_EN_MSK, regval);
+		break;
+	case IIO_MOD_Y:
+		axis_msk = BMI270_FEAT_MOTION_Y_EN_MSK;
+		axis_field_val = FIELD_PREP(BMI270_FEAT_MOTION_Y_EN_MSK, state);
+		axis_en = FIELD_GET(BMI270_FEAT_MOTION_X_EN_MSK, regval) |
+			  FIELD_GET(BMI270_FEAT_MOTION_Z_EN_MSK, regval);
+		break;
+	case IIO_MOD_Z:
+		axis_msk = BMI270_FEAT_MOTION_Z_EN_MSK;
+		axis_field_val = FIELD_PREP(BMI270_FEAT_MOTION_Z_EN_MSK, state);
+		axis_en = FIELD_GET(BMI270_FEAT_MOTION_X_EN_MSK, regval) |
+			  FIELD_GET(BMI270_FEAT_MOTION_Y_EN_MSK, regval);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = bmi270_update_feature_reg(data, BMI270_ANYMO1_REG, axis_msk,
+					axis_field_val);
+	if (ret)
+		return ret;
+
+	ret = bmi270_update_feature_reg(data, BMI270_ANYMO2_REG,
+					BMI270_FEAT_MOTION_ENABLE_MSK,
+					FIELD_PREP(BMI270_FEAT_MOTION_ENABLE_MSK,
+						   state || axis_en));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap, irq_reg,
+				  BMI270_INT_MAP_FEAT_ANYMOTION_MSK,
+				  FIELD_PREP(BMI270_INT_MAP_FEAT_ANYMOTION_MSK,
+					     state || axis_en));
+}
+
+static int bmi270_nomotion_event_en(struct bmi270_data *data, bool state)
+{
+	int ret, irq_reg;
+
+	irq_reg = bmi270_int_map_reg(data->irq_pin);
+	if (irq_reg < 0)
+		return irq_reg;
+
+	guard(mutex)(&data->mutex);
+
+	ret = bmi270_update_feature_reg(data, BMI270_NOMO1_REG,
+					BMI270_FEAT_MOTION_XYZ_EN_MSK,
+					FIELD_PREP(BMI270_FEAT_MOTION_XYZ_EN_MSK,
+						   state ? BMI270_MOTION_XYZ_MSK : 0));
+	if (ret)
+		return ret;
+
+	ret = bmi270_update_feature_reg(data, BMI270_NOMO2_REG,
+					BMI270_FEAT_MOTION_ENABLE_MSK,
+					FIELD_PREP(BMI270_FEAT_MOTION_ENABLE_MSK,
+						   state));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap, irq_reg,
+				  BMI270_INT_MAP_FEAT_NOMOTION_MSK,
+				  FIELD_PREP(BMI270_INT_MAP_FEAT_NOMOTION_MSK,
+					     state));
+}
+
 static int bmi270_set_scale(struct bmi270_data *data, int chan_type, int uscale)
 {
 	int i;
@@ -479,8 +645,6 @@ static int bmi270_get_scale(struct bmi270_data *data, int chan_type, int *scale,
 	unsigned int val;
 	struct bmi270_scale_item bmi270_scale_item;
 
-	guard(mutex)(&data->mutex);
-
 	switch (chan_type) {
 	case IIO_ACCEL:
 		ret = regmap_read(data->regmap, BMI270_ACC_CONF_RANGE_REG, &val);
@@ -614,6 +778,20 @@ static irqreturn_t bmi270_irq_thread_handler(int irq, void *private)
 	if (FIELD_GET(BMI270_INT_STATUS_1_ACC_GYR_DRDY_MSK, status1))
 		iio_trigger_poll_nested(data->trig);
 
+	if (FIELD_GET(BMI270_INT_STATUS_0_MOTION_MSK, status0))
+		iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							     IIO_MOD_X_OR_Y_OR_Z,
+							     IIO_EV_TYPE_MAG_ADAPTIVE,
+							     IIO_EV_DIR_RISING),
+			       timestamp);
+
+	if (FIELD_GET(BMI270_INT_STATUS_0_NOMOTION_MSK, status0))
+		iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+							     IIO_MOD_X_AND_Y_AND_Z,
+							     IIO_EV_TYPE_ROC,
+							     IIO_EV_DIR_RISING),
+			       timestamp);
+
 	if (FIELD_GET(BMI270_INT_STATUS_0_STEP_CNT_MSK, status0))
 		iio_push_event(indio_dev, IIO_UNMOD_EVENT_CODE(IIO_STEPS, 0,
 							       IIO_EV_TYPE_CHANGE,
@@ -827,6 +1005,40 @@ static int bmi270_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static ssize_t bmi270_show_accel_value_avail(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct bmi270_data *data = iio_priv(indio_dev);
+	int ret, scale, uscale;
+	unsigned int step, max;
+
+	ret = bmi270_get_scale(data, IIO_ACCEL, &scale, &uscale);
+	if (ret)
+		return ret;
+
+	max = BMI270_G_MICRO_M_S_2 / uscale;
+	step = max / BMI270_MOTION_THRES_FULL_SCALE;
+
+	return sysfs_emit(buf, "[0 %u %u]\n", step, max);
+}
+
+static IIO_DEVICE_ATTR(in_accel_value_available, 0444,
+		       bmi270_show_accel_value_avail, NULL, 0);
+
+static IIO_CONST_ATTR(in_accel_period_available, "[0.0 0.02 162.0]");
+
+static struct attribute *bmi270_event_attributes[] = {
+	&iio_dev_attr_in_accel_value_available.dev_attr.attr,
+	&iio_const_attr_in_accel_period_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group bmi270_event_attribute_group = {
+	.attrs = bmi270_event_attributes,
+};
+
 static int bmi270_write_event_config(struct iio_dev *indio_dev,
 				     const struct iio_chan_spec *chan,
 				     enum iio_event_type type,
@@ -835,6 +1047,10 @@ static int bmi270_write_event_config(struct iio_dev *indio_dev,
 	struct bmi270_data *data = iio_priv(indio_dev);
 
 	switch (type) {
+	case IIO_EV_TYPE_MAG_ADAPTIVE:
+		return bmi270_anymotion_event_en(data, chan, state);
+	case IIO_EV_TYPE_ROC:
+		return bmi270_nomotion_event_en(data, state);
 	case IIO_EV_TYPE_CHANGE:
 		return bmi270_step_wtrmrk_en(data, state);
 	default:
@@ -848,21 +1064,57 @@ static int bmi270_read_event_config(struct iio_dev *indio_dev,
 				    enum iio_event_direction dir)
 {
 	struct bmi270_data *data = iio_priv(indio_dev);
+	bool feat_en, axis_en;
 	int ret, reg, regval;
+	u16 motion_reg;
 
 	guard(mutex)(&data->mutex);
 
+	reg = bmi270_int_map_reg(data->irq_pin);
+	if (reg < 0)
+		return reg;
+
+	ret = regmap_read(data->regmap, reg, &regval);
+	if (ret)
+		return ret;
+
 	switch (chan->type) {
 	case IIO_STEPS:
-		reg = bmi270_int_map_reg(data->irq_pin);
-		if (reg)
-			return reg;
+		return FIELD_GET(BMI270_INT_MAP_FEAT_STEP_CNT_WTRMRK_MSK, regval) ?
+									1 : 0;
+	case IIO_ACCEL:
+		switch (type) {
+		case IIO_EV_TYPE_ROC:
+			return FIELD_GET(BMI270_INT_MAP_FEAT_NOMOTION_MSK,
+					 regval) ? 1 : 0;
+		case IIO_EV_TYPE_MAG_ADAPTIVE:
+			ret = bmi270_read_feature_reg(data, BMI270_ANYMO1_REG,
+						      &motion_reg);
+			if (ret)
+				return ret;
 
-		ret = regmap_read(data->regmap, reg, &regval);
-		if (ret)
-			return ret;
-		return FIELD_GET(BMI270_INT_MAP_FEAT_STEP_CNT_WTRMRK_MSK,
-				 regval) ? 1 : 0;
+			feat_en = FIELD_GET(BMI270_INT_MAP_FEAT_ANYMOTION_MSK,
+					    regval);
+			switch (chan->channel2) {
+			case IIO_MOD_X:
+				axis_en = FIELD_GET(BMI270_FEAT_MOTION_X_EN_MSK,
+						    motion_reg);
+				break;
+			case IIO_MOD_Y:
+				axis_en = FIELD_GET(BMI270_FEAT_MOTION_Y_EN_MSK,
+						    motion_reg);
+				break;
+			case IIO_MOD_Z:
+				axis_en = FIELD_GET(BMI270_FEAT_MOTION_Z_EN_MSK,
+						    motion_reg);
+				break;
+			default:
+				return -EINVAL;
+			}
+			return axis_en && feat_en;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -876,20 +1128,50 @@ static int bmi270_write_event_value(struct iio_dev *indio_dev,
 				    int val, int val2)
 {
 	struct bmi270_data *data = iio_priv(indio_dev);
-	unsigned int raw;
+	unsigned int raw, mask, regval;
+	int ret, reg, scale, uscale;
+	u64 tmp;
 
 	guard(mutex)(&data->mutex);
 
-	switch (type) {
-	case IIO_EV_TYPE_CHANGE:
+	if (type == IIO_EV_TYPE_CHANGE) {
 		if (!in_range(val, 0, BMI270_STEP_COUNTER_MAX + 1))
 			return -EINVAL;
 
 		raw = val / BMI270_STEP_COUNTER_FACTOR;
-		return bmi270_update_feature_reg(data, BMI270_SC_26_REG,
-						 BMI270_STEP_SC26_WTRMRK_MSK,
-						 FIELD_PREP(BMI270_STEP_SC26_WTRMRK_MSK,
-							    raw));
+		mask = BMI270_STEP_SC26_WTRMRK_MSK;
+		regval = FIELD_PREP(BMI270_STEP_SC26_WTRMRK_MSK, raw);
+		return bmi270_update_feature_reg(data, BMI270_SC_26_REG, mask,
+						 regval);
+	}
+
+	reg = bmi270_motion_reg(type, info);
+	if (reg < 0)
+		return reg;
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		ret = bmi270_get_scale(data, IIO_ACCEL, &scale, &uscale);
+		if (ret)
+			return ret;
+
+		if (!in_range(val, 0, (BMI270_G_MICRO_M_S_2 / uscale) + 1))
+			return -EINVAL;
+
+		tmp = (u64)val * BMI270_MOTION_THRES_FULL_SCALE * uscale;
+		raw = DIV_ROUND_CLOSEST_ULL(tmp, BMI270_G_MICRO_M_S_2);
+		mask = BMI270_FEAT_MOTION_THRESHOLD_MSK;
+		regval = FIELD_PREP(BMI270_FEAT_MOTION_THRESHOLD_MSK, raw);
+		return bmi270_update_feature_reg(data, reg, mask, regval);
+	case IIO_EV_INFO_PERIOD:
+		if (!in_range(val, 0, BMI270_MOTION_DURAT_MAX + 1))
+			return -EINVAL;
+
+		raw = BMI270_INT_MICRO_TO_RAW(val, val2,
+					      BMI270_MOTION_DURAT_SCALE);
+		mask = BMI270_FEAT_MOTION_DURATION_MSK;
+		regval = FIELD_PREP(BMI270_FEAT_MOTION_DURATION_MSK, raw);
+		return bmi270_update_feature_reg(data, reg, mask, regval);
 	default:
 		return -EINVAL;
 	}
@@ -903,14 +1185,14 @@ static int bmi270_read_event_value(struct iio_dev *indio_dev,
 				   int *val, int *val2)
 {
 	struct bmi270_data *data = iio_priv(indio_dev);
+	int ret, reg, scale, uscale;
 	unsigned int raw;
 	u16 regval;
-	int ret;
+	u64 tmp;
 
 	guard(mutex)(&data->mutex);
 
-	switch (type) {
-	case IIO_EV_TYPE_CHANGE:
+	if (type == IIO_EV_TYPE_CHANGE) {
 		ret = bmi270_read_feature_reg(data, BMI270_SC_26_REG, &regval);
 		if (ret)
 			return ret;
@@ -918,6 +1200,36 @@ static int bmi270_read_event_value(struct iio_dev *indio_dev,
 		raw = FIELD_GET(BMI270_STEP_SC26_WTRMRK_MSK, regval);
 		*val = raw * BMI270_STEP_COUNTER_FACTOR;
 		return IIO_VAL_INT;
+	}
+
+	reg = bmi270_motion_reg(type, info);
+	if (reg < 0)
+		return reg;
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		ret = bmi270_read_feature_reg(data, reg, &regval);
+		if (ret)
+			return ret;
+
+		ret = bmi270_get_scale(data, IIO_ACCEL, &scale, &uscale);
+		if (ret)
+			return ret;
+
+		raw = FIELD_GET(BMI270_FEAT_MOTION_THRESHOLD_MSK, regval);
+		tmp = (u64)raw * BMI270_G_MICRO_M_S_2;
+		*val = DIV_ROUND_CLOSEST_ULL(tmp,
+					     BMI270_MOTION_THRES_FULL_SCALE * uscale);
+		return IIO_VAL_INT;
+	case IIO_EV_INFO_PERIOD:
+		ret = bmi270_read_feature_reg(data, reg, &regval);
+		if (ret)
+			return ret;
+
+		raw = FIELD_GET(BMI270_FEAT_MOTION_DURATION_MSK, regval);
+		*val = raw / BMI270_MOTION_DURAT_SCALE;
+		*val2 = BMI270_RAW_TO_MICRO(raw, BMI270_MOTION_DURAT_SCALE);
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -929,6 +1241,20 @@ static const struct iio_event_spec bmi270_step_wtrmrk_event = {
 	.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE) | BIT(IIO_EV_INFO_VALUE),
 };
 
+static const struct iio_event_spec bmi270_anymotion_event = {
+	.type = IIO_EV_TYPE_MAG_ADAPTIVE,
+	.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),
+};
+
+static const struct iio_event_spec bmi270_nomotion_event = {
+	.type = IIO_EV_TYPE_ROC,
+	.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),
+};
+
 static const struct iio_info bmi270_info = {
 	.read_raw = bmi270_read_raw,
 	.write_raw = bmi270_write_raw,
@@ -937,6 +1263,7 @@ static const struct iio_info bmi270_info = {
 	.read_event_config = bmi270_read_event_config,
 	.write_event_value = bmi270_write_event_value,
 	.read_event_value = bmi270_read_event_value,
+	.event_attrs = &bmi270_event_attribute_group,
 };
 
 #define BMI270_ACCEL_CHANNEL(_axis) {				\
@@ -956,6 +1283,8 @@ static const struct iio_info bmi270_info = {
 		.storagebits = 16,				\
 		.endianness = IIO_LE,				\
 	},	                                                \
+	.event_spec = &bmi270_anymotion_event,			\
+	.num_event_specs = 1,					\
 }
 
 #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
@@ -1000,6 +1329,14 @@ static const struct iio_chan_spec bmi270_channels[] = {
 		.num_event_specs = 1,
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(BMI270_SCAN_TIMESTAMP),
+	{
+		.type = IIO_ACCEL,
+		.modified = 1,
+		.channel2 = IIO_MOD_X_AND_Y_AND_Z,
+		.scan_index = -1, /* Fake channel */
+		.event_spec = &bmi270_nomotion_event,
+		.num_event_specs = 1,
+	},
 };
 
 static int bmi270_int_pin_config(struct bmi270_data *data,
@@ -1107,6 +1444,13 @@ static int bmi270_trigger_probe(struct bmi270_data *data,
 		return dev_err_probe(data->dev, ret,
 				     "Trigger registration failed\n");
 
+	/* Disable axes for motion events */
+	ret = bmi270_update_feature_reg(data, BMI270_ANYMO1_REG,
+					BMI270_FEAT_MOTION_XYZ_EN_MSK,
+					FIELD_PREP(BMI270_FEAT_MOTION_XYZ_EN_MSK, 0));
+	if (ret)
+		return ret;
+
 	data->irq_pin = irq_pin;
 
 	return 0;
-- 
2.51.0
Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events
Posted by Andy Shevchenko 1 month ago
On Sat, Aug 30, 2025 at 2:58 PM Gustavo Silva <gustavograzs@gmail.com> wrote:
>
> Any-motion event can be enabled on a per-axis basis and triggers a
> combined event when motion is detected on any axis.
>
> No-motion event is triggered if the rate of change on all axes falls
> below a specified threshold for a configurable duration. A fake channel
> is used to report this event.
>
> Threshold and duration can be configured from userspace.

...

> +#define BMI270_INT_MICRO_TO_RAW(val, val2, scale) \
> +       ((val) * (scale) + ((val2) * (scale)) / MEGA)
> +#define BMI270_RAW_TO_MICRO(raw, scale) \
> +       ((((raw) % (scale)) * MEGA) / scale)

In the macro names "MICRO" in the implementation "MEGA", please make
this consistent.

...

> +static ssize_t bmi270_show_accel_value_avail(struct device *dev,
> +                                            struct device_attribute *attr,
> +                                            char *buf)
> +{
> +       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +       struct bmi270_data *data = iio_priv(indio_dev);
> +       int ret, scale, uscale;
> +       unsigned int step, max;
> +
> +       ret = bmi270_get_scale(data, IIO_ACCEL, &scale, &uscale);
> +       if (ret)
> +               return ret;
> +
> +       max = BMI270_G_MICRO_M_S_2 / uscale;
> +       step = max / BMI270_MOTION_THRES_FULL_SCALE;
> +
> +       return sysfs_emit(buf, "[0 %u %u]\n", step, max);

Do we need []? Is it common in IIO? The usual way in SW to have "$min
$max $step" (see `seq` in shell, range() in Python and so on).

> +}
> +
> +static IIO_DEVICE_ATTR(in_accel_value_available, 0444,
> +                      bmi270_show_accel_value_avail, NULL, 0);

IIO_DEVICE_ATTR_RO()

...

> +               return FIELD_GET(BMI270_INT_MAP_FEAT_STEP_CNT_WTRMRK_MSK, regval) ?
> +                                                                       1 : 0;

Make it one line by replacing ternary with !! (double exclamation mark).

...

> +                       return FIELD_GET(BMI270_INT_MAP_FEAT_NOMOTION_MSK,
> +                                        regval) ? 1 : 0;

Ditto.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events
Posted by Gustavo Silva 2 weeks, 6 days ago
On Sat, Aug 30, 2025 at 03:44:37PM +0300, Andy Shevchenko wrote:

Hi Andy. Thank you for the review. I just have one question before
sending v6.
> 
> > +#define BMI270_INT_MICRO_TO_RAW(val, val2, scale) \
> > +       ((val) * (scale) + ((val2) * (scale)) / MEGA)
> > +#define BMI270_RAW_TO_MICRO(raw, scale) \
> > +       ((((raw) % (scale)) * MEGA) / scale)
> 
> In the macro names "MICRO" in the implementation "MEGA", please make
> this consistent.
> 
These macros are intended to work with values of type
IIO_VAL_INT_PLUS_MICRO. The division by MEGA is what produces the
fractional part in micro, so the naming was chosen to reflect the
result.
If you prefer, I can rename them for clarity. But please note that the
same naming is already used in the BMI323 driver, and I kept it
consistent here.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events
Posted by Andy Shevchenko 2 weeks, 5 days ago
On Sat, Sep 13, 2025 at 3:45 PM Gustavo Silva <gustavograzs@gmail.com> wrote:
> On Sat, Aug 30, 2025 at 03:44:37PM +0300, Andy Shevchenko wrote:
>
> > > +#define BMI270_INT_MICRO_TO_RAW(val, val2, scale) \
> > > +       ((val) * (scale) + ((val2) * (scale)) / MEGA)
> > > +#define BMI270_RAW_TO_MICRO(raw, scale) \
> > > +       ((((raw) % (scale)) * MEGA) / scale)
> >
> > In the macro names "MICRO" in the implementation "MEGA", please make
> > this consistent.
> >
> These macros are intended to work with values of type
> IIO_VAL_INT_PLUS_MICRO. The division by MEGA is what produces the
> fractional part in micro, so the naming was chosen to reflect the
> result.
> If you prefer, I can rename them for clarity. But please note that the
> same naming is already used in the BMI323 driver, and I kept it
> consistent here.

Ah, okay, this makes sense. So, if we ever want to switch it's better
to do it with both (or more?) drivers.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events
Posted by Jonathan Cameron 1 month ago
Just picking out one thing to reply to...
> 
> > +static ssize_t bmi270_show_accel_value_avail(struct device *dev,
> > +                                            struct device_attribute *attr,
> > +                                            char *buf)
> > +{
> > +       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +       struct bmi270_data *data = iio_priv(indio_dev);
> > +       int ret, scale, uscale;
> > +       unsigned int step, max;
> > +
> > +       ret = bmi270_get_scale(data, IIO_ACCEL, &scale, &uscale);
> > +       if (ret)
> > +               return ret;
> > +
> > +       max = BMI270_G_MICRO_M_S_2 / uscale;
> > +       step = max / BMI270_MOTION_THRES_FULL_SCALE;
> > +
> > +       return sysfs_emit(buf, "[0 %u %u]\n", step, max);  
> 
> Do we need []? Is it common in IIO? The usual way in SW to have "$min
> $max $step" (see `seq` in shell, range() in Python and so on).
> 
yes. See ABI docs for _available.  It comes in two forms.
space separted list of values and this one which is used for specifying
limits and step.  The [] is how we tell which one it is.

For anything other than events we handle all this via callbacks and
masks. I've never bothered adding the complexity of that to events
as _available attributes are fairly rare.
Re: [PATCH v5 3/4] iio: imu: bmi270: add support for motion events
Posted by Andy Shevchenko 1 month ago
On Sat, Aug 30, 2025 at 3:44 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Aug 30, 2025 at 2:58 PM Gustavo Silva <gustavograzs@gmail.com> wrote:

...

> > +       return sysfs_emit(buf, "[0 %u %u]\n", step, max);
>
> Do we need []? Is it common in IIO? The usual way in SW to have "$min
> $max $step" (see `seq` in shell, range() in Python and so on).

I stand corrected `seq` actually gives what you used. So, the first
question is still relevant. Use what is the common case in the other
IIO drivers / core.


-- 
With Best Regards,
Andy Shevchenko