Re: [PATCH v7 05/10] rust: io: add IoLoc and IoWrite types

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

On Wed Mar 4, 2026 at 7:58 PM CET, Gary Guo wrote:
> On Wed Mar 4, 2026 at 6:39 PM GMT, Gary Guo wrote:
>> On Wed Mar 4, 2026 at 4:18 PM GMT, Danilo Krummrich wrote:
>>> On Tue Mar 3, 2026 at 3:55 PM CET, Alexandre Courbot wrote:
>>>> So, to get a better idea of these two options I have converted this
>>>> patchset to use the 2-arguments `write_with` method. Here is the
>>>> difference between the two - it is particularly interesting to see how
>>>> nova-core changes:
>>>>
>>>> https://github.com/Gnurou/linux/compare/register_1arg..Gnurou:linux:register_2args
>>>
>>> This looks good to me, but the fact that this turns out nicely has nothing to do
>>> with write() now taking two arguments. I.e. there is no reason why we couldn't
>>> have the exact same write_with() method together with the single argument
>>> write() method.
>>>
>>> The contention point for me with a two arguments write() method still remains
>>> that the arguments are redundant.
>>>
>>> I.e. you first have the location in form of an object instance of a ZST (which
>>> in the end is just a "trick" to pass in the type itself) and then we have the
>>> object that actually represents the entire register, describing both the
>>> location *and* the value.
>>>
>>> So, let's say a driver creates a register object with a custom constructor
>>>
>>> 	let reset = regs::MyReg::reset();
>>>
>>> then the two argument approach would be
>>>
>>> 	(1) bar.write(regs::MyReg, regs::MyReg::reset());
>>>
>>> whereas the single argument approach would just be
>>>
>>> 	(2) bar.write(regs::MyReg::reset());
>>
>> That's only for bit field registers that has unique types. I still believe types
>> of registers should not be tightly coupled with name of registeres.
>>
>> Allowing a value of register to be directly used for `write` is also confusing
>> if a value is not created immediately before written to.
>>
>>>
>>> So, if I would have to write (1), I'd probably be tempted to implement a reset()
>>> function that takes the bar as argument to hide this, i.e.
>>>
>>> 	regs::MyReg::reset(bar);
>>>
>>> I also can't agree with the argument that the notation of write(loc, val) - or
>>> write(val, loc) as the C side does it - is common and we should stick to it.
>>>
>>> This notation is only common because it is necessary when operating on
>>> primitives or when the two representing types are discrete.
>>>
>>> But this isn't the case here, a register object is already distinct in terms of
>>> its location and value.
>>
>> I see no reason why register values for different locations have to be distinct
>> in terms of value types.

That's not what the register!() macro currently does, a register type always has
a unique location, or is an array register, etc. In any case a register type is
assoiciated with a location.

If the proposal is to disconnect location and register type entirely, that would
be a change to the current design.

If we'd have this clear separation, I would obviously not object to this change,
but currently it's just unnecessary redundancy.

>> Even Nova today has quite a few registers that are just bitfields of a single
>> field that spans all bits. I think many simple driver would probably want to
>> just operate on primitives for these.
>
> I shall add that I think the fact that the registers that are *not* fields still
> gain their dedicated type in Nova driver is due to the limitation of the initial
> `register!` API design that *requires* unique types due to the `value.op(io)`
> design as opposed to `io.op(value)`.
>
> I think even these ones should eventually be replaced by just primitives
> eventually. I see no benefit of
>
>     bar.write(REG.init(|x| x.with_value(value)))
>
> as opposed to just
>
>     bar.write(REG, value)

Well, you don't have to make that we have to use init() with a closure for such
cases. We can also do something like:

	bar.write(Reg::from(value))