[PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`

Alexandre Courbot posted 10 patches 1 month ago
There is a newer version of this series
[PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Alexandre Courbot 1 month ago
Some I/O types, like fixed address registers, carry their location
alongside their values. For these types, the regular `Io::write` method
can lead into repeating the location information twice: once to provide
the location itself, another time to build the value.

Add a new `Io::write_val` convenience method that takes a single
argument implementing `IntoIoVal`, a trait that decomposes implementors
into a `(location, value)` tuple. This allows write operations on fixed
offset registers to be done while specifying their name only once.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 rust/kernel/io.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index ed6fab001a39..09a0fe06f201 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -216,6 +216,22 @@ fn offset(&self) -> usize {
 // Provide the ability to read any primitive type from a [`usize`].
 impl_usize_ioloc!(u8, u16, u32, u64);
 
+/// Trait implemented by items that contain both an I/O location and a value to assign to it.
+///
+/// Implementors can be used with [`Io::write_val`].
+pub trait IntoIoVal<T, L: IoLoc<T>> {
+    /// Consumes `self` and returns a `(location, value)` tuple describing a valid I/O write
+    /// operation.
+    fn into_io_val(self) -> (L, T);
+}
+
+/// `(location, value)` tuples can be used as [`IntoIoVal`]s.
+impl<T, L: IoLoc<T>> IntoIoVal<T, L> for (L, T) {
+    fn into_io_val(self) -> (L, T) {
+        self
+    }
+}
+
 /// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
 /// can perform I/O operations on regions of memory.
 ///
@@ -463,6 +479,32 @@ fn try_write<T, L>(&self, location: L, value: T) -> Result
         Ok(())
     }
 
+    /// Generic fallible write of value bearing its location, with runtime bounds check.
+    ///
+    /// # Examples
+    ///
+    /// Tuples carrying a location and a value can be used with this method:
+    ///
+    /// ```no_run
+    /// use kernel::io::{Io, Mmio};
+    ///
+    /// fn do_writes(io: &Mmio) -> Result {
+    ///     // 32-bit write of value `1` at address `0x10`.
+    ///     io.try_write_val((0x10, 1u32))
+    /// }
+    /// ```
+    #[inline(always)]
+    fn try_write_val<T, L, V>(&self, value: V) -> Result
+    where
+        L: IoLoc<T>,
+        V: IntoIoVal<T, L>,
+        Self: IoCapable<L::IoType>,
+    {
+        let (location, value) = value.into_io_val();
+
+        self.try_write(location, value)
+    }
+
     /// Generic fallible update with runtime bounds check.
     ///
     /// Caution: this does not perform any synchronization. Race conditions can occur in case of
@@ -559,6 +601,32 @@ fn write<T, L>(&self, location: L, value: T)
         unsafe { self.io_write(io_value, address) }
     }
 
+    /// Generic infallible write of value bearing its location, with compile-time bounds check.
+    ///
+    /// # Examples
+    ///
+    /// Tuples carrying a location and a value can be used with this method:
+    ///
+    /// ```no_run
+    /// use kernel::io::{Io, Mmio};
+    ///
+    /// fn do_writes(io: &Mmio) {
+    ///     // 32-bit write of value `1` at address `0x10`.
+    ///     io.write_val((0x10, 1u32));
+    /// }
+    /// ```
+    #[inline(always)]
+    fn write_val<T, L, V>(&self, value: V)
+    where
+        L: IoLoc<T>,
+        V: IntoIoVal<T, L>,
+        Self: IoKnownSize + IoCapable<L::IoType>,
+    {
+        let (location, value) = value.into_io_val();
+
+        self.write(location, value)
+    }
+
     /// Generic infallible update with compile-time bounds check.
     ///
     /// Caution: this does not perform any synchronization. Race conditions can occur in case of

-- 
2.53.0
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Danilo Krummrich 4 weeks, 1 day ago
On Mon Mar 9, 2026 at 4:14 PM CET, Alexandre Courbot wrote:
> +    /// Generic infallible write of value bearing its location, with compile-time bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Tuples carrying a location and a value can be used with this method:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_writes(io: &Mmio) {
> +    ///     // 32-bit write of value `1` at address `0x10`.
> +    ///     io.write_val((0x10, 1u32));
> +    /// }
> +    /// ```
> +    #[inline(always)]
> +    fn write_val<T, L, V>(&self, value: V)

Still not super satisfied with the name write_val() plus I have some more
considerations, sorry. :)

Let's look at this:

	let reg = bar.read(regs::NV_PMC_BOOT_0);

	// Pass around, do some modifications, etc.

	bar.write_val(reg);

In this case read() returns an object that encodes the absolute location and
value, i.e. read() pairs with write_val(), which is a bit odd.

In this example:

	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());

	// Pass around, do some modifications, etc.

	bar.write(WithBase::of::<E>(), reg);

read() pairs with write(), since the object returned by read() does only encode
a relative location, so we have to supply the base location through the first
argument of write().

Well, technically it also pairs with write_val(), as we could also pass this as
tuple to write_val().

	bar.write_val((WithBase::of::<E>(), reg));

However, my point is that this is a bit inconsistent and I think a user would
expect something more symmetric.

For instance, let's say write() would become the single argument one, then
read() could return an impl of IntoIoVal, so we would have

	let reg = bar.read(regs::NV_PMC_BOOT_0);
	bar.write(reg);

	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
	bar.write(reg);

and additionally we can have

	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
	bar.write_val(WithBase::of::<E>(), val);

The same goes for

	let val = bar.read_val(regs::NV_PMC_BOOT_0);
	bar.write_val(regs::NV_PMC_BOOT_0, val);

which for complex values does not achieve anything, but makes sense for the FIFO
register use-case, as val could also be a primitive, e.g. u32.

With a dynamic base, and a single field we could just do the following to
support such primitives:

	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
	bar.write_val(regs::NV_PFALCON_FALCON_RM::of::<E>(), val);

I think adding this symmetry should be trivial, as most of this already compiles
with the current code.

Also, let's not get into a discussion whether we name on or the other write()
vs.  write_foo(). I just turned it around as I think with the specific name
write_val() it makes more sense this way around.

But I'm perfectly happy either way as long as we find descriptive names that
actually make sense.

Unfortunately, I can't really think of a good name for write_val() with its
current semantics. If we flip it around as in my examples above, the _val()
prefix seems fine. Maybe write_reg() and read_reg()?

> +    where
> +        L: IoLoc<T>,
> +        V: IntoIoVal<T, L>,
> +        Self: IoKnownSize + IoCapable<L::IoType>,
> +    {
> +        let (location, value) = value.into_io_val();
> +
> +        self.write(location, value)
> +    }
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Alexandre Courbot 4 weeks ago
On Wed Mar 11, 2026 at 1:34 AM JST, Danilo Krummrich wrote:
> On Mon Mar 9, 2026 at 4:14 PM CET, Alexandre Courbot wrote:
>> +    /// Generic infallible write of value bearing its location, with compile-time bounds check.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// Tuples carrying a location and a value can be used with this method:
>> +    ///
>> +    /// ```no_run
>> +    /// use kernel::io::{Io, Mmio};
>> +    ///
>> +    /// fn do_writes(io: &Mmio) {
>> +    ///     // 32-bit write of value `1` at address `0x10`.
>> +    ///     io.write_val((0x10, 1u32));
>> +    /// }
>> +    /// ```
>> +    #[inline(always)]
>> +    fn write_val<T, L, V>(&self, value: V)
>
> Still not super satisfied with the name write_val() plus I have some more
> considerations, sorry. :)
>
> Let's look at this:
>
> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
>
> 	// Pass around, do some modifications, etc.
>
> 	bar.write_val(reg);
>
> In this case read() returns an object that encodes the absolute location and
> value, i.e. read() pairs with write_val(), which is a bit odd.
>
> In this example:
>
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> 	// Pass around, do some modifications, etc.
>
> 	bar.write(WithBase::of::<E>(), reg);
>
> read() pairs with write(), since the object returned by read() does only encode
> a relative location, so we have to supply the base location through the first
> argument of write().
>
> Well, technically it also pairs with write_val(), as we could also pass this as
> tuple to write_val().
>
> 	bar.write_val((WithBase::of::<E>(), reg));
>
> However, my point is that this is a bit inconsistent and I think a user would
> expect something more symmetric.
>
> For instance, let's say write() would become the single argument one, then
> read() could return an impl of IntoIoVal, so we would have
>
> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
> 	bar.write(reg);
>
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
> 	bar.write(reg);
>
> and additionally we can have
>
> 	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
> 	bar.write_val(WithBase::of::<E>(), val);
>
> The same goes for
>
> 	let val = bar.read_val(regs::NV_PMC_BOOT_0);
> 	bar.write_val(regs::NV_PMC_BOOT_0, val);
>
> which for complex values does not achieve anything, but makes sense for the FIFO
> register use-case, as val could also be a primitive, e.g. u32.
>
> With a dynamic base, and a single field we could just do the following to
> support such primitives:
>
> 	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
> 	bar.write_val(regs::NV_PFALCON_FALCON_RM::of::<E>(), val);
>
> I think adding this symmetry should be trivial, as most of this already compiles
> with the current code.

Not quite, if I understood your idea correctly.

The new `read` would need to return some new type (except for fixed
offset registers) that contains the corresponding `IoLoc`, creating
overhead for the default I/O methods (so at the very least, I think they
should be the ones with the suffix). Accessing the value of that new
type would require some indirection, and probably the return of
closures. And having two pairs of read/write methods that work eerily
similarly but are marginally distinct sounds to me like it could itself
become a source of confusion.

>
> Also, let's not get into a discussion whether we name on or the other write()
> vs.  write_foo(). I just turned it around as I think with the specific name
> write_val() it makes more sense this way around.
>
> But I'm perfectly happy either way as long as we find descriptive names that
> actually make sense.
>
> Unfortunately, I can't really think of a good name for write_val() with its
> current semantics. If we flip it around as in my examples above, the _val()
> prefix seems fine. Maybe write_reg() and read_reg()?

The fact is, there is a symmetry between `read` and `write`:

- `read` takes a location and returns a value,
- `write` takes a location and a value.

`write_val` is really nothing but a convenience shortcut that has
technically no strong reason to exist. As Gary pointed out, the
counterpart of

    let reg = bar.read(regs::NV_PMC_BOOT_0);

is

    bar.write(regs::NV_PMC_BOOT_0, reg);

... and we introduced `write_val` for those cases where the value
in itself provides its location, as `NV_PMC_BOOT_0` is redundant. But
the above statement could also be written:

    bar.write((), reg);

which is exactly the same length as the `write_val` equivalent - it's
just that you need to remember that `()` can be used in this case. But
if you can remember that your register type can be used with
`write_val`, then why not this? This actually makes me doubt that
`write_val` is needed at all, and if we get rid of it, then we have a
symmetric API.

We were so focused on this single issue for the last few revisions that
the single-argument write variant sounded like the only way to handle
this properly, but the `()` use proposed by Gary actually fulfills the
same role and doesn't introduce more burden when you think of it.

So why not try without `write_val` at first? We can always add it later
if we feel the need (and the same applies to a `(location, value)`
symmetric read/write API).

And most importantly, that way we also don't have to worry about its
name. :)
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Danilo Krummrich 4 weeks ago
On Wed Mar 11, 2026 at 2:28 PM CET, Alexandre Courbot wrote:
> The fact is, there is a symmetry between `read` and `write`:
>
> - `read` takes a location and returns a value,
> - `write` takes a location and a value.

This is not entirely true.

read() takes an absolute location and returns something that encodes the value
and a relative location, where for fixed registers the relative location is
equivalent to an absolute location.

write() works with both an absolute location and something that encodes the
value and a relative location, or a base location and something that encodes the
value and a relative location.

	// `reg` encodes the value and an absolute location.
	let reg = bar.read(regs::NV_PMC_BOOT_0);

	// `reg` encodes the value and a relative location.
	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());

	// First argument is an absolute location, second argument is a type
	// that encodes a value and a relative location.
	bar.write(regs::NV_PFALCON_FALCON_RM::of::<E>(), reg);

	// First argument is a base location that infers the relative location
	// from `reg`.
	bar.write(WithBase::of::<E>(), reg);

And yes, I am aware that the above wording around the difference between
regs::NV_PFALCON_FALCON_RM::of::<E>() and WithBase::of::<E>() is at least a bit
vague technically, but my point is about how this appears to users.

In any case, the fact that you can write WithBase::of::<E>() as a location
argument in the first place proves that `reg` is not *only* a value.

	fn write<T, L>(&self, location: L, value: T)
	where
	    L: IoLoc<T>,
	    Self: IoKnownSize + IoCapable<L::IoType>,

Which is the reason why L: IoLoc<T>, i.e. the relative location is on T, not on
L.

So, what you could say is

  - read() takes an absolute location and returns something that encodes a value
    and relative location

  - write() takes a base location (or absolute location) and something that
    encodes a value and relative location

But then there is the case with fixed registers that simply do not have a base,
but write() still asks for a base (or absolute) location.

> `write_val` is really nothing but a convenience shortcut that has
> technically no strong reason to exist. As Gary pointed out, the
> counterpart of
>
>     let reg = bar.read(regs::NV_PMC_BOOT_0);
>
> is
>
>     bar.write(regs::NV_PMC_BOOT_0, reg);

I do not agree for the reasons mentioned above.

In this case read() returns (location + value), while write() asks for location
*and* (location + value), i.e. there is no base location for fixed registers.

> ... and we introduced `write_val` for those cases where the value
> in itself provides its location, as `NV_PMC_BOOT_0` is redundant. But
> the above statement could also be written:
>
>     bar.write((), reg);

This makes more sense, as one could argue that a fixed register still requires a
base location, which is just always zero, but why would we bother users with
this implementation detail?

Is this really less confusing than an additional bar.write_reg(reg) that just
works with any register?

> which is exactly the same length as the `write_val` equivalent - it's
> just that you need to remember that `()` can be used in this case. But
> if you can remember that your register type can be used with
> `write_val`, then why not this? This actually makes me doubt that
> `write_val` is needed at all, and if we get rid of it, then we have a
> symmetric API.

Still not symmetric, and I also don't think we will have a lot of fun explaining
people why they have to call it as bar.write((), reg). :(

OOC, how would you explain it when the question is raised without arguing with
implementation details?

> We were so focused on this single issue for the last few revisions that
> the single-argument write variant sounded like the only way to handle
> this properly, but the `()` use proposed by Gary actually fulfills the
> same role and doesn't introduce more burden when you think of it.
>
> So why not try without `write_val` at first? We can always add it later
> if we feel the need (and the same applies to a `(location, value)`
> symmetric read/write API).

If you really think it's the best solution, I'm fine picking it up this way for
now, but to me it still sounds like we have no solution for a very simple case
that does not at least raise an eyebrow.

> And most importantly, that way we also don't have to worry about its
> name. :)
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Alexandre Courbot 3 weeks, 6 days ago
On Wed Mar 11, 2026 at 11:56 PM JST, Danilo Krummrich wrote:
> On Wed Mar 11, 2026 at 2:28 PM CET, Alexandre Courbot wrote:
>> The fact is, there is a symmetry between `read` and `write`:
>>
>> - `read` takes a location and returns a value,
>> - `write` takes a location and a value.
>
> This is not entirely true.
>
> read() takes an absolute location and returns something that encodes the value
> and a relative location, where for fixed registers the relative location is
> equivalent to an absolute location.

Let's look at the prototype of `read` (inherited constraints in parentheses):

    fn read<T, L>(&self, location: L) -> T
    where
        L: IoLoc<T>,
        Self: IoKnownSize + IoCapable<L::IoType>,
        (L::IoType: Into<T> + From<T>,)

It takes an absolute location and returns... something that can be
converted to/from the I/O type. There is no mention of a relative
location anywhere.

The very concept of a relative location is local to the `register`
module. `read` doesn't care about this at all. It just so happens that
register do carry that information in their type  - but that's not a
requirement of the read/write I/O API.

>
> write() works with both an absolute location and something that encodes the
> value and a relative location, or a base location and something that encodes the
> value and a relative location.

Again, looking at the prototype for `write`:

    fn write<T, L>(&self, location: L, value: T)
    where
        L: IoLoc<T>,
        Self: IoKnownSize + IoCapable<L::IoType>,
        (L::IoType: Into<T> + From<T>,)

The first argument is an absolute location. The second argument is just
a value - there is no requirement for any kind of partial location
information.

It is true that in the case of registers, we can use the second argument
to infer a generic parameter of the first that is needed to build that
absolute location, which makes the first argument look as if it
contained partial location information, but that's just because the
*register* module defines this relationship between its own location and
value types (and the compiler is nice enough to connect the dots for
us). This is again not the work of the read/write I/O API.

So we really have this:

- read(location) -> value
- write(location, value)

Otherwise we couldn't use primitive types with read/write, since they
don't carry any location information by themselves.

>
> 	// `reg` encodes the value and an absolute location.
> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
>
> 	// `reg` encodes the value and a relative location.
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> 	// First argument is an absolute location, second argument is a type
> 	// that encodes a value and a relative location.
> 	bar.write(regs::NV_PFALCON_FALCON_RM::of::<E>(), reg);
>
> 	// First argument is a base location that infers the relative location
> 	// from `reg`.
> 	bar.write(WithBase::of::<E>(), reg);
>
> And yes, I am aware that the above wording around the difference between
> regs::NV_PFALCON_FALCON_RM::of::<E>() and WithBase::of::<E>() is at least a bit
> vague technically, but my point is about how this appears to users.
>
> In any case, the fact that you can write WithBase::of::<E>() as a location
> argument in the first place proves that `reg` is not *only* a value.

But that is only true in the case of registers. The I/O module can and
does cover things other than registers - currently primitive values, but
we might want to extend that in future (don't ask me with what, but I
don't see a reason to close the possibility :)).

>
> 	fn write<T, L>(&self, location: L, value: T)
> 	where
> 	    L: IoLoc<T>,
> 	    Self: IoKnownSize + IoCapable<L::IoType>,
>
> Which is the reason why L: IoLoc<T>, i.e. the relative location is on T, not on
> L.
>
> So, what you could say is
>
>   - read() takes an absolute location and returns something that encodes a value
>     and relative location
>
>   - write() takes a base location (or absolute location) and something that
>     encodes a value and relative location
>
> But then there is the case with fixed registers that simply do not have a base,
> but write() still asks for a base (or absolute) location.
>
>> `write_val` is really nothing but a convenience shortcut that has
>> technically no strong reason to exist. As Gary pointed out, the
>> counterpart of
>>
>>     let reg = bar.read(regs::NV_PMC_BOOT_0);
>>
>> is
>>
>>     bar.write(regs::NV_PMC_BOOT_0, reg);
>
> I do not agree for the reasons mentioned above.
>
> In this case read() returns (location + value), while write() asks for location
> *and* (location + value), i.e. there is no base location for fixed registers.
>
>> ... and we introduced `write_val` for those cases where the value
>> in itself provides its location, as `NV_PMC_BOOT_0` is redundant. But
>> the above statement could also be written:
>>
>>     bar.write((), reg);
>
> This makes more sense, as one could argue that a fixed register still requires a
> base location, which is just always zero, but why would we bother users with
> this implementation detail?
>
> Is this really less confusing than an additional bar.write_reg(reg) that just
> works with any register?

I don't think it is less or more confusing, in the end they are really
equivalent.

What I would like to avoid is having register-specific functions spill
into the `io` module. I mean, I am not strictly opposed to it if we
reach the conclusion that it is more convenient to users - in this case
we could add an `impl Io` block in `register.rs` and add a `write_reg`
method (and possibly a `read` variant returning the full position
information?). But if we can just use the same 2 methods everywhere,
that's better IMHO.

>
>> which is exactly the same length as the `write_val` equivalent - it's
>> just that you need to remember that `()` can be used in this case. But
>> if you can remember that your register type can be used with
>> `write_val`, then why not this? This actually makes me doubt that
>> `write_val` is needed at all, and if we get rid of it, then we have a
>> symmetric API.
>
> Still not symmetric, and I also don't think we will have a lot of fun explaining
> people why they have to call it as bar.write((), reg). :(
>
> OOC, how would you explain it when the question is raised without arguing with
> implementation details?

This seems to indicate that instead of a `Io::write_val` method in `io.rs`,
we might need a `Io::write_reg` method in `register.rs` that is
dedicated to writing unambiguous registers exclusively. How does that
sound to you?

>
>> We were so focused on this single issue for the last few revisions that
>> the single-argument write variant sounded like the only way to handle
>> this properly, but the `()` use proposed by Gary actually fulfills the
>> same role and doesn't introduce more burden when you think of it.
>>
>> So why not try without `write_val` at first? We can always add it later
>> if we feel the need (and the same applies to a `(location, value)`
>> symmetric read/write API).
>
> If you really think it's the best solution, I'm fine picking it up this way for
> now, but to me it still sounds like we have no solution for a very simple case
> that does not at least raise an eyebrow.

I think the current `read` and `write` methods are going to stay the
basis of I/O, so I'm pretty confident we can commit to them. They also
allow to do everything we want to do with registers. So my suggestion
would be to see how it goes and add functionality as we get feedback and
more visibility into how the API is used.

After reading your reply I believe an `Io::write_reg` in `register.rs`
could solve the problem of writing fixed offset registers without making
a single-argument `write` part of the core I/O API (and save the trouble
of finding a generic-enough name for it!). It's just not clear to me
that we should add this right now - but I can of course do it in the
next revision if you think the idea is good.
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Danilo Krummrich 3 weeks, 6 days ago
On Thu Mar 12, 2026 at 2:53 PM CET, Alexandre Courbot wrote:
> On Wed Mar 11, 2026 at 11:56 PM JST, Danilo Krummrich wrote:
>> And yes, I am aware that the above wording around the difference between
>> regs::NV_PFALCON_FALCON_RM::of::<E>() and WithBase::of::<E>() is at least a bit
>> vague technically, but my point is about how this appears to users.
>>
>> In any case, the fact that you can write WithBase::of::<E>() as a location
>> argument in the first place proves that `reg` is not *only* a value.
>
> But that is only true in the case of registers.

Agree with this and all the above. But - and this is why I wrote "my point is
about how this appears to users" - users use this API with registers only and
except for the FIFO use-case, which is not implemented yet, it is obvious to
users that part of the location comes from the value type.

I'm not saying that this is a problem. Actually, quite the opposite.

When I proposed the register!() macro, one of the problems I wanted to solve was
that people can't read from a certain location and accidentally treat it as
something semantically different.

Or in other words, if you want to read register A, you shouldn't be able to
accidentally read from B and treat it as A.

> The I/O module can and does cover things other than registers - currently
> primitive values, but we might want to extend that in future (don't ask me
> with what, but I don't see a reason to close the possibility :)).

Neither do I. :)

But, as mentioned above, we should be aware that people notice that for
registers part of the location information comes from the value type (except for
the FIFO case of course) and registers are the thing that people deal with most.

Other use-cases such as I/O memcpy, etc. will be accssible through other APIs,
such as IoSlice, or more generally, IoView.

Having that said, my point is that considering the use-cases it also makes sense
to think about how this API will be perceived.

And from this context something like bar.write((), reg) looks pretty odd.

>> Is this really less confusing than an additional bar.write_reg(reg) that just
>> works with any register?
>
> I don't think it is less or more confusing, in the end they are really
> equivalent.

From your point of view being an expert on the I/O and register!()
implementation details, but please also consider the above.

Also, look at the Tyr patches for instance, they exclusively use write_val().

> What I would like to avoid is having register-specific functions spill
> into the `io` module. I mean, I am not strictly opposed to it if we
> reach the conclusion that it is more convenient to users - in this case
> we could add an `impl Io` block in `register.rs` and add a `write_reg`
> method (and possibly a `read` variant returning the full position
> information?). But if we can just use the same 2 methods everywhere,
> that's better IMHO.

Just leave it in the io module I'd say, register access is probably the most
essential part of I/O, I think there is no need to factor it out.

>>> which is exactly the same length as the `write_val` equivalent - it's
>>> just that you need to remember that `()` can be used in this case. But
>>> if you can remember that your register type can be used with
>>> `write_val`, then why not this? This actually makes me doubt that
>>> `write_val` is needed at all, and if we get rid of it, then we have a
>>> symmetric API.
>>
>> Still not symmetric, and I also don't think we will have a lot of fun explaining
>> people why they have to call it as bar.write((), reg). :(
>>
>> OOC, how would you explain it when the question is raised without arguing with
>> implementation details?
>
> This seems to indicate that instead of a `Io::write_val` method in `io.rs`,
> we might need a `Io::write_reg` method in `register.rs` that is
> dedicated to writing unambiguous registers exclusively. How does that
> sound to you?

I don't see it adds any value to factor it out with an extention trait, rename
to write_reg() seems fine.

Additionally, I'd like to leave it open for the future to add read_reg() method
returning a generic (loc, val) tuple dereferencing to the value in case we see a
repetition of the following pattern.

	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());

	// modify reg

	bar.write(WithBase::of::<E>(), reg)

The reason is the same as mentioned above. In most cases drivers don't want to
switch the base location between a read and a write, i.e. write the value from A
to B.

The most common case is to read from A and write back to A. For instance,
talking to the Tyr folks, they told me that for the array registers they will
have, they will never have the case that they want to write a register value
from A to B.

So, I still think it would be good to provide an option for drivers to prevent
any mistakes in the first place.

Now, obviously this would require that we also provide register accessors that
take a mutable reference for this to work, but that doesn't seem like a big
deal.

I also don't think we have to do this now, but I'd like to have something like
this in the future.

For now s/write_val/write_reg/ and we should be good to go. :)
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Alexandre Courbot 3 weeks, 5 days ago
On Fri Mar 13, 2026 at 12:54 AM JST, Danilo Krummrich wrote:
<snip>
>>>> which is exactly the same length as the `write_val` equivalent - it's
>>>> just that you need to remember that `()` can be used in this case. But
>>>> if you can remember that your register type can be used with
>>>> `write_val`, then why not this? This actually makes me doubt that
>>>> `write_val` is needed at all, and if we get rid of it, then we have a
>>>> symmetric API.
>>>
>>> Still not symmetric, and I also don't think we will have a lot of fun explaining
>>> people why they have to call it as bar.write((), reg). :(
>>>
>>> OOC, how would you explain it when the question is raised without arguing with
>>> implementation details?
>>
>> This seems to indicate that instead of a `Io::write_val` method in `io.rs`,
>> we might need a `Io::write_reg` method in `register.rs` that is
>> dedicated to writing unambiguous registers exclusively. How does that
>> sound to you?
>
> I don't see it adds any value to factor it out with an extention trait, rename
> to write_reg() seems fine.
>
> Additionally, I'd like to leave it open for the future to add read_reg() method
> returning a generic (loc, val) tuple dereferencing to the value in case we see a
> repetition of the following pattern.
>
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> 	// modify reg
>
> 	bar.write(WithBase::of::<E>(), reg)
>
> The reason is the same as mentioned above. In most cases drivers don't want to
> switch the base location between a read and a write, i.e. write the value from A
> to B.

That's definitely a pattern we are seeing a lot already. I agree there
is a use-case for that.

>
> The most common case is to read from A and write back to A. For instance,
> talking to the Tyr folks, they told me that for the array registers they will
> have, they will never have the case that they want to write a register value
> from A to B.
>
> So, I still think it would be good to provide an option for drivers to prevent
> any mistakes in the first place.
>
> Now, obviously this would require that we also provide register accessors that
> take a mutable reference for this to work, but that doesn't seem like a big
> deal.
>
> I also don't think we have to do this now, but I'd like to have something like
> this in the future.

There are several ways we can achieve this. One would be to use the
macro to forward the bitfield setter methods to the containing type and
return an updated one. Or, we implement `Deref` on the containing type,
and add a set of `set_<field>` methods that modify the value in-place.

That way `read_reg` and `write_reg` should be symmetric and we remove
another cause of typos and mistakes.

I'l start thinking about it once I post the extracted `bitfield`
patches, as it makes it easier to look at the problem space.
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Gary Guo 4 weeks ago
On Wed Mar 11, 2026 at 2:56 PM GMT, Danilo Krummrich wrote:
> On Wed Mar 11, 2026 at 2:28 PM CET, Alexandre Courbot wrote:
>> The fact is, there is a symmetry between `read` and `write`:
>>
>> - `read` takes a location and returns a value,
>> - `write` takes a location and a value.
>
> This is not entirely true.
>
> read() takes an absolute location and returns something that encodes the value
> and a relative location, where for fixed registers the relative location is
> equivalent to an absolute location.

The value returned by `read` does not encode relative location. If a primitive
is returned (of course, this is not currently supported by the `register!`
macro) for the FIFO register case, the returned integer has no location
informaiton whatsoever. What encodes the relative location is the type itself.

If you just construct the value yourself without using `read`, you can also use
`write_val` or `bar.write(WithBase...)`.

>
> write() works with both an absolute location and something that encodes the
> value and a relative location, or a base location and something that encodes the
> value and a relative location.
>
> 	// `reg` encodes the value and an absolute location.
> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
>
> 	// `reg` encodes the value and a relative location.
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> 	// First argument is an absolute location, second argument is a type
> 	// that encodes a value and a relative location.
> 	bar.write(regs::NV_PFALCON_FALCON_RM::of::<E>(), reg);
>
> 	// First argument is a base location that infers the relative location
> 	// from `reg`.
> 	bar.write(WithBase::of::<E>(), reg);
>
> And yes, I am aware that the above wording around the difference between
> regs::NV_PFALCON_FALCON_RM::of::<E>() and WithBase::of::<E>() is at least a bit
> vague technically, but my point is about how this appears to users.

The latter is just a type inferred version of the former, to avoid having you to
write the name twice. They're exactly the same.

>
> In any case, the fact that you can write WithBase::of::<E>() as a location
> argument in the first place proves that `reg` is not *only* a value.

Not true as discussed above.

>
> 	fn write<T, L>(&self, location: L, value: T)
> 	where
> 	    L: IoLoc<T>,
> 	    Self: IoKnownSize + IoCapable<L::IoType>,
>
> Which is the reason why L: IoLoc<T>, i.e. the relative location is on T, not on
> L.

The location has nothing to do with `value`, just the type `T`.

>
> So, what you could say is
>
>   - read() takes an absolute location and returns something that encodes a value
>     and relative location
>
>   - write() takes a base location (or absolute location) and something that
>     encodes a value and relative location

Only `location` gives you the location. Value has nothing to do with locations at all. Type
inference does the trick.

Best,
Gary

>
> But then there is the case with fixed registers that simply do not have a base,
> but write() still asks for a base (or absolute) location.
>
>> `write_val` is really nothing but a convenience shortcut that has
>> technically no strong reason to exist. As Gary pointed out, the
>> counterpart of
>>
>>     let reg = bar.read(regs::NV_PMC_BOOT_0);
>>
>> is
>>
>>     bar.write(regs::NV_PMC_BOOT_0, reg);
>
> I do not agree for the reasons mentioned above.
>
> In this case read() returns (location + value), while write() asks for location
> *and* (location + value), i.e. there is no base location for fixed registers.
>
>> ... and we introduced `write_val` for those cases where the value
>> in itself provides its location, as `NV_PMC_BOOT_0` is redundant. But
>> the above statement could also be written:
>>
>>     bar.write((), reg);
>
> This makes more sense, as one could argue that a fixed register still requires a
> base location, which is just always zero, but why would we bother users with
> this implementation detail?
>
> Is this really less confusing than an additional bar.write_reg(reg) that just
> works with any register?
>
>> which is exactly the same length as the `write_val` equivalent - it's
>> just that you need to remember that `()` can be used in this case. But
>> if you can remember that your register type can be used with
>> `write_val`, then why not this? This actually makes me doubt that
>> `write_val` is needed at all, and if we get rid of it, then we have a
>> symmetric API.
>
> Still not symmetric, and I also don't think we will have a lot of fun explaining
> people why they have to call it as bar.write((), reg). :(
>
> OOC, how would you explain it when the question is raised without arguing with
> implementation details?
>
>> We were so focused on this single issue for the last few revisions that
>> the single-argument write variant sounded like the only way to handle
>> this properly, but the `()` use proposed by Gary actually fulfills the
>> same role and doesn't introduce more burden when you think of it.
>>
>> So why not try without `write_val` at first? We can always add it later
>> if we feel the need (and the same applies to a `(location, value)`
>> symmetric read/write API).
>
> If you really think it's the best solution, I'm fine picking it up this way for
> now, but to me it still sounds like we have no solution for a very simple case
> that does not at least raise an eyebrow.
>
>> And most importantly, that way we also don't have to worry about its
>> name. :)
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Danilo Krummrich 4 weeks ago
On Wed Mar 11, 2026 at 4:42 PM CET, Gary Guo wrote:
> On Wed Mar 11, 2026 at 2:56 PM GMT, Danilo Krummrich wrote:
>>
>> 	fn write<T, L>(&self, location: L, value: T)
>> 	where
>> 	    L: IoLoc<T>,
>> 	    Self: IoKnownSize + IoCapable<L::IoType>,
>>
>> Which is the reason why L: IoLoc<T>, i.e. the relative location is on T, not on
>> L.
>
> The location has nothing to do with `value`, just the type `T`.

(Cutting all the above, since it all boils down to the same thing.)

So, that's what I mean, the relative location is on type T (the value type), not
on L (the location type), hence T can't just be an arbitrary primitive, no?

>> So, what you could say is
>>
>>   - read() takes an absolute location and returns something that encodes a value
>>     and relative location
>>
>>   - write() takes a base location (or absolute location) and something that
>>     encodes a value and relative location
>
> Only `location` gives you the location. Value has nothing to do with locations at all. Type
> inference does the trick.

The value does have something to do with the relative location, as it comes from
the value type. You can't pass an "independent" value whose type does not carry
location information, can you?
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Gary Guo 4 weeks ago
On Wed Mar 11, 2026 at 4:22 PM GMT, Danilo Krummrich wrote:
> On Wed Mar 11, 2026 at 4:42 PM CET, Gary Guo wrote:
>> On Wed Mar 11, 2026 at 2:56 PM GMT, Danilo Krummrich wrote:
>>>
>>> 	fn write<T, L>(&self, location: L, value: T)
>>> 	where
>>> 	    L: IoLoc<T>,
>>> 	    Self: IoKnownSize + IoCapable<L::IoType>,
>>>
>>> Which is the reason why L: IoLoc<T>, i.e. the relative location is on T, not on
>>> L.
>>
>> The location has nothing to do with `value`, just the type `T`.
>
> (Cutting all the above, since it all boils down to the same thing.)
>
> So, that's what I mean, the relative location is on type T (the value type), not
> on L (the location type), hence T can't just be an arbitrary primitive, no?

The location is on the trait impl of `L`, it just may depend on `T`.

For a register location, say, `UART_TX`, it is itself contain full information
on location; the `IoLoc<u8>` impl of it would be just used to constrain the type
so that no other types than u8 gets used.

Yes, for the case of `()` or `WithBase`, it is itself only partial information
and the location is only complete with an associated type. But this is more just
a type-system trick so that we can avoid spelling out the full register name.

For the `write_val` case for a bitfield, I would basically consider the canonical
way of doing a register write is

    bar.write(REGISTER_NAME, bitfield)

value. Where

    bar.write((), bitfield)

or (`write_val`) is a nice sugar to avoid writing the name multiple times.

>
>>> So, what you could say is
>>>
>>>   - read() takes an absolute location and returns something that encodes a value
>>>     and relative location
>>>
>>>   - write() takes a base location (or absolute location) and something that
>>>     encodes a value and relative location
>>
>> Only `location` gives you the location. Value has nothing to do with locations at all. Type
>> inference does the trick.
>
> The value does have something to do with the relative location, as it comes from
> the value type. You can't pass an "independent" value whose type does not carry
> location information, can you?

You can also not pass an "independent" value to register of a different type.
With my FIFO example, you cannot write

    bar.write(UART_TX, 0u32)

because `UART_TX: IoLoc<u8>` only. It doesn't mean that the `0u32` suddenly
start carrying location information.

I just view this as type system working as designed.

Best,
Gary
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Gary Guo 4 weeks, 1 day ago
On Tue Mar 10, 2026 at 4:34 PM GMT, Danilo Krummrich wrote:
> On Mon Mar 9, 2026 at 4:14 PM CET, Alexandre Courbot wrote:
>> +    /// Generic infallible write of value bearing its location, with compile-time bounds check.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// Tuples carrying a location and a value can be used with this method:
>> +    ///
>> +    /// ```no_run
>> +    /// use kernel::io::{Io, Mmio};
>> +    ///
>> +    /// fn do_writes(io: &Mmio) {
>> +    ///     // 32-bit write of value `1` at address `0x10`.
>> +    ///     io.write_val((0x10, 1u32));
>> +    /// }
>> +    /// ```
>> +    #[inline(always)]
>> +    fn write_val<T, L, V>(&self, value: V)
>
> Still not super satisfied with the name write_val() plus I have some more
> considerations, sorry. :)
>
> Let's look at this:
>
> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
>
> 	// Pass around, do some modifications, etc.
>
> 	bar.write_val(reg);
>
> In this case read() returns an object that encodes the absolute location and
> value, i.e. read() pairs with write_val(), which is a bit odd.

I mean, it also pairs with `bar.write(regs::NV_PMC_BOOT_0, reg)`, so it's not
totally odd. We just need to teach that "write_val" infers the register location
from value.

>
> In this example:
>
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> 	// Pass around, do some modifications, etc.
>
> 	bar.write(WithBase::of::<E>(), reg);
>
> read() pairs with write(), since the object returned by read() does only encode
> a relative location, so we have to supply the base location through the first
> argument of write().
>
> Well, technically it also pairs with write_val(), as we could also pass this as
> tuple to write_val().
>
> 	bar.write_val((WithBase::of::<E>(), reg));
>
> However, my point is that this is a bit inconsistent and I think a user would
> expect something more symmetric.
>
> For instance, let's say write() would become the single argument one, then
> read() could return an impl of IntoIoVal, so we would have
>
> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
> 	bar.write(reg);
>
> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
> 	bar.write(reg);

How would you modify the value then? `reg` becomes a combined pair with both
location and value, and we would start need to use a closure to mutate inner
values again, which in my opinion is bad.

This would again become the `IoWrite` design of the last iteration which is the
part that I disliked most.

>
> and additionally we can have
>
> 	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
> 	bar.write_val(WithBase::of::<E>(), val);
>
> The same goes for
>
> 	let val = bar.read_val(regs::NV_PMC_BOOT_0);
> 	bar.write_val(regs::NV_PMC_BOOT_0, val);
>
> which for complex values does not achieve anything, but makes sense for the FIFO
> register use-case, as val could also be a primitive, e.g. u32.
>
> With a dynamic base, and a single field we could just do the following to
> support such primitives:
>
> 	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
> 	bar.write_val(regs::NV_PFALCON_FALCON_RM::of::<E>(), val);
>
> I think adding this symmetry should be trivial, as most of this already compiles
> with the current code.
>
> Also, let's not get into a discussion whether we name on or the other write()
> vs.  write_foo(). I just turned it around as I think with the specific name
> write_val() it makes more sense this way around.

To me flipping this around is the odd one. If this is flipped around then this
should be named `write_at`, but then it becomes odd as it is the dual of `read`
and we won't have `read_at`.

Best,
Gary

>
> But I'm perfectly happy either way as long as we find descriptive names that
> actually make sense.
>
> Unfortunately, I can't really think of a good name for write_val() with its
> current semantics. If we flip it around as in my examples above, the _val()
> prefix seems fine. Maybe write_reg() and read_reg()?
>
>> +    where
>> +        L: IoLoc<T>,
>> +        V: IntoIoVal<T, L>,
>> +        Self: IoKnownSize + IoCapable<L::IoType>,
>> +    {
>> +        let (location, value) = value.into_io_val();
>> +
>> +        self.write(location, value)
>> +    }
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Danilo Krummrich 4 weeks ago
On Tue Mar 10, 2026 at 11:33 PM CET, Gary Guo wrote:
> On Tue Mar 10, 2026 at 4:34 PM GMT, Danilo Krummrich wrote:
>> On Mon Mar 9, 2026 at 4:14 PM CET, Alexandre Courbot wrote:
>>> +    /// Generic infallible write of value bearing its location, with compile-time bounds check.
>>> +    ///
>>> +    /// # Examples
>>> +    ///
>>> +    /// Tuples carrying a location and a value can be used with this method:
>>> +    ///
>>> +    /// ```no_run
>>> +    /// use kernel::io::{Io, Mmio};
>>> +    ///
>>> +    /// fn do_writes(io: &Mmio) {
>>> +    ///     // 32-bit write of value `1` at address `0x10`.
>>> +    ///     io.write_val((0x10, 1u32));
>>> +    /// }
>>> +    /// ```
>>> +    #[inline(always)]
>>> +    fn write_val<T, L, V>(&self, value: V)
>>
>> Still not super satisfied with the name write_val() plus I have some more
>> considerations, sorry. :)
>>
>> Let's look at this:
>>
>> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
>>
>> 	// Pass around, do some modifications, etc.
>>
>> 	bar.write_val(reg);
>>
>> In this case read() returns an object that encodes the absolute location and
>> value, i.e. read() pairs with write_val(), which is a bit odd.
>
> I mean, it also pairs with `bar.write(regs::NV_PMC_BOOT_0, reg)`, so it's not

s/also pairs with/also works with/

> totally odd. We just need to teach that "write_val" infers the register location
> from value.
>
>>
>> In this example:
>>
>> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>>
>> 	// Pass around, do some modifications, etc.
>>
>> 	bar.write(WithBase::of::<E>(), reg);
>>
>> read() pairs with write(), since the object returned by read() does only encode
>> a relative location, so we have to supply the base location through the first
>> argument of write().
>>
>> Well, technically it also pairs with write_val(), as we could also pass this as
>> tuple to write_val().
>>
>> 	bar.write_val((WithBase::of::<E>(), reg));
>>
>> However, my point is that this is a bit inconsistent and I think a user would
>> expect something more symmetric.
>>
>> For instance, let's say write() would become the single argument one, then
>> read() could return an impl of IntoIoVal, so we would have
>>
>> 	let reg = bar.read(regs::NV_PMC_BOOT_0);
>> 	bar.write(reg);
>>
>> 	let reg = bar.read(regs::NV_PFALCON_FALCON_RM::of::<E>());
>> 	bar.write(reg);
>
> How would you modify the value then? `reg` becomes a combined pair with both
> location and value, and we would start need to use a closure to mutate inner
> values again, which in my opinion is bad.

IIUC, the return value could be a generic tuple struct with <L: IoLoc<T>, V:
IntoIoVal<T, L>> that dereferences to V?

I.e. the current write_val() already allows this:

	bar.write_val((WithBase::of::<E>(), reg))

so, a corresponding read_val() - both write_val() and read_val() should really
have different names - could return the same thing, just as a generic type that
dereferences to self.1.

With this you could do

	let reg = bar.read_reg(regs::NV_PFALCON_FALCON_RM::of::<E>());

	reg.modify();

	bar.write_reg(reg);

without repeating the of::<E>() part while

	let val = regs::NV_PFALCON_FALCON_RM::zeroed();

	// Set a couple of fields.

	bar.write(WithBase::of::<E>(), val);

is still possible.

> This would again become the `IoWrite` design of the last iteration which is the
> part that I disliked most.
>
>>
>> and additionally we can have
>>
>> 	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
>> 	bar.write_val(WithBase::of::<E>(), val);
>>
>> The same goes for
>>
>> 	let val = bar.read_val(regs::NV_PMC_BOOT_0);
>> 	bar.write_val(regs::NV_PMC_BOOT_0, val);
>>
>> which for complex values does not achieve anything, but makes sense for the FIFO
>> register use-case, as val could also be a primitive, e.g. u32.
>>
>> With a dynamic base, and a single field we could just do the following to
>> support such primitives:
>>
>> 	let val = bar.read_val(regs::NV_PFALCON_FALCON_RM::of::<E>());
>> 	bar.write_val(regs::NV_PFALCON_FALCON_RM::of::<E>(), val);
>>
>> I think adding this symmetry should be trivial, as most of this already compiles
>> with the current code.
>>
>> Also, let's not get into a discussion whether we name on or the other write()
>> vs.  write_foo(). I just turned it around as I think with the specific name
>> write_val() it makes more sense this way around.
>
> To me flipping this around is the odd one. If this is flipped around then this
> should be named `write_at`, but then it becomes odd as it is the dual of `read`
> and we won't have `read_at`.

That's why I said I only flipped it because write_val() is an odd name for what
it does currently. Please also note the below.

>> But I'm perfectly happy either way as long as we find descriptive names that
>> actually make sense.
>>
>> Unfortunately, I can't really think of a good name for write_val() with its
>> current semantics. If we flip it around as in my examples above, the _val()
>> prefix seems fine. Maybe write_reg() and read_reg()?
>>
>>> +    where
>>> +        L: IoLoc<T>,
>>> +        V: IntoIoVal<T, L>,
>>> +        Self: IoKnownSize + IoCapable<L::IoType>,
>>> +    {
>>> +        let (location, value) = value.into_io_val();
>>> +
>>> +        self.write(location, value)
>>> +    }
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Alexandre Courbot 4 weeks ago
On Wed Mar 11, 2026 at 10:25 PM JST, Danilo Krummrich wrote:
<snip>
> IIUC, the return value could be a generic tuple struct with <L: IoLoc<T>, V:
> IntoIoVal<T, L>> that dereferences to V?
>
> I.e. the current write_val() already allows this:
>
> 	bar.write_val((WithBase::of::<E>(), reg))
>
> so, a corresponding read_val() - both write_val() and read_val() should really
> have different names - could return the same thing, just as a generic type that
> dereferences to self.1.
>
> With this you could do
>
> 	let reg = bar.read_reg(regs::NV_PFALCON_FALCON_RM::of::<E>());
>
> 	reg.modify();

That's the tricky part - the register methods don't modify in-place,
they return a new value. I don't see how we could avoid a closure (and
mutable variables) here.
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Gary Guo 1 month ago
On Mon Mar 9, 2026 at 3:14 PM GMT, Alexandre Courbot wrote:
> Some I/O types, like fixed address registers, carry their location
> alongside their values. For these types, the regular `Io::write` method
> can lead into repeating the location information twice: once to provide
> the location itself, another time to build the value.
>
> Add a new `Io::write_val` convenience method that takes a single
> argument implementing `IntoIoVal`, a trait that decomposes implementors
> into a `(location, value)` tuple. This allows write operations on fixed
> offset registers to be done while specifying their name only once.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  rust/kernel/io.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index ed6fab001a39..09a0fe06f201 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -216,6 +216,22 @@ fn offset(&self) -> usize {
>  // Provide the ability to read any primitive type from a [`usize`].
>  impl_usize_ioloc!(u8, u16, u32, u64);
>  
> +/// Trait implemented by items that contain both an I/O location and a value to assign to it.
> +///
> +/// Implementors can be used with [`Io::write_val`].
> +pub trait IntoIoVal<T, L: IoLoc<T>> {

Is this generics instead of associative types for some reason? If so, please
mention it in the commit mesasge.

> +    /// Consumes `self` and returns a `(location, value)` tuple describing a valid I/O write
> +    /// operation.
> +    fn into_io_val(self) -> (L, T);
> +}
> +
> +/// `(location, value)` tuples can be used as [`IntoIoVal`]s.
> +impl<T, L: IoLoc<T>> IntoIoVal<T, L> for (L, T) {

Missing inline annotations.

Best,
Gary

> +    fn into_io_val(self) -> (L, T) {
> +        self
> +    }
> +}
> +
>  /// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
>  /// can perform I/O operations on regions of memory.
>  ///
> @@ -463,6 +479,32 @@ fn try_write<T, L>(&self, location: L, value: T) -> Result
>          Ok(())
>      }
>  
> +    /// Generic fallible write of value bearing its location, with runtime bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Tuples carrying a location and a value can be used with this method:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_writes(io: &Mmio) -> Result {
> +    ///     // 32-bit write of value `1` at address `0x10`.
> +    ///     io.try_write_val((0x10, 1u32))
> +    /// }
> +    /// ```
> +    #[inline(always)]
> +    fn try_write_val<T, L, V>(&self, value: V) -> Result
> +    where
> +        L: IoLoc<T>,
> +        V: IntoIoVal<T, L>,
> +        Self: IoCapable<L::IoType>,
> +    {
> +        let (location, value) = value.into_io_val();
> +
> +        self.try_write(location, value)
> +    }
> +
>      /// Generic fallible update with runtime bounds check.
>      ///
>      /// Caution: this does not perform any synchronization. Race conditions can occur in case of
> @@ -559,6 +601,32 @@ fn write<T, L>(&self, location: L, value: T)
>          unsafe { self.io_write(io_value, address) }
>      }
>  
> +    /// Generic infallible write of value bearing its location, with compile-time bounds check.
> +    ///
> +    /// # Examples
> +    ///
> +    /// Tuples carrying a location and a value can be used with this method:
> +    ///
> +    /// ```no_run
> +    /// use kernel::io::{Io, Mmio};
> +    ///
> +    /// fn do_writes(io: &Mmio) {
> +    ///     // 32-bit write of value `1` at address `0x10`.
> +    ///     io.write_val((0x10, 1u32));
> +    /// }
> +    /// ```
> +    #[inline(always)]
> +    fn write_val<T, L, V>(&self, value: V)
> +    where
> +        L: IoLoc<T>,
> +        V: IntoIoVal<T, L>,
> +        Self: IoKnownSize + IoCapable<L::IoType>,
> +    {
> +        let (location, value) = value.into_io_val();
> +
> +        self.write(location, value)
> +    }
> +
>      /// Generic infallible update with compile-time bounds check.
>      ///
>      /// Caution: this does not perform any synchronization. Race conditions can occur in case of
Re: [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val`
Posted by Alexandre Courbot 1 month ago
On Tue Mar 10, 2026 at 12:30 AM JST, Gary Guo wrote:
> On Mon Mar 9, 2026 at 3:14 PM GMT, Alexandre Courbot wrote:
>> Some I/O types, like fixed address registers, carry their location
>> alongside their values. For these types, the regular `Io::write` method
>> can lead into repeating the location information twice: once to provide
>> the location itself, another time to build the value.
>>
>> Add a new `Io::write_val` convenience method that takes a single
>> argument implementing `IntoIoVal`, a trait that decomposes implementors
>> into a `(location, value)` tuple. This allows write operations on fixed
>> offset registers to be done while specifying their name only once.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  rust/kernel/io.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>
>> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
>> index ed6fab001a39..09a0fe06f201 100644
>> --- a/rust/kernel/io.rs
>> +++ b/rust/kernel/io.rs
>> @@ -216,6 +216,22 @@ fn offset(&self) -> usize {
>>  // Provide the ability to read any primitive type from a [`usize`].
>>  impl_usize_ioloc!(u8, u16, u32, u64);
>>  
>> +/// Trait implemented by items that contain both an I/O location and a value to assign to it.
>> +///
>> +/// Implementors can be used with [`Io::write_val`].
>> +pub trait IntoIoVal<T, L: IoLoc<T>> {
>
> Is this generics instead of associative types for some reason? If so, please
> mention it in the commit mesasge.

Associated types sound better in that situation, I don't think we can
have several implementors with different returned types for the same
value anyway.

>
>> +    /// Consumes `self` and returns a `(location, value)` tuple describing a valid I/O write
>> +    /// operation.
>> +    fn into_io_val(self) -> (L, T);
>> +}
>> +
>> +/// `(location, value)` tuples can be used as [`IntoIoVal`]s.
>> +impl<T, L: IoLoc<T>> IntoIoVal<T, L> for (L, T) {
>
> Missing inline annotations.

Fixed, thanks! (and elsewhere in this series as well)