[PATCH v2 0/16] bitfield: tidy up bitfield.h

[PATCH v2 0/16] bitfield: tidy up bitfield.h

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

I noticed some very long (18KB) error messages from the compiler.
Turned out they were errors on lines that passed GENMASK() to FIELD_PREP().
Since most of the #defines are already statement functions the values
can be copied to locals so the actual parameters only get expanded once.

The 'bloat' is reduced further by using a simple test to ensure 'reg'
is large enough, slightly simplifying the test for constant 'val' and
only checking 'reg' and 'val' when the parameters are present.

The first two patches are slightly problematic.

drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c manages to use
a #define that should be an internal to bitfield.h, the changed file
is actually more similar to the previous version.

drivers/thunderbolt/tb.h passes a bifield to FIELD_GET(), these can't
be used with sizeof, typeof or __auto_type.
The existing FIELD_GET() uses _Generic(); gcc treats 'u32 foo:8' as
'unsigned char' and clang treats 'u32 foo:n' as 'unsigned int'.
So the code currentyly compiles 'by accident', pass 'u32 foo:6' and
gcc will error the attempt to use typeof with a bitfield.
For v2 fixed by changing the structure definition to use u8 for the
relevant field.

Both changes may need to to through the same tree as the header file changes.

The changes are based on 'next' and contain the addition of field_prep()
and field_get() for non-constant values.

I also know it is the merge window.
I expect to be generating a v3 in the new year (someone always has a comment).

Changes for v2:
- Change thunderbolt header (see above).
- Fix variable name re-use in FIELD_PREP_WM16()
- Use 'mask' (not _mask) in __BF_SHIFT().
The changes to bitfield.h have been split into multiple patches,
but the actual final file only has whitespace differences.

David Laight (16):
  nfp: Call FIELD_PREP() in NFP_ETH_SET_BIT_CONFIG() wrapper
  thunderbolt: Don't pass a bitfield to FIELD_GET
  bitmap: Use FIELD_PREP() in expansion of FIELD_PREP_WM16()
  bitfield: Copy #define parameters to locals
  bitfield: Merge __field_prep/get() into field_prep/get()
  bitfield: Remove some pointless casts
  bitfield: FIELD_MODIFY: Only do a single read/write on  the target
  bitfield: Simplify __BF_FIELD_CHECK_REG()
  bitfield: Rename __FIELD_PREP/GET() to __BF_FIELD_PREP/GET()
  bitfield: Split the 'val' check out of __BF_FIELD_CHECK_MASK()
  bitfield: Common up validation of the mask parameter
  bitfield: Remove leading _ from #define formal parameter names
  bitfield: Reduce indentation
  bitfield: Add comment block for the host/fixed endian functions
  bitfield: Update comments for le/be functions
  build_bug.h; Remove __BUILD_BUG_ON_NOT_POWER_OF_2()

 .../netronome/nfp/nfpcore/nfp_nsp_eth.c       |  16 +-
 drivers/thunderbolt/tb_regs.h                 |  16 +-
 include/linux/bitfield.h                      | 278 ++++++++++--------
 include/linux/build_bug.h                     |   2 -
 include/linux/hw_bitfield.h                   |  21 +-
 5 files changed, 175 insertions(+), 158 deletions(-)

-- 
2.39.5

[PATCH v2 01/16] nfp: Call FIELD_PREP() in NFP_ETH_SET_BIT_CONFIG() wrapper

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Rather than use a define that should be internal to the implementation
of FIELD_PREP(), pass the shifted 'val' to nfp_eth_set_bit_config()
and change the test for 'value unchanged' to match.

This is a simpler change than the one used to avoid calling both
FIELD_GET() and FIELD_PREP() with non-constant mask values.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---

Unchanged for v2.

 .../ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
index 5cfddc9a5d87..4a71ff47fbef 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
@@ -509,8 +509,7 @@ int nfp_eth_set_configured(struct nfp_cpp *cpp, unsigned int idx, bool configed)
 
 static int
 nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
-		       const u64 mask, const unsigned int shift,
-		       u64 val, const u64 ctrl_bit)
+		       const u64 mask, u64 shifted_val, const u64 ctrl_bit)
 {
 	union eth_table_entry *entries = nfp_nsp_config_entries(nsp);
 	unsigned int idx = nfp_nsp_config_idx(nsp);
@@ -527,11 +526,11 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
 
 	/* Check if we are already in requested state */
 	reg = le64_to_cpu(entries[idx].raw[raw_idx]);
-	if (val == (reg & mask) >> shift)
+	if (shifted_val == (reg & mask))
 		return 0;
 
 	reg &= ~mask;
-	reg |= (val << shift) & mask;
+	reg |= shifted_val;
 	entries[idx].raw[raw_idx] = cpu_to_le64(reg);
 
 	entries[idx].control |= cpu_to_le64(ctrl_bit);
@@ -571,12 +570,9 @@ int nfp_eth_set_idmode(struct nfp_cpp *cpp, unsigned int idx, bool state)
 	return nfp_eth_config_commit_end(nsp);
 }
 
-#define NFP_ETH_SET_BIT_CONFIG(nsp, raw_idx, mask, val, ctrl_bit)	\
-	({								\
-		__BF_FIELD_CHECK(mask, 0ULL, val, "NFP_ETH_SET_BIT_CONFIG: "); \
-		nfp_eth_set_bit_config(nsp, raw_idx, mask, __bf_shf(mask), \
-				       val, ctrl_bit);			\
-	})
+#define NFP_ETH_SET_BIT_CONFIG(nsp, raw_idx, mask, val, ctrl_bit)	  \
+	nfp_eth_set_bit_config(nsp, raw_idx, mask, FIELD_PREP(mask, val), \
+			       ctrl_bit)
 
 /**
  * __nfp_eth_set_aneg() - set PHY autonegotiation control bit
-- 
2.39.5

[PATCH v2 02/16] thunderbolt: Don't pass a bitfield to FIELD_GET

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

None of sizeof(), typeof() or __auto_type can be used with bitfields
which makes it difficult to assign a #define parameter to a local
without promoting char and short to int.

Change:
	u32 thunderbolt_version:8;
to the equivalent:
	u8 thunderbolt_version;
(and the other three bytes of 'DWORD 4' to match).

This is necessary so that FIELD_GET can use sizeof() to verify 'reg'.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---

Changes for v2:
- Change structure definition instead of call to FIELD_GET().

FIELD_GET currently uses _Generic() which behaves differently for
gcc and clang (I suspect both are wrong!).
gcc treats 'u32 foo:8' as 'u8', but will take the 'default' for other
widths (which will generate an error in FIED_GET().
clang treats 'u32 foo:n' as 'u32'.

 drivers/thunderbolt/tb_regs.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index c0bf136236e6..f35f062beb34 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -180,14 +180,14 @@ struct tb_regs_switch_header {
 	u32 route_hi:31;
 	bool enabled:1;
 	/* DWORD 4 */
-	u32 plug_events_delay:8; /*
-				  * RW, pause between plug events in
-				  * milliseconds. Writing 0x00 is interpreted
-				  * as 255ms.
-				  */
-	u32 cmuv:8;
-	u32 __unknown4:8;
-	u32 thunderbolt_version:8;
+	u8 plug_events_delay; /*
+			       * RW, pause between plug events in
+			       * milliseconds. Writing 0x00 is interpreted
+			       * as 255ms.
+			       */
+	u8 cmuv;
+	u8 __unknown4;
+	u8 thunderbolt_version;
 } __packed;
 
 /* Used with the router thunderbolt_version */
-- 
2.39.5

[PATCH v2 03/16] bitmap: Use FIELD_PREP() in expansion of FIELD_PREP_WM16()

[david.laight.linux]

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.
The extra check is added after calling FIELD_PREP() to get a sane
error message if 'mask' isn't constant.

Remove the leading _ from the formal parameter names.
Prefix the local variables with _wm16_ to hopefully make them
unique.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---

Changes for v2:
- Update kerneldoc to match changed formal parameter names.
- Change local variables to not collide with those in FIELD_PREP().

Most of the examples are constants and get optimised away.

 include/linux/hw_bitfield.h | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/linux/hw_bitfield.h b/include/linux/hw_bitfield.h
index df202e167ce4..0bd1040a5f93 100644
--- a/include/linux/hw_bitfield.h
+++ b/include/linux/hw_bitfield.h
@@ -12,8 +12,8 @@
 
 /**
  * FIELD_PREP_WM16() - prepare a bitfield element with a mask in the upper half
- * @_mask: shifted mask defining the field's length and position
- * @_val:  value to put in the field
+ * @mask: shifted mask defining the field's length and position
+ * @val:  value to put in the field
  *
  * FIELD_PREP_WM16() masks and shifts up the value, as well as bitwise ORs the
  * result with the mask shifted up by 16.
@@ -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 _wm16_mask = mask;				\
+	u32 _wm16_val = FIELD_PREP(_wm16_mask, val);		\
+	BUILD_BUG_ON_MSG(_wm16_mask > 0xffffu,			\
+			 "FIELD_PREP_WM16: mask too large");	\
+	_wm16_val | (_wm16_mask << 16);				\
+})
 
 /**
  * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
-- 
2.39.5

[PATCH v2 04/16] bitfield: Copy #define parameters to locals

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Use __auto_type to take copies of parameters to both ensure they are
evaluated only once and to avoid bloating the pre-processor output.
In particular 'mask' is likely to be GENMASK() and the expension
of FIELD_GET() is then about 18KB.

The 'val' parameter is sometimes a bitfield (so _auto_type cannot be
used), the type of _val also needs to be large enough to hold the
shifted value, using '__auto_type _val = 1 ? (val) : _mask;' solves
both problems.

Update kerneldoc to match the renamed (no leading _) formal parameters.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 52 ++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 126dc5b380af..3800744281c7 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -99,40 +99,45 @@
 
 /**
  * FIELD_MAX() - produce the maximum value representable by a field
- * @_mask: shifted mask defining the field's length and position
+ * @mask: shifted mask defining the field's length and position
  *
  * FIELD_MAX() returns the maximum value that can be held in the field
- * specified by @_mask.
+ * specified by @mask.
  */
-#define FIELD_MAX(_mask)						\
+#define FIELD_MAX(mask)							\
 	({								\
+		__auto_type _mask = mask;				\
 		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_MAX: ");	\
 		(typeof(_mask))((_mask) >> __bf_shf(_mask));		\
 	})
 
 /**
  * FIELD_FIT() - check if value fits in the field
- * @_mask: shifted mask defining the field's length and position
- * @_val:  value to test against the field
+ * @mask: shifted mask defining the field's length and position
+ * @val:  value to test against the field
  *
- * Return: true if @_val can fit inside @_mask, false if @_val is too big.
+ * Return: true if @val can fit inside @mask, false if @val is too big.
  */
-#define FIELD_FIT(_mask, _val)						\
+#define FIELD_FIT(mask, val)						\
 	({								\
+		__auto_type _mask = mask;				\
+		__auto_type _val = 1 ? (val) : _mask;			\
 		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_FIT: ");	\
 		!((((typeof(_mask))_val) << __bf_shf(_mask)) & ~(_mask)); \
 	})
 
 /**
  * FIELD_PREP() - prepare a bitfield element
- * @_mask: shifted mask defining the field's length and position
- * @_val:  value to put in the field
+ * @mask: shifted mask defining the field's length and position
+ * @val:  value to put in the field
  *
  * FIELD_PREP() masks and shifts up the value.  The result should
  * be combined with other fields of the bitfield using logical OR.
  */
-#define FIELD_PREP(_mask, _val)						\
+#define FIELD_PREP(mask, val)						\
 	({								\
+		__auto_type _mask = mask;				\
+		__auto_type _val = 1 ? (val) : _mask;			\
 		__BF_FIELD_CHECK_REG(_mask, 0ULL, "FIELD_PREP: ");	\
 		__FIELD_PREP(_mask, _val, "FIELD_PREP: ");		\
 	})
@@ -165,29 +170,34 @@
 
 /**
  * FIELD_GET() - extract a bitfield element
- * @_mask: shifted mask defining the field's length and position
- * @_reg:  value of entire bitfield
+ * @mask: shifted mask defining the field's length and position
+ * @reg:  value of entire bitfield
  *
- * FIELD_GET() extracts the field specified by @_mask from the
- * bitfield passed in as @_reg by masking and shifting it down.
+ * FIELD_GET() extracts the field specified by @mask from the
+ * bitfield passed in as @reg by masking and shifting it down.
  */
-#define FIELD_GET(_mask, _reg)						\
+#define FIELD_GET(mask, reg)						\
 	({								\
+		__auto_type _mask = mask;				\
+		__auto_type _reg = reg;					\
 		__BF_FIELD_CHECK_REG(_mask, _reg, "FIELD_GET: ");	\
 		__FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
 	})
 
 /**
  * FIELD_MODIFY() - modify a bitfield element
- * @_mask: shifted mask defining the field's length and position
- * @_reg_p: pointer to the memory that should be updated
- * @_val: value to store in the bitfield
+ * @mask: shifted mask defining the field's length and position
+ * @reg_p: pointer to the memory that should be updated
+ * @val: value to store in the bitfield
  *
- * FIELD_MODIFY() modifies the set of bits in @_reg_p specified by @_mask,
- * by replacing them with the bitfield value passed in as @_val.
+ * FIELD_MODIFY() modifies the set of bits in @reg_p specified by @mask,
+ * by replacing them with the bitfield value passed in as @val.
  */
-#define FIELD_MODIFY(_mask, _reg_p, _val)						\
+#define FIELD_MODIFY(mask, reg_p, val)							\
 	({										\
+		__auto_type _mask = mask;						\
+		__auto_type _reg_p = reg_p;						\
+		__auto_type _val = 1 ? (val) : _mask;					\
 		typecheck_pointer(_reg_p);						\
 		__BF_FIELD_CHECK(_mask, *(_reg_p), _val, "FIELD_MODIFY: ");		\
 		*(_reg_p) &= ~(_mask);							\
-- 
2.39.5

[PATCH v2 05/16] bitfield: Merge __field_prep/get() into field_prep/get()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

field_prep() calls __FIELD_PREP() which really wants the parameters
copied to 'locals' - but this is done later in __field_prep().

Move the 'auto_type' lines to the outer function.
This only leaves the shift calculation and final expression inside
__field_prep(), move the shift calculation to a new define __BF_SHIFT()
and just inline the final expression into  field_prep().
Use a common #define for the shift expression.

Do the same for field_get().

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 42 ++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 3800744281c7..c30120535680 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -251,23 +251,9 @@ __MAKE_OP(64)
 #undef __MAKE_OP
 #undef ____MAKE_OP
 
-#define __field_prep(mask, val)						\
-	({								\
-		__auto_type __mask = (mask);				\
-		typeof(__mask) __val = (val);				\
-		unsigned int __shift = BITS_PER_TYPE(__mask) <= 32 ?	\
-				       __ffs(__mask) : __ffs64(__mask);	\
-		(__val << __shift) & __mask;				\
-	})
-
-#define __field_get(mask, reg)						\
-	({								\
-		__auto_type __mask = (mask);				\
-		typeof(__mask) __reg =  (reg);				\
-		unsigned int __shift = BITS_PER_TYPE(__mask) <= 32 ?	\
-				       __ffs(__mask) : __ffs64(__mask);	\
-		(__reg & __mask) >> __shift;				\
-	})
+/* As __bf_shf() but for non-zero variables */
+#define __BF_SHIFT(mask) \
+	(BITS_PER_TYPE(mask) <= 32 ? __ffs(mask) : __ffs64(mask))
 
 /**
  * field_prep() - prepare a bitfield element
@@ -285,9 +271,14 @@ __MAKE_OP(64)
  * If you want to ensure that @mask is a compile-time constant, please use
  * FIELD_PREP() directly instead.
  */
-#define field_prep(mask, val)						\
-	(__builtin_constant_p(mask) ? __FIELD_PREP(mask, val, "field_prep: ") \
-				    : __field_prep(mask, val))
+#define field_prep(mask, val)					\
+({								\
+	__auto_type _mask = mask;				\
+	__auto_type _val = 1 ? (val) : _mask;			\
+	__builtin_constant_p(_mask) ?				\
+		__FIELD_PREP(_mask, _val, "field_prep: ") :	\
+		(_val << __BF_SHIFT(_mask)) & _mask;		\
+})
 
 /**
  * field_get() - extract a bitfield element
@@ -305,8 +296,13 @@ __MAKE_OP(64)
  * If you want to ensure that @mask is a compile-time constant, please use
  * FIELD_GET() directly instead.
  */
-#define field_get(mask, reg)						\
-	(__builtin_constant_p(mask) ? __FIELD_GET(mask, reg, "field_get: ") \
-				    : __field_get(mask, reg))
+#define field_get(mask, reg)					\
+({								\
+	__auto_type _mask = mask;				\
+	__auto_type _reg = reg;					\
+	__builtin_constant_p(_mask) ?				\
+		__FIELD_GET(_mask, _reg, "field_get: ") :	\
+		(_reg & _mask) >> __BF_SHIFT(_mask);		\
+})
 
 #endif
-- 
2.39.5

[PATCH v2 06/16] bitfield: Remove some pointless casts

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Now that _val is always large enough to hold _mask there is no need
to cast it before the shift left.

This also corrects the incorrect parenthesis in FIELD_FIT(),
unlikely to be a real problem.

For FIELD_GET() and FIELD_MAX() there is no point casting the
result of the shift to the type of mask.
Even if mask were char/short the value will immediately be promoted
to 'signed int' as soon as it is used.
All that is likely to happen is an extra and with 0xff.

Remove the associated extra (...) around __auto_type variable.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index c30120535680..f1859e28df5d 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -88,13 +88,13 @@
 #define __FIELD_PREP(mask, val, pfx)					\
 	({								\
 		__BF_FIELD_CHECK_MASK(mask, val, pfx);			\
-		((typeof(mask))(val) << __bf_shf(mask)) & (mask);	\
+		((val) << __bf_shf(mask)) & (mask);			\
 	})
 
 #define __FIELD_GET(mask, reg, pfx)					\
 	({								\
 		__BF_FIELD_CHECK_MASK(mask, 0U, pfx);			\
-		(typeof(mask))(((reg) & (mask)) >> __bf_shf(mask));	\
+		((reg) & (mask)) >> __bf_shf(mask);			\
 	})
 
 /**
@@ -108,7 +108,7 @@
 	({								\
 		__auto_type _mask = mask;				\
 		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_MAX: ");	\
-		(typeof(_mask))((_mask) >> __bf_shf(_mask));		\
+		(_mask >> __bf_shf(_mask));				\
 	})
 
 /**
@@ -123,7 +123,7 @@
 		__auto_type _mask = mask;				\
 		__auto_type _val = 1 ? (val) : _mask;			\
 		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_FIT: ");	\
-		!((((typeof(_mask))_val) << __bf_shf(_mask)) & ~(_mask)); \
+		!((_val << __bf_shf(_mask)) & ~_mask);			\
 	})
 
 /**
@@ -201,7 +201,7 @@
 		typecheck_pointer(_reg_p);						\
 		__BF_FIELD_CHECK(_mask, *(_reg_p), _val, "FIELD_MODIFY: ");		\
 		*(_reg_p) &= ~(_mask);							\
-		*(_reg_p) |= (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask));	\
+		*(_reg_p) |= (_val << __bf_shf(_mask)) & _mask;				\
 	})
 
 extern void __compiletime_error("value doesn't fit into mask")
-- 
2.39.5

[PATCH v2 07/16] bitfield: FIELD_MODIFY: Only do a single read/write on the target

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Replace the:
        *reg &= ~mask; *reg |= new_value;
with a single assignment.
While the compiler will usually optimise the extra access away
it isn't guaranteed.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index f1859e28df5d..c354ca2ef1a0 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -200,8 +200,7 @@
 		__auto_type _val = 1 ? (val) : _mask;					\
 		typecheck_pointer(_reg_p);						\
 		__BF_FIELD_CHECK(_mask, *(_reg_p), _val, "FIELD_MODIFY: ");		\
-		*(_reg_p) &= ~(_mask);							\
-		*(_reg_p) |= (_val << __bf_shf(_mask)) & _mask;				\
+		*_reg_p = (*_reg_p & ~_mask) | ((_val << __bf_shf(_mask)) & _mask);	\
 	})
 
 extern void __compiletime_error("value doesn't fit into mask")
-- 
2.39.5

[PATCH v2 08/16] bitfield: Simplify __BF_FIELD_CHECK_REG()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Simplify the check for 'reg' being large enough to hold 'mask' using
sizeof (reg) rather than a convoluted scheme to generate an unsigned
type the same size as 'reg'.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index c354ca2ef1a0..7160b762c979 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -45,22 +45,6 @@
 
 #define __bf_shf(x) (__builtin_ffsll(x) - 1)
 
-#define __scalar_type_to_unsigned_cases(type)				\
-		unsigned type:	(unsigned type)0,			\
-		signed type:	(unsigned type)0
-
-#define __unsigned_scalar_typeof(x) typeof(				\
-		_Generic((x),						\
-			char:	(unsigned char)0,			\
-			__scalar_type_to_unsigned_cases(char),		\
-			__scalar_type_to_unsigned_cases(short),		\
-			__scalar_type_to_unsigned_cases(int),		\
-			__scalar_type_to_unsigned_cases(long),		\
-			__scalar_type_to_unsigned_cases(long long),	\
-			default: (x)))
-
-#define __bf_cast_unsigned(type, x)	((__unsigned_scalar_typeof(type))(x))
-
 #define __BF_FIELD_CHECK_MASK(_mask, _val, _pfx)			\
 	({								\
 		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
@@ -75,8 +59,8 @@
 	})
 
 #define __BF_FIELD_CHECK_REG(mask, reg, pfx)				\
-	BUILD_BUG_ON_MSG(__bf_cast_unsigned(mask, mask) >		\
-			 __bf_cast_unsigned(reg, ~0ull),		\
+	BUILD_BUG_ON_MSG((mask) + 0U + 0UL + 0ULL >			\
+			 ~0ULL >> (64 - 8 * sizeof (reg)),		\
 			 pfx "type of reg too small for mask")
 
 #define __BF_FIELD_CHECK(mask, reg, val, pfx)				\
-- 
2.39.5

[PATCH v2 09/16] bitfield: Rename __FIELD_PREP/GET() to __BF_FIELD_PREP/GET()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

This makes the names consistent with the other internal defines
that shouldn't be used outside tis file.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 7160b762c979..ed5735c13a64 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -69,13 +69,13 @@
 		__BF_FIELD_CHECK_REG(mask, reg, pfx);			\
 	})
 
-#define __FIELD_PREP(mask, val, pfx)					\
+#define __BF_FIELD_PREP(mask, val, pfx)					\
 	({								\
 		__BF_FIELD_CHECK_MASK(mask, val, pfx);			\
 		((val) << __bf_shf(mask)) & (mask);			\
 	})
 
-#define __FIELD_GET(mask, reg, pfx)					\
+#define __BF_FIELD_GET(mask, reg, pfx)					\
 	({								\
 		__BF_FIELD_CHECK_MASK(mask, 0U, pfx);			\
 		((reg) & (mask)) >> __bf_shf(mask);			\
@@ -123,7 +123,7 @@
 		__auto_type _mask = mask;				\
 		__auto_type _val = 1 ? (val) : _mask;			\
 		__BF_FIELD_CHECK_REG(_mask, 0ULL, "FIELD_PREP: ");	\
-		__FIELD_PREP(_mask, _val, "FIELD_PREP: ");		\
+		__BF_FIELD_PREP(_mask, _val, "FIELD_PREP: ");		\
 	})
 
 #define __BF_CHECK_POW2(n)	BUILD_BUG_ON_ZERO(((n) & ((n) - 1)) != 0)
@@ -165,7 +165,7 @@
 		__auto_type _mask = mask;				\
 		__auto_type _reg = reg;					\
 		__BF_FIELD_CHECK_REG(_mask, _reg, "FIELD_GET: ");	\
-		__FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
+		__BF_FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
 	})
 
 /**
@@ -259,7 +259,7 @@ __MAKE_OP(64)
 	__auto_type _mask = mask;				\
 	__auto_type _val = 1 ? (val) : _mask;			\
 	__builtin_constant_p(_mask) ?				\
-		__FIELD_PREP(_mask, _val, "field_prep: ") :	\
+		__BF_FIELD_PREP(_mask, _val, "field_prep: ") :	\
 		(_val << __BF_SHIFT(_mask)) & _mask;		\
 })
 
@@ -284,7 +284,7 @@ __MAKE_OP(64)
 	__auto_type _mask = mask;				\
 	__auto_type _reg = reg;					\
 	__builtin_constant_p(_mask) ?				\
-		__FIELD_GET(_mask, _reg, "field_get: ") :	\
+		__BF_FIELD_GET(_mask, _reg, "field_get: ") :	\
 		(_reg & _mask) >> __BF_SHIFT(_mask);		\
 })
 
-- 
2.39.5

[PATCH v2 10/16] bitfield: Split the 'val' check out of __BF_FIELD_CHECK_MASK()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Three of the five calls to __BF_FIELD_CHECK_MASK() don't have a 'value'
to check, separate out as was done to __BF_FIELD_CHECK_REG().

Since __BF_FIELD_CHECK_MASK() doesn't return a value, use
do { ... } while (0) rather than ({ ... }).

There is no point checking a 'val' of zero or a 'reg' of 0ULL (both
are placeholders) - remove/change the calls.

There should be a check of __BF_FIELD_CHECK_REG() when __BF_FIELD_GET()
is called from field_get().
Move the check from FIELD_GET() into __BF_FIELD_GET().

Delete the now-unused __BF_FIELD_CHECK().

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index ed5735c13a64..138f4c14786d 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -45,39 +45,36 @@
 
 #define __bf_shf(x) (__builtin_ffsll(x) - 1)
 
-#define __BF_FIELD_CHECK_MASK(_mask, _val, _pfx)			\
-	({								\
+#define __BF_FIELD_CHECK_MASK(_mask, _pfx)				\
+	do {								\
 		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
 				 _pfx "mask is not constant");		\
 		BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");	\
-		BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?		\
-				 ~((_mask) >> __bf_shf(_mask)) &	\
-					(0 + (_val)) : 0,		\
-				 _pfx "value too large for the field"); \
 		__BUILD_BUG_ON_NOT_POWER_OF_2((_mask) +			\
 					      (1ULL << __bf_shf(_mask))); \
-	})
+	} while (0)
+
+#define __BF_FIELD_CHECK_VAL(mask, val, pfx)			\
+	BUILD_BUG_ON_MSG(__builtin_constant_p(val) &&		\
+			 ~((mask) >> __bf_shf(mask)) & (val),	\
+			 pfx "value too large for the field")
 
 #define __BF_FIELD_CHECK_REG(mask, reg, pfx)				\
 	BUILD_BUG_ON_MSG((mask) + 0U + 0UL + 0ULL >			\
 			 ~0ULL >> (64 - 8 * sizeof (reg)),		\
 			 pfx "type of reg too small for mask")
 
-#define __BF_FIELD_CHECK(mask, reg, val, pfx)				\
-	({								\
-		__BF_FIELD_CHECK_MASK(mask, val, pfx);			\
-		__BF_FIELD_CHECK_REG(mask, reg, pfx);			\
-	})
-
 #define __BF_FIELD_PREP(mask, val, pfx)					\
 	({								\
-		__BF_FIELD_CHECK_MASK(mask, val, pfx);			\
+		__BF_FIELD_CHECK_MASK(mask, pfx);			\
+		__BF_FIELD_CHECK_VAL(mask, val, pfx);			\
 		((val) << __bf_shf(mask)) & (mask);			\
 	})
 
 #define __BF_FIELD_GET(mask, reg, pfx)					\
 	({								\
-		__BF_FIELD_CHECK_MASK(mask, 0U, pfx);			\
+		__BF_FIELD_CHECK_MASK(mask, pfx);			\
+		__BF_FIELD_CHECK_REG(mask, reg, pfx);			\
 		((reg) & (mask)) >> __bf_shf(mask);			\
 	})
 
@@ -91,7 +88,7 @@
 #define FIELD_MAX(mask)							\
 	({								\
 		__auto_type _mask = mask;				\
-		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_MAX: ");	\
+		__BF_FIELD_CHECK_MASK(_mask, "FIELD_MAX: ");		\
 		(_mask >> __bf_shf(_mask));				\
 	})
 
@@ -106,7 +103,7 @@
 	({								\
 		__auto_type _mask = mask;				\
 		__auto_type _val = 1 ? (val) : _mask;			\
-		__BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_FIT: ");	\
+		__BF_FIELD_CHECK_MASK(_mask, "FIELD_FIT: ");		\
 		!((_val << __bf_shf(_mask)) & ~_mask);			\
 	})
 
@@ -122,7 +119,6 @@
 	({								\
 		__auto_type _mask = mask;				\
 		__auto_type _val = 1 ? (val) : _mask;			\
-		__BF_FIELD_CHECK_REG(_mask, 0ULL, "FIELD_PREP: ");	\
 		__BF_FIELD_PREP(_mask, _val, "FIELD_PREP: ");		\
 	})
 
@@ -164,7 +160,6 @@
 	({								\
 		__auto_type _mask = mask;				\
 		__auto_type _reg = reg;					\
-		__BF_FIELD_CHECK_REG(_mask, _reg, "FIELD_GET: ");	\
 		__BF_FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
 	})
 
@@ -182,8 +177,9 @@
 		__auto_type _mask = mask;						\
 		__auto_type _reg_p = reg_p;						\
 		__auto_type _val = 1 ? (val) : _mask;					\
-		typecheck_pointer(_reg_p);						\
-		__BF_FIELD_CHECK(_mask, *(_reg_p), _val, "FIELD_MODIFY: ");		\
+		__BF_FIELD_CHECK_MASK(_mask, "FIELD_MODIFY: ");				\
+		__BF_FIELD_CHECK_VAL(_mask, _val, "FIELD_MODIFY: ");			\
+		__BF_FIELD_CHECK_REG(_mask, *_reg_p, "FIELD_MODIFY: ");			\
 		*_reg_p = (*_reg_p & ~_mask) | ((_val << __bf_shf(_mask)) & _mask);	\
 	})
 
-- 
2.39.5

[PATCH v2 11/16] bitfield: Common up validation of the mask parameter

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

There are three places where the mask is checked for being non-zero
and contiguous.
Add a simple expression that checks it and use in all three places.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 138f4c14786d..6f454ef43d24 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -45,13 +45,15 @@
 
 #define __bf_shf(x) (__builtin_ffsll(x) - 1)
 
+#define __BF_VALIDATE_MASK(mask) \
+	(!(mask) || ((mask) & ((mask) + ((mask) & -(mask)))))
+
 #define __BF_FIELD_CHECK_MASK(_mask, _pfx)				\
 	do {								\
 		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
 				 _pfx "mask is not constant");		\
-		BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");	\
-		__BUILD_BUG_ON_NOT_POWER_OF_2((_mask) +			\
-					      (1ULL << __bf_shf(_mask))); \
+		BUILD_BUG_ON_MSG(__BF_VALIDATE_MASK(_mask),		\
+				 _pfx "mask is zero or not contiguous");	\
 	} while (0)
 
 #define __BF_FIELD_CHECK_VAL(mask, val, pfx)			\
@@ -122,8 +124,6 @@
 		__BF_FIELD_PREP(_mask, _val, "FIELD_PREP: ");		\
 	})
 
-#define __BF_CHECK_POW2(n)	BUILD_BUG_ON_ZERO(((n) & ((n) - 1)) != 0)
-
 /**
  * FIELD_PREP_CONST() - prepare a constant bitfield element
  * @_mask: shifted mask defining the field's length and position
@@ -138,12 +138,10 @@
  */
 #define FIELD_PREP_CONST(_mask, _val)					\
 	(								\
-		/* mask must be non-zero */				\
-		BUILD_BUG_ON_ZERO((_mask) == 0) +			\
+		/* mask must be non-zero and contiguous */		\
+		BUILD_BUG_ON_ZERO(__BF_VALIDATE_MASK(_mask)) +		\
 		/* check if value fits */				\
 		BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
-		/* check if mask is contiguous */			\
-		__BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) +	\
 		/* and create the value */				\
 		(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))	\
 	)
@@ -189,7 +187,7 @@ extern void __compiletime_error("bad bitfield mask")
 __bad_mask(void);
 static __always_inline u64 field_multiplier(u64 field)
 {
-	if ((field | (field - 1)) & ((field | (field - 1)) + 1))
+	if (__BF_VALIDATE_MASK(field))
 		__bad_mask();
 	return field & -field;
 }
-- 
2.39.5

[PATCH v2 12/16] bitfield: Remove leading _ from #define formal parameter names

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

There is no need to 'protect' formal parameter names from anything
outside the #define body itself.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 6f454ef43d24..753032285754 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -48,12 +48,12 @@
 #define __BF_VALIDATE_MASK(mask) \
 	(!(mask) || ((mask) & ((mask) + ((mask) & -(mask)))))
 
-#define __BF_FIELD_CHECK_MASK(_mask, _pfx)				\
+#define __BF_FIELD_CHECK_MASK(mask, pfx)				\
 	do {								\
-		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
-				 _pfx "mask is not constant");		\
-		BUILD_BUG_ON_MSG(__BF_VALIDATE_MASK(_mask),		\
-				 _pfx "mask is zero or not contiguous");	\
+		BUILD_BUG_ON_MSG(!__builtin_constant_p(mask),		\
+				 pfx "mask is not constant");		\
+		BUILD_BUG_ON_MSG(__BF_VALIDATE_MASK(mask),		\
+				 pfx "mask is zero or not contiguous");	\
 	} while (0)
 
 #define __BF_FIELD_CHECK_VAL(mask, val, pfx)			\
@@ -126,8 +126,8 @@
 
 /**
  * FIELD_PREP_CONST() - prepare a constant bitfield element
- * @_mask: shifted mask defining the field's length and position
- * @_val:  value to put in the field
+ * @mask: shifted mask defining the field's length and position
+ * @val:  value to put in the field
  *
  * FIELD_PREP_CONST() masks and shifts up the value.  The result should
  * be combined with other fields of the bitfield using logical OR.
@@ -136,14 +136,14 @@
  * be used in initializers. Error checking is less comfortable for this
  * version, and non-constant masks cannot be used.
  */
-#define FIELD_PREP_CONST(_mask, _val)					\
+#define FIELD_PREP_CONST(mask, val)					\
 	(								\
 		/* mask must be non-zero and contiguous */		\
-		BUILD_BUG_ON_ZERO(__BF_VALIDATE_MASK(_mask)) +		\
+		BUILD_BUG_ON_ZERO(__BF_VALIDATE_MASK(mask)) +		\
 		/* check if value fits */				\
-		BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
+		BUILD_BUG_ON_ZERO(~((mask) >> __bf_shf(mask)) & (val)) + \
 		/* and create the value */				\
-		(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))	\
+		(((typeof(mask))(val) << __bf_shf(mask)) & (mask))	\
 	)
 
 /**
-- 
2.39.5

[PATCH v2 13/16] bitfield: Reduce indentation

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

There is no need to double indent the body of #defines.
Leave the opening ( and closing ) on their own lines.

Remove extra tabs before line continuations.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 132 +++++++++++++++++++--------------------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 753032285754..03206be4ab54 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -48,37 +48,37 @@
 #define __BF_VALIDATE_MASK(mask) \
 	(!(mask) || ((mask) & ((mask) + ((mask) & -(mask)))))
 
-#define __BF_FIELD_CHECK_MASK(mask, pfx)				\
-	do {								\
-		BUILD_BUG_ON_MSG(!__builtin_constant_p(mask),		\
-				 pfx "mask is not constant");		\
-		BUILD_BUG_ON_MSG(__BF_VALIDATE_MASK(mask),		\
-				 pfx "mask is zero or not contiguous");	\
-	} while (0)
+#define __BF_FIELD_CHECK_MASK(mask, pfx)			\
+do {								\
+	BUILD_BUG_ON_MSG(!__builtin_constant_p(mask),		\
+			 pfx "mask is not constant");		\
+	BUILD_BUG_ON_MSG(__BF_VALIDATE_MASK(mask),		\
+			 pfx "mask is zero or not contiguous");	\
+} while (0)
 
 #define __BF_FIELD_CHECK_VAL(mask, val, pfx)			\
 	BUILD_BUG_ON_MSG(__builtin_constant_p(val) &&		\
 			 ~((mask) >> __bf_shf(mask)) & (val),	\
 			 pfx "value too large for the field")
 
-#define __BF_FIELD_CHECK_REG(mask, reg, pfx)				\
-	BUILD_BUG_ON_MSG((mask) + 0U + 0UL + 0ULL >			\
-			 ~0ULL >> (64 - 8 * sizeof (reg)),		\
+#define __BF_FIELD_CHECK_REG(mask, reg, pfx)			\
+	BUILD_BUG_ON_MSG((mask) + 0U + 0UL + 0ULL >		\
+			 ~0ULL >> (64 - 8 * sizeof (reg)),	\
 			 pfx "type of reg too small for mask")
 
-#define __BF_FIELD_PREP(mask, val, pfx)					\
-	({								\
-		__BF_FIELD_CHECK_MASK(mask, pfx);			\
-		__BF_FIELD_CHECK_VAL(mask, val, pfx);			\
-		((val) << __bf_shf(mask)) & (mask);			\
-	})
+#define __BF_FIELD_PREP(mask, val, pfx)		\
+({						\
+	__BF_FIELD_CHECK_MASK(mask, pfx);	\
+	__BF_FIELD_CHECK_VAL(mask, val, pfx);	\
+	((val) << __bf_shf(mask)) & (mask);	\
+})
 
-#define __BF_FIELD_GET(mask, reg, pfx)					\
-	({								\
-		__BF_FIELD_CHECK_MASK(mask, pfx);			\
-		__BF_FIELD_CHECK_REG(mask, reg, pfx);			\
-		((reg) & (mask)) >> __bf_shf(mask);			\
-	})
+#define __BF_FIELD_GET(mask, reg, pfx)				\
+({								\
+	__BF_FIELD_CHECK_MASK(mask, pfx);			\
+	__BF_FIELD_CHECK_REG(mask, reg, pfx);			\
+	((reg) & (mask)) >> __bf_shf(mask);			\
+})
 
 /**
  * FIELD_MAX() - produce the maximum value representable by a field
@@ -87,12 +87,12 @@
  * FIELD_MAX() returns the maximum value that can be held in the field
  * specified by @mask.
  */
-#define FIELD_MAX(mask)							\
-	({								\
-		__auto_type _mask = mask;				\
-		__BF_FIELD_CHECK_MASK(_mask, "FIELD_MAX: ");		\
-		(_mask >> __bf_shf(_mask));				\
-	})
+#define FIELD_MAX(mask)					\
+({							\
+	__auto_type _mask = mask;			\
+	__BF_FIELD_CHECK_MASK(_mask, "FIELD_MAX: ");	\
+	(_mask >> __bf_shf(_mask));			\
+})
 
 /**
  * FIELD_FIT() - check if value fits in the field
@@ -101,13 +101,13 @@
  *
  * Return: true if @val can fit inside @mask, false if @val is too big.
  */
-#define FIELD_FIT(mask, val)						\
-	({								\
-		__auto_type _mask = mask;				\
-		__auto_type _val = 1 ? (val) : _mask;			\
-		__BF_FIELD_CHECK_MASK(_mask, "FIELD_FIT: ");		\
-		!((_val << __bf_shf(_mask)) & ~_mask);			\
-	})
+#define FIELD_FIT(mask, val)				\
+({							\
+	__auto_type _mask = mask;			\
+	__auto_type _val = 1 ? (val) : _mask;		\
+	__BF_FIELD_CHECK_MASK(_mask, "FIELD_FIT: ");	\
+	!((_val << __bf_shf(_mask)) & ~_mask);		\
+})
 
 /**
  * FIELD_PREP() - prepare a bitfield element
@@ -117,12 +117,12 @@
  * FIELD_PREP() masks and shifts up the value.  The result should
  * be combined with other fields of the bitfield using logical OR.
  */
-#define FIELD_PREP(mask, val)						\
-	({								\
-		__auto_type _mask = mask;				\
-		__auto_type _val = 1 ? (val) : _mask;			\
-		__BF_FIELD_PREP(_mask, _val, "FIELD_PREP: ");		\
-	})
+#define FIELD_PREP(mask, val)				\
+({							\
+	__auto_type _mask = mask;			\
+	__auto_type _val = 1 ? (val) : _mask;		\
+	__BF_FIELD_PREP(_mask, _val, "FIELD_PREP: ");	\
+})
 
 /**
  * FIELD_PREP_CONST() - prepare a constant bitfield element
@@ -136,15 +136,15 @@
  * be used in initializers. Error checking is less comfortable for this
  * version, and non-constant masks cannot be used.
  */
-#define FIELD_PREP_CONST(mask, val)					\
-	(								\
-		/* mask must be non-zero and contiguous */		\
-		BUILD_BUG_ON_ZERO(__BF_VALIDATE_MASK(mask)) +		\
-		/* check if value fits */				\
-		BUILD_BUG_ON_ZERO(~((mask) >> __bf_shf(mask)) & (val)) + \
-		/* and create the value */				\
-		(((typeof(mask))(val) << __bf_shf(mask)) & (mask))	\
-	)
+#define FIELD_PREP_CONST(mask, val)				\
+(								\
+	/* mask must be non-zero and contiguous */		\
+	BUILD_BUG_ON_ZERO(__BF_VALIDATE_MASK(mask)) +		\
+	/* check if value fits */				\
+	BUILD_BUG_ON_ZERO(~((mask) >> __bf_shf(mask)) & (val)) + \
+	/* and create the value */				\
+	(((typeof(mask))(val) << __bf_shf(mask)) & (mask))	\
+)
 
 /**
  * FIELD_GET() - extract a bitfield element
@@ -154,12 +154,12 @@
  * FIELD_GET() extracts the field specified by @mask from the
  * bitfield passed in as @reg by masking and shifting it down.
  */
-#define FIELD_GET(mask, reg)						\
-	({								\
-		__auto_type _mask = mask;				\
-		__auto_type _reg = reg;					\
-		__BF_FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
-	})
+#define FIELD_GET(mask, reg)				\
+({							\
+	__auto_type _mask = mask;			\
+	__auto_type _reg = reg;				\
+	__BF_FIELD_GET(_mask, _reg, "FIELD_GET: ");	\
+})
 
 /**
  * FIELD_MODIFY() - modify a bitfield element
@@ -170,16 +170,16 @@
  * FIELD_MODIFY() modifies the set of bits in @reg_p specified by @mask,
  * by replacing them with the bitfield value passed in as @val.
  */
-#define FIELD_MODIFY(mask, reg_p, val)							\
-	({										\
-		__auto_type _mask = mask;						\
-		__auto_type _reg_p = reg_p;						\
-		__auto_type _val = 1 ? (val) : _mask;					\
-		__BF_FIELD_CHECK_MASK(_mask, "FIELD_MODIFY: ");				\
-		__BF_FIELD_CHECK_VAL(_mask, _val, "FIELD_MODIFY: ");			\
-		__BF_FIELD_CHECK_REG(_mask, *_reg_p, "FIELD_MODIFY: ");			\
-		*_reg_p = (*_reg_p & ~_mask) | ((_val << __bf_shf(_mask)) & _mask);	\
-	})
+#define FIELD_MODIFY(mask, reg_p, val)						\
+({										\
+	__auto_type _mask = mask;						\
+	__auto_type _reg_p = reg_p;						\
+	__auto_type _val = 1 ? (val) : _mask;					\
+	__BF_FIELD_CHECK_MASK(_mask, "FIELD_MODIFY: ");				\
+	__BF_FIELD_CHECK_VAL(_mask, _val, "FIELD_MODIFY: ");			\
+	__BF_FIELD_CHECK_REG(_mask, *_reg_p, "FIELD_MODIFY: ");			\
+	*_reg_p = (*_reg_p & ~_mask) | ((_val << __bf_shf(_mask)) & _mask);	\
+})
 
 extern void __compiletime_error("value doesn't fit into mask")
 __field_overflow(void);
-- 
2.39.5

[PATCH v2 14/16] bitfield: Add comment block for the host/fixed endian functions

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Copied almost verbatim from the commit message that added the functions.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 43 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 03206be4ab54..3bf82121a282 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -181,6 +181,49 @@ do {								\
 	*_reg_p = (*_reg_p & ~_mask) | ((_val << __bf_shf(_mask)) & _mask);	\
 })
 
+/*
+ * Primitives for manipulating bitfields both in host- and fixed-endian.
+ *
+ * * u32 le32_get_bits(__le32 val, u32 field) extracts the contents of the
+ *   bitfield specified by @field in little-endian 32bit object @val and
+ *   converts it to host-endian.
+ *
+ * * void le32p_replace_bits(__le32 *p, u32 v, u32 field) replaces
+ *   the contents of the bitfield specified by @field in little-endian
+ *   32bit object pointed to by @p with the value of @v.  New value is
+ *   given in host-endian and stored as little-endian.
+ *
+ * * __le32 le32_replace_bits(__le32 old, u32 v, u32 field) is equivalent to
+ *   ({__le32 tmp = old; le32p_replace_bits(&tmp, v, field); tmp;})
+ *   In other words, instead of modifying an object in memory, it takes
+ *   the initial value and returns the modified one.
+ *
+ * * __le32 le32_encode_bits(u32 v, u32 field) is equivalent to
+ *   le32_replace_bits(0, v, field).  In other words, it returns a little-endian
+ *   32bit object with the bitfield specified by @field containing the
+ *   value of @v and all bits outside that bitfield being zero.
+ *
+ * Such set of helpers is defined for each of little-, big- and host-endian
+ * types; e.g. u64_get_bits(val, field) will return the contents of the bitfield
+ * specified by @field in host-endian 64bit object @val, etc.  Of course, for
+ * host-endian no conversion is involved.
+ *
+ * Fields to access are specified as GENMASK() values - an N-bit field
+ * starting at bit #M is encoded as GENMASK(M + N - 1, M).  Note that
+ * bit numbers refer to endianness of the object we are working with -
+ * e.g. GENMASK(11, 0) in __be16 refers to the second byte and the lower
+ * 4 bits of the first byte.  In __le16 it would refer to the first byte
+ * and the lower 4 bits of the second byte, etc.
+ *
+ * Field specification must be a constant; __builtin_constant_p() doesn't
+ * have to be true for it, but compiler must be able to evaluate it at
+ * build time.  If it cannot or if the value does not encode any bitfield,
+ * the build will fail.
+ *
+ * If the value being stored in a bitfield is a constant that does not fit
+ * into that bitfield, a warning will be generated at compile time.
+ */
+
 extern void __compiletime_error("value doesn't fit into mask")
 __field_overflow(void);
 extern void __compiletime_error("bad bitfield mask")
-- 
2.39.5

[PATCH v2 15/16] bitfield: Update comments for le/be functions

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Make it clear the the values are converted to host order before
being acted on.
Order the explantions with the simple functions first.

These still need converting to kerneldoc format.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/bitfield.h | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 3bf82121a282..3f43683ebe96 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -184,24 +184,24 @@ do {								\
 /*
  * Primitives for manipulating bitfields both in host- and fixed-endian.
  *
- * * u32 le32_get_bits(__le32 val, u32 field) extracts the contents of the
- *   bitfield specified by @field in little-endian 32bit object @val and
- *   converts it to host-endian.
  *
- * * void le32p_replace_bits(__le32 *p, u32 v, u32 field) replaces
- *   the contents of the bitfield specified by @field in little-endian
- *   32bit object pointed to by @p with the value of @v.  New value is
- *   given in host-endian and stored as little-endian.
+ * * u32 le32_get_bits(__le32 val, u32 field) converts the little-endian 32bit
+ *   object @val to host-endian then extracts the contents of the bitfield
+ *   specified by @field.
+ *
+ * * __le32 le32_encode_bits(u32 v, u32 field) encodes the value of @v into
+ *   the bitfield specified by @field then converts the value to little-endian.
+ *   All the bits outside that bitfield being zero.
  *
- * * __le32 le32_replace_bits(__le32 old, u32 v, u32 field) is equivalent to
- *   ({__le32 tmp = old; le32p_replace_bits(&tmp, v, field); tmp;})
- *   In other words, instead of modifying an object in memory, it takes
- *   the initial value and returns the modified one.
+ * * __le32 le32_replace_bits(__le32 old, u32 v, u32 field) converts the
+ *   little-endian 32bit object @old to host order, replaces the contents
+ *   of the bitfield specified by @field with @v, then returns the value
+ *   converted back to little-endian.
  *
- * * __le32 le32_encode_bits(u32 v, u32 field) is equivalent to
- *   le32_replace_bits(0, v, field).  In other words, it returns a little-endian
- *   32bit object with the bitfield specified by @field containing the
- *   value of @v and all bits outside that bitfield being zero.
+ * * void le32p_replace_bits(__le32 *p, u32 v, u32 field) replaces
+ *   the contents of the bitfield specified by @field in little-endian
+ *   32bit object pointed to by @p with the value of @v.
+ *   Equivalent to *p = le32_replace_bits(*p, v, field).
  *
  * Such set of helpers is defined for each of little-, big- and host-endian
  * types; e.g. u64_get_bits(val, field) will return the contents of the bitfield
@@ -210,15 +210,13 @@ do {								\
  *
  * Fields to access are specified as GENMASK() values - an N-bit field
  * starting at bit #M is encoded as GENMASK(M + N - 1, M).  Note that
- * bit numbers refer to endianness of the object we are working with -
+ * bit numbers refer to the value after being converted to host order -
  * e.g. GENMASK(11, 0) in __be16 refers to the second byte and the lower
  * 4 bits of the first byte.  In __le16 it would refer to the first byte
  * and the lower 4 bits of the second byte, etc.
  *
- * Field specification must be a constant; __builtin_constant_p() doesn't
- * have to be true for it, but compiler must be able to evaluate it at
- * build time.  If it cannot or if the value does not encode any bitfield,
- * the build will fail.
+ * Field specification must be a non-zero constant, otherwise the build
+ * will fail.
  *
  * If the value being stored in a bitfield is a constant that does not fit
  * into that bitfield, a warning will be generated at compile time.
-- 
2.39.5

[PATCH v2 16/16] build_bug.h; Remove __BUILD_BUG_ON_NOT_POWER_OF_2()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

It was added for, and only used by, bitfield.h.
That not longer uses it so it can be removed.

It should have been called BUILD_BUG_ON_NOT_ZERO_OR_POWER_OF_2()
but there is no real need for such a specialised function.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 include/linux/build_bug.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index 2cfbb4c65c78..0ca6cb79f704 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -17,8 +17,6 @@
 	__BUILD_BUG_ON_ZERO_MSG(e, ##__VA_ARGS__, #e " is true")
 
 /* Force a compilation error if a constant expression is not a power of 2 */
-#define __BUILD_BUG_ON_NOT_POWER_OF_2(n)	\
-	BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
 #define BUILD_BUG_ON_NOT_POWER_OF_2(n)			\
 	BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
 
-- 
2.39.5

Re: [PATCH v2 01/16] nfp: Call FIELD_PREP() in NFP_ETH_SET_BIT_CONFIG() wrapper

[Jakub Kicinski]

On Fri, 12 Dec 2025 19:37:06 +0000 david.laight.linux@gmail.com wrote:
> Unchanged for v2.

Again, nak, just keep the macro.

Re: [PATCH v2 02/16] thunderbolt: Don't pass a bitfield to FIELD_GET

[Yury Norov]

On Fri, Dec 12, 2025 at 07:37:07PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> None of sizeof(), typeof() or __auto_type can be used with bitfields
> which makes it difficult to assign a #define parameter to a local
> without promoting char and short to int.
> 
> Change:
> 	u32 thunderbolt_version:8;
> to the equivalent:
> 	u8 thunderbolt_version;
> (and the other three bytes of 'DWORD 4' to match).
> 
> This is necessary so that FIELD_GET can use sizeof() to verify 'reg'.
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> 
> Changes for v2:
> - Change structure definition instead of call to FIELD_GET().
> 
> FIELD_GET currently uses _Generic() which behaves differently for
> gcc and clang (I suspect both are wrong!).
> gcc treats 'u32 foo:8' as 'u8', but will take the 'default' for other
> widths (which will generate an error in FIED_GET().
> clang treats 'u32 foo:n' as 'u32'.

FIELD_GET() works just well with bitfields, and whatever you do breaks
it. I pointed that in v1, but instead of fixing it, you do really well
hiding the problem.

I see no reasons to hack a random victim because of your rework. So
NAK for this. 

In v3, please add an explicit test to make sure that bitfields are not
broken with new implementation.

Thanks,
Yury

Re: [PATCH v2 02/16] thunderbolt: Don't pass a bitfield to FIELD_GET

[David Laight]

On Fri, 12 Dec 2025 21:28:31 -0500
Yury Norov <yury.norov@gmail.com> wrote:

> On Fri, Dec 12, 2025 at 07:37:07PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > None of sizeof(), typeof() or __auto_type can be used with bitfields
> > which makes it difficult to assign a #define parameter to a local
> > without promoting char and short to int.
> > 
> > Change:
> > 	u32 thunderbolt_version:8;
> > to the equivalent:
> > 	u8 thunderbolt_version;
> > (and the other three bytes of 'DWORD 4' to match).
> > 
> > This is necessary so that FIELD_GET can use sizeof() to verify 'reg'.
> > 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> > 
> > Changes for v2:
> > - Change structure definition instead of call to FIELD_GET().
> > 
> > FIELD_GET currently uses _Generic() which behaves differently for
> > gcc and clang (I suspect both are wrong!).
> > gcc treats 'u32 foo:8' as 'u8', but will take the 'default' for other
> > widths (which will generate an error in FIED_GET().
> > clang treats 'u32 foo:n' as 'u32'.  
> 
> FIELD_GET() works just well with bitfields, and whatever you do breaks
> it. I pointed that in v1, but instead of fixing it, you do really well
> hiding the problem.

It doesn't, pass 'u32 foo:6' when using gcc.

	David

> 
> I see no reasons to hack a random victim because of your rework. So
> NAK for this. 
> 
> In v3, please add an explicit test to make sure that bitfields are not
> broken with new implementation.
> 
> Thanks,
> Yury

Re: [PATCH v2 02/16] thunderbolt: Don't pass a bitfield to FIELD_GET

[David Laight]

On Sat, 13 Dec 2025 10:01:36 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> On Fri, 12 Dec 2025 21:28:31 -0500
> Yury Norov <yury.norov@gmail.com> wrote:
> 
> > On Fri, Dec 12, 2025 at 07:37:07PM +0000, david.laight.linux@gmail.com wrote:  
> > > From: David Laight <david.laight.linux@gmail.com>
> > > 
> > > None of sizeof(), typeof() or __auto_type can be used with bitfields
> > > which makes it difficult to assign a #define parameter to a local
> > > without promoting char and short to int.
> > > 
> > > Change:
> > > 	u32 thunderbolt_version:8;
> > > to the equivalent:
> > > 	u8 thunderbolt_version;
> > > (and the other three bytes of 'DWORD 4' to match).
> > > 
> > > This is necessary so that FIELD_GET can use sizeof() to verify 'reg'.
> > > 
> > > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > > ---
> > > 
> > > Changes for v2:
> > > - Change structure definition instead of call to FIELD_GET().
> > > 
> > > FIELD_GET currently uses _Generic() which behaves differently for
> > > gcc and clang (I suspect both are wrong!).
> > > gcc treats 'u32 foo:8' as 'u8', but will take the 'default' for other
> > > widths (which will generate an error in FIED_GET().
> > > clang treats 'u32 foo:n' as 'u32'.    
> > 
> > FIELD_GET() works just well with bitfields, and whatever you do breaks
> > it. I pointed that in v1, but instead of fixing it, you do really well
> > hiding the problem.  
> 
> It doesn't, pass 'u32 foo:6' when using gcc.

I've been trying to look up how _Generic() should behave for bitfields.
Basically it doesn't seem to be specified in the C standard.
Pretty much all you can do is force an integer promotion - and that only
works if the size is <= 32.
(I really hope there aren't any bigger bitfields lurking...)
But FIELD_GET() wants to check that 'reg' is 'big enough' to hold 'mask'.
An integer promotion breaks that for char/short.

I did try erroring on statically_true(mask > reg), but that throws up
a lot of false positives (even for non-constant 'reg').
It really is amazing that you get FIELD_GET(GENMASK(7, 0), val) instead
of 'val & 0xff' (and I don't mean code that want the low 8 bits of some
device register that is made up of lots of fields).

FIELD_PREP() does support bitfields (for 'val'), they get an integer
promotion applied.

There seems to be exactly one instance of 'reg' being a bitfield,
bloating all the other expansions for a trivially changeable line of
code is just stupid.

	David

> 
> 	David
> 
> > 
> > I see no reasons to hack a random victim because of your rework. So
> > NAK for this. 
> > 
> > In v3, please add an explicit test to make sure that bitfields are not
> > broken with new implementation.
> > 
> > Thanks,
> > Yury  
> 

Re: [PATCH v2 11/16] bitfield: Common up validation of the mask parameter

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.19-rc1 next-20251212]
[cannot apply to westeri-thunderbolt/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/david-laight-linux-gmail-com/nfp-Call-FIELD_PREP-in-NFP_ETH_SET_BIT_CONFIG-wrapper/20251213-040625
base:   linus/master
patch link:    https://lore.kernel.org/r/20251212193721.740055-12-david.laight.linux%40gmail.com
patch subject: [PATCH v2 11/16] bitfield: Common up validation of the mask parameter
config: i386-randconfig-053-20251213 (https://download.01.org/0day-ci/archive/20251214/202512141305.J3aPiiBv-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251214/202512141305.J3aPiiBv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512141305.J3aPiiBv-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/xe/xe_guc.c:639:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     639 |                 klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);
         |                                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
>> drivers/gpu/drm/xe/xe_guc.c:639:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_guc.c:642:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     642 |                 klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH);
         |                                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
   drivers/gpu/drm/xe/xe_guc.c:642:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   4 errors generated.
--
>> drivers/gpu/drm/xe/xe_guc_submit.c:371:12: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     371 |         *emit++ = PREP_GUC_KLV_TAG(SCHEDULING_POLICIES_RENDER_COMPUTE_YIELD);
         |                   ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
>> drivers/gpu/drm/xe/xe_guc_submit.c:371:12: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   2 errors generated.
--
>> drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:163:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     163 |                 PREP_GUC_KLV_TAG(VF_CFG_GGTT_START),
         |                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
>> drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:163:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:166:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     166 |                 PREP_GUC_KLV_TAG(VF_CFG_GGTT_SIZE),
         |                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:166:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:177:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     177 |                 PREP_GUC_KLV_TAG(VF_CFG_BEGIN_CONTEXT_ID),
         |                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:177:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:179:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     179 |                 PREP_GUC_KLV_TAG(VF_CFG_NUM_CONTEXTS),
         |                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:179:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:189:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     189 |                 PREP_GUC_KLV_TAG(VF_CFG_BEGIN_DOORBELL_ID),
         |                 ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:189:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
      62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
         |         ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c:191:3: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     191 |                 PREP_GUC_KLV_TAG(VF_CFG_NUM_DOORBELLS),
         |                 ^
--
>> drivers/gpu/drm/xe/xe_sriov_packet.c:381:16: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     381 |         klvs[len++] = PREP_GUC_KLV_CONST(MIGRATION_KLV_DEVICE_DEVID_KEY,
         |                       ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
>> drivers/gpu/drm/xe/xe_sriov_packet.c:381:16: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   drivers/gpu/drm/xe/xe_sriov_packet.c:384:16: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
     384 |         klvs[len++] = PREP_GUC_KLV_CONST(MIGRATION_KLV_DEVICE_REVID_KEY,
         |                       ^
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
      36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
         |                                                          ^
   drivers/gpu/drm/xe/xe_sriov_packet.c:384:16: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]
   drivers/gpu/drm/xe/xe_guc_klv_helpers.h:39:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
      39 |          FIELD_PREP_CONST(GUC_KLV_0_LEN, (len)))
         |                           ^
   drivers/gpu/drm/xe/abi/guc_klvs_abi.h:37:35: note: expanded from macro 'GUC_KLV_0_LEN'
      37 | #define GUC_KLV_0_LEN                           (0xffffu << 0)
         |                                                          ^
   4 errors generated.


vim +639 drivers/gpu/drm/xe/xe_guc.c

9c7d93a8f1ec04 Daniele Ceraolo Spurio 2025-06-25  616  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  617  #define OPT_IN_MAX_DWORDS 16
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  618  int xe_guc_opt_in_features_enable(struct xe_guc *guc)
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  619  {
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  620  	struct xe_device *xe = guc_to_xe(guc);
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  621  	CLASS(xe_guc_buf, buf)(&guc->buf, OPT_IN_MAX_DWORDS);
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  622  	u32 count = 0;
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  623  	u32 *klvs;
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  624  	int ret;
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  625  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  626  	if (!xe_guc_buf_is_valid(buf))
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  627  		return -ENOBUFS;
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  628  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  629  	klvs = xe_guc_buf_cpu_ptr(buf);
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  630  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  631  	/*
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  632  	 * The extra CAT error type opt-in was added in GuC v70.17.0, which maps
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  633  	 * to compatibility version v1.7.0.
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  634  	 * Note that the GuC allows enabling this KLV even on platforms that do
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  635  	 * not support the extra type; in such case the returned type variable
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  636  	 * will be set to a known invalid value which we can check against.
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  637  	 */
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  638  	if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 7, 0))
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25 @639  		klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  640  
9c7d93a8f1ec04 Daniele Ceraolo Spurio 2025-06-25  641  	if (supports_dynamic_ics(guc))
9c7d93a8f1ec04 Daniele Ceraolo Spurio 2025-06-25  642  		klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_DYNAMIC_INHIBIT_CONTEXT_SWITCH);
9c7d93a8f1ec04 Daniele Ceraolo Spurio 2025-06-25  643  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  644  	if (count) {
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  645  		xe_assert(xe, count <= OPT_IN_MAX_DWORDS);
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  646  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  647  		ret = __guc_opt_in_features_enable(guc, xe_guc_buf_flush(buf), count);
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  648  		if (ret < 0) {
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  649  			xe_gt_err(guc_to_gt(guc),
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  650  				  "failed to enable GuC opt-in features: %pe\n",
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  651  				  ERR_PTR(ret));
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  652  			return ret;
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  653  		}
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  654  	}
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  655  
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  656  	return 0;
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  657  }
a7ffcea8631af9 Daniele Ceraolo Spurio 2025-06-25  658  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 11/16] bitfield: Common up validation of the mask parameter

[David Laight]

On Sun, 14 Dec 2025 14:19:30 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v6.19-rc1 next-20251212]
> [cannot apply to westeri-thunderbolt/next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/david-laight-linux-gmail-com/nfp-Call-FIELD_PREP-in-NFP_ETH_SET_BIT_CONFIG-wrapper/20251213-040625
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20251212193721.740055-12-david.laight.linux%40gmail.com
> patch subject: [PATCH v2 11/16] bitfield: Common up validation of the mask parameter
> config: i386-randconfig-053-20251213 (https://download.01.org/0day-ci/archive/20251214/202512141305.J3aPiiBv-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251214/202512141305.J3aPiiBv-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202512141305.J3aPiiBv-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/gpu/drm/xe/xe_guc.c:639:19: error: converting the result of '<<' to a boolean always evaluates to true [-Werror,-Wtautological-constant-compare]  
>      639 |                 klvs[count++] = PREP_GUC_KLV_TAG(OPT_IN_FEATURE_EXT_CAT_ERR_TYPE);
>          |                                 ^
>    drivers/gpu/drm/xe/xe_guc_klv_helpers.h:62:2: note: expanded from macro 'PREP_GUC_KLV_TAG'
>       62 |         PREP_GUC_KLV_CONST(MAKE_GUC_KLV_KEY(TAG), MAKE_GUC_KLV_LEN(TAG))
>          |         ^
>    drivers/gpu/drm/xe/xe_guc_klv_helpers.h:38:20: note: expanded from macro 'PREP_GUC_KLV_CONST'
>       38 |         (FIELD_PREP_CONST(GUC_KLV_0_KEY, (key)) | \
>          |                           ^
>    drivers/gpu/drm/xe/abi/guc_klvs_abi.h:36:35: note: expanded from macro 'GUC_KLV_0_KEY'
>       36 | #define GUC_KLV_0_KEY                           (0xffffu << 16)
>          |                                                          ^

I've just sent a patch to move that warning to W=2.
It is picking up the same sort of things that -Wtype-limits does - already in W=2.

	David