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

[Jonathan Cameron <jic23@kernel.org>]

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