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

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

On Mon Mar 9, 2026 at 6:34 PM CET, John Hubbard wrote:
> On 3/9/26 8:43 AM, 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.
>
> Yes, I think so too, except that "WithBase::of::<E>" makes a lot more
> sense on-screen, than "&E::ID". The latter just looks quite random,
> so this part is an improvement.
>
> Let's break down the remaining troublesome part a bit:
>
>     regs::NV_PFALCON_FALCON_MAILBOX1::zeroed().with_value(mbox1)
>
> * "regs::" can be omitted with a "use" statement, right?
>
> * ::zeroed() maybe should be the default behavior here, and then
>   it could also be omitted?
>
> * .with_value(mbox1) I'm sure this is necessary, but the construction
>   is unfortunately much less clear than .write(value)! Thoughts?

I think this could also just be

	bar.write(regs::NV_PFALCON_FALCON_MAILBOX1::of::<E>(), mbox1);

as we would want it for FIFO-like registers.