Nouveau - Oct 2025 - [PATCH v7.1 2/4] gpu: nova-core: bitfield: Move bitfield-specific code from register! into new macro

On Thu Oct 16, 2025 at 9:28 PM CEST, Joel Fernandes
wrote:
>> On Oct 16, 2025, at 1:48?PM, Yury Norov <yury.norov at gmail.com>
wrote:
>> ?On Thu, Oct 16, 2025 at 11:13:21AM -0400, Joel Fernandes wrote:
>>> +///
>>> +/// bitfield! {
>>> +///     struct ControlReg {
>>> +///         3:0 mode as u8 ?=> Mode;
>>> +///         7:7 state as bool => State;
>>> +///     }
>>> +/// }
>> 
>> This notation is really unwelcome this days. It may be OK for a random
>> macro in some local driver, but doesn't really work for a global
basic
>> data type:
>> 
>>
https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ
at mail.gmail.com/
>> 
>> I've already shared this link with you, and shared my concern.
>> 
>> I realize that rust/bitfield derives the GENMASK(hi, lo) notation here,
>> and GENMASK() derives verilog or hardware specs popular notations. But
>> software people prefer lo:hi. I'm probably OK if you choose C-style
>> start:nbits, if you prefer. But let's stop this hi:lo early,
please.
>> 
>> Let me quote Linus from the link above:
>> 
>>  It does "high, low", which is often very unintuitive, and in
fact the
>>  very commit that introduced this thing from hell had to convert the
>>  sane "low,high" cases to the other way around.
>
> I agree with Linus but I disagree with comparing it with these macros.
> I agree with Linus it is oddly unreadable when used as function parameters.
> But that is a different syntax. Over here we are using colons with
sufficient whitespace around hi:lo.
I agree with Joel here.

While I'm not super opinionated for general bitfields, for the register!()
infrastructure I very much prefer the hi:lo notation, as this is the common
notation in datasheets and TRMs.

However, if we use hi:lo, we should use it decending, i.e.:

	bitfield! {
	    struct ControlReg {
	        7:5 state as u8 => State;
	        3:0 mode as u8 ?=> Mode;
	    }
	}

- Danilo

On 10/16/25 12:34 PM, Danilo Krummrich wrote:
> On Thu Oct 16, 2025 at 9:28 PM CEST, Joel Fernandes wrote:
>>> On Oct 16, 2025, at 1:48?PM, Yury Norov <yury.norov at
gmail.com> wrote:
>>> ?On Thu, Oct 16, 2025 at 11:13:21AM -0400, Joel Fernandes wrote:
...
> While I'm not super opinionated for general bitfields, for the
register!()
> infrastructure I very much prefer the hi:lo notation, as this is the common
> notation in datasheets and TRMs.
> 
> However, if we use hi:lo, we should use it decending, i.e.:
> 
Sure, descending works.
> 	bitfield! {
> 	    struct ControlReg {
> 	        7:5 state as u8 => State;
> 	        3:0 mode as u8 ?=> Mode;
And hi:lo matches our HW reference manuals. And if you're dealing
with bitfields, you are often also dealing with HW, so this is
a reasonable place in the SW to use hi:lo.

Looks good to me.


thanks,
-- 
John Hubbard

> On Oct 16, 2025, at 3:34?PM, Danilo Krummrich <dakr at kernel.org>
wrote:
> 
> ?On Thu Oct 16, 2025 at 9:28 PM CEST, Joel Fernandes wrote:
>>>> On Oct 16, 2025, at 1:48?PM, Yury Norov <yury.norov at
gmail.com> wrote:
>>> ?On Thu, Oct 16, 2025 at 11:13:21AM -0400, Joel Fernandes wrote:
>>>> +///
>>>> +/// bitfield! {
>>>> +///     struct ControlReg {
>>>> +///         3:0 mode as u8 ?=> Mode;
>>>> +///         7:7 state as bool => State;
>>>> +///     }
>>>> +/// }
>>> 
>>> This notation is really unwelcome this days. It may be OK for a
random
>>> macro in some local driver, but doesn't really work for a
global basic
>>> data type:
>>> 
>>>
https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ
at mail.gmail.com/
>>> 
>>> I've already shared this link with you, and shared my concern.
>>> 
>>> I realize that rust/bitfield derives the GENMASK(hi, lo) notation
here,
>>> and GENMASK() derives verilog or hardware specs popular notations.
But
>>> software people prefer lo:hi. I'm probably OK if you choose
C-style
>>> start:nbits, if you prefer. But let's stop this hi:lo early,
please.
>>> 
>>> Let me quote Linus from the link above:
>>> 
>>> It does "high, low", which is often very unintuitive, and
in fact the
>>> very commit that introduced this thing from hell had to convert the
>>> sane "low,high" cases to the other way around.
>> 
>> I agree with Linus but I disagree with comparing it with these macros.
>> I agree with Linus it is oddly unreadable when used as function
parameters.
>> But that is a different syntax. Over here we are using colons with
sufficient whitespace around hi:lo.
> 
> I agree with Joel here.
> 
> While I'm not super opinionated for general bitfields, for the
register!()
> infrastructure I very much prefer the hi:lo notation, as this is the common
> notation in datasheets and TRMs.
> 
> However, if we use hi:lo, we should use it decending, i.e.:
> 
>    bitfield! {
>        struct ControlReg {
>            7:5 state as u8 => State;
>            3:0 mode as u8 ?=> Mode;
>        }
>    }
Makes sense. The macro currently supports both. I will fix the example docs.

thanks,

 - Joel

> 
> - Danilo