* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
@ 2025-10-25 18:01 Alexey Dobriyan
0 siblings, 0 replies; 15+ messages in thread
From: Alexey Dobriyan @ 2025-10-25 18:01 UTC (permalink / raw)
To: Yury Norov (NVIDIA)
Cc: Linus Walleij, Nicolas Frattaroli, Rasmus Villemoes, linux-kernel
> GENMASK(high, low) notation reflects a common pattern to describe
> hardware registers. However, out of drivers context it's confusing and
> error-prone.
>
> This series introduces alternatives for GENMASK():
>
> - BITS(low, high);
This is less bad (low bit then high), but in a perfect world high bit
should have been exclusive.
> - FIRST_BITS(nbits);
> - LAST_BITS(start);
Now it is 200% more confusing.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 19:44 ` Linus Torvalds
2025-11-26 22:17 ` david laight
@ 2025-11-29 11:46 ` david laight
1 sibling, 0 replies; 15+ messages in thread
From: david laight @ 2025-11-29 11:46 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rasmus Villemoes, Yury Norov (NVIDIA),
Linus Walleij, Nicolas Frattaroli, linux-kernel
On Wed, 26 Nov 2025 11:44:59 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, 26 Nov 2025 at 01:07, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >
> > There is, however, an alternative that resembles the syntax in data
> > sheets even more closely:
> >
> > #define BITS(low_high) GENMASK((0 ? low_high), (1 ? low_high))
>
> Oh, me likey. That's a much better idea than my crazy syntax.
>
> Linus
>
I've had a couple of further thoughts...
1) GENMASK() could just swap the two arguments if they are constant and
in the wrong order, so GENMASK(2, 4) and GENMASK(4, 2) are equivalent.
Easily done without too much bloat in a statement expression.
2) It a lot of cases the result from GENMASK() is only ever passed to FIELD_XXX().
The latter really don't want the mask - they want the two bit numbers.
(In particular the low bit number.)
So you could change the headers to have (eg):
#define REGISTER_BITS_FOR_XXX (5:7)
Then the expected calling 'convention' would be FIELD_XXX((lo:hi), val).
All done with:
#define _BIT_LO(lo_hi) (1 ? lo_hi)
#define _BIT_HI(lo_hi) (0 ? lo_hi)
#define FIELD_XXX(lo_hi, ...) ({
u32 _lo = _BIT_LO lo_hi;
u32 _hi = _BIT_HI lo_hi;
...
Any attempted use without the 'magic macros' will be a nice syntax error.
David
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 22:17 ` david laight
2025-11-26 23:47 ` Linus Torvalds
@ 2025-11-27 18:51 ` david laight
1 sibling, 0 replies; 15+ messages in thread
From: david laight @ 2025-11-27 18:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rasmus Villemoes, Yury Norov (NVIDIA),
Linus Walleij, Nicolas Frattaroli, linux-kernel
On Wed, 26 Nov 2025 22:17:18 +0000
david laight <david.laight@runbox.com> wrote:
...
> Both FIELD_PREP/GET(GENMASK(), v) are about 18k plus three copies of v.
> FIELD_GET(mask,v) has 18 expansions of 'mask'.
All the FIELD_XXX() are already statement expressions.
I think that means they should 'just work' if the parameters are
copied to locals - annoyingly there are a few bitfields...
Even after that a shed-load (or two) of bloat comes from __BF_FIELD_CHECK().
Not helped by expanding pointless checks because the same define is
used lots of times.
I'm not really sure the expensive test that uses __bf_cast_unsigned()
is actually worth doing - unless a trivial equivalent can be found.
Actually the 'mask' side can just be 'mask + 0u + 0ul + 0ull' which
zero-extends 'mask';
the other just '~0ull >> (64 - sizeof(_reg) * 8)'.
I see a patch lurking...
David
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-27 14:11 ` Nicolas Frattaroli
@ 2025-11-27 17:29 ` Linus Torvalds
0 siblings, 0 replies; 15+ messages in thread
From: Linus Torvalds @ 2025-11-27 17:29 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: david laight, Rasmus Villemoes, Yury Norov (NVIDIA),
Linus Walleij, linux-kernel
On Thu, 27 Nov 2025 at 06:12, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> 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.
So I did some very preliminary checking, and we do end up having more
GENMASK() users with non-constant arguments than I thought.
They are still a small minority - about 3% of the users - which is why
my first grep made me think "it's all just constants".
It's just not quite as tiny a minority as I thought from a quick visual grep.
IOW, there's about a thousand cases of GENMASK() with things that
aren't obvious constants.
So trying to get rid of them would be somewhat painful. Fairly easily
automated, but still..
Linus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 23:47 ` Linus Torvalds
2025-11-27 9:37 ` david laight
@ 2025-11-27 14:11 ` Nicolas Frattaroli
2025-11-27 17:29 ` Linus Torvalds
1 sibling, 1 reply; 15+ messages in thread
From: Nicolas Frattaroli @ 2025-11-27 14:11 UTC (permalink / raw)
To: david laight, Linus Torvalds
Cc: Rasmus Villemoes, Yury Norov (NVIDIA), Linus Walleij, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 23:47 ` Linus Torvalds
@ 2025-11-27 9:37 ` david laight
2025-11-27 14:11 ` Nicolas Frattaroli
1 sibling, 0 replies; 15+ messages in thread
From: david laight @ 2025-11-27 9:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rasmus Villemoes, Yury Norov (NVIDIA),
Linus Walleij, Nicolas Frattaroli, linux-kernel
On Wed, 26 Nov 2025 15:47:28 -0800
Linus Torvalds <torvalds@linux-foundation.org> 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.
There is the other 0.1% that actually need an 'integer constant expression'.
They stop you using ({ ...}).
I doubt there are many (if any) for FIELD_PREP() and FIELD_GET() and using
auto _mask = mask;
would shrink those massively.
(If there are any I suspect you are the only person who can fix them
in one release cycle.)
I've not looked closely at the expansions, but as well as some is_constexpr()
(that could probably be 'statically_true()') there is the _Generic() that
generates u8/u16/u32/u64 based on 'something'.
That is mostly entirely pointless, you just need to pick u32 or u64, the u8/u16
get promoted to 'int' as soon as they are used - so it only makes a difference
to code that looks at the type of the variable/result.
In spite of the faffing, the type of FIELD_GET() is u64.
David
>
> Linus
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 22:17 ` david laight
@ 2025-11-26 23:47 ` Linus Torvalds
2025-11-27 9:37 ` david laight
2025-11-27 14:11 ` Nicolas Frattaroli
2025-11-27 18:51 ` david laight
1 sibling, 2 replies; 15+ messages in thread
From: Linus Torvalds @ 2025-11-26 23:47 UTC (permalink / raw)
To: david laight
Cc: Rasmus Villemoes, Yury Norov (NVIDIA),
Linus Walleij, Nicolas Frattaroli, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 19:44 ` Linus Torvalds
@ 2025-11-26 22:17 ` david laight
2025-11-26 23:47 ` Linus Torvalds
2025-11-27 18:51 ` david laight
2025-11-29 11:46 ` david laight
1 sibling, 2 replies; 15+ messages in thread
From: david laight @ 2025-11-26 22:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rasmus Villemoes, Yury Norov (NVIDIA),
Linus Walleij, Nicolas Frattaroli, linux-kernel
On Wed, 26 Nov 2025 11:44:59 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, 26 Nov 2025 at 01:07, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >
> > There is, however, an alternative that resembles the syntax in data
> > sheets even more closely:
> >
> > #define BITS(low_high) GENMASK((0 ? low_high), (1 ? low_high))
>
> Oh, me likey. That's a much better idea than my crazy syntax.
Mark B. will accuse you of abusing ?: :-)
I've just looked at a .i file.
GENMASK() currently expands to 855 chars plus four copies of each argument.
Both FIELD_PREP/GET(GENMASK(), v) are about 18k plus three copies of v.
FIELD_GET(mask,v) has 18 expansions of 'mask'.
HWEIHT32(GENMASK(15,0)) is about 30k - don't ask why anyone would do it.
They all look excessive.
David
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 9:07 ` Rasmus Villemoes
2025-11-26 10:38 ` david laight
@ 2025-11-26 19:44 ` Linus Torvalds
2025-11-26 22:17 ` david laight
2025-11-29 11:46 ` david laight
1 sibling, 2 replies; 15+ messages in thread
From: Linus Torvalds @ 2025-11-26 19:44 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Yury Norov (NVIDIA), Linus Walleij, Nicolas Frattaroli, linux-kernel
On Wed, 26 Nov 2025 at 01:07, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> There is, however, an alternative that resembles the syntax in data
> sheets even more closely:
>
> #define BITS(low_high) GENMASK((0 ? low_high), (1 ? low_high))
Oh, me likey. That's a much better idea than my crazy syntax.
Linus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-11-26 9:07 ` Rasmus Villemoes
@ 2025-11-26 10:38 ` david laight
2025-11-26 19:44 ` Linus Torvalds
1 sibling, 0 replies; 15+ messages in thread
From: david laight @ 2025-11-26 10:38 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Linus Torvalds, Yury Norov (NVIDIA),
Linus Walleij, Nicolas Frattaroli, linux-kernel
On Wed, 26 Nov 2025 10:07:34 +0100
Rasmus Villemoes <ravi@prevas.dk> wrote:
> On Sat, Oct 25 2025, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> > On Sat, 25 Oct 2025 at 09:40, Yury Norov (NVIDIA) <yury.norov@gmail.com> wrote:
> >>
> >> GENMASK(high, low) notation reflects a common pattern to describe
> >> hardware registers. However, out of drivers context it's confusing and
> >> error-prone.
> >
> > So I obviously approve of the BITS() syntax, and am not a huge fan of
> > the odd "GENMASK()" argument ordering.
> >
> > That said, I'm not convinced about adding the other helpers. I don't
> > think "FIRST_BITS(8)" is any more readable than "BITS(0,7)", and I
> > think there's a real danger to having lots of specialized macros that
> > people then have to know what they mean.
> >
> > I have an absolutely *disgusting* trick to use the syntax
> >
> > BITS(3 ... 25)
> >
> > to do this all, but it's so disgusting and limited that I don't think
> > it's actually reasonable.
> >
> > In case somebody can come up with a cleaner model, here's the trick:
> >
> > #define LOWRANGE_0 0,
> > #define LOWRANGE_1 1,
> > #define LOWRANGE_2 2,
> > #define LOWRANGE_3 3,
> > #define LOWRANGE_4 4,
> > #define LOWRANGE_5 5,
> > // ..and so on
> >
> > #define _HIGH_VAL(x) (sizeof((char[]){[x]=1})-1)
> > #define __FIRST(x, ...) x
> > #define ___LOW_VAL(x) __FIRST(x)
> > #define __LOW_VAL(x) ___LOW_VAL(LOWRANGE_ ##x)
> > #define _LOW_VAL(x) __LOW_VAL(x)
> >
> > #define BITS(x) GENMASK(_HIGH_VAL(x), _LOW_VAL(x))
> >
> > #define TESTVAL 5
> > unsigned long val1 = BITS(3 ... 25);
> > unsigned long val2 = BITS(4);
> > unsigned long val3 = BITS(TESTVAL ... 31);
> >
> > and that syntax with either "3 ... 25" or just a plain "4" does
> > actually work. But only with fairly simple numbers.
> >
> > It doesn't work with more complex expressions (due to the nasty
> > preprocessor pasting hack), and I couldn't figure out a way to make it
> > do so.
>
> It's cute, but no, I also don't know how to make it work without the
> preprocessor concatenation stuff.
>
> There is, however, an alternative that resembles the syntax in data
> sheets even more closely:
>
> #define BITS(low_high) GENMASK((0 ? low_high), (1 ? low_high))
That is both beautiful and horrid...
>
> That'll work for
>
> unsigned long val1 = BITS(3:25);
> unsigned long val3 = BITS(TESTVAL:31);
>
> for most of the things TESTVAL might expand to - I'm not sure what would
> happen if one of the lo/hi values contains ternaries and isn't properly
> parenthesized, but nobody writes stuff like
>
> #define RESET_BIT(rev) rev == 0xaa ? 7 : 9
>
> It doesn't work for the single-bit case, but I don't think it's so bad
> to have to say
>
> unsigned long val2 = BIT(4);
>
> and obviously BITS(4:4) would work as well.
>
> It also doesn't do anything to prevent the hi-lo 11:7 order.
For constants that gets trapped.
If the correct expression in used in GENMASK() the compiler will reject
the negative shift - so doesn't need an explicit test.
Not directly related...
GENMASK() and the FIELD_GET() family need to go on massive diets.
Nest them and the pre-processor generates multi-kb lines.
Somewhere there is a (probably pointless) _Generic() for all the integer types.
(Especially since FIELD_GET() is always u64.)
FIELD_GET(mask, val) is just (val & mask) / (mask & -mask), but even that
really needs mask assigned to a local (just to reduce the line length).
For variable 'mask' you might worry about the cost of the 'divide by power of 2'
but for constants it doesn't matter.
FIELD_PREP(mask, val) is (val * (mask & -mask)) & mask
'Bloat city' comes from:
#define MASK GENMASK(9, 0);
x = FIELD_GET(MASK, MASK);
I've just looked at bitfield.h - no wonder it bloats the world.
95% of it needs deleting :-)
David
>
> Rasmus
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-10-25 17:56 ` Linus Torvalds
@ 2025-11-26 9:07 ` Rasmus Villemoes
2025-11-26 10:38 ` david laight
2025-11-26 19:44 ` Linus Torvalds
0 siblings, 2 replies; 15+ messages in thread
From: Rasmus Villemoes @ 2025-11-26 9:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: Yury Norov (NVIDIA), Linus Walleij, Nicolas Frattaroli, linux-kernel
On Sat, Oct 25 2025, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sat, 25 Oct 2025 at 09:40, Yury Norov (NVIDIA) <yury.norov@gmail.com> wrote:
>>
>> GENMASK(high, low) notation reflects a common pattern to describe
>> hardware registers. However, out of drivers context it's confusing and
>> error-prone.
>
> So I obviously approve of the BITS() syntax, and am not a huge fan of
> the odd "GENMASK()" argument ordering.
>
> That said, I'm not convinced about adding the other helpers. I don't
> think "FIRST_BITS(8)" is any more readable than "BITS(0,7)", and I
> think there's a real danger to having lots of specialized macros that
> people then have to know what they mean.
>
> I have an absolutely *disgusting* trick to use the syntax
>
> BITS(3 ... 25)
>
> to do this all, but it's so disgusting and limited that I don't think
> it's actually reasonable.
>
> In case somebody can come up with a cleaner model, here's the trick:
>
> #define LOWRANGE_0 0,
> #define LOWRANGE_1 1,
> #define LOWRANGE_2 2,
> #define LOWRANGE_3 3,
> #define LOWRANGE_4 4,
> #define LOWRANGE_5 5,
> // ..and so on
>
> #define _HIGH_VAL(x) (sizeof((char[]){[x]=1})-1)
> #define __FIRST(x, ...) x
> #define ___LOW_VAL(x) __FIRST(x)
> #define __LOW_VAL(x) ___LOW_VAL(LOWRANGE_ ##x)
> #define _LOW_VAL(x) __LOW_VAL(x)
>
> #define BITS(x) GENMASK(_HIGH_VAL(x), _LOW_VAL(x))
>
> #define TESTVAL 5
> unsigned long val1 = BITS(3 ... 25);
> unsigned long val2 = BITS(4);
> unsigned long val3 = BITS(TESTVAL ... 31);
>
> and that syntax with either "3 ... 25" or just a plain "4" does
> actually work. But only with fairly simple numbers.
>
> It doesn't work with more complex expressions (due to the nasty
> preprocessor pasting hack), and I couldn't figure out a way to make it
> do so.
It's cute, but no, I also don't know how to make it work without the
preprocessor concatenation stuff.
There is, however, an alternative that resembles the syntax in data
sheets even more closely:
#define BITS(low_high) GENMASK((0 ? low_high), (1 ? low_high))
That'll work for
unsigned long val1 = BITS(3:25);
unsigned long val3 = BITS(TESTVAL:31);
for most of the things TESTVAL might expand to - I'm not sure what would
happen if one of the lo/hi values contains ternaries and isn't properly
parenthesized, but nobody writes stuff like
#define RESET_BIT(rev) rev == 0xaa ? 7 : 9
It doesn't work for the single-bit case, but I don't think it's so bad
to have to say
unsigned long val2 = BIT(4);
and obviously BITS(4:4) would work as well.
It also doesn't do anything to prevent the hi-lo 11:7 order.
Rasmus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-10-25 16:39 Yury Norov (NVIDIA)
@ 2025-10-25 17:56 ` Linus Torvalds
2025-11-26 9:07 ` Rasmus Villemoes
0 siblings, 1 reply; 15+ messages in thread
From: Linus Torvalds @ 2025-10-25 17:56 UTC (permalink / raw)
To: Yury Norov (NVIDIA)
Cc: Linus Walleij, Nicolas Frattaroli, Rasmus Villemoes, linux-kernel
On Sat, 25 Oct 2025 at 09:40, Yury Norov (NVIDIA) <yury.norov@gmail.com> wrote:
>
> GENMASK(high, low) notation reflects a common pattern to describe
> hardware registers. However, out of drivers context it's confusing and
> error-prone.
So I obviously approve of the BITS() syntax, and am not a huge fan of
the odd "GENMASK()" argument ordering.
That said, I'm not convinced about adding the other helpers. I don't
think "FIRST_BITS(8)" is any more readable than "BITS(0,7)", and I
think there's a real danger to having lots of specialized macros that
people then have to know what they mean.
I have an absolutely *disgusting* trick to use the syntax
BITS(3 ... 25)
to do this all, but it's so disgusting and limited that I don't think
it's actually reasonable.
In case somebody can come up with a cleaner model, here's the trick:
#define LOWRANGE_0 0,
#define LOWRANGE_1 1,
#define LOWRANGE_2 2,
#define LOWRANGE_3 3,
#define LOWRANGE_4 4,
#define LOWRANGE_5 5,
// ..and so on
#define _HIGH_VAL(x) (sizeof((char[]){[x]=1})-1)
#define __FIRST(x, ...) x
#define ___LOW_VAL(x) __FIRST(x)
#define __LOW_VAL(x) ___LOW_VAL(LOWRANGE_ ##x)
#define _LOW_VAL(x) __LOW_VAL(x)
#define BITS(x) GENMASK(_HIGH_VAL(x), _LOW_VAL(x))
#define TESTVAL 5
unsigned long val1 = BITS(3 ... 25);
unsigned long val2 = BITS(4);
unsigned long val3 = BITS(TESTVAL ... 31);
and that syntax with either "3 ... 25" or just a plain "4" does
actually work. But only with fairly simple numbers.
It doesn't work with more complex expressions (due to the nasty
preprocessor pasting hack), and I couldn't figure out a way to make it
do so.
I also suspect that we shouldn't do the core code conversions without
having acks from maintainers. Some people may prefer the odd
"high,low" ordering due to it matching some equally odd hardware
documentation.
Linus
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 00/21] lib: add alternatives for GENMASK()
@ 2025-10-25 16:39 Yury Norov (NVIDIA)
2025-10-25 17:56 ` Linus Torvalds
0 siblings, 1 reply; 15+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:39 UTC (permalink / raw)
To: Linus Torvalds, Linus Walleij, Nicolas Frattaroli
Cc: Yury Norov (NVIDIA), Rasmus Villemoes, linux-kernel
GENMASK(high, low) notation reflects a common pattern to describe
hardware registers. However, out of drivers context it's confusing and
error-prone.
This series introduces alternatives for GENMASK():
- BITS(low, high);
- FIRST_BITS(nbits);
- LAST_BITS(start);
Patches 1-6 and 8 rename existing local BITS(). Patches 7 and 9 introduce
new generic macros. Patches 10-18 switch GENMASK() usage in core files to
better alternatives. Patch 19 adds Documentation section describing
preferred parameters ordering.
The series is inspired by:
https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
Yury Norov (NVIDIA) (21):
arc: disasm: rename BITS() for FIELD()
iwlwifi: drop unused BITS()
select: rename BITS() to FDS_BITS()
ALSA: rename BITS to R_BITS
zlib: BITS() to LOWBITS()
mfd: prepare to generalize BITS() macro
bits: Add BITS() macro
mfd: drop local BITS() macro
bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK
i2c: nomadik: don't use GENMASK()
drivers: don't use GENMASK() in FIELD_PREP_WM16()
bitmap: don't use GENMASK()
trace: don't use GENMASK()
lib: 842: don't use GENMASK_ULL()
bpf: don't use GENMASK()
kcsan: don't use GENMASK()
mm: don't use GENMASK()
fprobe: don't use GENMASK()
fs: don't use GENMASK()
fortify-string: don't use GENMASK()
Docs: add Functions parameters order section
Documentation/process/coding-style.rst | 48 ++++++++++++++
arch/arc/include/asm/disasm.h | 62 +++++++++----------
arch/arc/kernel/disasm.c | 46 +++++++-------
drivers/gpu/drm/rockchip/rockchip_lvds.h | 2 +-
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 4 +-
drivers/i2c/busses/i2c-nomadik.c | 44 ++++++-------
.../platform/synopsys/hdmirx/snps_hdmirx.h | 4 +-
drivers/mfd/db8500-prcmu-regs.h | 2 -
drivers/mmc/host/dw_mmc-rockchip.c | 4 +-
.../net/wireless/intel/iwlwifi/fw/api/coex.h | 2 -
drivers/soc/rockchip/grf.c | 4 +-
fs/erofs/internal.h | 2 +-
fs/f2fs/data.c | 2 +-
fs/f2fs/inode.c | 2 +-
fs/f2fs/segment.c | 2 +-
fs/f2fs/super.c | 2 +-
fs/proc/task_mmu.c | 2 +-
fs/resctrl/pseudo_lock.c | 2 +-
fs/select.c | 6 +-
include/asm-generic/fprobe.h | 7 +--
include/linux/bitmap.h | 11 ++--
include/linux/bits.h | 11 ++++
include/linux/f2fs_fs.h | 2 +-
include/linux/find.h | 34 +++++-----
include/linux/fortify-string.h | 4 +-
kernel/bpf/verifier.c | 4 +-
kernel/kcsan/encoding.h | 4 +-
kernel/trace/fgraph.c | 10 +--
kernel/trace/trace_probe.h | 2 +-
lib/842/842_compress.c | 2 +-
lib/842/842_decompress.c | 2 +-
lib/bitmap.c | 2 +-
lib/test_bitmap.c | 14 ++---
lib/zlib_inflate/inflate.c | 48 +++++++-------
mm/debug_vm_pgtable.c | 2 +-
sound/core/oss/rate.c | 12 ++--
sound/soc/rockchip/rockchip_i2s_tdm.h | 2 +-
37 files changed, 235 insertions(+), 180 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 00/21] lib: add alternatives for GENMASK()
2025-10-25 16:28 Yury Norov (NVIDIA)
@ 2025-10-25 16:37 ` Yury Norov
0 siblings, 0 replies; 15+ messages in thread
From: Yury Norov @ 2025-10-25 16:37 UTC (permalink / raw)
To: Linus Torvalds, Linus Walleij, Nicolas Frattaroli
Cc: Rasmus Villemoes, linux-kernel
Please disregard it. The patches 8-21 have the TO list corrupted. I'll
resend shortly.
On Sat, Oct 25, 2025 at 12:28:36PM -0400, Yury Norov (NVIDIA) wrote:
> GENMASK(high, low) notation reflects a common pattern to describe
> hardware registers. However, out of drivers context it's confusing and
> error-prone.
>
> This series introduces alternatives for GENMASK():
>
> - BITS(low, high);
> - FIRST_BITS(nbits);
> - LAST_BITS(start);
>
> Patches 1-6 and 8 rename existing local BITS(). Patches 7 and 9 introduce
> new generic macros. Patches 10-18 switch GENMASK() usage in core files to
> better alternatives. Patch 19 adds Documentation section describing
> preferred parameters ordering.
>
> The series is inspired by:
>
> https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
>
> Yury Norov (NVIDIA) (21):
> arc: disasm: rename BITS() for FIELD()
> iwlwifi: drop unused BITS()
> select: rename BITS() to FDS_BITS()
> ALSA: rename BITS to R_BITS
> zlib: BITS() to LOWBITS()
> mfd: prepare to generalize BITS() macro
> bits: Add BITS() macro
> mfd: drop local BITS() macro
> bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK
> i2c: nomadik: don't use GENMASK()
> drivers: don't use GENMASK() in FIELD_PREP_WM16()
> bitmap: don't use GENMASK()
> trace: don't use GENMASK()
> lib: 842: don't use GENMASK_ULL()
> bpf: don't use GENMASK()
> kcsan: don't use GENMASK()
> mm: don't use GENMASK()
> fprobe: don't use GENMASK()
> fs: don't use GENMASK()
> fortify-string: don't use GENMASK()
> Docs: add Functions parameters order section
>
> Documentation/process/coding-style.rst | 48 ++++++++++++++
> arch/arc/include/asm/disasm.h | 62 +++++++++----------
> arch/arc/kernel/disasm.c | 46 +++++++-------
> drivers/gpu/drm/rockchip/rockchip_lvds.h | 2 +-
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 4 +-
> drivers/i2c/busses/i2c-nomadik.c | 44 ++++++-------
> .../platform/synopsys/hdmirx/snps_hdmirx.h | 4 +-
> drivers/mfd/db8500-prcmu-regs.h | 2 -
> drivers/mmc/host/dw_mmc-rockchip.c | 4 +-
> .../net/wireless/intel/iwlwifi/fw/api/coex.h | 2 -
> drivers/soc/rockchip/grf.c | 4 +-
> fs/erofs/internal.h | 2 +-
> fs/f2fs/data.c | 2 +-
> fs/f2fs/inode.c | 2 +-
> fs/f2fs/segment.c | 2 +-
> fs/f2fs/super.c | 2 +-
> fs/proc/task_mmu.c | 2 +-
> fs/resctrl/pseudo_lock.c | 2 +-
> fs/select.c | 6 +-
> include/asm-generic/fprobe.h | 7 +--
> include/linux/bitmap.h | 11 ++--
> include/linux/bits.h | 11 ++++
> include/linux/f2fs_fs.h | 2 +-
> include/linux/find.h | 34 +++++-----
> include/linux/fortify-string.h | 4 +-
> kernel/bpf/verifier.c | 4 +-
> kernel/kcsan/encoding.h | 4 +-
> kernel/trace/fgraph.c | 10 +--
> kernel/trace/trace_probe.h | 2 +-
> lib/842/842_compress.c | 2 +-
> lib/842/842_decompress.c | 2 +-
> lib/bitmap.c | 2 +-
> lib/test_bitmap.c | 14 ++---
> lib/zlib_inflate/inflate.c | 48 +++++++-------
> mm/debug_vm_pgtable.c | 2 +-
> sound/core/oss/rate.c | 12 ++--
> sound/soc/rockchip/rockchip_i2s_tdm.h | 2 +-
> 37 files changed, 235 insertions(+), 180 deletions(-)
>
> --
> 2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 00/21] lib: add alternatives for GENMASK()
@ 2025-10-25 16:28 Yury Norov (NVIDIA)
2025-10-25 16:37 ` Yury Norov
0 siblings, 1 reply; 15+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:28 UTC (permalink / raw)
To: Linus Torvalds, Linus Walleij, Nicolas Frattaroli
Cc: Yury Norov (NVIDIA), Rasmus Villemoes, linux-kernel
GENMASK(high, low) notation reflects a common pattern to describe
hardware registers. However, out of drivers context it's confusing and
error-prone.
This series introduces alternatives for GENMASK():
- BITS(low, high);
- FIRST_BITS(nbits);
- LAST_BITS(start);
Patches 1-6 and 8 rename existing local BITS(). Patches 7 and 9 introduce
new generic macros. Patches 10-18 switch GENMASK() usage in core files to
better alternatives. Patch 19 adds Documentation section describing
preferred parameters ordering.
The series is inspired by:
https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
Yury Norov (NVIDIA) (21):
arc: disasm: rename BITS() for FIELD()
iwlwifi: drop unused BITS()
select: rename BITS() to FDS_BITS()
ALSA: rename BITS to R_BITS
zlib: BITS() to LOWBITS()
mfd: prepare to generalize BITS() macro
bits: Add BITS() macro
mfd: drop local BITS() macro
bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK
i2c: nomadik: don't use GENMASK()
drivers: don't use GENMASK() in FIELD_PREP_WM16()
bitmap: don't use GENMASK()
trace: don't use GENMASK()
lib: 842: don't use GENMASK_ULL()
bpf: don't use GENMASK()
kcsan: don't use GENMASK()
mm: don't use GENMASK()
fprobe: don't use GENMASK()
fs: don't use GENMASK()
fortify-string: don't use GENMASK()
Docs: add Functions parameters order section
Documentation/process/coding-style.rst | 48 ++++++++++++++
arch/arc/include/asm/disasm.h | 62 +++++++++----------
arch/arc/kernel/disasm.c | 46 +++++++-------
drivers/gpu/drm/rockchip/rockchip_lvds.h | 2 +-
drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 4 +-
drivers/i2c/busses/i2c-nomadik.c | 44 ++++++-------
.../platform/synopsys/hdmirx/snps_hdmirx.h | 4 +-
drivers/mfd/db8500-prcmu-regs.h | 2 -
drivers/mmc/host/dw_mmc-rockchip.c | 4 +-
.../net/wireless/intel/iwlwifi/fw/api/coex.h | 2 -
drivers/soc/rockchip/grf.c | 4 +-
fs/erofs/internal.h | 2 +-
fs/f2fs/data.c | 2 +-
fs/f2fs/inode.c | 2 +-
fs/f2fs/segment.c | 2 +-
fs/f2fs/super.c | 2 +-
fs/proc/task_mmu.c | 2 +-
fs/resctrl/pseudo_lock.c | 2 +-
fs/select.c | 6 +-
include/asm-generic/fprobe.h | 7 +--
include/linux/bitmap.h | 11 ++--
include/linux/bits.h | 11 ++++
include/linux/f2fs_fs.h | 2 +-
include/linux/find.h | 34 +++++-----
include/linux/fortify-string.h | 4 +-
kernel/bpf/verifier.c | 4 +-
kernel/kcsan/encoding.h | 4 +-
kernel/trace/fgraph.c | 10 +--
kernel/trace/trace_probe.h | 2 +-
lib/842/842_compress.c | 2 +-
lib/842/842_decompress.c | 2 +-
lib/bitmap.c | 2 +-
lib/test_bitmap.c | 14 ++---
lib/zlib_inflate/inflate.c | 48 +++++++-------
mm/debug_vm_pgtable.c | 2 +-
sound/core/oss/rate.c | 12 ++--
sound/soc/rockchip/rockchip_i2s_tdm.h | 2 +-
37 files changed, 235 insertions(+), 180 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-11-29 11:46 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-25 18:01 [PATCH 00/21] lib: add alternatives for GENMASK() Alexey Dobriyan
-- strict thread matches above, loose matches on Subject: below --
2025-10-25 16:39 Yury Norov (NVIDIA)
2025-10-25 17:56 ` Linus Torvalds
2025-11-26 9:07 ` Rasmus Villemoes
2025-11-26 10:38 ` david laight
2025-11-26 19:44 ` Linus Torvalds
2025-11-26 22:17 ` david laight
2025-11-26 23:47 ` Linus Torvalds
2025-11-27 9:37 ` david laight
2025-11-27 14:11 ` Nicolas Frattaroli
2025-11-27 17:29 ` Linus Torvalds
2025-11-27 18:51 ` david laight
2025-11-29 11:46 ` david laight
2025-10-25 16:28 Yury Norov (NVIDIA)
2025-10-25 16:37 ` Yury Norov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®