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);

It's error-prone because it relies on user to provide the correct
index of the most significant bit.

This series adds a signed version of FIELD_GET(), which is the more
convenient and compiles (on x86_64) to just a couple instructions:
shl and sar.

Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
tree-wide.

Yury Norov (9):
  bitfield: add FIELD_GET_SIGNED()
  x86/extable: switch to using FIELD_GET_SIGNED()
  iio: intel_dc_ti_adc: switch to using
  iio: magnetometer: yas530: switch to using FIELD_GET_SIGNED()
  iio: pressure: bmp280: switch to using
  iio: mcp9600: switch to using FIELD_GET_SIGNED()
  wifi: rtw89: switch to using FIELD_GET_SIGNED()
  rtc: rv3032: switch to using FIELD_GET_SIGNED()
  ptp: switch to using FIELD_GET_SIGNED()

 arch/x86/include/asm/extable_fixup_types.h       | 13 ++++---------
 arch/x86/mm/extable.c                            |  2 +-
 drivers/iio/adc/intel_dc_ti_adc.c                |  4 ++--
 drivers/iio/magnetometer/yamaha-yas530.c         | 12 ++++++------
 drivers/iio/pressure/bmp280-core.c               |  2 +-
 drivers/iio/temperature/mcp9600.c                |  2 +-
 .../net/wireless/realtek/rtw89/rtw8852a_rfk.c    |  4 ++--
 .../net/wireless/realtek/rtw89/rtw8852b_common.c |  4 ++--
 .../net/wireless/realtek/rtw89/rtw8852b_rfk.c    |  4 ++--
 drivers/net/wireless/realtek/rtw89/rtw8852c.c    |  4 ++--
 drivers/ptp/ptp_fc3.c                            |  4 ++--
 drivers/rtc/rtc-rv3032.c                         |  2 +-
 include/linux/bitfield.h                         | 16 ++++++++++++++++
 13 files changed, 42 insertions(+), 31 deletions(-)

-- 
2.51.0

On Fri, Apr 17, 2026 at 01:36:11PM -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);
> 
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit.
> 
> This series adds a signed version of FIELD_GET(), which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
> 
> Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> tree-wide.

Here the example is missing.

Nevertheless, I looked at the implementation a bit and wondering how would it
work for 64-bit mask of say GENMASK_ULL(63, 60)? Wouldn't it give an overflow?

-- 
With Best Regards,
Andy Shevchenko

On Fri, Apr 17, 2026 at 09:23:02PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 17, 2026 at 01:36:11PM -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);
> > 
> > It's error-prone because it relies on user to provide the correct
> > index of the most significant bit.
> > 
> > This series adds a signed version of FIELD_GET(), which is the more
> > convenient and compiles (on x86_64) to just a couple instructions:
> > shl and sar.
> > 
> > Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> > tree-wide.
> 
> Here the example is missing.

This series is full of examples... I'll add one here if you prefer, if
it comes to v2.
 
> Nevertheless, I looked at the implementation a bit and wondering how would it
> work for 64-bit mask of say GENMASK_ULL(63, 60)? Wouldn't it give an overflow?

In that case, the '<< __builtin_clzll(mask)' part becomes a NOP, and
the compiler only emits a single sar:

   long long foo(long long reg)
  {
    10:   f3 0f 1e fa             endbr64
          return FIELD_GET_SIGNED(GENMASK_ULL(63, 60), reg);
    14:   48 89 f8                mov    %rdi,%rax
    17:   48 c1 f8 3c             sar    $0x3c,%rax
  }

Just tested it with a real kernel build with gcc-15.2, and it works as
intended.

On Fri, 17 Apr 2026 13:36:11 -0400
Yury Norov <ynorov@nvidia.com> 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);
> 
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit.
> 
> This series adds a signed version of FIELD_GET(), which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
> 
> Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> tree-wide.
> 

Just a quick heads up that I'm beginning to assume that this series
will land in some form.  If it does can we do it as an immutable branch
as I'm suggesting it gets used in some other patches in that should land
in the new cycle.

Thanks,

Jonathan

> Yury Norov (9):
>   bitfield: add FIELD_GET_SIGNED()
>   x86/extable: switch to using FIELD_GET_SIGNED()
>   iio: intel_dc_ti_adc: switch to using
>   iio: magnetometer: yas530: switch to using FIELD_GET_SIGNED()
>   iio: pressure: bmp280: switch to using
>   iio: mcp9600: switch to using FIELD_GET_SIGNED()
>   wifi: rtw89: switch to using FIELD_GET_SIGNED()
>   rtc: rv3032: switch to using FIELD_GET_SIGNED()
>   ptp: switch to using FIELD_GET_SIGNED()
> 
>  arch/x86/include/asm/extable_fixup_types.h       | 13 ++++---------
>  arch/x86/mm/extable.c                            |  2 +-
>  drivers/iio/adc/intel_dc_ti_adc.c                |  4 ++--
>  drivers/iio/magnetometer/yamaha-yas530.c         | 12 ++++++------
>  drivers/iio/pressure/bmp280-core.c               |  2 +-
>  drivers/iio/temperature/mcp9600.c                |  2 +-
>  .../net/wireless/realtek/rtw89/rtw8852a_rfk.c    |  4 ++--
>  .../net/wireless/realtek/rtw89/rtw8852b_common.c |  4 ++--
>  .../net/wireless/realtek/rtw89/rtw8852b_rfk.c    |  4 ++--
>  drivers/net/wireless/realtek/rtw89/rtw8852c.c    |  4 ++--
>  drivers/ptp/ptp_fc3.c                            |  4 ++--
>  drivers/rtc/rtc-rv3032.c                         |  2 +-
>  include/linux/bitfield.h                         | 16 ++++++++++++++++
>  13 files changed, 42 insertions(+), 31 deletions(-)
>

On Fri, Apr 24, 2026 at 01:09:27PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Apr 2026 13:36:11 -0400
> Yury Norov <ynorov@nvidia.com> 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);
> > 
> > It's error-prone because it relies on user to provide the correct
> > index of the most significant bit.
> > 
> > This series adds a signed version of FIELD_GET(), which is the more
> > convenient and compiles (on x86_64) to just a couple instructions:
> > shl and sar.
> > 
> > Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> > tree-wide.
> > 
> 
> Just a quick heads up that I'm beginning to assume that this series
> will land in some form.  If it does can we do it as an immutable branch
> as I'm suggesting it gets used in some other patches in that should land
> in the new cycle.

I'm going to submit v2 soon, as seemingly the discussion is boiled
down, and then will likely merge it with my tree. I'll create an
immutable branch for you before the end of day.

Thanks,
Yury

On Fri, Apr 24, 2026 at 11:50:10AM -0400, Yury Norov wrote:
> On Fri, Apr 24, 2026 at 01:09:27PM +0100, Jonathan Cameron wrote:
> > On Fri, 17 Apr 2026 13:36:11 -0400
> > Yury Norov <ynorov@nvidia.com> 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);
> > > 
> > > It's error-prone because it relies on user to provide the correct
> > > index of the most significant bit.
> > > 
> > > This series adds a signed version of FIELD_GET(), which is the more
> > > convenient and compiles (on x86_64) to just a couple instructions:
> > > shl and sar.
> > > 
> > > Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> > > tree-wide.
> > > 
> > 
> > Just a quick heads up that I'm beginning to assume that this series
> > will land in some form.  If it does can we do it as an immutable branch
> > as I'm suggesting it gets used in some other patches in that should land
> > in the new cycle.
> 
> I'm going to submit v2 soon, as seemingly the discussion is boiled
> down, and then will likely merge it with my tree. I'll create an
> immutable branch for you before the end of day.

Here it is:

https://github.com/norov/linux/pull/new/fgsv2

It builds well for me, but I'll wait for a while for robots feedback
before making it 'officially' immutable.

Thanks,
Yury