Re: [PATCH v8 2/5] rust: io: factor common I/O helpers into Io trait

Danilo Krummrich
2026-01-13T19:44:44Z

On Tue Jan 13, 2026 at 10:22 AM CET, Zhi Wang wrote:
> drivers/gpu/nova-core/gsp/sequencer.rs | 5 +-
> drivers/gpu/nova-core/regs/macros.rs | 90 ++++----
> drivers/gpu/nova-core/vbios.rs | 1 +
> rust/kernel/devres.rs | 14 +-
> rust/kernel/io.rs | 271 ++++++++++++++++++-------
> rust/kernel/io/mem.rs | 16 +-
> rust/kernel/io/poll.rs | 8 +-
> rust/kernel/pci/io.rs | 12 +-
> samples/rust/rust_driver_pci.rs | 2 +
> 9 files changed, 288 insertions(+), 131 deletions(-)

I think you did forget to update the Tyr driver.

> +/// Represents a region of I/O space of a fixed size.
> +///
> +/// Provides common helpers for offset validation and address
> +/// calculation on top of a base address and maximum size.

Maybe you can expand this a bit, i.e. explaining that this is a abstract
representation to be implemented by arbitrary I/O backends pointing out some
examples, such as MMIO, I2C, etc.

> +///

Spurious newline.

> +pub trait IoBase {

<snip>

> +/// Types implementing this trait (e.g. MMIO BARs or PCI config
> +/// regions) can share the same KnownSize accessors.

Please expand this a bit and add some more detailed explanation what "known
size" means in this context, i.e. it is the minimum size requested from the I/O
backend, hence we know that we are safe within this boundary.

However, the size of the requested I/O region might still be larger than the
"known size".

> +pub trait IoKnownSize: IoBase {
> + /// Infallible 8-bit read with compile-time bounds check.
> + fn read8(&self, offset: usize) -> u8;
> +
> + /// Infallible 16-bit read with compile-time bounds check.
> + fn read16(&self, offset: usize) -> u16;
> +
> + /// Infallible 32-bit read with compile-time bounds check.
> + fn read32(&self, offset: usize) -> u32;
> +
> + /// Infallible 8-bit write with compile-time bounds check.
> + fn write8(&self, value: u8, offset: usize);
> +
> + /// Infallible 16-bit write with compile-time bounds check.
> + fn write16(&self, value: u16, offset: usize);
> +
> + /// Infallible 32-bit write with compile-time bounds check.
> + fn write32(&self, value: u32, offset: usize);
> +}
> +
> +/// Types implementing this trait (e.g. MMIO BARs or PCI config
> +/// regions) can share the same Io accessors.

Same here, please add some more detail, e.g. how does it differ from
IoKnownSize, in which cases should it be used, etc.

> +pub trait Io: IoBase {
> + /// Fallible 8-bit read with runtime bounds check.
> + fn try_read8(&self, offset: usize) -> Result<u8>;
> +
> + /// Fallible 16-bit read with runtime bounds check.
> + fn try_read16(&self, offset: usize) -> Result<u16>;
> +
> + /// Fallible 32-bit read with runtime bounds check.
> + fn try_read32(&self, offset: usize) -> Result<u32>;
> +
> + /// Fallible 8-bit write with runtime bounds check.
> + fn try_write8(&self, value: u8, offset: usize) -> Result;
> +
> + /// Fallible 16-bit write with runtime bounds check.
> + fn try_write16(&self, value: u16, offset: usize) -> Result;
> +
> + /// Fallible 32-bit write with runtime bounds check.
> + fn try_write32(&self, value: u32, offset: usize) -> Result;
> +}
> +
> +/// Represents a region of I/O space of a fixed size with 64-bit accessors.
> +/// Types implementing this trait can share the same Infallible accessors.

Please refer to IoKnownSize for details.

> +#[cfg(CONFIG_64BIT)]
> +pub trait IoKnownSize64: IoKnownSize {
> + /// Infallible 64-bit read with compile-time bounds check.
> + fn read64(&self, offset: usize) -> u64;
>
> - define_read!(read8, try_read8, readb -> u8);
> - define_read!(read16, try_read16, readw -> u16);
> - define_read!(read32, try_read32, readl -> u32);
> + /// Infallible 64-bit write with compile-time bounds check.
> + fn write64(&self, value: u64, offset: usize);
> +}
> +
> +/// Types implementing this trait can share the same Fallible accessors.
> +#[cfg(CONFIG_64BIT)]
> +pub trait Io64: Io {

Please refer to Io for details.

> diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
> index b1a2570364f4..65d5a370ed14 100644
> --- a/rust/kernel/io/poll.rs
> +++ b/rust/kernel/io/poll.rs
> @@ -45,12 +45,12 @@
> /// # Examples
> ///
> /// ```no_run
> -/// use kernel::io::{Io, poll::read_poll_timeout};
> +/// use kernel::io::{Io, Mmio, poll::read_poll_timeout};

Please switch to vertical style while at it, here and below.

> /// use kernel::time::Delta;
> ///
> /// const HW_READY: u16 = 0x01;
> ///
> -/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result {
> +/// fn wait_for_hardware<const SIZE: usize>(io: &Mmio<SIZE>) -> Result {
> /// read_poll_timeout(
> /// // The `op` closure reads the value of a specific status register.
> /// || io.try_read16(0x1000),
> @@ -128,12 +128,12 @@ pub fn read_poll_timeout<Op, Cond, T>(
> /// # Examples
> ///
> /// ```no_run
> -/// use kernel::io::{poll::read_poll_timeout_atomic, Io};
> +/// use kernel::io::{poll::read_poll_timeout_atomic, Io, Mmio};
> /// use kernel::time::Delta;
> ///
> /// const HW_READY: u16 = 0x01;
> ///
> -/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result {
> +/// fn wait_for_hardware<const SIZE: usize>(io: &Mmio<SIZE>) -> Result {
> /// read_poll_timeout_atomic(
> /// // The `op` closure reads the value of a specific status register.
> /// || io.try_read16(0x1000),