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

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

On Wed Mar 4, 2026 at 8:37 PM GMT, Danilo Krummrich wrote:
> On Wed Mar 4, 2026 at 8:48 PM CET, Gary Guo wrote:
>> On Wed Mar 4, 2026 at 7:38 PM GMT, Danilo Krummrich wrote:
>>> 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.
>>
>> It's not what the macro do today, but I don't want to ask Alex to change it
>> further before landing the series. I do think it's a worthy follow-up to add the
>> ability to decouple the location and type. It's not incompatible with current
>> design anyway.
>
> I'm not sure there are any relevant use-cases for this. Do you have real
> examples that would not be represented with array registers?

Even for the cases where there's a PIO register, I think it's beneficial to just
get a value without a type.

I don't see why we want people to write

    self.io.read(UART_RX).value()

vs

    self.io.read(UART_RX)

or

    self.io.write(UART_TX::from(byte))

vs

    self.io.write(UART_TX, byte)

what benefit does additional type provide?

>
> Otherwise, I think this disconnect has no advantages. Actually, I think it may
> even be the opposite, as it would allow for confusing the location.

The two ways of writing has the exact same possibility of location confusion.
It's just one uses comma and another uses `::from()`. Arguably the comma form is
better because you always spell out the location, *and* it's still type checked.

The comma one is more C like, less typing, and is more clear visually. The one
operand approach is just aesthetically bad.

I also have to say that I am not a big fan of adding additional types when
things can be just done with constant values. That's more work for compiler and
code monomorphization in general, without significant added value.

>
>>> 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))
>>
>> This won't work for the array case, right? For array you'd have
>>
>>     bar.write(ARRAY.try_at(idx).ok_or(EINVAL)?.set(Reg::from(value)))
>>
>> and now the register name is repeating twice rather than just
>>
>>     bar.write(ARRAY.try_at(idx).ok_or(EINVAL)?, value)
>
> We should be able to just do
>
> 	bar.write(ARRAY.try_at(idx).ok_or(EINVAL)?.set(value.into()))

Sure, more typing, more parenthesis nesting, and more cluttered visually. I
don't understand why is this preferrable.

The `io.write(loc, value)` approach is very simple to understand also very
straightforward to implement. `loc` is just an offset that also restricts the
type, `value` is a typed value, that can be bitfields or some other used-defined
types.

Want things to be just integers? Sure, you can have it. Want everything to be
fully typed? Sure, you can have it with bitfields. Custom types with custom
conversion logic? Implement your own conversion with a `IoCapable` primitive and
you can have it too.

I don't see a need to have a more restrictive API, and also have something that
is less clear visually, and, most important to me, ugly.

Best,
Gary