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

["Danilo Krummrich" <dakr@kernel.org>]

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)
> +    }