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

["Danilo Krummrich" <dakr@kernel.org>]

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@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@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