Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()

[Yury Norov]

On Mon, Apr 20, 2026 at 10:43:08AM +0200, Johannes Berg wrote:
> On Fri, 2026-04-17 at 13:36 -0400, Yury Norov wrote:
> > The bitfields are designed in assumption that fields contain unsigned
> > integer values, thus extracting the values from the field implies
> > zero-extending.
> > 
> > Some drivers need to sign-extend their fields, and currently do it like:
> > 
> > 	dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > 	dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
> 
> That's indeed pretty awful...
> 
> 
> > +#define FIELD_GET_SIGNED(mask, reg)					\
> > 
> 
> [...]
> 
> I (personally) tend to prefer the "__MAKE_OP" versions (*_get_bits()
> etc.), in particular because WiFi and firmware interfaces deal a lot
> with fixed endian fields.

I don't like that __MAKE_OP magic because whatever it generates is not
greppable. And because we disable strict type checks for kernel, but
this API claims to typecheck the parameters for the user. So, the
following compiles well:

        u64 val = 0;
        ret = le16_get_bits(val, GENMASK(15, 10));

I don't like autogeneration in general. We generate, for example,
be32_get_bits(), but never use it. We don't even know the level of
the bloat.
 
> Any chance it'd be simple to generate u32_get_bits_signed() etc.? Could
> be especially useful for le32_get_bits_signed() for example, to have the
> endian conversion built-in unlike FIELD_GET_SIGNED().

Maybe this:

        FIELD_GET_SIGNED(mask, le32_to_cpu(reg))

Thanks,
Yury