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

From: Danilo Krummrich

Date: Wed Mar 04 2026 - 16:38:51 EST


On Wed Mar 4, 2026 at 10:13 PM CET, Gary Guo wrote:
> 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?

Well, for FIFO registers this is indeed better. However, my main concern was
this

bar.write(regs::MyReg, regs::MyReg::foo())

for the reasons explained above.

As for FIFO registers, another option could be to leverage the raw accessors and
make a location type dereferece to the offset, e.g.

bar.write8(regs::TX_FIFO, byte)

The same goes for the array case of course.