From: David Laight <david.laight.linux@gmail.com>
Instead of directly expanding __BF_FIELD_CHECK() (which really ought
not be used outside bitfield) and open-coding the generation of the
masked value, just call FIELD_PREP() and add an extra check for
the mask being at most 16 bits.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
include/linux/hw_bitfield.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/include/linux/hw_bitfield.h b/include/linux/hw_bitfield.h
index df202e167ce4..d7f21b60449b 100644
--- a/include/linux/hw_bitfield.h
+++ b/include/linux/hw_bitfield.h
@@ -23,15 +23,14 @@
* register, a bit in the lower half is only updated if the corresponding bit
* in the upper half is high.
*/
-#define FIELD_PREP_WM16(_mask, _val) \
- ({ \
- typeof(_val) __val = _val; \
- typeof(_mask) __mask = _mask; \
- __BF_FIELD_CHECK(__mask, ((u16)0U), __val, \
- "HWORD_UPDATE: "); \
- (((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) | \
- ((__mask) << 16); \
- })
+#define FIELD_PREP_WM16(mask, val) \
+({ \
+ __auto_type _mask = mask; \
+ u32 _val = FIELD_PREP(_mask, val); \
+ BUILD_BUG_ON_MSG(_mask > 0xffffu, \
+ "FIELD_PREP_WM16: mask too large"); \
+ _val | (_mask << 16); \
+})
/**
* FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
--
2.39.5On Tue, Dec 09, 2025 at 10:03:07AM +0000, david.laight.linux@gmail.com wrote:
> Instead of directly expanding __BF_FIELD_CHECK() (which really ought
> not be used outside bitfield) and open-coding the generation of the
> masked value, just call FIELD_PREP() and add an extra check for
> the mask being at most 16 bits.
...
> +#define FIELD_PREP_WM16(mask, val) \
> +({ \
> + __auto_type _mask = mask; \
> + u32 _val = FIELD_PREP(_mask, val); \
> + BUILD_BUG_ON_MSG(_mask > 0xffffu, \
> + "FIELD_PREP_WM16: mask too large"); \
Can it be static_assert() instead?
> + _val | (_mask << 16); \
> +})
--
With Best Regards,
Andy Shevchenko
On Tue, 9 Dec 2025 17:46:21 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Tue, Dec 09, 2025 at 10:03:07AM +0000, david.laight.linux@gmail.com wrote:
>
> > Instead of directly expanding __BF_FIELD_CHECK() (which really ought
> > not be used outside bitfield) and open-coding the generation of the
> > masked value, just call FIELD_PREP() and add an extra check for
> > the mask being at most 16 bits.
>
> ...
>
> > +#define FIELD_PREP_WM16(mask, val) \
> > +({ \
> > + __auto_type _mask = mask; \
> > + u32 _val = FIELD_PREP(_mask, val); \
>
> > + BUILD_BUG_ON_MSG(_mask > 0xffffu, \
> > + "FIELD_PREP_WM16: mask too large"); \
>
> Can it be static_assert() instead?
No, they have to be 'integer constant expressions' not just
'compile time constants'.
Pretty useless for anything non-trivial.
David
>
> > + _val | (_mask << 16); \
> > +})
>
On Tuesday, 9 December 2025 11:03:07 Central European Standard Time david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> Instead of directly expanding __BF_FIELD_CHECK() (which really ought
> not be used outside bitfield) and open-coding the generation of the
> masked value, just call FIELD_PREP() and add an extra check for
> the mask being at most 16 bits.
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> include/linux/hw_bitfield.h | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/hw_bitfield.h b/include/linux/hw_bitfield.h
> index df202e167ce4..d7f21b60449b 100644
> --- a/include/linux/hw_bitfield.h
> +++ b/include/linux/hw_bitfield.h
> @@ -23,15 +23,14 @@
> * register, a bit in the lower half is only updated if the corresponding bit
> * in the upper half is high.
> */
> -#define FIELD_PREP_WM16(_mask, _val) \
> - ({ \
> - typeof(_val) __val = _val; \
> - typeof(_mask) __mask = _mask; \
> - __BF_FIELD_CHECK(__mask, ((u16)0U), __val, \
> - "HWORD_UPDATE: "); \
> - (((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) | \
> - ((__mask) << 16); \
> - })
> +#define FIELD_PREP_WM16(mask, val) \
> +({ \
> + __auto_type _mask = mask; \
> + u32 _val = FIELD_PREP(_mask, val); \
> + BUILD_BUG_ON_MSG(_mask > 0xffffu, \
> + "FIELD_PREP_WM16: mask too large"); \
> + _val | (_mask << 16); \
> +})
>
> /**
> * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
>
This breaks the build for at least one driver that uses
FIELD_PREP_WM16, namely phy-rockchip-emmc.c:
drivers/phy/rockchip/phy-rockchip-emmc.c:109:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
109 | HIWORD_UPDATE(PHYCTRL_PDB_PWR_OFF,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:114:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
114 | HIWORD_UPDATE(PHYCTRL_ENDLL_DISABLE,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:167:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
167 | HIWORD_UPDATE(PHYCTRL_PDB_PWR_ON,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:190:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
190 | HIWORD_UPDATE(freqsel, PHYCTRL_FREQSEL_MASK,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:196:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
196 | HIWORD_UPDATE(PHYCTRL_ENDLL_ENABLE,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:291:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
291 | HIWORD_UPDATE(rk_phy->drive_impedance,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:298:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
298 | HIWORD_UPDATE(PHYCTRL_OTAPDLYENA,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:305:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
305 | HIWORD_UPDATE(rk_phy->output_tapdelay_select,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:312:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
312 | HIWORD_UPDATE(rk_phy->enable_strobe_pulldown,
| ^
drivers/phy/rockchip/phy-rockchip-emmc.c:25:4: note: expanded from macro 'HIWORD_UPDATE'
25 | (FIELD_PREP_WM16((mask) << (shift), (val)))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
Maybe the wrapping in HIWORD_UPDATE (which was done to make the
transitionary patch easier) is playing a role here.
pcie-dw-rockchip.c is similarly broken by this change, except
without the superfluous wrapper:
drivers/pci/controller/dwc/pcie-dw-rockchip.c:191:37: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
191 | rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:43:35: note: expanded from macro 'PCIE_CLIENT_ENABLE_LTSSM'
43 | #define PCIE_CLIENT_ENABLE_LTSSM FIELD_PREP_WM16(BIT(2), 1)
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:197:37: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
197 | rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_DISABLE_LTSSM,
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:44:36: note: expanded from macro 'PCIE_CLIENT_DISABLE_LTSSM'
44 | #define PCIE_CLIENT_DISABLE_LTSSM FIELD_PREP_WM16(BIT(2), 0)
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:222:38: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
222 | rockchip_pcie_writel_apb(rockchip, PCIE_CLKREQ_READY,
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:67:29: note: expanded from macro 'PCIE_CLKREQ_READY'
67 | #define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:234:6: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
234 | PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:69:33: note: expanded from macro 'PCIE_CLKREQ_PULL_DOWN'
69 | #define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:234:30: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
234 | PCIE_CLKREQ_PULL_DOWN | PCIE_CLKREQ_NOT_READY,
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:68:33: note: expanded from macro 'PCIE_CLKREQ_NOT_READY'
68 | #define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:535:9: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
535 | val = FIELD_PREP_WM16(PCIE_LTSSM_APP_DLY2_DONE, 1);
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:574:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
574 | val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:578:6: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
578 | PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_RC),
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:41:34: note: expanded from macro 'PCIE_CLIENT_SET_MODE'
41 | #define PCIE_CLIENT_SET_MODE(x) FIELD_PREP_WM16(PCIE_CLIENT_MODE_MASK, (x))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:592:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
592 | val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0);
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:624:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
624 | val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1) |
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:625:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
625 | FIELD_PREP_WM16(PCIE_LTSSM_APP_DLY2_EN, 1);
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:629:6: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
629 | PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_EP),
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:41:34: note: expanded from macro 'PCIE_CLIENT_SET_MODE'
41 | #define PCIE_CLIENT_SET_MODE(x) FIELD_PREP_WM16(PCIE_CLIENT_MODE_MASK, (x))
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:653:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
653 | val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0) |
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
drivers/pci/controller/dwc/pcie-dw-rockchip.c:654:8: error: variable '_mask' declared with deduced type '__auto_type' cannot appear in its own initializer
654 | FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
| ^
include/linux/hw_bitfield.h:29:24: note: expanded from macro 'FIELD_PREP_WM16'
29 | u32 _val = FIELD_PREP(_mask, val); \
| ^
14 errors generated.
Kind regards,
Nicolas Frattaroli
On Wed, 10 Dec 2025 20:18:30 +0100
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:
> On Tuesday, 9 December 2025 11:03:07 Central European Standard Time david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> >
> > Instead of directly expanding __BF_FIELD_CHECK() (which really ought
> > not be used outside bitfield) and open-coding the generation of the
> > masked value, just call FIELD_PREP() and add an extra check for
> > the mask being at most 16 bits.
> >
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> > include/linux/hw_bitfield.h | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/hw_bitfield.h b/include/linux/hw_bitfield.h
> > index df202e167ce4..d7f21b60449b 100644
> > --- a/include/linux/hw_bitfield.h
> > +++ b/include/linux/hw_bitfield.h
> > @@ -23,15 +23,14 @@
> > * register, a bit in the lower half is only updated if the corresponding bit
> > * in the upper half is high.
> > */
> > -#define FIELD_PREP_WM16(_mask, _val) \
> > - ({ \
> > - typeof(_val) __val = _val; \
> > - typeof(_mask) __mask = _mask; \
> > - __BF_FIELD_CHECK(__mask, ((u16)0U), __val, \
> > - "HWORD_UPDATE: "); \
> > - (((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) | \
> > - ((__mask) << 16); \
> > - })
> > +#define FIELD_PREP_WM16(mask, val) \
> > +({ \
> > + __auto_type _mask = mask; \
> > + u32 _val = FIELD_PREP(_mask, val); \
> > + BUILD_BUG_ON_MSG(_mask > 0xffffu, \
> > + "FIELD_PREP_WM16: mask too large"); \
> > + _val | (_mask << 16); \
> > +})
> >
> > /**
> > * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
> >
>
> This breaks the build for at least one driver that uses
> FIELD_PREP_WM16, namely phy-rockchip-emmc.c:
Not in my allmodconfig build.
...
> pcie-dw-rockchip.c is similarly broken by this change, except
> without the superfluous wrapper:
That one did get built.
The problem is that FIELD_PREP_WM16() needs to use different 'local'
variables than FIELD_PREP().
The 'proper' fix is to use unique names (as min() and max() do), but that
makes the whole thing unreadable and is best avoided unless nesting is
likely.
In this case s/mask/wm16_mask/ and s/val/wm16_val/ might be best.
David
On Wednesday, 10 December 2025 21:59:15 Central European Standard Time David Laight wrote:
> On Wed, 10 Dec 2025 20:18:30 +0100
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:
>
> > On Tuesday, 9 December 2025 11:03:07 Central European Standard Time david.laight.linux@gmail.com wrote:
> > > From: David Laight <david.laight.linux@gmail.com>
> > >
> > > Instead of directly expanding __BF_FIELD_CHECK() (which really ought
> > > not be used outside bitfield) and open-coding the generation of the
> > > masked value, just call FIELD_PREP() and add an extra check for
> > > the mask being at most 16 bits.
> > >
> > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > ---
> > > include/linux/hw_bitfield.h | 17 ++++++++---------
> > > 1 file changed, 8 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/include/linux/hw_bitfield.h b/include/linux/hw_bitfield.h
> > > index df202e167ce4..d7f21b60449b 100644
> > > --- a/include/linux/hw_bitfield.h
> > > +++ b/include/linux/hw_bitfield.h
> > > @@ -23,15 +23,14 @@
> > > * register, a bit in the lower half is only updated if the corresponding bit
> > > * in the upper half is high.
> > > */
> > > -#define FIELD_PREP_WM16(_mask, _val) \
> > > - ({ \
> > > - typeof(_val) __val = _val; \
> > > - typeof(_mask) __mask = _mask; \
> > > - __BF_FIELD_CHECK(__mask, ((u16)0U), __val, \
> > > - "HWORD_UPDATE: "); \
> > > - (((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) | \
> > > - ((__mask) << 16); \
> > > - })
> > > +#define FIELD_PREP_WM16(mask, val) \
> > > +({ \
> > > + __auto_type _mask = mask; \
> > > + u32 _val = FIELD_PREP(_mask, val); \
> > > + BUILD_BUG_ON_MSG(_mask > 0xffffu, \
> > > + "FIELD_PREP_WM16: mask too large"); \
> > > + _val | (_mask << 16); \
> > > +})
> > >
> > > /**
> > > * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
> > >
> >
> > This breaks the build for at least one driver that uses
> > FIELD_PREP_WM16, namely phy-rockchip-emmc.c:
>
> Not in my allmodconfig build.
> ...
> > pcie-dw-rockchip.c is similarly broken by this change, except
> > without the superfluous wrapper:
>
> That one did get built.
I build with clang 21.1.6 for arm64, in case that's any help.
I don't see how pcie-dw-rockchip.c built for you if FIELD_PREP
and FIELD_PREP_WM16 have conflicting symbol names?
>
> The problem is that FIELD_PREP_WM16() needs to use different 'local'
> variables than FIELD_PREP().
> The 'proper' fix is to use unique names (as min() and max() do), but that
> makes the whole thing unreadable and is best avoided unless nesting is
> likely.
> In this case s/mask/wm16_mask/ and s/val/wm16_val/ might be best.
>
> David
>
>
On Thu, 11 Dec 2025 13:50:28 +0100
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:
> On Wednesday, 10 December 2025 21:59:15 Central European Standard Time David Laight wrote:
> > On Wed, 10 Dec 2025 20:18:30 +0100
> > Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:
> >
> > > On Tuesday, 9 December 2025 11:03:07 Central European Standard Time david.laight.linux@gmail.com wrote:
> > > > From: David Laight <david.laight.linux@gmail.com>
> > > >
> > > > Instead of directly expanding __BF_FIELD_CHECK() (which really ought
> > > > not be used outside bitfield) and open-coding the generation of the
> > > > masked value, just call FIELD_PREP() and add an extra check for
> > > > the mask being at most 16 bits.
> > > >
> > > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > > ---
> > > > include/linux/hw_bitfield.h | 17 ++++++++---------
> > > > 1 file changed, 8 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/include/linux/hw_bitfield.h b/include/linux/hw_bitfield.h
> > > > index df202e167ce4..d7f21b60449b 100644
> > > > --- a/include/linux/hw_bitfield.h
> > > > +++ b/include/linux/hw_bitfield.h
> > > > @@ -23,15 +23,14 @@
> > > > * register, a bit in the lower half is only updated if the corresponding bit
> > > > * in the upper half is high.
> > > > */
> > > > -#define FIELD_PREP_WM16(_mask, _val) \
> > > > - ({ \
> > > > - typeof(_val) __val = _val; \
> > > > - typeof(_mask) __mask = _mask; \
> > > > - __BF_FIELD_CHECK(__mask, ((u16)0U), __val, \
> > > > - "HWORD_UPDATE: "); \
> > > > - (((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) | \
> > > > - ((__mask) << 16); \
> > > > - })
> > > > +#define FIELD_PREP_WM16(mask, val) \
> > > > +({ \
> > > > + __auto_type _mask = mask; \
> > > > + u32 _val = FIELD_PREP(_mask, val); \
> > > > + BUILD_BUG_ON_MSG(_mask > 0xffffu, \
> > > > + "FIELD_PREP_WM16: mask too large"); \
> > > > + _val | (_mask << 16); \
> > > > +})
> > > >
> > > > /**
> > > > * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
> > > >
> > >
> > > This breaks the build for at least one driver that uses
> > > FIELD_PREP_WM16, namely phy-rockchip-emmc.c:
> >
> > Not in my allmodconfig build.
> > ...
> > > pcie-dw-rockchip.c is similarly broken by this change, except
> > > without the superfluous wrapper:
> >
> > That one did get built.
>
> I build with clang 21.1.6 for arm64, in case that's any help.
> I don't see how pcie-dw-rockchip.c built for you if FIELD_PREP
> and FIELD_PREP_WM16 have conflicting symbol names?
I would expect gcc to have found that 'silly' error.
I'll generate a v2 soon.
David
>
> >
> > The problem is that FIELD_PREP_WM16() needs to use different 'local'
> > variables than FIELD_PREP().
> > The 'proper' fix is to use unique names (as min() and max() do), but that
> > makes the whole thing unreadable and is best avoided unless nesting is
> > likely.
> > In this case s/mask/wm16_mask/ and s/val/wm16_val/ might be best.
> >
> > David
> >
> >
>
>
>
>
>
© 2016 - 2026 Red Hat, Inc.