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

[Gary Guo]

On Wed Mar 4, 2026 at 9:38 PM GMT, Danilo Krummrich wrote:
> 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())

This specific case is indeed more cumbersome with the two argument approach,
although given Alex's nova diff I think the occurance shouldn't be that
frequent.

It's also not that the two argument approach would preclude us from having a
single argument option. In fact, with the two-argument design as the basis, we
can implement such a helper function cleaner than Alex's PATCH 10/10 (which uses
`Into<IoWrite>`:

    /// Indicates that this type is always associated with a specific fixed I/O
    /// location.
    ///
    /// This allows use of `io.bikeshed_shorthand_name(value)` instead of specifying
    /// the register name explicitly `io.write(REG, value)`.
    trait FixedIoLocation {
        type IoLocType: IoLoc<Self>;
        const IO_LOCATION: Self::IoLocType;
    }

    trait Io {
        fn bikeshed_shorthand_name<T>(&self, value: T)
            where T: FixedIoLocation +
                  Self: IoCapable<<T::IoLocType as IoLoc<T>>::IoType>,
        {
            self.write(T::IO_LOCATION, value)
        }
    }

No need for a `IoWrite` type, everything is done via traits.

Anyhow I don't think we should close door for a more universal approach because
a specific use case is shorter to write.

Best,
Gary

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