Re: [PATCH 00/21] lib: add alternatives for GENMASK()

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

On Thursday, 27 November 2025 00:47:28 Central European Standard Time Linus Torvalds wrote:
> On Wed, 26 Nov 2025 at 14:17, david laight <david.laight@runbox.com> wrote:
> >
> > Mark B. will accuse you of abusing ?: :-)
> 
> Compared to '__is_constexpr()', this is child's play.  Not *that* is
> abusing the ternary op.
> 
> > I've just looked at a .i file.
> > GENMASK() currently expands to 855 chars plus four copies of each argument.
> 
> Yeah, I don't love it. It's a horrid macro. But because of the odd
> order of arguments, it needs all that crazy checking.
> 
> That said, I do think it could be simplified. Particularly if we just
> make the rule be that GENMASK _has_ to take just constant values.
> 
> Right now, I think 99.9% of all users are constants, and we spend a
> lot of effort on the 0.1% that isn't.
> 
>                Linus
> 

As a datapoint from my hw_bitfield.h series, all but 3 drivers were
trivial to convert to using constant bit masks, which is required by
both variants of the FIELD_PREP_WM16* macro.

One of the big outliers was the Rockchip clock driver, which needed
non-const things all over the place, and wouldn't have been that
easy to refactor.

Another one worth calling out is phy-rockchip-inno-csidphy.c, where
the non-constness of the mask was inherited from a pointer dereference
of what is a const value, but the indirection makes it non-const,
and I didn't want to have to deal with that 20 patches in without a
good way to test it. Entirely possible I was a doofus and didn't try
making the pointer itself const as well.

Occasionally, masks in code are non-constant for frivolous reasons,
like using function arguments as the mask because they're assumed
to be always called with one of a set of known values, but that set
is never made obvious unless you check all the call sites. The
spicier variant of that is masks read from hardware registers or
firmware, because at that point you have no view into the
possibilities, but I've yet to come across this.

I think enforcing GENMASK to take constant values is a good idea in
light of that, because people who really need a non-const version
should make this explicit so that reviewers can catch it as a
potential code smell, in addition to the less horrid preprocessor
output.

Kind regards,
Nicolas Frattaroli