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

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

On Fri Mar 6, 2026 at 11:10 AM GMT, Alexandre Courbot wrote:
> On Fri Mar 6, 2026 at 7:42 PM JST, Gary Guo wrote:
>> On Fri Mar 6, 2026 at 5:37 AM GMT, Alexandre Courbot wrote:
>>> On Thu Mar 5, 2026 at 7:15 AM JST, Gary Guo wrote:
>>>> 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:
>>>>>> 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.
>>>
>>> That's cool but will only work for fixed registers. If you work with, say, an
>>> array of registers, cannot implement this trait on a value as the value
>>> doesn't have an index assigned - meaning you would have to build a
>>> location in addition of it.
>>
>> For array registers I think it makes more sense to use the two-argument version,
>> no?
>>
>> The example here is to demonstrate that we can add a shorthand version for the
>> fixed register version that can write a value to register without mentioning its
>> name (as a supplemental helper), and the basic write method is the two-argument
>> one.
>>
>> For cases where the type doesn't guarantee a fixed location like FIFO register
>> or an array register, mentioning the name twice is fine.
>
> It's still tedious, and a step back compared to the one-argument version
> imho.
>
>>
>> [
>>
>>     For array case, you *could* also do
>>
>>     impl IoLoc<RegisterName> for usize {
>>         fn offset(self) -> usize {
>>             self * stride + fixed_base
>>         }
>>     }
>>
>>
>>     and now you can do `self.write(index, reg_value)`, although I think this
>>     might confuse some people.
>
> Yes, in this case the semantics of write's first argument would be
> dependent on the second argument... I think that's a potential footgun.

I mean, `bar.write(Reg::at(10, regs::MyRegArray::foo()))` in your example is
also kind of "first argument depends on the second argument" situation, just
with a bit more boilerplate.

If you want to make things more explicit you could also have
`bar.write(at_array(10), ...)` or something similar.

For the array case I really think trying to shove everything into a single
argument is a footgun. The type of value in this case *doesn't* tell us the
location, and the location needs to be explicit.

Best,
Gary

>
>>
>>     For the fixed case you could do
>>
>>     impl IoLoc<RegisterName> for () {
>>         fn offset(self) -> usize {
>>             fixed_loc
>>         }
>>     }
>>
>>     which means you can do `self.write((), value)`. I think this looks a bit
>>     uglier compared to a dedicated method, but TBH it isn't too terrible.
>
> There is another potential solution I have played with:
>
>     https://lore.kernel.org/all/DGVJ7VQX3TD5.2UYW004QJPI6N@nvidia.com/
>
> It would let us keep the 2-arguments as a base, while letting us take
> advantage of the one-argument syntax you proposed in [1] and extending
> it to support other register types.
>
> [1] https://lore.kernel.org/all/DGU9AZ43QK6Y.115RDSK0M9JY5@garyguo.net/
>
> I think it turns out pretty nice and the supporting code is also not too
> ugly. I'll try to send a diff a bit later so you can see how it behaves
> in practice.