Re: [PATCH v2 08/10] gpu: nova-core: convert falcon registers to kernel register macro

[Danilo Krummrich <dakr@kernel.org>]

On 3/20/26 8:52 PM, John Hubbard wrote:
> On 3/20/26 10:38 AM, Joel Fernandes wrote:
>> Hi Alex,
>>
>> On 3/20/2026 8:19 AM, Alexandre Courbot wrote:
>>>      /// Reset the controller, select the falcon core, and wait for memory scrubbing to complete.
>>> @@ -616,9 +462,10 @@ pub(crate) fn reset(&self, bar: &Bar0) -> Result {
>>>          self.hal.select_core(self, bar)?;
>>>          self.hal.reset_wait_mem_scrubbing(bar)?;
>>>  
>>> -        regs::NV_PFALCON_FALCON_RM::default()
>>> -            .set_value(bar.read(regs::NV_PMC_BOOT_0).into())
>>> -            .write(bar, &E::ID);
>>> +        bar.write(
>>> +            WithBase::of::<E>(),
>>> +            regs::NV_PFALCON_FALCON_RM::from(bar.read(regs::NV_PMC_BOOT_0).into_raw()),
>>> +        );
>>>  
>>
>> Overall, I think the series is good improvement but I still feel this part is a
>> step back in readability, and we should come up with something better. I don't
>> think there's any chance anyone can memorize this syntax.
> 
> I must reluctantly (because I know this conversation has gone very
> long, across so many versions) agree. That .write() statement is just
> brutal, and we will be relying on AI in order to even figure it out,
> I fear.
> 
> I'd hoped that there were other, simpler forms to express this,
> is that not the case here?

I mean, it is barely different from the above, except for the
WithBase::of::<E>() argument instead of &E::ID.

It becomes much more readable if you break it down (which is what we should
probably do):

	let boot0 = bar.read(regs::NV_PMC_BOOT_0).into_raw();

	bar.write(WithBase::of::<E>(), regs::NV_PFALCON_FALCON_RM::from(boot0));

For other cases where we read a register and write the same register we will
have a way in the future that can omit WithBase::of::<E>() entirely, see also my
reply in [1].

I want to expand write_reg() to also be able to handle variable offset
registers, e.g.:

	let reg = bar.read_reg(regs::NV_PFALCON_FALCON_RM::of::<E>());

	// modify reg

	bar.write_reg(reg);

[1] https://lore.kernel.org/all/DH0XBLXZD81K.22SWIZ1ZAOW1@kernel.org/