1. Patchew
  2. linux
  3. [PATCH v2 0/9] st_lsm6dsx: add tap event detection
  4. View patch

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

Francesco Lavra posted 9 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Francesco Lavra 1 week, 4 days 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>
---
 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
Posted by Lorenzo Bianconi 1 week, 3 days 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>

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
>
Re: [PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Francesco Lavra 1 week, 3 days ago 
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/
Re: [PATCH v2 5/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Andy Shevchenko 1 week, 3 days ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.