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

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

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

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

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

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

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

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

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

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

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

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

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

Thanks,
Yury

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

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

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

	David

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

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

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

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

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

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

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

	David

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