[PATCH v2 0/9] st_lsm6dsx: add tap event detection

[PATCH v2 0/9] st_lsm6dsx: add tap event detection

[Francesco Lavra]

The bulk of this patch set consists of reworking the existing code for
event detection (which supports IIO_EV_TYPE_THRESH events only) in order to
make it generic to accommodate different event types. Actual support for
tap events is implemented in the last patch.
Tested on LSMDSV16X.

Changes from v1 [1]:
- added Fixes tag to patch 1/9 (Andy, Lorenzo)
- added st_lsm6dsx_field_get() temporary macro, to be removed once the
  generic FIELD_GET() macro lands (Andy)
- refactored st_lsm6dsx_write_event_config() with the addition of two
  helper functions st_lsm6dsx_check_other_events() and
  st_lsm6dsx_events_enable() (Andy)
- replaced dynamic allocation of iio_chan_spec and iio_event_spec arrays
  with the addition of new static arrays (Jonathan)
- changed line wrapping to 80 characters (Lorenzo, Andy)
- changed line wrapping for commit messages to 75 characters (Jonathan)
- added comments to st_lsm6dsx_event_setup() and st_lsm6dsx_get_event_reg()
  (Andy)
- miscellaneous stylistic changes (Andy)

[1] https://lore.kernel.org/linux-iio/20251030072752.349633-1-flavra@baylibre.com/T/

Francesco Lavra (9):
  iio: imu: st_lsm6dsx: fix iio_chan_spec for sensors without event
    detection
  iio: imu: st_lsm6dsx: make event_settings more generic
  iio: imu: st_lsm6dsx: move wakeup event enable mask to event_src
  iio: imu: st_lsm6dsx: rework code to check for enabled events
  iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
  iio: imu: st_lsm6dsx: make event management functions generic
  iio: imu: st_lsm6dsx: add event configurability on a per axis basis
  iio: imu: st_lsm6dsx: add event spec parameter to iio_chan_spec
    initializer
  iio: imu: st_lsm6dsx: add tap event detection

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  56 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 538 +++++++++++++------
 2 files changed, 395 insertions(+), 199 deletions(-)

-- 
2.39.5

[PATCH v2 1/9] iio: imu: st_lsm6dsx: fix iio_chan_spec for sensors without event detection

[Francesco Lavra]

The st_lsm6dsx_acc_channels array of struct iio_chan_spec has a non-NULL
event_spec field, indicating support for IIO events. However, event
detection is not supported for all sensors, and if userspace tries to
configure accelerometer wakeup events on a sensor device that does not
support them (e.g. LSM6DS0), st_lsm6dsx_write_event() dereferences a NULL
pointer when trying to write to the wakeup register.
Define an additional struct iio_chan_spec array whose members have a NULL
event_spec field, and use this array instead of st_lsm6dsx_acc_channels for
sensors without event detection capability.

Fixes: b5969abfa8b8 ("iio: imu: st_lsm6dsx: add motion events")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 216160549b5a..a09df9d772dd 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -103,6 +103,13 @@ static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+static const struct iio_chan_spec st_lsm6ds0_acc_channels[] = {
+	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
+	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
+	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
 static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
 	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x22, IIO_MOD_X, 0),
 	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x24, IIO_MOD_Y, 1),
@@ -144,8 +151,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 		},
 		.channels = {
 			[ST_LSM6DSX_ID_ACC] = {
-				.chan = st_lsm6dsx_acc_channels,
-				.len = ARRAY_SIZE(st_lsm6dsx_acc_channels),
+				.chan = st_lsm6ds0_acc_channels,
+				.len = ARRAY_SIZE(st_lsm6ds0_acc_channels),
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.chan = st_lsm6ds0_gyro_channels,
@@ -1445,8 +1452,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 		},
 		.channels = {
 			[ST_LSM6DSX_ID_ACC] = {
-				.chan = st_lsm6dsx_acc_channels,
-				.len = ARRAY_SIZE(st_lsm6dsx_acc_channels),
+				.chan = st_lsm6ds0_acc_channels,
+				.len = ARRAY_SIZE(st_lsm6ds0_acc_channels),
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.chan = st_lsm6dsx_gyro_channels,
-- 
2.39.5

Re: [PATCH v2 1/9] iio: imu: st_lsm6dsx: fix iio_chan_spec for sensors without event detection

[Lorenzo Bianconi]

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

> The st_lsm6dsx_acc_channels array of struct iio_chan_spec has a non-NULL
> event_spec field, indicating support for IIO events. However, event
> detection is not supported for all sensors, and if userspace tries to
> configure accelerometer wakeup events on a sensor device that does not
> support them (e.g. LSM6DS0), st_lsm6dsx_write_event() dereferences a NULL
> pointer when trying to write to the wakeup register.
> Define an additional struct iio_chan_spec array whose members have a NULL
> event_spec field, and use this array instead of st_lsm6dsx_acc_channels for
> sensors without event detection capability.
> 
> Fixes: b5969abfa8b8 ("iio: imu: st_lsm6dsx: add motion events")
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 216160549b5a..a09df9d772dd 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -103,6 +103,13 @@ static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
> +static const struct iio_chan_spec st_lsm6ds0_acc_channels[] = {
> +	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
> +	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
> +	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
>  static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
>  	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x22, IIO_MOD_X, 0),
>  	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x24, IIO_MOD_Y, 1),
> @@ -144,8 +151,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  		},
>  		.channels = {
>  			[ST_LSM6DSX_ID_ACC] = {
> -				.chan = st_lsm6dsx_acc_channels,
> -				.len = ARRAY_SIZE(st_lsm6dsx_acc_channels),
> +				.chan = st_lsm6ds0_acc_channels,
> +				.len = ARRAY_SIZE(st_lsm6ds0_acc_channels),
>  			},
>  			[ST_LSM6DSX_ID_GYRO] = {
>  				.chan = st_lsm6ds0_gyro_channels,
> @@ -1445,8 +1452,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  		},
>  		.channels = {
>  			[ST_LSM6DSX_ID_ACC] = {
> -				.chan = st_lsm6dsx_acc_channels,
> -				.len = ARRAY_SIZE(st_lsm6dsx_acc_channels),
> +				.chan = st_lsm6ds0_acc_channels,
> +				.len = ARRAY_SIZE(st_lsm6ds0_acc_channels),
>  			},
>  			[ST_LSM6DSX_ID_GYRO] = {
>  				.chan = st_lsm6dsx_gyro_channels,
> -- 
> 2.39.5
> 

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

[PATCH v2 2/9] iio: imu: st_lsm6dsx: make event_settings more generic

[Francesco Lavra]

The st_lsm6dsx_event_settings structure contains fields specific for one
event type (wakeup). In preparation for adding support for more event
types, introduce an event id enum and a generic event source structure, and
replace wakeup-specific data in struct st_lsm6dsx_event_settings with an
array of event source structures.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  21 ++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 134 +++++++++++--------
 2 files changed, 95 insertions(+), 60 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index a4f558899767..4c3ff1cc0097 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -242,14 +242,23 @@ struct st_lsm6dsx_shub_settings {
 	u8 pause;
 };
 
+enum st_lsm6dsx_event_id {
+	ST_LSM6DSX_EVENT_WAKEUP,
+	ST_LSM6DSX_EVENT_MAX
+};
+
+struct st_lsm6dsx_event_src {
+	struct st_lsm6dsx_reg value;
+	u8 status_reg;
+	u8 status_mask;
+	u8 status_x_mask;
+	u8 status_y_mask;
+	u8 status_z_mask;
+};
+
 struct st_lsm6dsx_event_settings {
 	struct st_lsm6dsx_reg enable_reg;
-	struct st_lsm6dsx_reg wakeup_reg;
-	u8 wakeup_src_reg;
-	u8 wakeup_src_status_mask;
-	u8 wakeup_src_z_mask;
-	u8 wakeup_src_y_mask;
-	u8 wakeup_src_x_mask;
+	struct st_lsm6dsx_event_src sources[ST_LSM6DSX_EVENT_MAX];
 };
 
 enum st_lsm6dsx_ext_sensor_id {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index a09df9d772dd..a71174e75f44 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -388,15 +388,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 			},
 		},
 		.event_settings = {
-			.wakeup_reg = {
-				.addr = 0x5B,
-				.mask = GENMASK(5, 0),
+			.sources = {
+				[ST_LSM6DSX_EVENT_WAKEUP] = {
+					.value = {
+						.addr = 0x5b,
+						.mask = GENMASK(5, 0),
+					},
+					.status_reg = 0x1b,
+					.status_mask = BIT(3),
+					.status_z_mask = BIT(0),
+					.status_y_mask = BIT(1),
+					.status_x_mask = BIT(2),
+				},
 			},
-			.wakeup_src_reg = 0x1b,
-			.wakeup_src_status_mask = BIT(3),
-			.wakeup_src_z_mask = BIT(0),
-			.wakeup_src_y_mask = BIT(1),
-			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -554,15 +558,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 			},
 		},
 		.event_settings = {
-			.wakeup_reg = {
-				.addr = 0x5B,
-				.mask = GENMASK(5, 0),
+			.sources = {
+				[ST_LSM6DSX_EVENT_WAKEUP] = {
+					.value = {
+						.addr = 0x5b,
+						.mask = GENMASK(5, 0),
+					},
+					.status_reg = 0x1b,
+					.status_mask = BIT(3),
+					.status_z_mask = BIT(0),
+					.status_y_mask = BIT(1),
+					.status_x_mask = BIT(2),
+				},
 			},
-			.wakeup_src_reg = 0x1b,
-			.wakeup_src_status_mask = BIT(3),
-			.wakeup_src_z_mask = BIT(0),
-			.wakeup_src_y_mask = BIT(1),
-			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -791,15 +799,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x58,
 				.mask = BIT(7),
 			},
-			.wakeup_reg = {
-				.addr = 0x5B,
-				.mask = GENMASK(5, 0),
+			.sources = {
+				[ST_LSM6DSX_EVENT_WAKEUP] = {
+					.value = {
+						.addr = 0x5b,
+						.mask = GENMASK(5, 0),
+					},
+					.status_reg = 0x1b,
+					.status_mask = BIT(3),
+					.status_z_mask = BIT(0),
+					.status_y_mask = BIT(1),
+					.status_x_mask = BIT(2),
+				},
 			},
-			.wakeup_src_reg = 0x1b,
-			.wakeup_src_status_mask = BIT(3),
-			.wakeup_src_z_mask = BIT(0),
-			.wakeup_src_y_mask = BIT(1),
-			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -1028,15 +1040,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x58,
 				.mask = BIT(7),
 			},
-			.wakeup_reg = {
-				.addr = 0x5b,
-				.mask = GENMASK(5, 0),
+			.sources = {
+				[ST_LSM6DSX_EVENT_WAKEUP] = {
+					.value = {
+						.addr = 0x5b,
+						.mask = GENMASK(5, 0),
+					},
+					.status_reg = 0x1b,
+					.status_mask = BIT(3),
+					.status_z_mask = BIT(0),
+					.status_y_mask = BIT(1),
+					.status_x_mask = BIT(2),
+				},
 			},
-			.wakeup_src_reg = 0x1b,
-			.wakeup_src_status_mask = BIT(3),
-			.wakeup_src_z_mask = BIT(0),
-			.wakeup_src_y_mask = BIT(1),
-			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -1209,15 +1225,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x58,
 				.mask = BIT(7),
 			},
-			.wakeup_reg = {
-				.addr = 0x5B,
-				.mask = GENMASK(5, 0),
+			.sources = {
+				[ST_LSM6DSX_EVENT_WAKEUP] = {
+					.value = {
+						.addr = 0x5b,
+						.mask = GENMASK(5, 0),
+					},
+					.status_reg = 0x1b,
+					.status_mask = BIT(3),
+					.status_z_mask = BIT(0),
+					.status_y_mask = BIT(1),
+					.status_x_mask = BIT(2),
+				},
 			},
-			.wakeup_src_reg = 0x1b,
-			.wakeup_src_status_mask = BIT(3),
-			.wakeup_src_z_mask = BIT(0),
-			.wakeup_src_y_mask = BIT(1),
-			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -1415,15 +1435,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x50,
 				.mask = BIT(7),
 			},
-			.wakeup_reg = {
-				.addr = 0x5b,
-				.mask = GENMASK(5, 0),
+			.sources = {
+				[ST_LSM6DSX_EVENT_WAKEUP] = {
+					.value = {
+						.addr = 0x5b,
+						.mask = GENMASK(5, 0),
+					},
+					.status_reg = 0x45,
+					.status_mask = BIT(3),
+					.status_z_mask = BIT(0),
+					.status_y_mask = BIT(1),
+					.status_x_mask = BIT(2),
+				},
 			},
-			.wakeup_src_reg = 0x45,
-			.wakeup_src_status_mask = BIT(3),
-			.wakeup_src_z_mask = BIT(0),
-			.wakeup_src_y_mask = BIT(1),
-			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -1935,7 +1959,7 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
 	if (val < 0 || val > 31)
 		return -EINVAL;
 
-	reg = &hw->settings->event_settings.wakeup_reg;
+	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
 	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
 	err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
 					    reg->mask, data);
@@ -2420,6 +2444,7 @@ static bool
 st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 {
 	const struct st_lsm6dsx_event_settings *event_settings;
+	const struct st_lsm6dsx_event_src *src;
 	int err, data;
 	s64 timestamp;
 
@@ -2427,13 +2452,14 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 		return false;
 
 	event_settings = &hw->settings->event_settings;
-	err = st_lsm6dsx_read_locked(hw, event_settings->wakeup_src_reg,
+	src = &event_settings->sources[ST_LSM6DSX_EVENT_WAKEUP];
+	err = st_lsm6dsx_read_locked(hw, src->status_reg,
 				     &data, sizeof(data));
 	if (err < 0)
 		return false;
 
 	timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
-	if ((data & hw->settings->event_settings.wakeup_src_z_mask) &&
+	if ((data & src->status_z_mask) &&
 	    (hw->enable_event & BIT(IIO_MOD_Z)))
 		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
@@ -2443,7 +2469,7 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 						  IIO_EV_DIR_EITHER),
 						  timestamp);
 
-	if ((data & hw->settings->event_settings.wakeup_src_y_mask) &&
+	if ((data & src->status_y_mask) &&
 	    (hw->enable_event & BIT(IIO_MOD_Y)))
 		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
@@ -2453,7 +2479,7 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 						  IIO_EV_DIR_EITHER),
 						  timestamp);
 
-	if ((data & hw->settings->event_settings.wakeup_src_x_mask) &&
+	if ((data & src->status_x_mask) &&
 	    (hw->enable_event & BIT(IIO_MOD_X)))
 		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
@@ -2463,7 +2489,7 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 						  IIO_EV_DIR_EITHER),
 						  timestamp);
 
-	return data & event_settings->wakeup_src_status_mask;
+	return data & src->status_mask;
 }
 
 static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
-- 
2.39.5

Re: [PATCH v2 2/9] iio: imu: st_lsm6dsx: make event_settings more generic

[Lorenzo Bianconi]

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

> The st_lsm6dsx_event_settings structure contains fields specific for one
> event type (wakeup). In preparation for adding support for more event
> types, introduce an event id enum and a generic event source structure, and
> replace wakeup-specific data in struct st_lsm6dsx_event_settings with an
> array of event source structures.

Hi Francesco,

just one nit inline, other than that:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

Regards,
Lorenzo

> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  21 ++-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 134 +++++++++++--------
>  2 files changed, 95 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index a4f558899767..4c3ff1cc0097 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -242,14 +242,23 @@ struct st_lsm6dsx_shub_settings {
>  	u8 pause;
>  };
>  
> +enum st_lsm6dsx_event_id {
> +	ST_LSM6DSX_EVENT_WAKEUP,
> +	ST_LSM6DSX_EVENT_MAX
> +};
> +
> +struct st_lsm6dsx_event_src {
> +	struct st_lsm6dsx_reg value;
> +	u8 status_reg;
> +	u8 status_mask;

I think you can use st_lsm6dsx_reg struct here:

	struct st_lsm6dsx_reg status;

> +	u8 status_x_mask;
> +	u8 status_y_mask;
> +	u8 status_z_mask;
> +};
> +
>  struct st_lsm6dsx_event_settings {
>  	struct st_lsm6dsx_reg enable_reg;
> -	struct st_lsm6dsx_reg wakeup_reg;
> -	u8 wakeup_src_reg;
> -	u8 wakeup_src_status_mask;
> -	u8 wakeup_src_z_mask;
> -	u8 wakeup_src_y_mask;
> -	u8 wakeup_src_x_mask;
> +	struct st_lsm6dsx_event_src sources[ST_LSM6DSX_EVENT_MAX];
>  };
>  
>  enum st_lsm6dsx_ext_sensor_id {
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index a09df9d772dd..a71174e75f44 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -388,15 +388,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  			},
>  		},
>  		.event_settings = {
> -			.wakeup_reg = {
> -				.addr = 0x5B,
> -				.mask = GENMASK(5, 0),
> +			.sources = {
> +				[ST_LSM6DSX_EVENT_WAKEUP] = {
> +					.value = {
> +						.addr = 0x5b,
> +						.mask = GENMASK(5, 0),
> +					},
> +					.status_reg = 0x1b,
> +					.status_mask = BIT(3),
> +					.status_z_mask = BIT(0),
> +					.status_y_mask = BIT(1),
> +					.status_x_mask = BIT(2),
> +				},
>  			},
> -			.wakeup_src_reg = 0x1b,
> -			.wakeup_src_status_mask = BIT(3),
> -			.wakeup_src_z_mask = BIT(0),
> -			.wakeup_src_y_mask = BIT(1),
> -			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -554,15 +558,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  			},
>  		},
>  		.event_settings = {
> -			.wakeup_reg = {
> -				.addr = 0x5B,
> -				.mask = GENMASK(5, 0),
> +			.sources = {
> +				[ST_LSM6DSX_EVENT_WAKEUP] = {
> +					.value = {
> +						.addr = 0x5b,
> +						.mask = GENMASK(5, 0),
> +					},
> +					.status_reg = 0x1b,
> +					.status_mask = BIT(3),
> +					.status_z_mask = BIT(0),
> +					.status_y_mask = BIT(1),
> +					.status_x_mask = BIT(2),
> +				},
>  			},
> -			.wakeup_src_reg = 0x1b,
> -			.wakeup_src_status_mask = BIT(3),
> -			.wakeup_src_z_mask = BIT(0),
> -			.wakeup_src_y_mask = BIT(1),
> -			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -791,15 +799,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x58,
>  				.mask = BIT(7),
>  			},
> -			.wakeup_reg = {
> -				.addr = 0x5B,
> -				.mask = GENMASK(5, 0),
> +			.sources = {
> +				[ST_LSM6DSX_EVENT_WAKEUP] = {
> +					.value = {
> +						.addr = 0x5b,
> +						.mask = GENMASK(5, 0),
> +					},
> +					.status_reg = 0x1b,
> +					.status_mask = BIT(3),
> +					.status_z_mask = BIT(0),
> +					.status_y_mask = BIT(1),
> +					.status_x_mask = BIT(2),
> +				},
>  			},
> -			.wakeup_src_reg = 0x1b,
> -			.wakeup_src_status_mask = BIT(3),
> -			.wakeup_src_z_mask = BIT(0),
> -			.wakeup_src_y_mask = BIT(1),
> -			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -1028,15 +1040,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x58,
>  				.mask = BIT(7),
>  			},
> -			.wakeup_reg = {
> -				.addr = 0x5b,
> -				.mask = GENMASK(5, 0),
> +			.sources = {
> +				[ST_LSM6DSX_EVENT_WAKEUP] = {
> +					.value = {
> +						.addr = 0x5b,
> +						.mask = GENMASK(5, 0),
> +					},
> +					.status_reg = 0x1b,
> +					.status_mask = BIT(3),
> +					.status_z_mask = BIT(0),
> +					.status_y_mask = BIT(1),
> +					.status_x_mask = BIT(2),
> +				},
>  			},
> -			.wakeup_src_reg = 0x1b,
> -			.wakeup_src_status_mask = BIT(3),
> -			.wakeup_src_z_mask = BIT(0),
> -			.wakeup_src_y_mask = BIT(1),
> -			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -1209,15 +1225,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x58,
>  				.mask = BIT(7),
>  			},
> -			.wakeup_reg = {
> -				.addr = 0x5B,
> -				.mask = GENMASK(5, 0),
> +			.sources = {
> +				[ST_LSM6DSX_EVENT_WAKEUP] = {
> +					.value = {
> +						.addr = 0x5b,
> +						.mask = GENMASK(5, 0),
> +					},
> +					.status_reg = 0x1b,
> +					.status_mask = BIT(3),
> +					.status_z_mask = BIT(0),
> +					.status_y_mask = BIT(1),
> +					.status_x_mask = BIT(2),
> +				},
>  			},
> -			.wakeup_src_reg = 0x1b,
> -			.wakeup_src_status_mask = BIT(3),
> -			.wakeup_src_z_mask = BIT(0),
> -			.wakeup_src_y_mask = BIT(1),
> -			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -1415,15 +1435,19 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x50,
>  				.mask = BIT(7),
>  			},
> -			.wakeup_reg = {
> -				.addr = 0x5b,
> -				.mask = GENMASK(5, 0),
> +			.sources = {
> +				[ST_LSM6DSX_EVENT_WAKEUP] = {
> +					.value = {
> +						.addr = 0x5b,
> +						.mask = GENMASK(5, 0),
> +					},
> +					.status_reg = 0x45,
> +					.status_mask = BIT(3),
> +					.status_z_mask = BIT(0),
> +					.status_y_mask = BIT(1),
> +					.status_x_mask = BIT(2),
> +				},
>  			},
> -			.wakeup_src_reg = 0x45,
> -			.wakeup_src_status_mask = BIT(3),
> -			.wakeup_src_z_mask = BIT(0),
> -			.wakeup_src_y_mask = BIT(1),
> -			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -1935,7 +1959,7 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
>  	if (val < 0 || val > 31)
>  		return -EINVAL;
>  
> -	reg = &hw->settings->event_settings.wakeup_reg;
> +	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
>  	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
>  	err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
>  					    reg->mask, data);
> @@ -2420,6 +2444,7 @@ static bool
>  st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  {
>  	const struct st_lsm6dsx_event_settings *event_settings;
> +	const struct st_lsm6dsx_event_src *src;
>  	int err, data;
>  	s64 timestamp;
>  
> @@ -2427,13 +2452,14 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  		return false;
>  
>  	event_settings = &hw->settings->event_settings;
> -	err = st_lsm6dsx_read_locked(hw, event_settings->wakeup_src_reg,
> +	src = &event_settings->sources[ST_LSM6DSX_EVENT_WAKEUP];
> +	err = st_lsm6dsx_read_locked(hw, src->status_reg,
>  				     &data, sizeof(data));
>  	if (err < 0)
>  		return false;
>  
>  	timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
> -	if ((data & hw->settings->event_settings.wakeup_src_z_mask) &&
> +	if ((data & src->status_z_mask) &&
>  	    (hw->enable_event & BIT(IIO_MOD_Z)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> @@ -2443,7 +2469,7 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  						  IIO_EV_DIR_EITHER),
>  						  timestamp);
>  
> -	if ((data & hw->settings->event_settings.wakeup_src_y_mask) &&
> +	if ((data & src->status_y_mask) &&
>  	    (hw->enable_event & BIT(IIO_MOD_Y)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> @@ -2453,7 +2479,7 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  						  IIO_EV_DIR_EITHER),
>  						  timestamp);
>  
> -	if ((data & hw->settings->event_settings.wakeup_src_x_mask) &&
> +	if ((data & src->status_x_mask) &&
>  	    (hw->enable_event & BIT(IIO_MOD_X)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> @@ -2463,7 +2489,7 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  						  IIO_EV_DIR_EITHER),
>  						  timestamp);
>  
> -	return data & event_settings->wakeup_src_status_mask;
> +	return data & src->status_mask;
>  }
>  
>  static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
> -- 
> 2.39.5
> 

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

[PATCH v2 3/9] iio: imu: st_lsm6dsx: move wakeup event enable mask to event_src

[Francesco Lavra]

The mask value being assigned to the irq1_func and irq2_func fields of the
irq_config struct is specific to a single event source (i.e. the wakeup
event), and as such it should be separate from the definition of the
interrupt function registers, which cover multiple event sources.
In preparation for adding support for more event types, change the
irq1_func and irq2_func type from an {address, mask} pair to an address,
and move the mask value to a new field of struct st_lsm6dsx_event_src. No
functional changes.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  7 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 81 +++++++-------------
 2 files changed, 31 insertions(+), 57 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 4c3ff1cc0097..bbb967b2754b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -249,6 +249,7 @@ enum st_lsm6dsx_event_id {
 
 struct st_lsm6dsx_event_src {
 	struct st_lsm6dsx_reg value;
+	u8 enable_mask;
 	u8 status_reg;
 	u8 status_mask;
 	u8 status_x_mask;
@@ -344,8 +345,8 @@ struct st_lsm6dsx_settings {
 	struct {
 		struct st_lsm6dsx_reg irq1;
 		struct st_lsm6dsx_reg irq2;
-		struct st_lsm6dsx_reg irq1_func;
-		struct st_lsm6dsx_reg irq2_func;
+		u8 irq1_func;
+		u8 irq2_func;
 		struct st_lsm6dsx_reg lir;
 		struct st_lsm6dsx_reg clear_on_read;
 		struct st_lsm6dsx_reg hla;
@@ -444,7 +445,7 @@ struct st_lsm6dsx_hw {
 	u8 ts_sip;
 	u8 sip;
 
-	const struct st_lsm6dsx_reg *irq_routing;
+	u8 irq_routing;
 	u8 event_threshold;
 	u8 enable_event;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index a71174e75f44..ce5f9213d476 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -328,14 +328,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x58,
 				.mask = BIT(0),
 			},
-			.irq1_func = {
-				.addr = 0x5e,
-				.mask = BIT(5),
-			},
-			.irq2_func = {
-				.addr = 0x5f,
-				.mask = BIT(5),
-			},
+			.irq1_func = 0x5e,
+			.irq2_func = 0x5f,
 			.hla = {
 				.addr = 0x12,
 				.mask = BIT(5),
@@ -394,6 +388,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 						.addr = 0x5b,
 						.mask = GENMASK(5, 0),
 					},
+					.enable_mask = BIT(5),
 					.status_reg = 0x1b,
 					.status_mask = BIT(3),
 					.status_z_mask = BIT(0),
@@ -498,14 +493,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x58,
 				.mask = BIT(0),
 			},
-			.irq1_func = {
-				.addr = 0x5e,
-				.mask = BIT(5),
-			},
-			.irq2_func = {
-				.addr = 0x5f,
-				.mask = BIT(5),
-			},
+			.irq1_func = 0x5e,
+			.irq2_func = 0x5f,
 			.hla = {
 				.addr = 0x12,
 				.mask = BIT(5),
@@ -564,6 +553,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 						.addr = 0x5b,
 						.mask = GENMASK(5, 0),
 					},
+					.enable_mask = BIT(5),
 					.status_reg = 0x1b,
 					.status_mask = BIT(3),
 					.status_z_mask = BIT(0),
@@ -698,14 +688,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x58,
 				.mask = BIT(0),
 			},
-			.irq1_func = {
-				.addr = 0x5e,
-				.mask = BIT(5),
-			},
-			.irq2_func = {
-				.addr = 0x5f,
-				.mask = BIT(5),
-			},
+			.irq1_func = 0x5e,
+			.irq2_func = 0x5f,
 			.hla = {
 				.addr = 0x12,
 				.mask = BIT(5),
@@ -805,6 +789,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 						.addr = 0x5b,
 						.mask = GENMASK(5, 0),
 					},
+					.enable_mask = BIT(5),
 					.status_reg = 0x1b,
 					.status_mask = BIT(3),
 					.status_z_mask = BIT(0),
@@ -951,14 +936,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x56,
 				.mask = BIT(6),
 			},
-			.irq1_func = {
-				.addr = 0x5e,
-				.mask = BIT(5),
-			},
-			.irq2_func = {
-				.addr = 0x5f,
-				.mask = BIT(5),
-			},
+			.irq1_func = 0x5e,
+			.irq2_func = 0x5f,
 			.hla = {
 				.addr = 0x12,
 				.mask = BIT(5),
@@ -1046,6 +1025,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 						.addr = 0x5b,
 						.mask = GENMASK(5, 0),
 					},
+					.enable_mask = BIT(5),
 					.status_reg = 0x1b,
 					.status_mask = BIT(3),
 					.status_z_mask = BIT(0),
@@ -1168,14 +1148,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x56,
 				.mask = BIT(6),
 			},
-			.irq1_func = {
-				.addr = 0x5e,
-				.mask = BIT(5),
-			},
-			.irq2_func = {
-				.addr = 0x5f,
-				.mask = BIT(5),
-			},
+			.irq1_func = 0x5e,
+			.irq2_func = 0x5f,
 			.hla = {
 				.addr = 0x12,
 				.mask = BIT(5),
@@ -1231,6 +1205,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 						.addr = 0x5b,
 						.mask = GENMASK(5, 0),
 					},
+					.enable_mask = BIT(5),
 					.status_reg = 0x1b,
 					.status_mask = BIT(3),
 					.status_z_mask = BIT(0),
@@ -1347,14 +1322,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x56,
 				.mask = BIT(0),
 			},
-			.irq1_func = {
-				.addr = 0x5e,
-				.mask = BIT(5),
-			},
-			.irq2_func = {
-				.addr = 0x5f,
-				.mask = BIT(5),
-			},
+			.irq1_func = 0x5e,
+			.irq2_func = 0x5f,
 			.hla = {
 				.addr = 0x03,
 				.mask = BIT(4),
@@ -1441,6 +1410,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 						.addr = 0x5b,
 						.mask = GENMASK(5, 0),
 					},
+					.enable_mask = BIT(5),
 					.status_reg = 0x45,
 					.status_mask = BIT(3),
 					.status_z_mask = BIT(0),
@@ -1899,10 +1869,12 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
 static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
 {
 	const struct st_lsm6dsx_reg *reg;
+	const struct st_lsm6dsx_event_src *src;
+	u8 enable_mask;
 	unsigned int data;
 	int err;
 
-	if (!hw->settings->irq_config.irq1_func.addr)
+	if (!hw->irq_routing)
 		return -ENOTSUPP;
 
 	reg = &hw->settings->event_settings.enable_reg;
@@ -1915,9 +1887,10 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
 	}
 
 	/* Enable wakeup interrupt */
-	data = ST_LSM6DSX_SHIFT_VAL(state, hw->irq_routing->mask);
-	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing->addr,
-					     hw->irq_routing->mask, data);
+	src = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP];
+	enable_mask = src->enable_mask;
+	data = ST_LSM6DSX_SHIFT_VAL(state, enable_mask);
+	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing, enable_mask, data);
 }
 
 static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
@@ -2171,11 +2144,11 @@ st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw,
 
 	switch (drdy_pin) {
 	case 1:
-		hw->irq_routing = &hw->settings->irq_config.irq1_func;
+		hw->irq_routing = hw->settings->irq_config.irq1_func;
 		*drdy_reg = &hw->settings->irq_config.irq1;
 		break;
 	case 2:
-		hw->irq_routing = &hw->settings->irq_config.irq2_func;
+		hw->irq_routing = hw->settings->irq_config.irq2_func;
 		*drdy_reg = &hw->settings->irq_config.irq2;
 		break;
 	default:
-- 
2.39.5

Re: [PATCH v2 3/9] iio: imu: st_lsm6dsx: move wakeup event enable mask to event_src

[Lorenzo Bianconi]

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

> The mask value being assigned to the irq1_func and irq2_func fields of the
> irq_config struct is specific to a single event source (i.e. the wakeup
> event), and as such it should be separate from the definition of the
> interrupt function registers, which cover multiple event sources.
> In preparation for adding support for more event types, change the
> irq1_func and irq2_func type from an {address, mask} pair to an address,
> and move the mask value to a new field of struct st_lsm6dsx_event_src. No
> functional changes.

Just a nit inline. Fixing it:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  7 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 81 +++++++-------------
>  2 files changed, 31 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 4c3ff1cc0097..bbb967b2754b 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -249,6 +249,7 @@ enum st_lsm6dsx_event_id {
>  
>  struct st_lsm6dsx_event_src {
>  	struct st_lsm6dsx_reg value;
> +	u8 enable_mask;
>  	u8 status_reg;
>  	u8 status_mask;
>  	u8 status_x_mask;
> @@ -344,8 +345,8 @@ struct st_lsm6dsx_settings {
>  	struct {
>  		struct st_lsm6dsx_reg irq1;
>  		struct st_lsm6dsx_reg irq2;
> -		struct st_lsm6dsx_reg irq1_func;
> -		struct st_lsm6dsx_reg irq2_func;
> +		u8 irq1_func;
> +		u8 irq2_func;
>  		struct st_lsm6dsx_reg lir;
>  		struct st_lsm6dsx_reg clear_on_read;
>  		struct st_lsm6dsx_reg hla;
> @@ -444,7 +445,7 @@ struct st_lsm6dsx_hw {
>  	u8 ts_sip;
>  	u8 sip;
>  
> -	const struct st_lsm6dsx_reg *irq_routing;
> +	u8 irq_routing;
>  	u8 event_threshold;
>  	u8 enable_event;
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index a71174e75f44..ce5f9213d476 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -328,14 +328,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x58,
>  				.mask = BIT(0),
>  			},
> -			.irq1_func = {
> -				.addr = 0x5e,
> -				.mask = BIT(5),
> -			},
> -			.irq2_func = {
> -				.addr = 0x5f,
> -				.mask = BIT(5),
> -			},
> +			.irq1_func = 0x5e,
> +			.irq2_func = 0x5f,
>  			.hla = {
>  				.addr = 0x12,
>  				.mask = BIT(5),
> @@ -394,6 +388,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  						.addr = 0x5b,
>  						.mask = GENMASK(5, 0),
>  					},
> +					.enable_mask = BIT(5),
>  					.status_reg = 0x1b,
>  					.status_mask = BIT(3),
>  					.status_z_mask = BIT(0),
> @@ -498,14 +493,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x58,
>  				.mask = BIT(0),
>  			},
> -			.irq1_func = {
> -				.addr = 0x5e,
> -				.mask = BIT(5),
> -			},
> -			.irq2_func = {
> -				.addr = 0x5f,
> -				.mask = BIT(5),
> -			},
> +			.irq1_func = 0x5e,
> +			.irq2_func = 0x5f,
>  			.hla = {
>  				.addr = 0x12,
>  				.mask = BIT(5),
> @@ -564,6 +553,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  						.addr = 0x5b,
>  						.mask = GENMASK(5, 0),
>  					},
> +					.enable_mask = BIT(5),
>  					.status_reg = 0x1b,
>  					.status_mask = BIT(3),
>  					.status_z_mask = BIT(0),
> @@ -698,14 +688,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x58,
>  				.mask = BIT(0),
>  			},
> -			.irq1_func = {
> -				.addr = 0x5e,
> -				.mask = BIT(5),
> -			},
> -			.irq2_func = {
> -				.addr = 0x5f,
> -				.mask = BIT(5),
> -			},
> +			.irq1_func = 0x5e,
> +			.irq2_func = 0x5f,
>  			.hla = {
>  				.addr = 0x12,
>  				.mask = BIT(5),
> @@ -805,6 +789,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  						.addr = 0x5b,
>  						.mask = GENMASK(5, 0),
>  					},
> +					.enable_mask = BIT(5),
>  					.status_reg = 0x1b,
>  					.status_mask = BIT(3),
>  					.status_z_mask = BIT(0),
> @@ -951,14 +936,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x56,
>  				.mask = BIT(6),
>  			},
> -			.irq1_func = {
> -				.addr = 0x5e,
> -				.mask = BIT(5),
> -			},
> -			.irq2_func = {
> -				.addr = 0x5f,
> -				.mask = BIT(5),
> -			},
> +			.irq1_func = 0x5e,
> +			.irq2_func = 0x5f,
>  			.hla = {
>  				.addr = 0x12,
>  				.mask = BIT(5),
> @@ -1046,6 +1025,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  						.addr = 0x5b,
>  						.mask = GENMASK(5, 0),
>  					},
> +					.enable_mask = BIT(5),
>  					.status_reg = 0x1b,
>  					.status_mask = BIT(3),
>  					.status_z_mask = BIT(0),
> @@ -1168,14 +1148,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x56,
>  				.mask = BIT(6),
>  			},
> -			.irq1_func = {
> -				.addr = 0x5e,
> -				.mask = BIT(5),
> -			},
> -			.irq2_func = {
> -				.addr = 0x5f,
> -				.mask = BIT(5),
> -			},
> +			.irq1_func = 0x5e,
> +			.irq2_func = 0x5f,
>  			.hla = {
>  				.addr = 0x12,
>  				.mask = BIT(5),
> @@ -1231,6 +1205,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  						.addr = 0x5b,
>  						.mask = GENMASK(5, 0),
>  					},
> +					.enable_mask = BIT(5),
>  					.status_reg = 0x1b,
>  					.status_mask = BIT(3),
>  					.status_z_mask = BIT(0),
> @@ -1347,14 +1322,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.addr = 0x56,
>  				.mask = BIT(0),
>  			},
> -			.irq1_func = {
> -				.addr = 0x5e,
> -				.mask = BIT(5),
> -			},
> -			.irq2_func = {
> -				.addr = 0x5f,
> -				.mask = BIT(5),
> -			},
> +			.irq1_func = 0x5e,
> +			.irq2_func = 0x5f,
>  			.hla = {
>  				.addr = 0x03,
>  				.mask = BIT(4),
> @@ -1441,6 +1410,7 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  						.addr = 0x5b,
>  						.mask = GENMASK(5, 0),
>  					},
> +					.enable_mask = BIT(5),
>  					.status_reg = 0x45,
>  					.status_mask = BIT(3),
>  					.status_z_mask = BIT(0),
> @@ -1899,10 +1869,12 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
>  static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
>  {
>  	const struct st_lsm6dsx_reg *reg;
> +	const struct st_lsm6dsx_event_src *src;
> +	u8 enable_mask;
>  	unsigned int data;
>  	int err;
>  
> -	if (!hw->settings->irq_config.irq1_func.addr)
> +	if (!hw->irq_routing)
>  		return -ENOTSUPP;
>  
>  	reg = &hw->settings->event_settings.enable_reg;
> @@ -1915,9 +1887,10 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
>  	}
>  
>  	/* Enable wakeup interrupt */
> -	data = ST_LSM6DSX_SHIFT_VAL(state, hw->irq_routing->mask);
> -	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing->addr,
> -					     hw->irq_routing->mask, data);
> +	src = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP];
> +	enable_mask = src->enable_mask;

I think you can drop enable_mask here and just use src->enable_mask directly
here.

> +	data = ST_LSM6DSX_SHIFT_VAL(state, enable_mask);
> +	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing, enable_mask, data);
>  }
>  
>  static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
> @@ -2171,11 +2144,11 @@ st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw,
>  
>  	switch (drdy_pin) {
>  	case 1:
> -		hw->irq_routing = &hw->settings->irq_config.irq1_func;
> +		hw->irq_routing = hw->settings->irq_config.irq1_func;
>  		*drdy_reg = &hw->settings->irq_config.irq1;
>  		break;
>  	case 2:
> -		hw->irq_routing = &hw->settings->irq_config.irq2_func;
> +		hw->irq_routing = hw->settings->irq_config.irq2_func;
>  		*drdy_reg = &hw->settings->irq_config.irq2;
>  		break;
>  	default:
> -- 
> 2.39.5
> 

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

[PATCH v2 4/9] iio: imu: st_lsm6dsx: rework code to check for enabled events

[Francesco Lavra]

The enable_event field in struct st_lsm6dsx_hw does not lend itself well to
handling multiple event sources, so it will have to be modified to add
support for more event sources. As a preparatory step, remove references to
this field from code that does not deal with event management; rework the
st_lsm6dsx_check_events() function so that it returns whether any events
are currently enabled on a given sensor.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index ce5f9213d476..287a85d4bd58 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1744,11 +1744,11 @@ __st_lsm6dsx_sensor_set_enable(struct st_lsm6dsx_sensor *sensor,
 }
 
 static int
-st_lsm6dsx_check_events(struct st_lsm6dsx_sensor *sensor, bool enable)
+st_lsm6dsx_check_events(struct st_lsm6dsx_sensor *sensor)
 {
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 
-	if (sensor->id == ST_LSM6DSX_ID_GYRO || enable)
+	if (sensor->id != ST_LSM6DSX_ID_ACC)
 		return 0;
 
 	return hw->enable_event;
@@ -1757,7 +1757,7 @@ st_lsm6dsx_check_events(struct st_lsm6dsx_sensor *sensor, bool enable)
 int st_lsm6dsx_sensor_set_enable(struct st_lsm6dsx_sensor *sensor,
 				 bool enable)
 {
-	if (st_lsm6dsx_check_events(sensor, enable))
+	if (st_lsm6dsx_check_events(sensor))
 		return 0;
 
 	return __st_lsm6dsx_sensor_set_enable(sensor, enable);
@@ -1785,11 +1785,9 @@ static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
 	if (err < 0)
 		return err;
 
-	if (!hw->enable_event) {
-		err = st_lsm6dsx_sensor_set_enable(sensor, false);
-		if (err < 0)
-			return err;
-	}
+	err = st_lsm6dsx_sensor_set_enable(sensor, false);
+	if (err < 0)
+		return err;
 
 	*val = (s16)le16_to_cpu(data);
 
@@ -2751,7 +2749,7 @@ static int st_lsm6dsx_suspend(struct device *dev)
 			continue;
 
 		if (device_may_wakeup(dev) &&
-		    sensor->id == ST_LSM6DSX_ID_ACC && hw->enable_event) {
+		    st_lsm6dsx_check_events(sensor)) {
 			/* Enable wake from IRQ */
 			enable_irq_wake(hw->irq);
 			continue;
@@ -2782,7 +2780,7 @@ static int st_lsm6dsx_resume(struct device *dev)
 
 		sensor = iio_priv(hw->iio_devs[i]);
 		if (device_may_wakeup(dev) &&
-		    sensor->id == ST_LSM6DSX_ID_ACC && hw->enable_event)
+		    st_lsm6dsx_check_events(sensor))
 			disable_irq_wake(hw->irq);
 
 		if (!(hw->suspend_mask & BIT(sensor->id)))
-- 
2.39.5

[PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct

[Francesco Lavra]

This field is used to store the wakeup event detection threshold value.
When adding support for more event types, some of which may have different
threshold values for different axes, storing all threshold values for all
event sources would be cumbersome. Thus, remove this field altogether, and
read the currently configured value from the sensor when requested by
userspace.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  3 +--
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index bbb967b2754b..e727a87413e5 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -79,6 +79,7 @@ enum st_lsm6dsx_hw_id {
 #define ST_LSM6DSX_MAX_TAGGED_WORD_LEN	((32 / ST_LSM6DSX_TAGGED_SAMPLE_SIZE) \
 					 * ST_LSM6DSX_TAGGED_SAMPLE_SIZE)
 #define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
+#define st_lsm6dsx_field_get(mask, reg)	((reg & mask) >> __ffs(mask))
 
 #define ST_LSM6DSX_CHANNEL_ACC(chan_type, addr, mod, scan_idx)		\
 {									\
@@ -422,7 +423,6 @@ struct st_lsm6dsx_sensor {
  * @sip: Total number of samples (acc/gyro/ts) in a given pattern.
  * @buff: Device read buffer.
  * @irq_routing: pointer to interrupt routing configuration.
- * @event_threshold: wakeup event threshold.
  * @enable_event: enabled event bitmask.
  * @iio_devs: Pointers to acc/gyro iio_dev instances.
  * @settings: Pointer to the specific sensor settings in use.
@@ -446,7 +446,6 @@ struct st_lsm6dsx_hw {
 	u8 sip;
 
 	u8 irq_routing;
-	u8 event_threshold;
 	u8 enable_event;
 
 	u8 *buff;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 287a85d4bd58..117ecb080d8e 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1900,12 +1900,20 @@ static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
 {
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
 	struct st_lsm6dsx_hw *hw = sensor->hw;
+	const struct st_lsm6dsx_reg *reg;
+	u8 data;
+	int err;
 
 	if (type != IIO_EV_TYPE_THRESH)
 		return -EINVAL;
 
+	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
+	err = st_lsm6dsx_read_locked(hw, reg->addr, &data, sizeof(data));
+	if (err < 0)
+		return err;
+
 	*val2 = 0;
-	*val = hw->event_threshold;
+	*val = st_lsm6dsx_field_get(reg->mask, data);
 
 	return IIO_VAL_INT;
 }
@@ -1937,8 +1945,6 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
 	if (err < 0)
 		return -EINVAL;
 
-	hw->event_threshold = val;
-
 	return 0;
 }
 
-- 
2.39.5

Re: [PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct

[Lorenzo Bianconi]

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

> This field is used to store the wakeup event detection threshold value.
> When adding support for more event types, some of which may have different
> threshold values for different axes, storing all threshold values for all
> event sources would be cumbersome. Thus, remove this field altogether, and
> read the currently configured value from the sensor when requested by
> userspace.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>

Just a nit inline, fixing it:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  3 +--
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index bbb967b2754b..e727a87413e5 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -79,6 +79,7 @@ enum st_lsm6dsx_hw_id {
>  #define ST_LSM6DSX_MAX_TAGGED_WORD_LEN	((32 / ST_LSM6DSX_TAGGED_SAMPLE_SIZE) \
>  					 * ST_LSM6DSX_TAGGED_SAMPLE_SIZE)
>  #define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
> +#define st_lsm6dsx_field_get(mask, reg)	((reg & mask) >> __ffs(mask))

To be aligned with the rest of the code, I guess we could use capital letters
here:

	#define ST_LSM6DSX_FIELD_GET(mask, reg)▸     ((reg & mask) >> __ffs(mask))

>  
>  #define ST_LSM6DSX_CHANNEL_ACC(chan_type, addr, mod, scan_idx)		\
>  {									\
> @@ -422,7 +423,6 @@ struct st_lsm6dsx_sensor {
>   * @sip: Total number of samples (acc/gyro/ts) in a given pattern.
>   * @buff: Device read buffer.
>   * @irq_routing: pointer to interrupt routing configuration.
> - * @event_threshold: wakeup event threshold.
>   * @enable_event: enabled event bitmask.
>   * @iio_devs: Pointers to acc/gyro iio_dev instances.
>   * @settings: Pointer to the specific sensor settings in use.
> @@ -446,7 +446,6 @@ struct st_lsm6dsx_hw {
>  	u8 sip;
>  
>  	u8 irq_routing;
> -	u8 event_threshold;
>  	u8 enable_event;
>  
>  	u8 *buff;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 287a85d4bd58..117ecb080d8e 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1900,12 +1900,20 @@ static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
>  {
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	const struct st_lsm6dsx_reg *reg;
> +	u8 data;
> +	int err;
>  
>  	if (type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
> +	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
> +	err = st_lsm6dsx_read_locked(hw, reg->addr, &data, sizeof(data));
> +	if (err < 0)
> +		return err;
> +
>  	*val2 = 0;
> -	*val = hw->event_threshold;
> +	*val = st_lsm6dsx_field_get(reg->mask, data);
>  
>  	return IIO_VAL_INT;
>  }
> @@ -1937,8 +1945,6 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
>  	if (err < 0)
>  		return -EINVAL;
>  
> -	hw->event_threshold = val;
> -
>  	return 0;
>  }
>  
> -- 
> 2.39.5
> 

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

Re: [PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct

[Francesco Lavra]

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

On Fri, 2025-11-21 at 09:14 +0100, Lorenzo Bianconi wrote:
> > This field is used to store the wakeup event detection threshold value.
> > When adding support for more event types, some of which may have
> > different
> > threshold values for different axes, storing all threshold values for
> > all
> > event sources would be cumbersome. Thus, remove this field altogether,
> > and
> > read the currently configured value from the sensor when requested by
> > userspace.
> > 
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> 
> Just a nit inline, fixing it:
> 
> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  3 +--
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++++++---
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index bbb967b2754b..e727a87413e5 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -79,6 +79,7 @@ enum st_lsm6dsx_hw_id {
> >  #define ST_LSM6DSX_MAX_TAGGED_WORD_LEN ((32 /
> > ST_LSM6DSX_TAGGED_SAMPLE_SIZE) \
> >                                          *
> > ST_LSM6DSX_TAGGED_SAMPLE_SIZE)
> >  #define ST_LSM6DSX_SHIFT_VAL(val, mask)        (((val) << __ffs(mask))
> > & (mask))
> > +#define st_lsm6dsx_field_get(mask, reg)        ((reg & mask) >>
> > __ffs(mask))
> 
> To be aligned with the rest of the code, I guess we could use capital
> letters
> here:
> 
>         #define ST_LSM6DSX_FIELD_GET(mask, reg)▸     ((reg & mask) >>
> __ffs(mask))

That's what I proposed initially, but Andy suggested [1] using small
letters instead.

https://lore.kernel.org/linux-iio/aQh4m25uVBV8A09F@smile.fi.intel.com/


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct

[Andy Shevchenko]

On Fri, Nov 21, 2025 at 09:43:28AM +0100, Francesco Lavra wrote:
> On Fri, 2025-11-21 at 09:14 +0100, Lorenzo Bianconi wrote:
> > > This field is used to store the wakeup event detection threshold value.
> > > When adding support for more event types, some of which may have
> > > different
> > > threshold values for different axes, storing all threshold values for
> > > all
> > > event sources would be cumbersome. Thus, remove this field altogether,
> > > and
> > > read the currently configured value from the sensor when requested by
> > > userspace.
> > > 
> > > Signed-off-by: Francesco Lavra <flavra@baylibre.com>

> > Just a nit inline, fixing it:

...

> > > +#define st_lsm6dsx_field_get(mask, reg)        ((reg & mask) >>
> > > __ffs(mask))
> > 
> > To be aligned with the rest of the code, I guess we could use capital
> > letters
> > here:
> > 
> >         #define ST_LSM6DSX_FIELD_GET(mask, reg)▸     ((reg & mask) >>
> > __ffs(mask))
> 
> That's what I proposed initially, but Andy suggested [1] using small
> letters instead.
> 
> https://lore.kernel.org/linux-iio/aQh4m25uVBV8A09F@smile.fi.intel.com/

Yes, this should be temporary as there is a pending series to make field_get()
to appear in the common header, so this one will be removed, we may even do
that after v6.19-rc1 as a (semi-)fix if Jonathan agrees on that.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 6/9] iio: imu: st_lsm6dsx: make event management functions generic

[Francesco Lavra]

In preparation for adding support for more event types, use an array
indexed by event ID instead of a scalar value to store enabled events, and
refactor the functions to configure and report events so that their
implementation is not specific for wakeup events. Move the logic to update
the global event interrupt enable flag from st_lsm6dsx_event_setup() to its
calling function, so that it can take into account also event sources
different from the source being configured. While changing the signature of
the st_lsm6dsx_event_setup() function, opportunistically add the currently
unused `axis` parameter, which will be used when adding support for
enabling and disabling events on a per axis basis.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |   2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 149 ++++++++++++++-----
 2 files changed, 109 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index e727a87413e5..037b3b817e83 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -446,7 +446,7 @@ struct st_lsm6dsx_hw {
 	u8 sip;
 
 	u8 irq_routing;
-	u8 enable_event;
+	u8 enable_event[ST_LSM6DSX_EVENT_MAX];
 
 	u8 *buff;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 117ecb080d8e..18a09ed6907c 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1747,11 +1747,16 @@ static int
 st_lsm6dsx_check_events(struct st_lsm6dsx_sensor *sensor)
 {
 	struct st_lsm6dsx_hw *hw = sensor->hw;
+	int event;
 
 	if (sensor->id != ST_LSM6DSX_ID_ACC)
 		return 0;
 
-	return hw->enable_event;
+	for (event = 0; event < ST_LSM6DSX_EVENT_MAX; event++) {
+		if (hw->enable_event[event])
+			return true;
+	}
+	return false;
 }
 
 int st_lsm6dsx_sensor_set_enable(struct st_lsm6dsx_sensor *sensor,
@@ -1864,9 +1869,10 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
 	return err;
 }
 
-static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
+static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw,
+				  enum st_lsm6dsx_event_id event, int axis,
+				  bool state)
 {
-	const struct st_lsm6dsx_reg *reg;
 	const struct st_lsm6dsx_event_src *src;
 	u8 enable_mask;
 	unsigned int data;
@@ -1875,22 +1881,24 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
 	if (!hw->irq_routing)
 		return -ENOTSUPP;
 
-	reg = &hw->settings->event_settings.enable_reg;
-	if (reg->addr) {
-		data = ST_LSM6DSX_SHIFT_VAL(state, reg->mask);
-		err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
-						    reg->mask, data);
-		if (err < 0)
-			return err;
-	}
-
-	/* Enable wakeup interrupt */
-	src = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP];
+	/* Enable/disable event interrupt */
+	src = &hw->settings->event_settings.sources[event];
 	enable_mask = src->enable_mask;
 	data = ST_LSM6DSX_SHIFT_VAL(state, enable_mask);
 	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing, enable_mask, data);
 }
 
+static enum st_lsm6dsx_event_id
+st_lsm6dsx_get_event_id(enum iio_event_type type)
+{
+	switch (type) {
+	case IIO_EV_TYPE_THRESH:
+		return ST_LSM6DSX_EVENT_WAKEUP;
+	default:
+		return ST_LSM6DSX_EVENT_MAX;
+	}
+}
+
 static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
 				 const struct iio_chan_spec *chan,
 				 enum iio_event_type type,
@@ -1898,16 +1906,17 @@ static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
 				 enum iio_event_info info,
 				 int *val, int *val2)
 {
+	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	const struct st_lsm6dsx_reg *reg;
 	u8 data;
 	int err;
 
-	if (type != IIO_EV_TYPE_THRESH)
+	if (event == ST_LSM6DSX_EVENT_MAX)
 		return -EINVAL;
 
-	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
+	reg = &hw->settings->event_settings.sources[event].value;
 	err = st_lsm6dsx_read_locked(hw, reg->addr, &data, sizeof(data));
 	if (err < 0)
 		return err;
@@ -1926,19 +1935,20 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
 		       enum iio_event_info info,
 		       int val, int val2)
 {
+	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	const struct st_lsm6dsx_reg *reg;
 	unsigned int data;
 	int err;
 
-	if (type != IIO_EV_TYPE_THRESH)
+	if (event == ST_LSM6DSX_EVENT_MAX)
 		return -EINVAL;
 
 	if (val < 0 || val > 31)
 		return -EINVAL;
 
-	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
+	reg = &hw->settings->event_settings.sources[event].value;
 	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
 	err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
 					    reg->mask, data);
@@ -1954,13 +1964,53 @@ st_lsm6dsx_read_event_config(struct iio_dev *iio_dev,
 			     enum iio_event_type type,
 			     enum iio_event_direction dir)
 {
+	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 
-	if (type != IIO_EV_TYPE_THRESH)
+	if (event == ST_LSM6DSX_EVENT_MAX)
 		return -EINVAL;
 
-	return !!(hw->enable_event & BIT(chan->channel2));
+	return !!(hw->enable_event[event] & BIT(chan->channel2));
+}
+
+/**
+ * st_lsm6dsx_check_other_events - Check for enabled sensor events.
+ * @hw: Sensor hardware instance.
+ * @curr: Current event type.
+ *
+ * Return: whether any events other than @curr are enabled.
+ */
+static bool st_lsm6dsx_check_other_events(struct st_lsm6dsx_hw *hw,
+					  enum st_lsm6dsx_event_id curr)
+{
+	enum st_lsm6dsx_event_id other;
+
+	for (other = 0; other < ST_LSM6DSX_EVENT_MAX; other++) {
+		if (other != curr && hw->enable_event[other])
+			return true;
+	}
+	return false;
+}
+
+static int st_lsm6dsx_events_enable(struct st_lsm6dsx_sensor *sensor,
+				    bool state)
+{
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+	const struct st_lsm6dsx_reg *reg;
+
+	reg = &hw->settings->event_settings.enable_reg;
+	if (reg->addr) {
+		int err;
+
+		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
+					 ST_LSM6DSX_SHIFT_VAL(state, reg->mask));
+		if (err)
+			return err;
+	}
+	if (state || !(hw->fifo_mask & BIT(sensor->id)))
+		return __st_lsm6dsx_sensor_set_enable(sensor, state);
+	return 0;
 }
 
 static int
@@ -1969,22 +2019,24 @@ st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 			      enum iio_event_type type,
 			      enum iio_event_direction dir, bool state)
 {
+	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
 	struct st_lsm6dsx_hw *hw = sensor->hw;
+	int axis = chan->channel2;
 	u8 enable_event;
 	int err;
 
-	if (type != IIO_EV_TYPE_THRESH)
+	if (event == ST_LSM6DSX_EVENT_MAX)
 		return -EINVAL;
 
 	if (state) {
-		enable_event = hw->enable_event | BIT(chan->channel2);
+		enable_event = hw->enable_event[event] | BIT(axis);
 
 		/* do not enable events if they are already enabled */
-		if (hw->enable_event)
+		if (hw->enable_event[event])
 			goto out;
 	} else {
-		enable_event = hw->enable_event & ~BIT(chan->channel2);
+		enable_event = hw->enable_event[event] & ~BIT(axis);
 
 		/* only turn off sensor if no events is enabled */
 		if (enable_event)
@@ -1992,22 +2044,24 @@ st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 	}
 
 	/* stop here if no changes have been made */
-	if (hw->enable_event == enable_event)
+	if (hw->enable_event[event] == enable_event)
 		return 0;
 
-	err = st_lsm6dsx_event_setup(hw, state);
+	err = st_lsm6dsx_event_setup(hw, event, axis, state);
 	if (err < 0)
 		return err;
 
 	mutex_lock(&hw->conf_lock);
-	if (enable_event || !(hw->fifo_mask & BIT(sensor->id)))
-		err = __st_lsm6dsx_sensor_set_enable(sensor, state);
+	if (enable_event)
+		err = st_lsm6dsx_events_enable(sensor, true);
+	else if (!st_lsm6dsx_check_other_events(hw, event))
+		err = st_lsm6dsx_events_enable(sensor, false);
 	mutex_unlock(&hw->conf_lock);
 	if (err < 0)
 		return err;
 
 out:
-	hw->enable_event = enable_event;
+	hw->enable_event[event] = enable_event;
 
 	return 0;
 }
@@ -2418,18 +2472,20 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 }
 
 static bool
-st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
+st_lsm6dsx_report_events(struct st_lsm6dsx_hw *hw, enum st_lsm6dsx_event_id id,
+			 enum iio_event_type type, enum iio_event_direction dir)
 {
+	u8 enable_event = hw->enable_event[id];
 	const struct st_lsm6dsx_event_settings *event_settings;
 	const struct st_lsm6dsx_event_src *src;
 	int err, data;
 	s64 timestamp;
 
-	if (!hw->enable_event)
+	if (!enable_event)
 		return false;
 
 	event_settings = &hw->settings->event_settings;
-	src = &event_settings->sources[ST_LSM6DSX_EVENT_WAKEUP];
+	src = &event_settings->sources[id];
 	err = st_lsm6dsx_read_locked(hw, src->status_reg,
 				     &data, sizeof(data));
 	if (err < 0)
@@ -2437,38 +2493,49 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 
 	timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
 	if ((data & src->status_z_mask) &&
-	    (hw->enable_event & BIT(IIO_MOD_Z)))
+	    (enable_event & BIT(IIO_MOD_Z)))
 		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
 						  IIO_MOD_Z,
-						  IIO_EV_TYPE_THRESH,
-						  IIO_EV_DIR_EITHER),
+						  type,
+						  dir),
 						  timestamp);
 
 	if ((data & src->status_y_mask) &&
-	    (hw->enable_event & BIT(IIO_MOD_Y)))
+	    (enable_event & BIT(IIO_MOD_Y)))
 		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
 						  IIO_MOD_Y,
-						  IIO_EV_TYPE_THRESH,
-						  IIO_EV_DIR_EITHER),
+						  type,
+						  dir),
 						  timestamp);
 
 	if ((data & src->status_x_mask) &&
-	    (hw->enable_event & BIT(IIO_MOD_X)))
+	    (enable_event & BIT(IIO_MOD_X)))
 		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
 						  IIO_MOD_X,
-						  IIO_EV_TYPE_THRESH,
-						  IIO_EV_DIR_EITHER),
+						  type,
+						  dir),
 						  timestamp);
 
 	return data & src->status_mask;
 }
 
+static bool st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
+{
+	bool events_found;
+
+	events_found = st_lsm6dsx_report_events(hw, ST_LSM6DSX_EVENT_WAKEUP,
+						IIO_EV_TYPE_THRESH,
+						IIO_EV_DIR_EITHER);
+
+	return events_found;
+}
+
 static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
 {
 	struct st_lsm6dsx_hw *hw = private;
-- 
2.39.5

Re: [PATCH v2 6/9] iio: imu: st_lsm6dsx: make event management functions generic

[Lorenzo Bianconi]

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

> In preparation for adding support for more event types, use an array
> indexed by event ID instead of a scalar value to store enabled events, and
> refactor the functions to configure and report events so that their
> implementation is not specific for wakeup events. Move the logic to update
> the global event interrupt enable flag from st_lsm6dsx_event_setup() to its
> calling function, so that it can take into account also event sources
> different from the source being configured. While changing the signature of
> the st_lsm6dsx_event_setup() function, opportunistically add the currently
> unused `axis` parameter, which will be used when adding support for
> enabling and disabling events on a per axis basis.

Just some nits inline. Fixing them:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |   2 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 149 ++++++++++++++-----
>  2 files changed, 109 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index e727a87413e5..037b3b817e83 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -446,7 +446,7 @@ struct st_lsm6dsx_hw {
>  	u8 sip;
>  
>  	u8 irq_routing;
> -	u8 enable_event;
> +	u8 enable_event[ST_LSM6DSX_EVENT_MAX];
>  
>  	u8 *buff;
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 117ecb080d8e..18a09ed6907c 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1747,11 +1747,16 @@ static int
>  st_lsm6dsx_check_events(struct st_lsm6dsx_sensor *sensor)
>  {
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	int event;
>  
>  	if (sensor->id != ST_LSM6DSX_ID_ACC)
>  		return 0;
>  
> -	return hw->enable_event;
> +	for (event = 0; event < ST_LSM6DSX_EVENT_MAX; event++) {
> +		if (hw->enable_event[event])
> +			return true;
> +	}
> +	return false;
>  }
>  
>  int st_lsm6dsx_sensor_set_enable(struct st_lsm6dsx_sensor *sensor,
> @@ -1864,9 +1869,10 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
>  	return err;
>  }
>  
> -static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
> +static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw,
> +				  enum st_lsm6dsx_event_id event, int axis,
> +				  bool state)
>  {
> -	const struct st_lsm6dsx_reg *reg;
>  	const struct st_lsm6dsx_event_src *src;
>  	u8 enable_mask;
>  	unsigned int data;
> @@ -1875,22 +1881,24 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, bool state)
>  	if (!hw->irq_routing)
>  		return -ENOTSUPP;
>  
> -	reg = &hw->settings->event_settings.enable_reg;
> -	if (reg->addr) {
> -		data = ST_LSM6DSX_SHIFT_VAL(state, reg->mask);
> -		err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
> -						    reg->mask, data);
> -		if (err < 0)
> -			return err;
> -	}
> -
> -	/* Enable wakeup interrupt */
> -	src = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP];
> +	/* Enable/disable event interrupt */
> +	src = &hw->settings->event_settings.sources[event];
>  	enable_mask = src->enable_mask;
>  	data = ST_LSM6DSX_SHIFT_VAL(state, enable_mask);
>  	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing, enable_mask, data);
>  }
>  
> +static enum st_lsm6dsx_event_id
> +st_lsm6dsx_get_event_id(enum iio_event_type type)
> +{
> +	switch (type) {
> +	case IIO_EV_TYPE_THRESH:
> +		return ST_LSM6DSX_EVENT_WAKEUP;
> +	default:
> +		return ST_LSM6DSX_EVENT_MAX;
> +	}
> +}
> +
>  static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
>  				 const struct iio_chan_spec *chan,
>  				 enum iio_event_type type,
> @@ -1898,16 +1906,17 @@ static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
>  				 enum iio_event_info info,
>  				 int *val, int *val2)
>  {
> +	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
>  	const struct st_lsm6dsx_reg *reg;
>  	u8 data;
>  	int err;
>  
> -	if (type != IIO_EV_TYPE_THRESH)
> +	if (event == ST_LSM6DSX_EVENT_MAX)
>  		return -EINVAL;
>  
> -	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
> +	reg = &hw->settings->event_settings.sources[event].value;
>  	err = st_lsm6dsx_read_locked(hw, reg->addr, &data, sizeof(data));
>  	if (err < 0)
>  		return err;
> @@ -1926,19 +1935,20 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
>  		       enum iio_event_info info,
>  		       int val, int val2)
>  {
> +	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
>  	const struct st_lsm6dsx_reg *reg;
>  	unsigned int data;
>  	int err;
>  
> -	if (type != IIO_EV_TYPE_THRESH)
> +	if (event == ST_LSM6DSX_EVENT_MAX)
>  		return -EINVAL;
>  
>  	if (val < 0 || val > 31)
>  		return -EINVAL;
>  
> -	reg = &hw->settings->event_settings.sources[ST_LSM6DSX_EVENT_WAKEUP].value;
> +	reg = &hw->settings->event_settings.sources[event].value;
>  	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
>  	err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
>  					    reg->mask, data);
> @@ -1954,13 +1964,53 @@ st_lsm6dsx_read_event_config(struct iio_dev *iio_dev,
>  			     enum iio_event_type type,
>  			     enum iio_event_direction dir)
>  {
> +	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
>  
> -	if (type != IIO_EV_TYPE_THRESH)
> +	if (event == ST_LSM6DSX_EVENT_MAX)
>  		return -EINVAL;
>  
> -	return !!(hw->enable_event & BIT(chan->channel2));
> +	return !!(hw->enable_event[event] & BIT(chan->channel2));
> +}
> +
> +/**
> + * st_lsm6dsx_check_other_events - Check for enabled sensor events.
> + * @hw: Sensor hardware instance.
> + * @curr: Current event type.
> + *
> + * Return: whether any events other than @curr are enabled.
> + */
> +static bool st_lsm6dsx_check_other_events(struct st_lsm6dsx_hw *hw,
> +					  enum st_lsm6dsx_event_id curr)
> +{
> +	enum st_lsm6dsx_event_id other;
> +
> +	for (other = 0; other < ST_LSM6DSX_EVENT_MAX; other++) {
> +		if (other != curr && hw->enable_event[other])
> +			return true;
> +	}

new line here

> +	return false;
> +}
> +
> +static int st_lsm6dsx_events_enable(struct st_lsm6dsx_sensor *sensor,
> +				    bool state)
> +{
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	const struct st_lsm6dsx_reg *reg;
> +
> +	reg = &hw->settings->event_settings.enable_reg;
> +	if (reg->addr) {
> +		int err;
> +
> +		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> +					 ST_LSM6DSX_SHIFT_VAL(state, reg->mask));
> +		if (err)
> +			return err;
> +	}

new line here

> +	if (state || !(hw->fifo_mask & BIT(sensor->id)))
> +		return __st_lsm6dsx_sensor_set_enable(sensor, state);

new line here

> +	return 0;
>  }
>  
>  static int
> @@ -1969,22 +2019,24 @@ st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>  			      enum iio_event_type type,
>  			      enum iio_event_direction dir, bool state)
>  {
> +	enum st_lsm6dsx_event_id event = st_lsm6dsx_get_event_id(type);
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	int axis = chan->channel2;

I think you can drop axis here and just use chan->channel2 directly.

>  	u8 enable_event;
>  	int err;
>  
> -	if (type != IIO_EV_TYPE_THRESH)
> +	if (event == ST_LSM6DSX_EVENT_MAX)
>  		return -EINVAL;
>  
>  	if (state) {
> -		enable_event = hw->enable_event | BIT(chan->channel2);
> +		enable_event = hw->enable_event[event] | BIT(axis);
>  
>  		/* do not enable events if they are already enabled */
> -		if (hw->enable_event)
> +		if (hw->enable_event[event])
>  			goto out;
>  	} else {
> -		enable_event = hw->enable_event & ~BIT(chan->channel2);
> +		enable_event = hw->enable_event[event] & ~BIT(axis);
>  
>  		/* only turn off sensor if no events is enabled */
>  		if (enable_event)
> @@ -1992,22 +2044,24 @@ st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>  	}
>  
>  	/* stop here if no changes have been made */
> -	if (hw->enable_event == enable_event)
> +	if (hw->enable_event[event] == enable_event)
>  		return 0;
>  
> -	err = st_lsm6dsx_event_setup(hw, state);
> +	err = st_lsm6dsx_event_setup(hw, event, axis, state);
>  	if (err < 0)
>  		return err;
>  
>  	mutex_lock(&hw->conf_lock);
> -	if (enable_event || !(hw->fifo_mask & BIT(sensor->id)))
> -		err = __st_lsm6dsx_sensor_set_enable(sensor, state);
> +	if (enable_event)
> +		err = st_lsm6dsx_events_enable(sensor, true);
> +	else if (!st_lsm6dsx_check_other_events(hw, event))
> +		err = st_lsm6dsx_events_enable(sensor, false);
>  	mutex_unlock(&hw->conf_lock);
>  	if (err < 0)
>  		return err;
>  
>  out:
> -	hw->enable_event = enable_event;
> +	hw->enable_event[event] = enable_event;
>  
>  	return 0;
>  }
> @@ -2418,18 +2472,20 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  }
>  
>  static bool
> -st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
> +st_lsm6dsx_report_events(struct st_lsm6dsx_hw *hw, enum st_lsm6dsx_event_id id,
> +			 enum iio_event_type type, enum iio_event_direction dir)
>  {
> +	u8 enable_event = hw->enable_event[id];

same here, can you just use hw->enable_event[id] directly?

>  	const struct st_lsm6dsx_event_settings *event_settings;
>  	const struct st_lsm6dsx_event_src *src;
>  	int err, data;
>  	s64 timestamp;
>  
> -	if (!hw->enable_event)
> +	if (!enable_event)
>  		return false;
>  
>  	event_settings = &hw->settings->event_settings;
> -	src = &event_settings->sources[ST_LSM6DSX_EVENT_WAKEUP];
> +	src = &event_settings->sources[id];
>  	err = st_lsm6dsx_read_locked(hw, src->status_reg,
>  				     &data, sizeof(data));
>  	if (err < 0)
> @@ -2437,38 +2493,49 @@ st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  
>  	timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
>  	if ((data & src->status_z_mask) &&
> -	    (hw->enable_event & BIT(IIO_MOD_Z)))
> +	    (enable_event & BIT(IIO_MOD_Z)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
>  						  IIO_MOD_Z,
> -						  IIO_EV_TYPE_THRESH,
> -						  IIO_EV_DIR_EITHER),
> +						  type,
> +						  dir),
>  						  timestamp);
>  
>  	if ((data & src->status_y_mask) &&
> -	    (hw->enable_event & BIT(IIO_MOD_Y)))
> +	    (enable_event & BIT(IIO_MOD_Y)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
>  						  IIO_MOD_Y,
> -						  IIO_EV_TYPE_THRESH,
> -						  IIO_EV_DIR_EITHER),
> +						  type,
> +						  dir),
>  						  timestamp);
>  
>  	if ((data & src->status_x_mask) &&
> -	    (hw->enable_event & BIT(IIO_MOD_X)))
> +	    (enable_event & BIT(IIO_MOD_X)))
>  		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
>  						  IIO_MOD_X,
> -						  IIO_EV_TYPE_THRESH,
> -						  IIO_EV_DIR_EITHER),
> +						  type,
> +						  dir),
>  						  timestamp);
>  
>  	return data & src->status_mask;
>  }
>  
> +static bool st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
> +{
> +	bool events_found;
> +
> +	events_found = st_lsm6dsx_report_events(hw, ST_LSM6DSX_EVENT_WAKEUP,
> +						IIO_EV_TYPE_THRESH,
> +						IIO_EV_DIR_EITHER);
> +
> +	return events_found;
> +}
> +
>  static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
>  {
>  	struct st_lsm6dsx_hw *hw = private;
> -- 
> 2.39.5
> 

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

[PATCH v2 7/9] iio: imu: st_lsm6dsx: add event configurability on a per axis basis

[Francesco Lavra]

In order to be able to configure event detection on a per axis
basis (for either setting an event threshold/sensitivity value, or
enabling/disabling event detection), add new axis-specific fields
to struct st_lsm6dsx_event_src, and modify the logic that handles
event configuration to properly handle axis-specific settings when
supported by a given event source.
A future commit will add actual event sources with per-axis
configurability.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  7 ++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 93 +++++++++++++++++---
 2 files changed, 86 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 037b3b817e83..6f68508e5769 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -250,7 +250,14 @@ enum st_lsm6dsx_event_id {
 
 struct st_lsm6dsx_event_src {
 	struct st_lsm6dsx_reg value;
+	struct st_lsm6dsx_reg x_value;
+	struct st_lsm6dsx_reg y_value;
+	struct st_lsm6dsx_reg z_value;
 	u8 enable_mask;
+	u8 enable_axis_reg;
+	u8 enable_x_mask;
+	u8 enable_y_mask;
+	u8 enable_z_mask;
 	u8 status_reg;
 	u8 status_mask;
 	u8 status_x_mask;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 18a09ed6907c..da914a58a875 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1877,12 +1877,48 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw,
 	u8 enable_mask;
 	unsigned int data;
 	int err;
+	u8 old_enable, new_enable;
 
 	if (!hw->irq_routing)
 		return -ENOTSUPP;
 
 	/* Enable/disable event interrupt */
 	src = &hw->settings->event_settings.sources[event];
+	if (src->enable_axis_reg) {
+		switch (axis) {
+		case IIO_MOD_X:
+			enable_mask = src->enable_x_mask;
+			break;
+		case IIO_MOD_Y:
+			enable_mask = src->enable_y_mask;
+			break;
+		case IIO_MOD_Z:
+			enable_mask = src->enable_z_mask;
+			break;
+		default:
+			enable_mask = 0;
+		}
+		if (enable_mask) {
+			data = ST_LSM6DSX_SHIFT_VAL(state, enable_mask);
+			err = st_lsm6dsx_update_bits_locked(hw,
+							    src->enable_axis_reg,
+							    enable_mask, data);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	/*
+	 * If the set of axes for which the event source is enabled does not
+	 * change from empty to non-empty or vice versa, there is nothing else
+	 * to do.
+	 */
+	old_enable = hw->enable_event[event];
+	new_enable = state ? (old_enable | BIT(axis)) :
+			     (old_enable & ~BIT(axis));
+	if (!!old_enable == !!new_enable)
+		return 0;
+
 	enable_mask = src->enable_mask;
 	data = ST_LSM6DSX_SHIFT_VAL(state, enable_mask);
 	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing, enable_mask, data);
@@ -1899,6 +1935,39 @@ st_lsm6dsx_get_event_id(enum iio_event_type type)
 	}
 }
 
+static const struct st_lsm6dsx_reg *
+st_lsm6dsx_get_event_reg(struct st_lsm6dsx_hw *hw,
+			 enum st_lsm6dsx_event_id event,
+			 const struct iio_chan_spec *chan)
+{
+	const struct st_lsm6dsx_event_src *src;
+	const struct st_lsm6dsx_reg *reg;
+
+	src = &hw->settings->event_settings.sources[event];
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		reg = &src->x_value;
+		break;
+	case IIO_MOD_Y:
+		reg = &src->y_value;
+		break;
+	case IIO_MOD_Z:
+		reg = &src->z_value;
+		break;
+	default:
+		return NULL;
+	}
+	if (reg->addr)
+		return reg;
+
+	/*
+	 * The sensor does not support configuring this event source on a per
+	 * axis basis: return the register to configure the event source for all
+	 * axes.
+	 */
+	return &src->value;
+}
+
 static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
 				 const struct iio_chan_spec *chan,
 				 enum iio_event_type type,
@@ -1916,7 +1985,10 @@ static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
 	if (event == ST_LSM6DSX_EVENT_MAX)
 		return -EINVAL;
 
-	reg = &hw->settings->event_settings.sources[event].value;
+	reg = st_lsm6dsx_get_event_reg(hw, event, chan);
+	if (!reg)
+		return -EINVAL;
+
 	err = st_lsm6dsx_read_locked(hw, reg->addr, &data, sizeof(data));
 	if (err < 0)
 		return err;
@@ -1948,7 +2020,10 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
 	if (val < 0 || val > 31)
 		return -EINVAL;
 
-	reg = &hw->settings->event_settings.sources[event].value;
+	reg = st_lsm6dsx_get_event_reg(hw, event, chan);
+	if (!reg)
+		return -EINVAL;
+
 	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
 	err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
 					    reg->mask, data);
@@ -2029,20 +2104,11 @@ st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 	if (event == ST_LSM6DSX_EVENT_MAX)
 		return -EINVAL;
 
-	if (state) {
+	if (state)
 		enable_event = hw->enable_event[event] | BIT(axis);
-
-		/* do not enable events if they are already enabled */
-		if (hw->enable_event[event])
-			goto out;
-	} else {
+	else
 		enable_event = hw->enable_event[event] & ~BIT(axis);
 
-		/* only turn off sensor if no events is enabled */
-		if (enable_event)
-			goto out;
-	}
-
 	/* stop here if no changes have been made */
 	if (hw->enable_event[event] == enable_event)
 		return 0;
@@ -2060,7 +2126,6 @@ st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 	if (err < 0)
 		return err;
 
-out:
 	hw->enable_event[event] = enable_event;
 
 	return 0;
-- 
2.39.5

Re: [PATCH v2 7/9] iio: imu: st_lsm6dsx: add event configurability on a per axis basis

[Andy Shevchenko]

On Thu, Nov 20, 2025 at 09:26:13AM +0100, Francesco Lavra wrote:
> In order to be able to configure event detection on a per axis
> basis (for either setting an event threshold/sensitivity value, or
> enabling/disabling event detection), add new axis-specific fields
> to struct st_lsm6dsx_event_src, and modify the logic that handles
> event configuration to properly handle axis-specific settings when
> supported by a given event source.
> A future commit will add actual event sources with per-axis
> configurability.

...

> +	/*
> +	 * If the set of axes for which the event source is enabled does not
> +	 * change from empty to non-empty or vice versa, there is nothing else
> +	 * to do.
> +	 */
> +	old_enable = hw->enable_event[event];
> +	new_enable = state ? (old_enable | BIT(axis)) :
> +			     (old_enable & ~BIT(axis));
> +	if (!!old_enable == !!new_enable)
> +		return 0;

Oh, seems I forgot to click "send" for the previous reply.
In the old round I proposed better way of doing that (spoiler, no hweight()
needed at all).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 8/9] iio: imu: st_lsm6dsx: add event spec parameter to iio_chan_spec initializer

[Francesco Lavra]

In preparation for adding support for more event sources, add to the
ST_LSM6DSX_CHANNEL_ACC() iio_chan_spec initializer macro an iio_event_spec
array argument, so that this macro can be used with different arrays by
sensors that support different event sources; change the st_lsm6dsx_event
struct declaration to an array (renamed as st_lsm6dsx_ev_motion) so that it
can be passed to the macro (and opportunistically move it from the header
file where it does not belong to the C file where it is used).
In addition, remove from this macro the channel type parameter and
hard-code IIO_ACCEL in the macro definition, since all callers use
IIO_ACCEL as channel type argument.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      | 15 ++++-----------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 15 ++++++++++++---
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 6f68508e5769..3b90261e6bb1 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -81,9 +81,9 @@ enum st_lsm6dsx_hw_id {
 #define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
 #define st_lsm6dsx_field_get(mask, reg)	((reg & mask) >> __ffs(mask))
 
-#define ST_LSM6DSX_CHANNEL_ACC(chan_type, addr, mod, scan_idx)		\
+#define ST_LSM6DSX_CHANNEL_ACC(addr, mod, scan_idx, events)		\
 {									\
-	.type = chan_type,						\
+	.type = IIO_ACCEL,						\
 	.address = addr,						\
 	.modified = 1,							\
 	.channel2 = mod,						\
@@ -97,9 +97,9 @@ enum st_lsm6dsx_hw_id {
 		.storagebits = 16,					\
 		.endianness = IIO_LE,					\
 	},								\
-	.event_spec = &st_lsm6dsx_event,				\
+	.event_spec = events,						\
+	.num_event_specs = ARRAY_SIZE(events),				\
 	.ext_info = st_lsm6dsx_ext_info,				\
-	.num_event_specs = 1,						\
 }
 
 #define ST_LSM6DSX_CHANNEL(chan_type, addr, mod, scan_idx)		\
@@ -469,13 +469,6 @@ struct st_lsm6dsx_hw {
 	} scan[ST_LSM6DSX_ID_MAX];
 };
 
-static __maybe_unused const struct iio_event_spec st_lsm6dsx_event = {
-	.type = IIO_EV_TYPE_THRESH,
-	.dir = IIO_EV_DIR_EITHER,
-	.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			 BIT(IIO_EV_INFO_ENABLE)
-};
-
 static __maybe_unused const unsigned long st_lsm6dsx_available_scan_masks[] = {
 	0x7, 0x0,
 };
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index da914a58a875..562544636f36 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -96,10 +96,19 @@
 
 #define ST_LSM6DSX_TS_SENSITIVITY		25000UL /* 25us */
 
+static const struct iio_event_spec st_lsm6dsx_ev_motion[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
 static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
-	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
-	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
-	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
+	ST_LSM6DSX_CHANNEL_ACC(0x28, IIO_MOD_X, 0, st_lsm6dsx_ev_motion),
+	ST_LSM6DSX_CHANNEL_ACC(0x2a, IIO_MOD_Y, 1, st_lsm6dsx_ev_motion),
+	ST_LSM6DSX_CHANNEL_ACC(0x2c, IIO_MOD_Z, 2, st_lsm6dsx_ev_motion),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
-- 
2.39.5

[PATCH v2 9/9] iio: imu: st_lsm6dsx: add tap event detection

[Francesco Lavra]

In order to allow sensors to advertise tap event capability and report tap
events, define a new struct iio_event_spec array that includes a tap event
spec, and a new struct iio_chan_spec array that references the new
iio_event_spec array; for the LSM6DSV chip family, use the new
iio_chan_spec array and define an event source for tap events.
Tested on LSMDSV16X.

Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  1 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 55 +++++++++++++++++++-
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 3b90261e6bb1..5d820f2a0bce 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -245,6 +245,7 @@ struct st_lsm6dsx_shub_settings {
 
 enum st_lsm6dsx_event_id {
 	ST_LSM6DSX_EVENT_WAKEUP,
+	ST_LSM6DSX_EVENT_TAP,
 	ST_LSM6DSX_EVENT_MAX
 };
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 562544636f36..55cd63643c52 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -105,6 +105,21 @@ static const struct iio_event_spec st_lsm6dsx_ev_motion[] = {
 	},
 };
 
+static const struct iio_event_spec st_lsm6dsx_ev_motion_tap[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+	{
+		.type = IIO_EV_TYPE_GESTURE,
+		.dir = IIO_EV_DIR_SINGLETAP,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
 static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
 	ST_LSM6DSX_CHANNEL_ACC(0x28, IIO_MOD_X, 0, st_lsm6dsx_ev_motion),
 	ST_LSM6DSX_CHANNEL_ACC(0x2a, IIO_MOD_Y, 1, st_lsm6dsx_ev_motion),
@@ -119,6 +134,13 @@ static const struct iio_chan_spec st_lsm6ds0_acc_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+static const struct iio_chan_spec st_lsm6dsx_acc_tap_channels[] = {
+	ST_LSM6DSX_CHANNEL_ACC(0x28, IIO_MOD_X, 0, st_lsm6dsx_ev_motion_tap),
+	ST_LSM6DSX_CHANNEL_ACC(0x2a, IIO_MOD_Y, 1, st_lsm6dsx_ev_motion_tap),
+	ST_LSM6DSX_CHANNEL_ACC(0x2c, IIO_MOD_Z, 2, st_lsm6dsx_ev_motion_tap),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
 static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
 	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x22, IIO_MOD_X, 0),
 	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x24, IIO_MOD_Y, 1),
@@ -1250,8 +1272,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 		},
 		.channels = {
 			[ST_LSM6DSX_ID_ACC] = {
-				.chan = st_lsm6dsx_acc_channels,
-				.len = ARRAY_SIZE(st_lsm6dsx_acc_channels),
+				.chan = st_lsm6dsx_acc_tap_channels,
+				.len = ARRAY_SIZE(st_lsm6dsx_acc_tap_channels),
 			},
 			[ST_LSM6DSX_ID_GYRO] = {
 				.chan = st_lsm6dsx_gyro_channels,
@@ -1426,6 +1448,30 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 					.status_y_mask = BIT(1),
 					.status_x_mask = BIT(2),
 				},
+				[ST_LSM6DSX_EVENT_TAP] = {
+					.x_value = {
+						.addr = 0x57,
+						.mask = GENMASK(4, 0),
+					},
+					.y_value = {
+						.addr = 0x58,
+						.mask = GENMASK(4, 0),
+					},
+					.z_value = {
+						.addr = 0x59,
+						.mask = GENMASK(4, 0),
+					},
+					.enable_mask = BIT(6),
+					.enable_axis_reg = 0x56,
+					.enable_x_mask = BIT(3),
+					.enable_y_mask = BIT(2),
+					.enable_z_mask = BIT(1),
+					.status_reg = 0x46,
+					.status_mask = BIT(5),
+					.status_x_mask = BIT(2),
+					.status_y_mask = BIT(1),
+					.status_z_mask = BIT(0),
+				},
 			},
 		},
 	},
@@ -1939,6 +1985,8 @@ st_lsm6dsx_get_event_id(enum iio_event_type type)
 	switch (type) {
 	case IIO_EV_TYPE_THRESH:
 		return ST_LSM6DSX_EVENT_WAKEUP;
+	case IIO_EV_TYPE_GESTURE:
+		return ST_LSM6DSX_EVENT_TAP;
 	default:
 		return ST_LSM6DSX_EVENT_MAX;
 	}
@@ -2606,6 +2654,9 @@ static bool st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
 	events_found = st_lsm6dsx_report_events(hw, ST_LSM6DSX_EVENT_WAKEUP,
 						IIO_EV_TYPE_THRESH,
 						IIO_EV_DIR_EITHER);
+	events_found |= st_lsm6dsx_report_events(hw, ST_LSM6DSX_EVENT_TAP,
+						 IIO_EV_TYPE_GESTURE,
+						 IIO_EV_DIR_SINGLETAP);
 
 	return events_found;
 }
-- 
2.39.5

Re: [PATCH v2 9/9] iio: imu: st_lsm6dsx: add tap event detection

[Lorenzo Bianconi]

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

> In order to allow sensors to advertise tap event capability and report tap
> events, define a new struct iio_event_spec array that includes a tap event
> spec, and a new struct iio_chan_spec array that references the new
> iio_event_spec array; for the LSM6DSV chip family, use the new
> iio_chan_spec array and define an event source for tap events.
> Tested on LSMDSV16X.
> 
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  1 +
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 55 +++++++++++++++++++-
>  2 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 3b90261e6bb1..5d820f2a0bce 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -245,6 +245,7 @@ struct st_lsm6dsx_shub_settings {
>  
>  enum st_lsm6dsx_event_id {
>  	ST_LSM6DSX_EVENT_WAKEUP,
> +	ST_LSM6DSX_EVENT_TAP,
>  	ST_LSM6DSX_EVENT_MAX
>  };
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 562544636f36..55cd63643c52 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -105,6 +105,21 @@ static const struct iio_event_spec st_lsm6dsx_ev_motion[] = {
>  	},
>  };
>  
> +static const struct iio_event_spec st_lsm6dsx_ev_motion_tap[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				 BIT(IIO_EV_INFO_ENABLE),
> +	},
> +	{
> +		.type = IIO_EV_TYPE_GESTURE,
> +		.dir = IIO_EV_DIR_SINGLETAP,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				 BIT(IIO_EV_INFO_ENABLE),
> +	},
> +};
> +
>  static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
>  	ST_LSM6DSX_CHANNEL_ACC(0x28, IIO_MOD_X, 0, st_lsm6dsx_ev_motion),
>  	ST_LSM6DSX_CHANNEL_ACC(0x2a, IIO_MOD_Y, 1, st_lsm6dsx_ev_motion),
> @@ -119,6 +134,13 @@ static const struct iio_chan_spec st_lsm6ds0_acc_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
> +static const struct iio_chan_spec st_lsm6dsx_acc_tap_channels[] = {
> +	ST_LSM6DSX_CHANNEL_ACC(0x28, IIO_MOD_X, 0, st_lsm6dsx_ev_motion_tap),
> +	ST_LSM6DSX_CHANNEL_ACC(0x2a, IIO_MOD_Y, 1, st_lsm6dsx_ev_motion_tap),
> +	ST_LSM6DSX_CHANNEL_ACC(0x2c, IIO_MOD_Z, 2, st_lsm6dsx_ev_motion_tap),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
>  static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
>  	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x22, IIO_MOD_X, 0),
>  	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, 0x24, IIO_MOD_Y, 1),
> @@ -1250,8 +1272,8 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  		},
>  		.channels = {
>  			[ST_LSM6DSX_ID_ACC] = {
> -				.chan = st_lsm6dsx_acc_channels,
> -				.len = ARRAY_SIZE(st_lsm6dsx_acc_channels),
> +				.chan = st_lsm6dsx_acc_tap_channels,
> +				.len = ARRAY_SIZE(st_lsm6dsx_acc_tap_channels),
>  			},
>  			[ST_LSM6DSX_ID_GYRO] = {
>  				.chan = st_lsm6dsx_gyro_channels,
> @@ -1426,6 +1448,30 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  					.status_y_mask = BIT(1),
>  					.status_x_mask = BIT(2),
>  				},
> +				[ST_LSM6DSX_EVENT_TAP] = {
> +					.x_value = {
> +						.addr = 0x57,
> +						.mask = GENMASK(4, 0),
> +					},
> +					.y_value = {
> +						.addr = 0x58,
> +						.mask = GENMASK(4, 0),
> +					},
> +					.z_value = {
> +						.addr = 0x59,
> +						.mask = GENMASK(4, 0),
> +					},
> +					.enable_mask = BIT(6),
> +					.enable_axis_reg = 0x56,
> +					.enable_x_mask = BIT(3),
> +					.enable_y_mask = BIT(2),
> +					.enable_z_mask = BIT(1),
> +					.status_reg = 0x46,
> +					.status_mask = BIT(5),
> +					.status_x_mask = BIT(2),
> +					.status_y_mask = BIT(1),
> +					.status_z_mask = BIT(0),
> +				},
>  			},
>  		},
>  	},
> @@ -1939,6 +1985,8 @@ st_lsm6dsx_get_event_id(enum iio_event_type type)
>  	switch (type) {
>  	case IIO_EV_TYPE_THRESH:
>  		return ST_LSM6DSX_EVENT_WAKEUP;
> +	case IIO_EV_TYPE_GESTURE:
> +		return ST_LSM6DSX_EVENT_TAP;
>  	default:
>  		return ST_LSM6DSX_EVENT_MAX;
>  	}
> @@ -2606,6 +2654,9 @@ static bool st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw)
>  	events_found = st_lsm6dsx_report_events(hw, ST_LSM6DSX_EVENT_WAKEUP,
>  						IIO_EV_TYPE_THRESH,
>  						IIO_EV_DIR_EITHER);
> +	events_found |= st_lsm6dsx_report_events(hw, ST_LSM6DSX_EVENT_TAP,
> +						 IIO_EV_TYPE_GESTURE,
> +						 IIO_EV_DIR_SINGLETAP);
>  
>  	return events_found;
>  }
> -- 
> 2.39.5
> 

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

Re: [PATCH v2 0/9] st_lsm6dsx: add tap event detection

[Andy Shevchenko]

On Thu, Nov 20, 2025 at 09:26:06AM +0100, Francesco Lavra wrote:
> The bulk of this patch set consists of reworking the existing code for
> event detection (which supports IIO_EV_TYPE_THRESH events only) in order to
> make it generic to accommodate different event types. Actual support for
> tap events is implemented in the last patch.
> Tested on LSMDSV16X.

Still one piece to get better, but overall looks good to me now,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
(at least for the patches with no comments from me, and for the one more
 after addressing the comment)

-- 
With Best Regards,
Andy Shevchenko