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

Francesco Lavra posted 9 patches 2 months, 1 week ago
[PATCH v4 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Francesco Lavra 2 months, 1 week ago
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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.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 4200e5231950..b27a833d5107 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)		\
 {									\
@@ -421,7 +422,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.
@@ -445,7 +445,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 828e495c870c..dbdf9bb9e258 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1911,12 +1911,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;
 }
@@ -1948,8 +1956,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 v4 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Jonathan Cameron 2 months ago
On Mon,  1 Dec 2025 11:00:14 +0100
Francesco Lavra <flavra@baylibre.com> 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>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.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 4200e5231950..b27a833d5107 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))

I'm not going to fuss too much about this as expectation is that
this will be replaced soon anyway with a generic version but convention
would be to (reg) & (mask) to avoid precedence of operator problems if
there are any in the parameters passed.  The generic version will I guess also deal
with avoiding multiple evaluation of mask.

Anyhow, doesn't matter here given the simple user.

Applied.

Jonathan
Re: [PATCH v4 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Andy Shevchenko 2 months ago
On Sun, Dec 07, 2025 at 03:31:13PM +0000, Jonathan Cameron wrote:
> On Mon,  1 Dec 2025 11:00:14 +0100
> Francesco Lavra <flavra@baylibre.com> 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.

...

> >  #define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
> > +#define st_lsm6dsx_field_get(mask, reg)	((reg & mask) >> __ffs(mask))
> 
> I'm not going to fuss too much about this as expectation is that
> this will be replaced soon anyway with a generic version but convention
> would be to (reg) & (mask) to avoid precedence of operator problems if
> there are any in the parameters passed.  The generic version will I guess also deal
> with avoiding multiple evaluation of mask.
> 
> Anyhow, doesn't matter here given the simple user.
> 
> Applied.

Note, the new API is already in Linus' tree.

-- 
With Best Regards,
Andy Shevchenko