[PATCH 0/9] bitfield: tidy up bitfield.h

[PATCH 0/9] 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 or __auto_type. The usual solution is to add zero,
but that can't be done in FIELD_GET() because it doesn't want the value
promoted to 'int' (no idea how _Generic() treated it.)
The fix is just to add zero at the call site.
(The bitfield seems to be in a structure rad from hardware - no idea
how that works on BE (or any LE that uses an unusual order for bitfields.)

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 v2 in the new year (someone always has a comment).

David Laight (9):
  nfp: Call FIELD_PREP() in NFP_ETH_SET_BIT_CONFIG() wrapper
  thunderblot: 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: FIELD_MODIFY: Only do a single read/write on the target
  bitfield: Update sanity checks
  bitfield: Reduce indentation
  bitfield: Add comment block for the host/fixed endian functions
  bitfield: Update comments for le/be functions

 .../netronome/nfp/nfpcore/nfp_nsp_eth.c       |  16 +-
 drivers/thunderbolt/tb.h                      |   2 +-
 include/linux/bitfield.h                      | 278 ++++++++++--------
 include/linux/hw_bitfield.h                   |  17 +-
 4 files changed, 166 insertions(+), 147 deletions(-)

-- 
2.39.5

[PATCH 1/9] 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>
---
 .../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 2/9] thunderblot: Don't pass a bitfield to FIELD_GET

[david.laight.linux]

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

FIELD_GET needs to use __auto_type to get the value of the 'reg'
parameter, this can't be used with bifields.

FIELD_GET also want to verify the size of 'reg' so can't add zero
to force the type to int.

So add a zero here.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 drivers/thunderbolt/tb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index e96474f17067..7ca2b5a0f01e 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1307,7 +1307,7 @@ static inline struct tb_retimer *tb_to_retimer(struct device *dev)
  */
 static inline unsigned int usb4_switch_version(const struct tb_switch *sw)
 {
-	return FIELD_GET(USB4_VERSION_MAJOR_MASK, sw->config.thunderbolt_version);
+	return FIELD_GET(USB4_VERSION_MAJOR_MASK, sw->config.thunderbolt_version + 0);
 }
 
 /**
-- 
2.39.5

[PATCH 3/9] 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.

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.5

[PATCH 4/9] 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.

Remove any extra (), update kerneldoc.

Consistently use xxx for #define formal parameters and _xxx for
local variables.

Rather than use (typeof(mask))(val) to ensure bits aren't lost when
val is shifted left, use '__auto_type _val = 1 ? (val) : _mask;'
relying on the ?: operator to generate a type that is large enough.

Remove the (typeof(mask)) cast from __FIELD_GET(), it can only make
a difference if 'reg' is larger than 'mask' and the caller cares about
the actual type.
Note that mask usually comes from GENMASK() and is then 'unsigned long'.

Rename the internal defines __FIELD_PREP to __BF_FIELD_PREP and
__FIELD_GET to __BF_FIELD_GET.

Now that field_prep() and field_get() copy their parameters there is
no need for the __field_prep() and __field_get() defines.
But add a define to generate the required 'shift' to use in both defines.

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

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 126dc5b380af..3e013da9ea12 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -61,17 +61,17 @@
 
 #define __bf_cast_unsigned(type, x)	((__unsigned_scalar_typeof(type))(x))
 
-#define __BF_FIELD_CHECK_MASK(_mask, _val, _pfx)			\
+#define __BF_FIELD_CHECK_MASK(mask, val, pfx)				\
 	({								\
-		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))); \
+		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))); \
 	})
 
 #define __BF_FIELD_CHECK_REG(mask, reg, pfx)				\
@@ -85,64 +85,69 @@
 		__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);			\
-		((typeof(mask))(val) << __bf_shf(mask)) & (mask);	\
+		((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);			\
-		(typeof(mask))(((reg) & (mask)) >> __bf_shf(mask));	\
+		((reg) & (mask)) >> __bf_shf(mask);			\
 	})
 
 /**
  * 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));		\
+		(_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)); \
+		!((_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: ");		\
+		__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
- * @_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.
@@ -151,47 +156,52 @@
  * 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 */				\
-		BUILD_BUG_ON_ZERO((_mask) == 0) +			\
+		BUILD_BUG_ON_ZERO((mask) == 0) +			\
 		/* check if value fits */				\
-		BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
+		BUILD_BUG_ON_ZERO(~((mask) >> __bf_shf(mask)) & (val)) + \
 		/* check if mask is contiguous */			\
-		__BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) +	\
+		__BF_CHECK_POW2((mask) + (1ULL << __bf_shf(mask))) +	\
 		/* and create the value */				\
-		(((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask))	\
+		(((typeof(mask))(val) << __bf_shf(mask)) & (mask))	\
 	)
 
 /**
  * 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: ");		\
+		__BF_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);							\
-		*(_reg_p) |= (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask));	\
+		__BF_FIELD_CHECK(_mask, *_reg_p, _val, "FIELD_MODIFY: ");		\
+		*_reg_p &= ~_mask;							\
+		*_reg_p |= ((_val << __bf_shf(_mask)) & _mask);				\
 	})
 
 extern void __compiletime_error("value doesn't fit into mask")
@@ -241,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
@@ -275,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) ?				\
+		__BF_FIELD_PREP(_mask, _val, "field_prep: ") :	\
+		(_val << __BF_SHIFT(_mask)) & _mask;		\
+})
 
 /**
  * field_get() - extract a bitfield element
@@ -295,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) ?				\
+		__BF_FIELD_GET(_mask, _reg, "field_get: ") :	\
+		(_reg & _mask) >> __BF_SHIFT(_mask);		\
+})
 
 #endif
-- 
2.39.5

[PATCH 5/9] 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 3e013da9ea12..3e0e8533bb66 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 6/9] bitfield: Update sanity checks

[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'.

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.

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().

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 | 74 ++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 48 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 3e0e8533bb66..7e8d436b6571 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -45,55 +45,38 @@
 
 #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 __BF_VALIDATE_MASK(mask) \
+	(!(mask) || ((mask) & ((mask) + ((mask) & -(mask)))))
 
-#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)				\
-	({								\
+#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))); \
-	})
+		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(__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)				\
-	({								\
-		__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);			\
 	})
 
@@ -107,7 +90,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));				\
 	})
 
@@ -122,7 +105,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); 			\
 	})
 
@@ -138,12 +121,9 @@
 	({								\
 		__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: ");		\
 	})
 
-#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
@@ -158,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))	\
 	)
@@ -180,7 +158,6 @@
 	({								\
 		__auto_type _mask = mask;				\
 		__auto_type _reg = reg;					\
-		__BF_FIELD_CHECK_REG(_mask, _reg, "FIELD_GET: ");	\
 		__BF_FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
 	})
 
@@ -198,8 +175,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);	\
 	})
 
@@ -209,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 7/9] 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.

Delete extra tabs before continuation markers.

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 7e8d436b6571..bfd80ebd25b1 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 8/9] 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 bfd80ebd25b1..9feb489a8da3 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 9/9] 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 9feb489a8da3..dcc4809b4706 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

Re: [PATCH 2/9] thunderblot: Don't pass a bitfield to FIELD_GET

[Greg KH]

On Mon, Dec 08, 2025 at 10:42:43PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> FIELD_GET needs to use __auto_type to get the value of the 'reg'
> parameter, this can't be used with bifields.
> 
> FIELD_GET also want to verify the size of 'reg' so can't add zero
> to force the type to int.
> 
> So add a zero here.
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  drivers/thunderbolt/tb.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index e96474f17067..7ca2b5a0f01e 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -1307,7 +1307,7 @@ static inline struct tb_retimer *tb_to_retimer(struct device *dev)
>   */
>  static inline unsigned int usb4_switch_version(const struct tb_switch *sw)
>  {
> -	return FIELD_GET(USB4_VERSION_MAJOR_MASK, sw->config.thunderbolt_version);
> +	return FIELD_GET(USB4_VERSION_MAJOR_MASK, sw->config.thunderbolt_version + 0);

This is going to be very confusing to people who see this line only.
Can we add a comment here to explain why we have to do a "+ 0" and why
it can't be removed?  Otherwise I'm going to get a bunch of "cleanup"
patches attempting to "fix" this over the next few years.

thanks,

greg k-h

Re: [PATCH 3/9] bitmap: Use FIELD_PREP() in expansion of FIELD_PREP_WM16()

[Yury Norov]

+ Nicolas Frattaroli <nicolas.frattaroli@collabora.com> 

It's always good to CC an author of the original implementation,
isn't?

On Mon, Dec 08, 2025 at 10:42:44PM +0000, 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()

That's fair.

> and add an extra check for
> the mask being at most 16 bits.

Maybe it's time to introduce FIELD_PREP16()?

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

Is __auto_type any better than typeof()?

> +	u32 _val = FIELD_PREP(_mask, val);			\
> +	BUILD_BUG_ON_MSG(_mask > 0xffffu,			\
> +			 "FIELD_PREP_WM16: mask too large");	\

Not necessary to split this line.

> +	_val | (_mask << 16);					\
> +})

Can you share bloat-o-meter and code generation examples?

For the next version please try to keep as much history
untouched as possible.

Thanks,
Yury

>  
>  /**
>   * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
> -- 
> 2.39.5

Re: [PATCH 0/9] bitfield: tidy up bitfield.h

[Mika Westerberg]

Hi David,

On Mon, Dec 08, 2025 at 10:42:41PM +0000, david.laight.linux@gmail.com wrote:
> 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 or __auto_type. The usual solution is to add zero,
> but that can't be done in FIELD_GET() because it doesn't want the value
> promoted to 'int' (no idea how _Generic() treated it.)
> The fix is just to add zero at the call site.
> (The bitfield seems to be in a structure rad from hardware - no idea
> how that works on BE (or any LE that uses an unusual order for bitfields.)

Okay but can you CC me the actual patch too? I only got the cover letter
;-)

> 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 v2 in the new year (someone always has a comment).
> 
> David Laight (9):
>   nfp: Call FIELD_PREP() in NFP_ETH_SET_BIT_CONFIG() wrapper
>   thunderblot: 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: FIELD_MODIFY: Only do a single read/write on the target
>   bitfield: Update sanity checks
>   bitfield: Reduce indentation
>   bitfield: Add comment block for the host/fixed endian functions
>   bitfield: Update comments for le/be functions
> 
>  .../netronome/nfp/nfpcore/nfp_nsp_eth.c       |  16 +-
>  drivers/thunderbolt/tb.h                      |   2 +-
>  include/linux/bitfield.h                      | 278 ++++++++++--------
>  include/linux/hw_bitfield.h                   |  17 +-
>  4 files changed, 166 insertions(+), 147 deletions(-)
> 
> -- 
> 2.39.5

Re: [PATCH 0/9] bitfield: tidy up bitfield.h

[Jakub Kicinski]

On Tue, 9 Dec 2025 08:08:06 +0100 Mika Westerberg wrote:
> > drivers/thunderbolt/tb.h passes a bifield to FIELD_GET(), these can't
> > be used with sizeof or __auto_type. The usual solution is to add zero,
> > but that can't be done in FIELD_GET() because it doesn't want the value
> > promoted to 'int' (no idea how _Generic() treated it.)
> > The fix is just to add zero at the call site.
> > (The bitfield seems to be in a structure rad from hardware - no idea
> > how that works on BE (or any LE that uses an unusual order for bitfields.)  
> 
> Okay but can you CC me the actual patch too? I only got the cover letter
> ;-)

Right, David, please fix your CC lists. kernel dev 101..

Re: [PATCH 0/9] bitfield: tidy up bitfield.h

[David Laight]

On Tue, 9 Dec 2025 08:08:06 +0100
Mika Westerberg <mika.westerberg@linux.intel.com> wrote:

> Hi David,
> 
> On Mon, Dec 08, 2025 at 10:42:41PM +0000, david.laight.linux@gmail.com wrote:
> > 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 or __auto_type. The usual solution is to add zero,
> > but that can't be done in FIELD_GET() because it doesn't want the value
> > promoted to 'int' (no idea how _Generic() treated it.)
> > The fix is just to add zero at the call site.
> > (The bitfield seems to be in a structure rad from hardware - no idea
> > how that works on BE (or any LE that uses an unusual order for bitfields.)  
> 
> Okay but can you CC me the actual patch too? I only got the cover letter
> ;-)

Ah, sorry I'd changed the git settings..
I'll resend it all.

	David

Re: [PATCH 3/9] bitmap: Use FIELD_PREP() in expansion of FIELD_PREP_WM16()

[David Laight]

On Mon, 8 Dec 2025 19:54:37 -0500
Yury Norov <yury.norov@gmail.com> wrote:

> + Nicolas Frattaroli <nicolas.frattaroli@collabora.com> 
> 
> It's always good to CC an author of the original implementation,
> isn't?
> 
> On Mon, Dec 08, 2025 at 10:42:44PM +0000, 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()  
> 
> That's fair.
> 
> > and add an extra check for
> > the mask being at most 16 bits.  
> 
> Maybe it's time to introduce FIELD_PREP16()?
> 
> > 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;				\  
> 
> Is __auto_type any better than typeof()?

Generally yes.
Mainly because you only get one expansion of the #define parameter.

> 
> > +	u32 _val = FIELD_PREP(_mask, val);			\
> > +	BUILD_BUG_ON_MSG(_mask > 0xffffu,			\
> > +			 "FIELD_PREP_WM16: mask too large");	\  
> 
> Not necessary to split this line.
> 
> > +	_val | (_mask << 16);					\
> > +})  
> 
> Can you share bloat-o-meter and code generation examples?

The generated code is going to be pretty much unchanged.
The changes only really affect the compile-time checks.

> 
> For the next version please try to keep as much history
> untouched as possible.

I'm going to resend the same patches with the full cc list for now.

	David

> 
> Thanks,
> Yury
> 
> >  
> >  /**
> >   * FIELD_PREP_WM16_CONST() - prepare a constant bitfield element with a mask in
> > -- 
> > 2.39.5  

[PATCH 6/9] bitfield: Update sanity checks

[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'.

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.

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().

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 | 74 ++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 48 deletions(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 3e0e8533bb66..7e8d436b6571 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -45,55 +45,38 @@
 
 #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 __BF_VALIDATE_MASK(mask) \
+	(!(mask) || ((mask) & ((mask) + ((mask) & -(mask)))))
 
-#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)				\
-	({								\
+#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))); \
-	})
+		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(__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)				\
-	({								\
-		__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);			\
 	})
 
@@ -107,7 +90,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));				\
 	})
 
@@ -122,7 +105,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); 			\
 	})
 
@@ -138,12 +121,9 @@
 	({								\
 		__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: ");		\
 	})
 
-#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
@@ -158,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))	\
 	)
@@ -180,7 +158,6 @@
 	({								\
 		__auto_type _mask = mask;				\
 		__auto_type _reg = reg;					\
-		__BF_FIELD_CHECK_REG(_mask, _reg, "FIELD_GET: ");	\
 		__BF_FIELD_GET(_mask, _reg, "FIELD_GET: ");		\
 	})
 
@@ -198,8 +175,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);	\
 	})
 
@@ -209,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

Re: [PATCH 2/9] thunderblot: Don't pass a bitfield to FIELD_GET

[David Laight]

On Tue, 9 Dec 2025 07:56:35 +0900
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Dec 08, 2025 at 10:42:43PM +0000, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > FIELD_GET needs to use __auto_type to get the value of the 'reg'
> > parameter, this can't be used with bifields.
> > 
> > FIELD_GET also want to verify the size of 'reg' so can't add zero
> > to force the type to int.
> > 
> > So add a zero here.
> > 
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> >  drivers/thunderbolt/tb.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> > index e96474f17067..7ca2b5a0f01e 100644
> > --- a/drivers/thunderbolt/tb.h
> > +++ b/drivers/thunderbolt/tb.h
> > @@ -1307,7 +1307,7 @@ static inline struct tb_retimer *tb_to_retimer(struct device *dev)
> >   */
> >  static inline unsigned int usb4_switch_version(const struct tb_switch *sw)
> >  {
> > -	return FIELD_GET(USB4_VERSION_MAJOR_MASK, sw->config.thunderbolt_version);
> > +	return FIELD_GET(USB4_VERSION_MAJOR_MASK, sw->config.thunderbolt_version + 0);  
> 
> This is going to be very confusing to people who see this line only.
> Can we add a comment here to explain why we have to do a "+ 0" and why
> it can't be removed?  Otherwise I'm going to get a bunch of "cleanup"
> patches attempting to "fix" this over the next few years.

I've added a comment for v2 (but not the resend I did for the cc 'issue').
I've also fixed the subject blot => bolt.

I also noticed that the 'int thunderbolt_version:8' seems to be in a structure
that maps something read from the hardware.
That isn't going to be right on any BE system, and isn't guaranteed on LE ones.

	David

> 
> thanks,
> 
> greg k-h

Re: [PATCH 6/9] bitfield: Update sanity checks

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence static check first_new_problem: drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse: sparse: __auto_type on self-init var"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251209100313.2867-7-david.laight.linux@gmail.com>
References: <20251209100313.2867-7-david.laight.linux@gmail.com>
TO: david.laight.linux@gmail.com
TO: Yury Norov <yury.norov@gmail.com>
TO: Rasmus Villemoes <linux@rasmusvillemoes.dk>
TO: linux-kernel@vger.kernel.org
TO: linux-usb@vger.kernel.org
TO: Geert Uytterhoeven <geert+renesas@glider.be>
TO: Alexandre Belloni <alexandre.belloni@bootlin.com>
TO: Jonathan Cameron <Jonathan.Cameron@huawei.com>
TO: Crt Mori <cmo@melexis.com>
TO: Richard Genoud <richard.genoud@bootlin.com>
TO: Andy Shevchenko <andriy.shevchenko@intel.com>
TO: Luo Jie <quic_luoj@quicinc.com>
TO: Peter Zijlstra <peterz@infradead.org>
TO: Jakub Kicinski <kuba@kernel.org>
TO: netdev@vger.kernel.org
TO: "David S . Miller" <davem@davemloft.net>
TO: Simon Horman <simon.horman@netronome.com>
TO: Mika Westerberg <mika.westerberg@linux.intel.com>
TO: Andreas Noever <andreas.noever@gmail.com>
TO: Yehezkel Bernat <YehezkelShB@gmail.com>
TO: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
CC: David Laight <david.laight.linux@gmail.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20251211]
[cannot apply to westeri-thunderbolt/next v6.18]
[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/20251209-181248
base:   linus/master
patch link:    https://lore.kernel.org/r/20251209100313.2867-7-david.laight.linux%40gmail.com
patch subject: [PATCH 6/9] bitfield: Update sanity checks
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: nios2-randconfig-r133-20251211 (https://download.01.org/0day-ci/archive/20251211/202512112136.QAv8Vduf-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251211/202512112136.QAv8Vduf-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/r/202512112136.QAv8Vduf-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse: sparse: __auto_type on self-init var
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse: sparse: __auto_type on self-init var
   drivers/mmc/host/dw_mmc-rockchip.c:595:1: sparse: sparse: bad integer constant expression
   drivers/mmc/host/dw_mmc-rockchip.c:595:1: sparse: sparse: static assertion failed: "MODULE_INFO(author, ...) contains embedded NUL byte"
   drivers/mmc/host/dw_mmc-rockchip.c:596:1: sparse: sparse: bad integer constant expression
   drivers/mmc/host/dw_mmc-rockchip.c:596:1: sparse: sparse: static assertion failed: "MODULE_INFO(description, ...) contains embedded NUL byte"
   drivers/mmc/host/dw_mmc-rockchip.c:597:1: sparse: sparse: bad integer constant expression
   drivers/mmc/host/dw_mmc-rockchip.c:597:1: sparse: sparse: static assertion failed: "MODULE_INFO(alias, ...) contains embedded NUL byte"
   drivers/mmc/host/dw_mmc-rockchip.c:598:1: sparse: sparse: bad integer constant expression
   drivers/mmc/host/dw_mmc-rockchip.c:598:1: sparse: sparse: static assertion failed: "MODULE_INFO(file, ...) contains embedded NUL byte"
   drivers/mmc/host/dw_mmc-rockchip.c:598:1: sparse: sparse: bad integer constant expression
   drivers/mmc/host/dw_mmc-rockchip.c:598:1: sparse: sparse: static assertion failed: "MODULE_INFO(license, ...) contains embedded NUL byte"
>> drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse: sparse: incompatible types in conditional expression (different base types):
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    unsigned int
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    bad type
>> drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse: sparse: incompatible type for operation (-):
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    bad type _mask
>> drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse: sparse: incompatible types for operation (>>):
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    bad type _mask
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    int
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse: sparse: incompatible types for operation (<<):
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    bad type _val
   drivers/mmc/host/dw_mmc-rockchip.c:152:17: sparse:    int
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse: sparse: incompatible types in conditional expression (different base types):
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    unsigned int
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    bad type
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse: sparse: incompatible type for operation (-):
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    bad type _mask
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse: sparse: incompatible types for operation (>>):
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    bad type _mask
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    int
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse: sparse: incompatible types for operation (<<):
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    bad type _val
   drivers/mmc/host/dw_mmc-rockchip.c:155:17: sparse:    int

vim +152 drivers/mmc/host/dw_mmc-rockchip.c

59903441f5e49d Shawn Lin          2024-08-28   87  
59903441f5e49d Shawn Lin          2024-08-28   88  static int rockchip_mmc_set_internal_phase(struct dw_mci *host, bool sample, int degrees)
59903441f5e49d Shawn Lin          2024-08-28   89  {
739f04f4a46237 Shawn Lin          2025-11-04   90  	unsigned long rate = clk_get_rate(host->ciu_clk) / RK3288_CLKGEN_DIV;
59903441f5e49d Shawn Lin          2024-08-28   91  	u8 nineties, remainder;
59903441f5e49d Shawn Lin          2024-08-28   92  	u8 delay_num;
59903441f5e49d Shawn Lin          2024-08-28   93  	u32 raw_value;
59903441f5e49d Shawn Lin          2024-08-28   94  	u32 delay;
59903441f5e49d Shawn Lin          2024-08-28   95  
59903441f5e49d Shawn Lin          2024-08-28   96  	/*
59903441f5e49d Shawn Lin          2024-08-28   97  	 * The below calculation is based on the output clock from
59903441f5e49d Shawn Lin          2024-08-28   98  	 * MMC host to the card, which expects the phase clock inherits
59903441f5e49d Shawn Lin          2024-08-28   99  	 * the clock rate from its parent, namely the output clock
59903441f5e49d Shawn Lin          2024-08-28  100  	 * provider of MMC host. However, things may go wrong if
59903441f5e49d Shawn Lin          2024-08-28  101  	 * (1) It is orphan.
59903441f5e49d Shawn Lin          2024-08-28  102  	 * (2) It is assigned to the wrong parent.
59903441f5e49d Shawn Lin          2024-08-28  103  	 *
59903441f5e49d Shawn Lin          2024-08-28  104  	 * This check help debug the case (1), which seems to be the
59903441f5e49d Shawn Lin          2024-08-28  105  	 * most likely problem we often face and which makes it difficult
59903441f5e49d Shawn Lin          2024-08-28  106  	 * for people to debug unstable mmc tuning results.
59903441f5e49d Shawn Lin          2024-08-28  107  	 */
59903441f5e49d Shawn Lin          2024-08-28  108  	if (!rate) {
59903441f5e49d Shawn Lin          2024-08-28  109  		dev_err(host->dev, "%s: invalid clk rate\n", __func__);
59903441f5e49d Shawn Lin          2024-08-28  110  		return -EINVAL;
59903441f5e49d Shawn Lin          2024-08-28  111  	}
59903441f5e49d Shawn Lin          2024-08-28  112  
59903441f5e49d Shawn Lin          2024-08-28  113  	nineties = degrees / 90;
59903441f5e49d Shawn Lin          2024-08-28  114  	remainder = (degrees % 90);
59903441f5e49d Shawn Lin          2024-08-28  115  
59903441f5e49d Shawn Lin          2024-08-28  116  	/*
59903441f5e49d Shawn Lin          2024-08-28  117  	 * Due to the inexact nature of the "fine" delay, we might
59903441f5e49d Shawn Lin          2024-08-28  118  	 * actually go non-monotonic.  We don't go _too_ monotonic
59903441f5e49d Shawn Lin          2024-08-28  119  	 * though, so we should be OK.  Here are options of how we may
59903441f5e49d Shawn Lin          2024-08-28  120  	 * work:
59903441f5e49d Shawn Lin          2024-08-28  121  	 *
59903441f5e49d Shawn Lin          2024-08-28  122  	 * Ideally we end up with:
59903441f5e49d Shawn Lin          2024-08-28  123  	 *   1.0, 2.0, ..., 69.0, 70.0, ...,  89.0, 90.0
59903441f5e49d Shawn Lin          2024-08-28  124  	 *
59903441f5e49d Shawn Lin          2024-08-28  125  	 * On one extreme (if delay is actually 44ps):
59903441f5e49d Shawn Lin          2024-08-28  126  	 *   .73, 1.5, ..., 50.6, 51.3, ...,  65.3, 90.0
59903441f5e49d Shawn Lin          2024-08-28  127  	 * The other (if delay is actually 77ps):
59903441f5e49d Shawn Lin          2024-08-28  128  	 *   1.3, 2.6, ..., 88.6. 89.8, ..., 114.0, 90
59903441f5e49d Shawn Lin          2024-08-28  129  	 *
59903441f5e49d Shawn Lin          2024-08-28  130  	 * It's possible we might make a delay that is up to 25
59903441f5e49d Shawn Lin          2024-08-28  131  	 * degrees off from what we think we're making.  That's OK
59903441f5e49d Shawn Lin          2024-08-28  132  	 * though because we should be REALLY far from any bad range.
59903441f5e49d Shawn Lin          2024-08-28  133  	 */
59903441f5e49d Shawn Lin          2024-08-28  134  
59903441f5e49d Shawn Lin          2024-08-28  135  	/*
59903441f5e49d Shawn Lin          2024-08-28  136  	 * Convert to delay; do a little extra work to make sure we
59903441f5e49d Shawn Lin          2024-08-28  137  	 * don't overflow 32-bit / 64-bit numbers.
59903441f5e49d Shawn Lin          2024-08-28  138  	 */
59903441f5e49d Shawn Lin          2024-08-28  139  	delay = 10000000; /* PSECS_PER_SEC / 10000 / 10 */
59903441f5e49d Shawn Lin          2024-08-28  140  	delay *= remainder;
59903441f5e49d Shawn Lin          2024-08-28  141  	delay = DIV_ROUND_CLOSEST(delay,
59903441f5e49d Shawn Lin          2024-08-28  142  			(rate / 1000) * 36 *
59903441f5e49d Shawn Lin          2024-08-28  143  				(ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10));
59903441f5e49d Shawn Lin          2024-08-28  144  
59903441f5e49d Shawn Lin          2024-08-28  145  	delay_num = (u8) min_t(u32, delay, 255);
59903441f5e49d Shawn Lin          2024-08-28  146  
59903441f5e49d Shawn Lin          2024-08-28  147  	raw_value = delay_num ? ROCKCHIP_MMC_DELAY_SEL : 0;
59903441f5e49d Shawn Lin          2024-08-28  148  	raw_value |= delay_num << ROCKCHIP_MMC_DELAYNUM_OFFSET;
59903441f5e49d Shawn Lin          2024-08-28  149  	raw_value |= nineties;
59903441f5e49d Shawn Lin          2024-08-28  150  
59903441f5e49d Shawn Lin          2024-08-28  151  	if (sample)
47975a878c0669 Nicolas Frattaroli 2025-08-25 @152  		mci_writel(host, TIMING_CON1,
47975a878c0669 Nicolas Frattaroli 2025-08-25  153  			   FIELD_PREP_WM16(GENMASK(11, 1), raw_value));
59903441f5e49d Shawn Lin          2024-08-28  154  	else
47975a878c0669 Nicolas Frattaroli 2025-08-25  155  		mci_writel(host, TIMING_CON0,
47975a878c0669 Nicolas Frattaroli 2025-08-25  156  			   FIELD_PREP_WM16(GENMASK(11, 1), raw_value));
59903441f5e49d Shawn Lin          2024-08-28  157  
59903441f5e49d Shawn Lin          2024-08-28  158  	dev_dbg(host->dev, "set %s_phase(%d) delay_nums=%u actual_degrees=%d\n",
59903441f5e49d Shawn Lin          2024-08-28  159  		sample ? "sample" : "drv", degrees, delay_num,
59903441f5e49d Shawn Lin          2024-08-28  160  		rockchip_mmc_get_phase(host, sample)
59903441f5e49d Shawn Lin          2024-08-28  161  	);
59903441f5e49d Shawn Lin          2024-08-28  162  
59903441f5e49d Shawn Lin          2024-08-28  163  	return 0;
59903441f5e49d Shawn Lin          2024-08-28  164  }
59903441f5e49d Shawn Lin          2024-08-28  165  

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

Re: [PATCH 3/9] bitmap: Use FIELD_PREP() in expansion of FIELD_PREP_WM16()

[David Laight]

On Mon, 8 Dec 2025 19:54:37 -0500
Yury Norov <yury.norov@gmail.com> wrote:

> + Nicolas Frattaroli <nicolas.frattaroli@collabora.com> 
> 
> It's always good to CC an author of the original implementation,
> isn't?
> 
> On Mon, Dec 08, 2025 at 10:42:44PM +0000, 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()  
> 
> That's fair.
> 
> > and add an extra check for
> > the mask being at most 16 bits.  
> 
> Maybe it's time to introduce FIELD_PREP16()?
> 
> > 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;				\  
> 
> Is __auto_type any better than typeof()?

There is likely to be pressure for a global change soon.
6.19 contains '#define auto __auto_type' to match C23.

> 
> > +	u32 _val = FIELD_PREP(_mask, val);			\
> > +	BUILD_BUG_ON_MSG(_mask > 0xffffu,			\
> > +			 "FIELD_PREP_WM16: mask too large");	\  
> 
> Not necessary to split this line.

Only because I removed a level on indentation.

> 
> > +	_val | (_mask << 16);					\
> > +})  
> 
> Can you share bloat-o-meter and code generation examples?
> 
> For the next version please try to keep as much history
> untouched as possible.

Even the minimal changes (not moving the continuation markers) change pretty
much all the lines.
So It really isn't worth trying to keep it.

	David

Re: [PATCH 6/9] bitfield: Update sanity checks

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence static check first_new_problem: drivers/pci/controller/pcie-rockchip.c:158:16: sparse: sparse: __auto_type on self-init var"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251209100313.2867-7-david.laight.linux@gmail.com>
References: <20251209100313.2867-7-david.laight.linux@gmail.com>
TO: david.laight.linux@gmail.com
TO: Yury Norov <yury.norov@gmail.com>
TO: Rasmus Villemoes <linux@rasmusvillemoes.dk>
TO: linux-kernel@vger.kernel.org
TO: linux-usb@vger.kernel.org
TO: Geert Uytterhoeven <geert+renesas@glider.be>
TO: Alexandre Belloni <alexandre.belloni@bootlin.com>
TO: Jonathan Cameron <Jonathan.Cameron@huawei.com>
TO: Crt Mori <cmo@melexis.com>
TO: Richard Genoud <richard.genoud@bootlin.com>
TO: Andy Shevchenko <andriy.shevchenko@intel.com>
TO: Luo Jie <quic_luoj@quicinc.com>
TO: Peter Zijlstra <peterz@infradead.org>
TO: Jakub Kicinski <kuba@kernel.org>
TO: netdev@vger.kernel.org
TO: "David S . Miller" <davem@davemloft.net>
TO: Simon Horman <simon.horman@netronome.com>
TO: Mika Westerberg <mika.westerberg@linux.intel.com>
TO: Andreas Noever <andreas.noever@gmail.com>
TO: Yehezkel Bernat <YehezkelShB@gmail.com>
TO: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
CC: David Laight <david.laight.linux@gmail.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20251212]
[cannot apply to westeri-thunderbolt/next v6.18]
[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/20251209-181248
base:   linus/master
patch link:    https://lore.kernel.org/r/20251209100313.2867-7-david.laight.linux%40gmail.com
patch subject: [PATCH 6/9] bitfield: Update sanity checks
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: um-randconfig-r132-20251212 (https://download.01.org/0day-ci/archive/20251212/202512121232.PvAIXM4T-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251212/202512121232.PvAIXM4T-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/r/202512121232.PvAIXM4T-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/pcie-rockchip.c:158:16: sparse: sparse: __auto_type on self-init var
>> drivers/pci/controller/pcie-rockchip.c:158:16: sparse: sparse: incompatible types in conditional expression (different base types):
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    unsigned int
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    bad type
>> drivers/pci/controller/pcie-rockchip.c:158:16: sparse: sparse: incompatible type for operation (-):
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    bad type _mask
>> drivers/pci/controller/pcie-rockchip.c:158:16: sparse: sparse: incompatible types for operation (>>):
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    bad type _mask
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    int
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse: sparse: incompatible types for operation (<<):
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    bad type _val
   drivers/pci/controller/pcie-rockchip.c:158:16: sparse:    int

vim +158 drivers/pci/controller/pcie-rockchip.c

9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  114  
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  115  int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  116  {
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  117  	struct device *dev = rockchip->dev;
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  118  	int err, i;
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  119  	u32 regs;
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  120  
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  121  	err = reset_control_bulk_assert(ROCKCHIP_NUM_PM_RSTS,
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  122  					rockchip->pm_rsts);
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  123  	if (err)
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  124  		return dev_err_probe(dev, err, "Couldn't assert PM resets\n");
31a3a7b5b26f75 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-11-10  125  
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  126  	for (i = 0; i < MAX_LANE_NUM; i++) {
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  127  		err = phy_init(rockchip->phys[i]);
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  128  		if (err) {
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  129  			dev_err(dev, "init phy%d err %d\n", i, err);
8c595dd1fd97d8 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-23  130  			goto err_exit_phy;
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  131  		}
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  132  	}
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  133  
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  134  	err = reset_control_bulk_assert(ROCKCHIP_NUM_CORE_RSTS,
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  135  					rockchip->core_rsts);
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  136  	if (err) {
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  137  		dev_err_probe(dev, err, "Couldn't assert Core resets\n");
8c595dd1fd97d8 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-23  138  		goto err_exit_phy;
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  139  	}
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  140  
0722bdd2962a4a drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-11-24  141  	udelay(10);
0722bdd2962a4a drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-11-24  142  
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  143  	err = reset_control_bulk_deassert(ROCKCHIP_NUM_PM_RSTS,
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  144  					  rockchip->pm_rsts);
0722bdd2962a4a drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-11-24  145  	if (err) {
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  146  		dev_err(dev, "Couldn't deassert PM resets %d\n", err);
8c595dd1fd97d8 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-23  147  		goto err_exit_phy;
0722bdd2962a4a drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-11-24  148  	}
0722bdd2962a4a drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-11-24  149  
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  150  	if (rockchip->link_gen == 2)
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  151  		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_2,
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  152  				    PCIE_CLIENT_CONFIG);
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  153  	else
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  154  		rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1,
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  155  				    PCIE_CLIENT_CONFIG);
f2fb5b8f2a0cb6 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-12-07  156  
091022f5f94513 drivers/pci/controller/pcie-rockchip.c Damien Le Moal   2024-10-17  157  	regs = PCIE_CLIENT_ARI_ENABLE |
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09 @158  	       PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes);
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  159  
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  160  	if (rockchip->is_rc)
091022f5f94513 drivers/pci/controller/pcie-rockchip.c Damien Le Moal   2024-10-17  161  		regs |= PCIE_CLIENT_LINK_TRAIN_ENABLE |
091022f5f94513 drivers/pci/controller/pcie-rockchip.c Damien Le Moal   2024-10-17  162  			PCIE_CLIENT_CONF_ENABLE | PCIE_CLIENT_MODE_RC;
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  163  	else
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  164  		regs |= PCIE_CLIENT_CONF_DISABLE | PCIE_CLIENT_MODE_EP;
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  165  
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  166  	rockchip_pcie_write(rockchip, regs, PCIE_CLIENT_CONFIG);
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  167  
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  168  	for (i = 0; i < MAX_LANE_NUM; i++) {
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  169  		err = phy_power_on(rockchip->phys[i]);
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  170  		if (err) {
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  171  			dev_err(dev, "power on phy%d err %d\n", i, err);
8c595dd1fd97d8 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-23  172  			goto err_power_off_phy;
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  173  		}
9e87240c460637 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-25  174  	}
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  175  
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  176  	err = readx_poll_timeout(rockchip_pcie_read_addr,
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  177  				 PCIE_CLIENT_SIDE_BAND_STATUS,
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  178  				 regs, !(regs & PCIE_CLIENT_PHY_ST),
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  179  				 RK_PHY_PLL_LOCK_SLEEP_US,
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  180  				 RK_PHY_PLL_LOCK_TIMEOUT_US);
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  181  	if (err) {
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  182  		dev_err(dev, "PHY PLLs could not lock, %d\n", err);
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  183  		goto err_power_off_phy;
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  184  	}
9dd3c7c4c8c3f7 drivers/pci/controller/pcie-rockchip.c Rick Wertenbroek 2023-04-18  185  
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  186  	err = reset_control_bulk_deassert(ROCKCHIP_NUM_CORE_RSTS,
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  187  					  rockchip->core_rsts);
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  188  	if (err) {
18715931a5c08d drivers/pci/controller/pcie-rockchip.c Anand Moon       2024-12-02  189  		dev_err(dev, "Couldn't deassert Core reset %d\n", err);
8c595dd1fd97d8 drivers/pci/host/pcie-rockchip.c       Shawn Lin        2017-08-23  190  		goto err_power_off_phy;
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  191  	}
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  192  
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  193  	return 0;
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  194  err_power_off_phy:
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  195  	while (i--)
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  196  		phy_power_off(rockchip->phys[i]);
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  197  	i = MAX_LANE_NUM;
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  198  err_exit_phy:
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  199  	while (i--)
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  200  		phy_exit(rockchip->phys[i]);
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  201  	return err;
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  202  }
3593709f2651da drivers/pci/host/pcie-rockchip.c       Shawn Lin        2018-05-09  203  EXPORT_SYMBOL_GPL(rockchip_pcie_init_port);
e77f847df54c6b drivers/pci/host/pcie-rockchip.c       Shawn Lin        2016-09-03  204  

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

Re: [PATCH 6/9] bitfield: Update sanity checks

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence static check first_new_problem: drivers/iommu/riscv/iommu.c:1596:13: sparse: sparse: trying to concatenate 15816-character string (8191 bytes max)"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251209100313.2867-7-david.laight.linux@gmail.com>
References: <20251209100313.2867-7-david.laight.linux@gmail.com>
TO: david.laight.linux@gmail.com
TO: Yury Norov <yury.norov@gmail.com>
TO: Rasmus Villemoes <linux@rasmusvillemoes.dk>
TO: linux-kernel@vger.kernel.org
TO: linux-usb@vger.kernel.org
TO: Geert Uytterhoeven <geert+renesas@glider.be>
TO: Alexandre Belloni <alexandre.belloni@bootlin.com>
TO: Jonathan Cameron <Jonathan.Cameron@huawei.com>
TO: Crt Mori <cmo@melexis.com>
TO: Richard Genoud <richard.genoud@bootlin.com>
TO: Andy Shevchenko <andriy.shevchenko@intel.com>
TO: Luo Jie <quic_luoj@quicinc.com>
TO: Peter Zijlstra <peterz@infradead.org>
TO: Jakub Kicinski <kuba@kernel.org>
TO: netdev@vger.kernel.org
TO: "David S . Miller" <davem@davemloft.net>
TO: Simon Horman <simon.horman@netronome.com>
TO: Mika Westerberg <mika.westerberg@linux.intel.com>
TO: Andreas Noever <andreas.noever@gmail.com>
TO: Yehezkel Bernat <YehezkelShB@gmail.com>
TO: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
CC: David Laight <david.laight.linux@gmail.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20251212]
[cannot apply to westeri-thunderbolt/next v6.18]
[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/20251209-181248
base:   linus/master
patch link:    https://lore.kernel.org/r/20251209100313.2867-7-david.laight.linux%40gmail.com
patch subject: [PATCH 6/9] bitfield: Update sanity checks
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: riscv-randconfig-r113-20251212 (https://download.01.org/0day-ci/archive/20251213/202512130436.cX0B0hF7-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 1335a05ab8bc8339ce24be3a9da89d8c3f4e0571)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251213/202512130436.cX0B0hF7-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/r/202512130436.cX0B0hF7-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   WARNING: invalid argument to '-march': '_zacas_zabha'
>> drivers/iommu/riscv/iommu.c:1596:13: sparse: sparse: trying to concatenate 15816-character string (8191 bytes max)
   drivers/iommu/riscv/iommu.c:163:29: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *base @@     got void [noderef] __iomem * @@
   drivers/iommu/riscv/iommu.c:163:29: sparse:     expected void *base
   drivers/iommu/riscv/iommu.c:163:29: sparse:     got void [noderef] __iomem *
>> drivers/iommu/riscv/iommu.c:697:41: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected unsigned long long [usertype] *ddt_root @@     got void [noderef] __iomem * @@
   drivers/iommu/riscv/iommu.c:697:41: sparse:     expected unsigned long long [usertype] *ddt_root
   drivers/iommu/riscv/iommu.c:697:41: sparse:     got void [noderef] __iomem *

vim +1596 drivers/iommu/riscv/iommu.c

822e8bc68505820 Tomasz Jeznach 2024-10-15  1550  
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1551  static int riscv_iommu_init_check(struct riscv_iommu_device *iommu)
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1552  {
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1553  	u64 ddtp;
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1554  
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1555  	/*
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1556  	 * Make sure the IOMMU is switched off or in pass-through mode during
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1557  	 * regular boot flow and disable translation when we boot into a kexec
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1558  	 * kernel and the previous kernel left them enabled.
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1559  	 */
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1560  	ddtp = riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1561  	if (ddtp & RISCV_IOMMU_DDTP_BUSY)
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1562  		return -EBUSY;
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1563  
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1564  	if (FIELD_GET(RISCV_IOMMU_DDTP_IOMMU_MODE, ddtp) >
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1565  	     RISCV_IOMMU_DDTP_IOMMU_MODE_BARE) {
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1566  		if (!is_kdump_kernel())
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1567  			return -EBUSY;
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1568  		riscv_iommu_disable(iommu);
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1569  	}
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1570  
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1571  	/* Configure accesses to in-memory data structures for CPU-native byte order. */
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1572  	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) !=
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1573  	    !!(iommu->fctl & RISCV_IOMMU_FCTL_BE)) {
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1574  		if (!(iommu->caps & RISCV_IOMMU_CAPABILITIES_END))
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1575  			return -EINVAL;
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1576  		riscv_iommu_writel(iommu, RISCV_IOMMU_REG_FCTL,
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1577  				   iommu->fctl ^ RISCV_IOMMU_FCTL_BE);
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1578  		iommu->fctl = riscv_iommu_readl(iommu, RISCV_IOMMU_REG_FCTL);
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1579  		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) !=
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1580  		    !!(iommu->fctl & RISCV_IOMMU_FCTL_BE))
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1581  			return -EINVAL;
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1582  	}
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1583  
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1584  	/*
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1585  	 * Distribute interrupt vectors, always use first vector for CIV.
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1586  	 * At least one interrupt is required. Read back and verify.
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1587  	 */
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1588  	if (!iommu->irqs_count)
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1589  		return -EINVAL;
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1590  
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1591  	iommu->icvec = FIELD_PREP(RISCV_IOMMU_ICVEC_FIV, 1 % iommu->irqs_count) |
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1592  		       FIELD_PREP(RISCV_IOMMU_ICVEC_PIV, 2 % iommu->irqs_count) |
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1593  		       FIELD_PREP(RISCV_IOMMU_ICVEC_PMIV, 3 % iommu->irqs_count);
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1594  	riscv_iommu_writeq(iommu, RISCV_IOMMU_REG_ICVEC, iommu->icvec);
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1595  	iommu->icvec = riscv_iommu_readq(iommu, RISCV_IOMMU_REG_ICVEC);
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15 @1596  	if (max(max(FIELD_GET(RISCV_IOMMU_ICVEC_CIV, iommu->icvec),
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1597  		    FIELD_GET(RISCV_IOMMU_ICVEC_FIV, iommu->icvec)),
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1598  		max(FIELD_GET(RISCV_IOMMU_ICVEC_PIV, iommu->icvec),
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1599  		    FIELD_GET(RISCV_IOMMU_ICVEC_PMIV, iommu->icvec))) >= iommu->irqs_count)
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1600  		return -EINVAL;
856c0cfe5c5f6a2 Tomasz Jeznach 2024-10-15  1601  
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1602  	return 0;
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1603  }
5c0ebbd3c6c6e00 Tomasz Jeznach 2024-10-15  1604  

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