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

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

Francesco Lavra posted 9 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Francesco Lavra 3 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>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  2 --
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 12 +++++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index ec4efb29c4cc..98aa3cfb711b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -398,7 +398,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.
@@ -422,7 +421,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 157bc2615dc6..ea145e15cd36 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1825,12 +1825,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 = (data & reg->mask) >> __ffs(reg->mask);
 
 	return IIO_VAL_INT;
 }
@@ -1862,8 +1870,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 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Andy Shevchenko 3 months, 1 week ago 
On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra 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.

...

> +	*val = (data & reg->mask) >> __ffs(reg->mask);

Seems like yet another candidate for field_get() macro.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Francesco Lavra 3 months, 1 week ago 
On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:
> On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra 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.
> 
> ...
> 
> > +       *val = (data & reg->mask) >> __ffs(reg->mask);
> 
> Seems like yet another candidate for field_get() macro.

FIELD_GET() can only be used with compile-time constant masks.
And apparently this is the case with u8_get_bits() too, because you get a
"bad bitfield mask" compiler error if you try to use u8_get_bits().
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Andy Shevchenko 3 months, 1 week ago 
On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:
> On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:
> > On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:

...

> > > +       *val = (data & reg->mask) >> __ffs(reg->mask);
> > 
> > Seems like yet another candidate for field_get() macro.
> 
> FIELD_GET() can only be used with compile-time constant masks.
> And apparently this is the case with u8_get_bits() too, because you get a
> "bad bitfield mask" compiler error if you try to use u8_get_bits().

We are talking about different things.
Here are the pointers to what I'm talking:

- git grep -n -w 'field_get'
- https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Jonathan Cameron 3 months, 1 week ago 
On Thu, 30 Oct 2025 15:49:37 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:
> > On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:  
> > > On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:  
> 
> ...
> 
> > > > +       *val = (data & reg->mask) >> __ffs(reg->mask);  
> > > 
> > > Seems like yet another candidate for field_get() macro.  
> > 
> > FIELD_GET() can only be used with compile-time constant masks.
> > And apparently this is the case with u8_get_bits() too, because you get a
> > "bad bitfield mask" compiler error if you try to use u8_get_bits().  
> 
> We are talking about different things.
> Here are the pointers to what I'm talking:
> 
> - git grep -n -w 'field_get'
> - https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/
> 
True that it will be a usecase for that, but given plan is to merge that through
a different tree in next merge window, it's not available for us yet.  Hence would
be a follow up patch next cycle.

Jonathan
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Andy Shevchenko 3 months, 1 week ago 
On Sun, Nov 2, 2025 at 1:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Thu, 30 Oct 2025 15:49:37 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:
> > > On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:
> > > > On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:

...

> > > > > +       *val = (data & reg->mask) >> __ffs(reg->mask);
> > > >
> > > > Seems like yet another candidate for field_get() macro.
> > >
> > > FIELD_GET() can only be used with compile-time constant masks.
> > > And apparently this is the case with u8_get_bits() too, because you get a
> > > "bad bitfield mask" compiler error if you try to use u8_get_bits().
> >
> > We are talking about different things.
> > Here are the pointers to what I'm talking:
> >
> > - git grep -n -w 'field_get'
> > - https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/
> >
> True that it will be a usecase for that, but given plan is to merge that through
> a different tree in next merge window, it's not available for us yet.  Hence would
> be a follow up patch next cycle.

Yes, but we can still define them here. Dunno either with #under or
under (namespaced) names, but still possible to use now.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Francesco Lavra 3 months, 1 week ago 
On Sun, 2025-11-02 at 15:45 +0200, Andy Shevchenko wrote:
> On Sun, Nov 2, 2025 at 1:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > On Thu, 30 Oct 2025 15:49:37 +0200
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:
> > > > On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:
> > > > > On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:
> 
> ...
> 
> > > > > > +       *val = (data & reg->mask) >> __ffs(reg->mask);
> > > > > 
> > > > > Seems like yet another candidate for field_get() macro.
> > > > 
> > > > FIELD_GET() can only be used with compile-time constant masks.
> > > > And apparently this is the case with u8_get_bits() too, because you
> > > > get a
> > > > "bad bitfield mask" compiler error if you try to use u8_get_bits().
> > > 
> > > We are talking about different things.
> > > Here are the pointers to what I'm talking:
> > > 
> > > - git grep -n -w 'field_get'
> > > -
> > > https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/
> > > 
> > True that it will be a usecase for that, but given plan is to merge
> > that through
> > a different tree in next merge window, it's not available for us yet. 
> > Hence would
> > be a follow up patch next cycle.
> 
> Yes, but we can still define them here. Dunno either with #under or
> under (namespaced) names, but still possible to use now.

OK, I will define an ST_LSM6DSX_FIELD_GET() macro.
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by David Lechner 3 months, 1 week ago 
On 11/3/25 3:34 AM, Francesco Lavra wrote:
> On Sun, 2025-11-02 at 15:45 +0200, Andy Shevchenko wrote:
>> On Sun, Nov 2, 2025 at 1:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
>>> On Thu, 30 Oct 2025 15:49:37 +0200
>>> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>>>> On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:
>>>>> On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:
>>>>>> On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:
>>
>> ...
>>
>>>>>>> +       *val = (data & reg->mask) >> __ffs(reg->mask);
>>>>>>
>>>>>> Seems like yet another candidate for field_get() macro.
>>>>>
>>>>> FIELD_GET() can only be used with compile-time constant masks.
>>>>> And apparently this is the case with u8_get_bits() too, because you
>>>>> get a
>>>>> "bad bitfield mask" compiler error if you try to use u8_get_bits().
>>>>
>>>> We are talking about different things.
>>>> Here are the pointers to what I'm talking:
>>>>
>>>> - git grep -n -w 'field_get'
>>>> -
>>>> https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/
>>>>
>>> True that it will be a usecase for that, but given plan is to merge
>>> that through
>>> a different tree in next merge window, it's not available for us yet. 
>>> Hence would
>>> be a follow up patch next cycle.
>>
>> Yes, but we can still define them here. Dunno either with #under or
>> under (namespaced) names, but still possible to use now.
> 
> OK, I will define an ST_LSM6DSX_FIELD_GET() macro.

The macro should be named exactly `field_get()`, otherwise we will
have to rename all of the callers later after the series adding it
to linux/bitfield.h is merged. And it should have an
`#undef field_get` just like the other patches that series. Then
later, we only need to remove the #undef and #def lines and not
change the rest of the code.
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Jonathan Cameron 3 months ago 
On Mon, 3 Nov 2025 08:53:44 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 11/3/25 3:34 AM, Francesco Lavra wrote:
> > On Sun, 2025-11-02 at 15:45 +0200, Andy Shevchenko wrote:  
> >> On Sun, Nov 2, 2025 at 1:30 PM Jonathan Cameron <jic23@kernel.org> wrote:  
> >>> On Thu, 30 Oct 2025 15:49:37 +0200
> >>> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> >>>> On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:  
> >>>>> On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:  
> >>>>>> On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:  
> >>
> >> ...
> >>  
> >>>>>>> +       *val = (data & reg->mask) >> __ffs(reg->mask);  
> >>>>>>
> >>>>>> Seems like yet another candidate for field_get() macro.  
> >>>>>
> >>>>> FIELD_GET() can only be used with compile-time constant masks.
> >>>>> And apparently this is the case with u8_get_bits() too, because you
> >>>>> get a
> >>>>> "bad bitfield mask" compiler error if you try to use u8_get_bits().  
> >>>>
> >>>> We are talking about different things.
> >>>> Here are the pointers to what I'm talking:
> >>>>
> >>>> - git grep -n -w 'field_get'
> >>>> -
> >>>> https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/
> >>>>  
> >>> True that it will be a usecase for that, but given plan is to merge
> >>> that through
> >>> a different tree in next merge window, it's not available for us yet. 
> >>> Hence would
> >>> be a follow up patch next cycle.  
> >>
> >> Yes, but we can still define them here. Dunno either with #under or
> >> under (namespaced) names, but still possible to use now.  
> > 
> > OK, I will define an ST_LSM6DSX_FIELD_GET() macro.  
> 
> The macro should be named exactly `field_get()`, otherwise we will
> have to rename all of the callers later after the series adding it
> to linux/bitfield.h is merged. And it should have an
> `#undef field_get` just like the other patches that series. Then
> later, we only need to remove the #undef and #def lines and not
> change the rest of the code.

Carrying undefs for reasons we can't see in this code is for me something I'd
consider problematic.

For a case like this where we have just once instance I'd just prefer
either not using the macros, or namespacing them then replacing next cycle.
If there are loads of uses, then maybe on the undef nastiness.
These tend not to be common in drivers though and so far I've not seen
a case where the undef is worth doing (except maybe in the series adding
these new helpers where it is a very temporary thing)

Jonathan
Re: [PATCH 6/9] iio: imu: st_lsm6dsx: remove event_threshold field from hw struct
Posted by Andy Shevchenko 3 months, 1 week ago 
On Mon, Nov 03, 2025 at 10:34:28AM +0100, Francesco Lavra wrote:
> On Sun, 2025-11-02 at 15:45 +0200, Andy Shevchenko wrote:
> > On Sun, Nov 2, 2025 at 1:30 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Thu, 30 Oct 2025 15:49:37 +0200
> > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > > On Thu, Oct 30, 2025 at 12:10:08PM +0100, Francesco Lavra wrote:
> > > > > On Thu, 2025-10-30 at 10:01 +0200, Andy Shevchenko wrote:
> > > > > > On Thu, Oct 30, 2025 at 08:27:49AM +0100, Francesco Lavra wrote:

...

> > > > > > > +       *val = (data & reg->mask) >> __ffs(reg->mask);
> > > > > > 
> > > > > > Seems like yet another candidate for field_get() macro.
> > > > > 
> > > > > FIELD_GET() can only be used with compile-time constant masks.
> > > > > And apparently this is the case with u8_get_bits() too, because you
> > > > > get a
> > > > > "bad bitfield mask" compiler error if you try to use u8_get_bits().
> > > > 
> > > > We are talking about different things.
> > > > Here are the pointers to what I'm talking:
> > > > 
> > > > - git grep -n -w 'field_get'
> > > > -
> > > > https://lore.kernel.org/linux-gpio/cover.1761588465.git.geert+renesas@glider.be/
> > > > 
> > > True that it will be a usecase for that, but given plan is to merge
> > > that through
> > > a different tree in next merge window, it's not available for us yet. 
> > > Hence would
> > > be a follow up patch next cycle.
> > 
> > Yes, but we can still define them here. Dunno either with #under or
> > under (namespaced) names, but still possible to use now.
> 
> OK, I will define an ST_LSM6DSX_FIELD_GET() macro.

With small letters, as this is about run-time helpers (non-constant).

-- 
With Best Regards,
Andy Shevchenko

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.