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

["Alexandre Courbot" <acourbot@nvidia.com>]

On Mon Mar 2, 2026 at 12:11 AM JST, Gary Guo wrote:
> On Sat Feb 28, 2026 at 12:33 AM GMT, Alexandre Courbot wrote:
>> On Sat Feb 28, 2026 at 3:02 AM JST, Gary Guo wrote:
>> <snip>
>>>> +/// A pending I/O write operation, bundling a value with the [`IoLoc`] it should be written to.
>>>> +///
>>>> +/// Created by [`IoLoc::set`], [`IoLoc::zeroed`], [`IoLoc::default`], [`IoLoc::init`], or
>>>> +/// [`IoLoc::init_default`], and consumed by [`Io::write`] or [`Io::try_write`] to perform the
>>>> +/// actual write.
>>>> +///
>>>> +/// The value can be modified before writing using [`IoWrite::update`] or [`IoWrite::try_update`],
>>>> +/// enabling a builder pattern:
>>>> +///
>>>> +/// ```ignore
>>>> +/// io.write(REGISTER.init(|v| v.with_field(x)));
>>>> +/// ```
>>>
>>> Thinking about this again, I think we might still want to write
>>>
>>>     io.write(REGISTER, value)
>>
>> This was the original design, but as you point out below this makes the
>> very common case of writing a register value built from scratch more
>> verbose than it needs. Real-world examples are significantly worse, e.g:
>>
>>     bar.write(
>>     regs::NV_PFALCON_FALCON_DMATRFMOFFS::of::<E>()
>>         .try_init(|r| r.try_with_offs(load_offsets.dst_start + pos))?,
>>     );
>
> My main dissatisfaction with this is with the function call
>
> I wonder if we can just have
>
>     io.write(loc, value)
>     io.write_with(loc, updater)
>
> where the latter simply is a function that does
>
>     io.write(loc, updater(T::zeroed()))
>
> then the example would be
>
>     bar.write_with(
>         regs::NV_PFALCON_FALCON_DMATRFMOFFS::of::<E>(),
>         |r| r.try_with_offs(load_offsets.dst_start + pos)
>     )

That should be doable. Note that we currently support `zeroed` and
`default` as initializers, so having the same level of coverage would
require two `write` variants. I'd like to hear what Danilo thinks.

>
> [ Note: it's possible to have write_with that works with both fallible and
> non-fallible callbacks. You can define a trait like a monad (well, not fully
> a monad, because this just has a map, not a return and a bind):
>
> trait Map<T> {
>     type Mapped<U>;
>     
>     fn map<U>(self, f: impl FnOnce(T) -> U) -> Self::Mapped<U>;
> }
>
> impl<T> Map<T> for T {
>     type Mapped<U> = U;
>     
>     fn map<U>(self, f: impl FnOnce(T) -> U) -> Self::Mapped<U> {
>         f(self)
>     }
> }
>
> impl<T, E> Map<T> for Result<T, E> {
>     type Mapped<U> = Result<U, E>;
>     
>     fn map<U>(self, f: impl FnOnce(T) -> U) -> Self::Mapped<U> {
>         Ok(f(self?))
>     }
> }
>
> impl Io {    
>     fn write_with<R: IoLoc<T>, U: Map<T>, T: Zeroable, F: FnOnce(T) -> U>(&self, loc: R, f: F) -> U::Mapped<()> {
>         f(T::zeroed).map(|x| self.write(loc, x))
>     }
> }
>
> then this returns `()` if closure cannot fail, and returns `Result<()>` if it
> fails).

This approach look like it could also be used for I/O in general - right
now we do not handle bus errors, but we definitely should.

>
> end of note ]
>
> BTW, I am also not very happy with the `::<E>()` syntax. I think with the I/O
> projection upcoming, we might be able to get rid of relative offseting
> completely by requiring user to first project into `View<'_, .., FalconBase>` and then have
> some registers that can only be used on `Io<Type = FalconBase>` but nothing else.

That's something we can always update once I/O projection is available
if it can indeed be applied (using a two-steps approach if necessary). I
can already hear the pitchforks if we don't deliver some basic register
support for 7.1. :)