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

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

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),