linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Yury Norov <yury.norov@gmail.com>,
	Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org,  linux-crypto@vger.kernel.org,
	qat-linux@intel.com, linux-gpio@vger.kernel.org,
		linux-aspeed@lists.ozlabs.org, linux-iio@vger.kernel.org,
		linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
	Michael Turquette	 <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Nicolas Ferre	 <nicolas.ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	 Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	Giovanni Cabiddu <giovanni.cabiddu@intel.com>,
	Herbert Xu	 <herbert@gondor.apana.org.au>,
	"David S . Miller" <davem@davemloft.net>,
	 Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Joel Stanley <joel@jms.id.au>,
	 Andrew Jeffery <andrew@codeconstruct.com.au>,
	Crt Mori <cmo@melexis.com>, Jonathan Cameron <jic23@kernel.org>,
	 Lars-Peter Clausen	 <lars@metafoo.de>,
	Jacky Huang <ychuang3@nuvoton.com>,
	Shan-Chun Hung	 <schung@nuvoton.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Jakub Kicinski <kuba@kernel.org>, Alex Elder <elder@ieee.org>
Subject: Re: [PATCH treewide v2 1/3] bitfield: Add non-constant field_{prep,get}() helpers
Date: Mon, 03 Feb 2025 08:44:06 +0100	[thread overview]
Message-ID: <74cab7d1ec31e7531cdda0f1eb47acdebd5c8d3f.camel@sipsolutions.net> (raw)
In-Reply-To: <Z5-xMUqrDuaE8Eo_@thinkpad>

On Sun, 2025-02-02 at 12:53 -0500, Yury Norov wrote:
> 
> > Instead of creating another variant for
> > non-constant bitfields, wouldn't it be better to make the existing macro
> > accept both?
> 
> Yes, it would definitely be better IMO.

On the flip side, there have been discussions in the past (though I
think not all, if any, on the list(s)) about the argument order. Since
the value is typically not a constant, requiring the mask to be a
constant has ensured that the argument order isn't as easily mixed up as
otherwise.

With a non-constant mask there can also be no validation that the mask
is contiguous etc.

Now that doesn't imply a strong objection - personally I've come to
prefer the lower-case typed versions anyway - but something to keep in
mind when doing this.

However, the suggested change to BUILD_BUG_ON_NOT_POWER_OF_2 almost
certainly shouldn't be done for the same reason - not compiling for non-
constant values is [IMHO] part of the API contract for that macro. This
can be important for the same reasons.

(Obviously, doing that change now doesn't invalidate existing code, but
it does remove checks that may have been intended to be present in the
code.)

johannes

  reply	other threads:[~2025-02-03  7:44 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-31 13:46 [PATCH v2 0/3] Non-const bitfield helpers Geert Uytterhoeven
2025-01-31 13:46 ` [PATCH treewide v2 1/3] bitfield: Add non-constant field_{prep,get}() helpers Geert Uytterhoeven
2025-01-31 16:29   ` Alexandre Belloni
2025-01-31 16:32   ` Jonathan Cameron
2025-01-31 19:03   ` David Laight
2025-02-14 10:28     ` Geert Uytterhoeven
2025-02-02  8:26   ` Vincent Mailhol
2025-02-02 17:53     ` Yury Norov
2025-02-03  7:44       ` Johannes Berg [this message]
2025-02-03 13:36         ` Vincent Mailhol
2025-02-03 13:59           ` Geert Uytterhoeven
2025-02-03 15:41             ` Vincent Mailhol
2025-02-03 16:48               ` Yury Norov
2025-02-14 11:03                 ` Geert Uytterhoeven
2025-02-14 14:39                   ` Yury Norov
2025-02-03 15:31           ` Johannes Berg
2025-02-04 15:30     ` Jakub Kicinski
2025-02-14 11:00       ` Geert Uytterhoeven
2025-01-31 13:46 ` [PATCH v2 2/3] clk: renesas: Use bitfield helpers Geert Uytterhoeven
2025-01-31 13:46 ` [PATCH v2 3/3] soc: " Geert Uytterhoeven

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=74cab7d1ec31e7531cdda0f1eb47acdebd5c8d3f.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@codeconstruct.com.au \
    --cc=brgl@bgdev.pl \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=cmo@melexis.com \
    --cc=davem@davemloft.net \
    --cc=elder@ieee.org \
    --cc=geert+renesas@glider.be \
    --cc=giovanni.cabiddu@intel.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=jic23@kernel.org \
    --cc=joel@jms.id.au \
    --cc=kuba@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=mturquette@baylibre.com \
    --cc=nicolas.ferre@microchip.com \
    --cc=perex@perex.cz \
    --cc=qat-linux@intel.com \
    --cc=sboyd@kernel.org \
    --cc=schung@nuvoton.com \
    --cc=tiwai@suse.com \
    --cc=ychuang3@nuvoton.com \
    --cc=yury.norov@gmail.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).