[PATCH v2 08/10] gpu: nova-core: convert falcon registers to kernel register macro
Danilo Krummrich
dakr at kernel.org
Fri Mar 20 13:07:38 PDT 2026
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/
More information about the linux-riscv
mailing list