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

["Gary Guo" <gary@garyguo.net>]

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.
>
> 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)

Best,
Gary

>
> Another example is that if there're multiple registers where fields have the
> same meaning (e.g. a device address), then a user might want to just have the
> same type for all these values. We've already have an example of register arrays
> that share types, and I see no reason to forbid non-array registers from also
> sharing types.
>
> Forcing all registers to have different types is a design direction that I don't
> want to take.
>
> Best,
> Gary
>
>>
>> So, unless there is a strong argument why this improves anything on the user
>> side of the API, I still feel like we should keep write() as is with a single
>> argument; having an additional write_with() method seems fine though.