Re: [PATCH FOR REFERENCE v8 10/10] gpu: nova-core: use the kernel `register!` macro

[Alexandre Courbot]

On Tue Mar 10, 2026 at 12:43 AM JST, Joel Fernandes wrote:
>
> On 3/9/2026 11:14 AM, Alexandre Courbot wrote:
>>
>> @@ -797,26 +792,30 @@ pub(crate) fn start(&self, bar: &Bar0) -> Result<()> {
>>      /// Writes values to the mailbox registers if provided.
>>      pub(crate) fn write_mailboxes(&self, bar: &Bar0, mbox0: Option<u32>, mbox1: Option<u32>) {
>>          if let Some(mbox0) = mbox0 {
>> -            regs::NV_PFALCON_FALCON_MAILBOX0::default()
>> -                .set_value(mbox0)
>> -                .write(bar, &E::ID);
>> +            bar.write(
>> +                WithBase::of::<E>(),
>> +                regs::NV_PFALCON_FALCON_MAILBOX0::zeroed().with_value(mbox0),
>> +            );
>>          }
>>  
>>          if let Some(mbox1) = mbox1 {
>> -            regs::NV_PFALCON_FALCON_MAILBOX1::default()
>> -                .set_value(mbox1)
>> -                .write(bar, &E::ID);
>> +            bar.write(
>> +                WithBase::of::<E>(),
>> +                regs::NV_PFALCON_FALCON_MAILBOX1::zeroed().with_value(mbox1),
>> +            );
>>          }
>>      }
>
> TBH, this is quite a readability hit, the previous was much more readable IMHO.
>
> Why doesn't the previous caller-side syntax still work?

Looking down the discussion is looks like almost everyone found a way to
work with this API that they like, but here are the reasons for the
record.

The previous caller-side syntax had many problems:

* Every register must have its own ~6 (try_)read/write/update methods
generated by the macro, creating a rift in how I/O is used only for
registers.
* The number of arguments these methods take is dependent on the
register type, which is awkward.
* The I/O type is passed as an argument, meaning we cannot use
Deref coercion on it, further complicating things when we have more than
one layer of indirection. For instance, it didn't work well with both
Nova and the Rust PCI driver sample (one of them had to `&` or `*` its
I/O type with every call).

With the current revision, there is only one site where I/O methods are
defined (io.rs), to which registers cleanly integrate like any other I/O
type (i.e. registers are not "special"). Deref coercion on the `Io` type
can be used without limitation. There is much less code generated by the
register macro, which has become much simpler as a result.

This syntax might require one more line here and there, but after
working with it a bit I think it is actually much more readable and
well-formatted than before. It makes it clear from the get go what kind
of register you are accessing, and we *do* want to be meticulous when
working with registers.

It looks better both on the inside and the outside, and I hold that view
not as an opinion, but an objective fact. :)