netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: david.laight.linux@gmail.com
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Crt Mori <cmo@melexis.com>,
	Richard Genoud <richard.genoud@bootlin.com>,
	Andy Shevchenko <andriy.shevchenko@intel.com>,
	Luo Jie <quic_luoj@quicinc.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andreas Noever <andreas.noever@gmail.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Subject: Re: [PATCH v2 02/16] thunderbolt: Don't pass a bitfield to FIELD_GET
Date: Fri, 12 Dec 2025 21:28:31 -0500	[thread overview]
Message-ID: <aTzPT2kAt96ypGU-@yury> (raw)
In-Reply-To: <20251212193721.740055-3-david.laight.linux@gmail.com>

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

  reply	other threads:[~2025-12-13  2:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-12 19:37 [PATCH v2 0/16] bitfield: tidy up bitfield.h david.laight.linux
2025-12-12 19:37 ` [PATCH v2 01/16] nfp: Call FIELD_PREP() in NFP_ETH_SET_BIT_CONFIG() wrapper david.laight.linux
2025-12-12 23:10   ` Jakub Kicinski
2025-12-12 19:37 ` [PATCH v2 02/16] thunderbolt: Don't pass a bitfield to FIELD_GET david.laight.linux
2025-12-13  2:28   ` Yury Norov [this message]
2025-12-13 10:01     ` David Laight
2025-12-13 22:14       ` David Laight
2025-12-12 19:37 ` [PATCH v2 03/16] bitmap: Use FIELD_PREP() in expansion of FIELD_PREP_WM16() david.laight.linux
2025-12-17 13:22   ` Nicolas Frattaroli
2025-12-18  0:16     ` Yury Norov
2025-12-18  8:44       ` David Laight
2025-12-19 13:11       ` Nicolas Frattaroli
2025-12-12 19:37 ` [PATCH v2 04/16] bitfield: Copy #define parameters to locals david.laight.linux
2025-12-12 19:37 ` [PATCH v2 05/16] bitfield: Merge __field_prep/get() into field_prep/get() david.laight.linux
2025-12-12 19:37 ` [PATCH v2 06/16] bitfield: Remove some pointless casts david.laight.linux
2025-12-12 19:37 ` [PATCH v2 07/16] bitfield: FIELD_MODIFY: Only do a single read/write on the target david.laight.linux
2025-12-12 19:37 ` [PATCH v2 08/16] bitfield: Simplify __BF_FIELD_CHECK_REG() david.laight.linux
2025-12-17 10:26   ` Jonathan Cameron
2025-12-17 22:31     ` David Laight
2025-12-18  0:10       ` Yury Norov
2025-12-28 18:53       ` Andy Shevchenko
2025-12-28 22:50         ` David Laight
2025-12-12 19:37 ` [PATCH v2 09/16] bitfield: Rename __FIELD_PREP/GET() to __BF_FIELD_PREP/GET() david.laight.linux
2025-12-12 19:37 ` [PATCH v2 10/16] bitfield: Split the 'val' check out of __BF_FIELD_CHECK_MASK() david.laight.linux
2025-12-12 19:37 ` [PATCH v2 11/16] bitfield: Common up validation of the mask parameter david.laight.linux
2025-12-14  6:19   ` kernel test robot
2025-12-14 13:17     ` David Laight
2025-12-12 19:37 ` [PATCH v2 12/16] bitfield: Remove leading _ from #define formal parameter names david.laight.linux
2025-12-12 19:37 ` [PATCH v2 13/16] bitfield: Reduce indentation david.laight.linux
2025-12-12 19:37 ` [PATCH v2 14/16] bitfield: Add comment block for the host/fixed endian functions david.laight.linux
2025-12-12 19:37 ` [PATCH v2 15/16] bitfield: Update comments for le/be functions david.laight.linux
2025-12-12 19:37 ` [PATCH v2 16/16] build_bug.h; Remove __BUILD_BUG_ON_NOT_POWER_OF_2() david.laight.linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aTzPT2kAt96ypGU-@yury \
    --to=yury.norov@gmail.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=YehezkelShB@gmail.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andreas.noever@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=cmo@melexis.com \
    --cc=davem@davemloft.net \
    --cc=david.laight.linux@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mika.westerberg@linux.intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.frattaroli@collabora.com \
    --cc=peterz@infradead.org \
    --cc=quic_luoj@quicinc.com \
    --cc=richard.genoud@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).