The previous Io<SIZE> type combined both the generic I/O access helpers
and MMIO implementation details in a single struct.

To establish a cleaner layering between the I/O interface and its concrete
backends, paving the way for supporting additional I/O mechanisms in the
future, Io<SIZE> need to be factored.

Factor the common helpers into a new Io trait, and moves the MMIO-specific
logic into a dedicated Mmio<SIZE> type implementing that trait. Rename the
IoRaw to MmioRaw and pdate the bus MMIO implementations to use MmioRaw.

No functional change intended.

Cc: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 drivers/gpu/nova-core/regs/macros.rs | 36 +++++------
 rust/kernel/io.rs                    | 89 +++++++++++++++++-----------
 rust/kernel/io/mem.rs                | 16 ++---
 rust/kernel/pci.rs                   | 10 ++--
 4 files changed, 87 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
index 8058e1696df9..c2a6547d58cd 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -609,7 +609,7 @@ impl $name {
             /// Read the register from its address in `io`.
             #[inline(always)]
             pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
             {
                 Self(io.read32($offset))
             }
@@ -617,7 +617,7 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
             /// Write the value contained in `self` to the register address in `io`.
             #[inline(always)]
             pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
             {
                 io.write32(self.0, $offset)
             }
@@ -629,7 +629,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
                 io: &T,
                 f: F,
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 F: ::core::ops::FnOnce(Self) -> Self,
             {
                 let reg = f(Self::read(io));
@@ -652,7 +652,7 @@ pub(crate) fn read<const SIZE: usize, T, B>(
                 #[allow(unused_variables)]
                 base: &B,
             ) -> Self where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
             {
                 const OFFSET: usize = $name::OFFSET;
@@ -673,7 +673,7 @@ pub(crate) fn write<const SIZE: usize, T, B>(
                 #[allow(unused_variables)]
                 base: &B,
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
             {
                 const OFFSET: usize = $name::OFFSET;
@@ -693,7 +693,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
                 base: &B,
                 f: F,
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
                 F: ::core::ops::FnOnce(Self) -> Self,
             {
@@ -717,7 +717,7 @@ pub(crate) fn read<const SIZE: usize, T>(
                 io: &T,
                 idx: usize,
             ) -> Self where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
             {
                 build_assert!(idx < Self::SIZE);
 
@@ -734,7 +734,7 @@ pub(crate) fn write<const SIZE: usize, T>(
                 io: &T,
                 idx: usize
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
             {
                 build_assert!(idx < Self::SIZE);
 
@@ -751,7 +751,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
                 idx: usize,
                 f: F,
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 F: ::core::ops::FnOnce(Self) -> Self,
             {
                 let reg = f(Self::read(io, idx));
@@ -767,7 +767,7 @@ pub(crate) fn try_read<const SIZE: usize, T>(
                 io: &T,
                 idx: usize,
             ) -> ::kernel::error::Result<Self> where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
             {
                 if idx < Self::SIZE {
                     Ok(Self::read(io, idx))
@@ -786,7 +786,7 @@ pub(crate) fn try_write<const SIZE: usize, T>(
                 io: &T,
                 idx: usize,
             ) -> ::kernel::error::Result where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
             {
                 if idx < Self::SIZE {
                     Ok(self.write(io, idx))
@@ -806,7 +806,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, F>(
                 idx: usize,
                 f: F,
             ) -> ::kernel::error::Result where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 F: ::core::ops::FnOnce(Self) -> Self,
             {
                 if idx < Self::SIZE {
@@ -838,7 +838,7 @@ pub(crate) fn read<const SIZE: usize, T, B>(
                 base: &B,
                 idx: usize,
             ) -> Self where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
             {
                 build_assert!(idx < Self::SIZE);
@@ -860,7 +860,7 @@ pub(crate) fn write<const SIZE: usize, T, B>(
                 base: &B,
                 idx: usize
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
             {
                 build_assert!(idx < Self::SIZE);
@@ -881,7 +881,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
                 idx: usize,
                 f: F,
             ) where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
                 F: ::core::ops::FnOnce(Self) -> Self,
             {
@@ -900,7 +900,7 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
                 base: &B,
                 idx: usize,
             ) -> ::kernel::error::Result<Self> where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
             {
                 if idx < Self::SIZE {
@@ -922,7 +922,7 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
                 base: &B,
                 idx: usize,
             ) -> ::kernel::error::Result where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
             {
                 if idx < Self::SIZE {
@@ -945,7 +945,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
                 idx: usize,
                 f: F,
             ) -> ::kernel::error::Result where
-                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
                 B: crate::regs::macros::RegisterBase<$base>,
                 F: ::core::ops::FnOnce(Self) -> Self,
             {
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index ee182b0b5452..78413dc7ffcc 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -18,16 +18,16 @@
 /// By itself, the existence of an instance of this structure does not provide any guarantees that
 /// the represented MMIO region does exist or is properly mapped.
 ///
-/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
-/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
-/// any guarantees are given.
-pub struct IoRaw<const SIZE: usize = 0> {
+/// Instead, the bus specific MMIO implementation must convert this raw representation into an
+/// `Mmio` instance providing the actual memory accessors. Only by the conversion into an `Mmio`
+/// structure any guarantees are given.
+pub struct MmioRaw<const SIZE: usize = 0> {
     addr: usize,
     maxsize: usize,
 }
 
-impl<const SIZE: usize> IoRaw<SIZE> {
-    /// Returns a new `IoRaw` instance on success, an error otherwise.
+impl<const SIZE: usize> MmioRaw<SIZE> {
+    /// Returns a new `MmioRaw` instance on success, an error otherwise.
     pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
         if maxsize < SIZE {
             return Err(EINVAL);
@@ -62,11 +62,11 @@ pub fn maxsize(&self) -> usize {
 /// # Examples
 ///
 /// ```no_run
-/// # use kernel::{bindings, ffi::c_void, io::{Io, IoRaw}};
+/// # use kernel::{bindings, ffi::c_void, io::{Mmio, MmioRaw}};
 /// # use core::ops::Deref;
 ///
 /// // See also [`pci::Bar`] for a real example.
-/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>);
 ///
 /// impl<const SIZE: usize> IoMem<SIZE> {
 ///     /// # Safety
@@ -81,7 +81,7 @@ pub fn maxsize(&self) -> usize {
 ///             return Err(ENOMEM);
 ///         }
 ///
-///         Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+///         Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?))
 ///     }
 /// }
 ///
@@ -93,11 +93,11 @@ pub fn maxsize(&self) -> usize {
 /// }
 ///
 /// impl<const SIZE: usize> Deref for IoMem<SIZE> {
-///    type Target = Io<SIZE>;
+///    type Target = Mmio<SIZE>;
 ///
 ///    fn deref(&self) -> &Self::Target {
 ///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
-///         unsafe { Io::from_raw(&self.0) }
+///         unsafe { Mmio::from_raw(&self.0) }
 ///    }
 /// }
 ///
@@ -111,7 +111,7 @@ pub fn maxsize(&self) -> usize {
 /// # }
 /// ```
 #[repr(transparent)]
-pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
+pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
 
 macro_rules! define_read {
     ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident -> $type_name:ty) => {
@@ -172,32 +172,24 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
     };
 }
 
-impl<const SIZE: usize> Io<SIZE> {
-    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
-    ///
-    /// # Safety
-    ///
-    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
-    /// `maxsize`.
-    pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
-        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
-        unsafe { &*core::ptr::from_ref(raw).cast() }
-    }
-
+/// 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.
+///
+/// Types implementing this trait (e.g. MMIO BARs or PCI config
+/// regions) can share the same accessors.
+pub trait Io<const SIZE: usize> {
     /// Returns the base address of this mapping.
-    #[inline]
-    pub fn addr(&self) -> usize {
-        self.0.addr()
-    }
+    fn addr(&self) -> usize;
 
     /// Returns the maximum size of this mapping.
-    #[inline]
-    pub fn maxsize(&self) -> usize {
-        self.0.maxsize()
-    }
+    fn maxsize(&self) -> usize;
 
+    /// Checks whether an access of type `U` at the given `offset`
+    /// is valid within this region.
     #[inline]
-    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
+    fn offset_valid<U>(offset: usize, size: usize) -> bool {
         let type_size = core::mem::size_of::<U>();
         if let Some(end) = offset.checked_add(type_size) {
             end <= size && offset % type_size == 0
@@ -206,6 +198,8 @@ const fn offset_valid<U>(offset: usize, size: usize) -> bool {
         }
     }
 
+    /// Returns the absolute I/O address for a given `offset`.
+    /// Performs runtime bounds checks using [`offset_valid`]
     #[inline]
     fn io_addr<U>(&self, offset: usize) -> Result<usize> {
         if !Self::offset_valid::<U>(offset, self.maxsize()) {
@@ -217,12 +211,41 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
         self.addr().checked_add(offset).ok_or(EINVAL)
     }
 
+    /// Returns the absolute I/O address for a given `offset`,
+    /// performing compile-time bound checks.
     #[inline]
     fn io_addr_assert<U>(&self, offset: usize) -> usize {
         build_assert!(Self::offset_valid::<U>(offset, SIZE));
 
         self.addr() + offset
     }
+}
+
+impl<const SIZE: usize> Io<SIZE> for Mmio<SIZE> {
+    /// Returns the base address of this mapping.
+    #[inline]
+    fn addr(&self) -> usize {
+        self.0.addr()
+    }
+
+    /// Returns the maximum size of this mapping.
+    #[inline]
+    fn maxsize(&self) -> usize {
+        self.0.maxsize()
+    }
+}
+
+impl<const SIZE: usize> Mmio<SIZE> {
+    /// Converts an `MmioRaw` into an `Mmio` instance, providing the accessors to the MMIO mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
+    /// `maxsize`.
+    pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self {
+        // SAFETY: `Mmio` is a transparent wrapper around `MmioRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
+    }
 
     define_read!(read8, try_read8, readb -> u8);
     define_read!(read16, try_read16, readw -> u16);
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
index 6f99510bfc3a..93cad8539b18 100644
--- a/rust/kernel/io/mem.rs
+++ b/rust/kernel/io/mem.rs
@@ -11,8 +11,8 @@
 use crate::io;
 use crate::io::resource::Region;
 use crate::io::resource::Resource;
-use crate::io::Io;
-use crate::io::IoRaw;
+use crate::io::Mmio;
+use crate::io::MmioRaw;
 use crate::prelude::*;
 
 /// An IO request for a specific device and resource.
@@ -195,7 +195,7 @@ pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit<Devres<Self>, Error> +
 }
 
 impl<const SIZE: usize> Deref for ExclusiveIoMem<SIZE> {
-    type Target = Io<SIZE>;
+    type Target = Mmio<SIZE>;
 
     fn deref(&self) -> &Self::Target {
         &self.iomem
@@ -209,10 +209,10 @@ fn deref(&self) -> &Self::Target {
 ///
 /// # Invariants
 ///
-/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the
+/// [`IoMem`] always holds an [`MmioRaw`] instance that holds a valid pointer to the
 /// start of the I/O memory mapped region.
 pub struct IoMem<const SIZE: usize = 0> {
-    io: IoRaw<SIZE>,
+    io: MmioRaw<SIZE>,
 }
 
 impl<const SIZE: usize> IoMem<SIZE> {
@@ -247,7 +247,7 @@ fn ioremap(resource: &Resource) -> Result<Self> {
             return Err(ENOMEM);
         }
 
-        let io = IoRaw::new(addr as usize, size)?;
+        let io = MmioRaw::new(addr as usize, size)?;
         let io = IoMem { io };
 
         Ok(io)
@@ -270,10 +270,10 @@ fn drop(&mut self) {
 }
 
 impl<const SIZE: usize> Deref for IoMem<SIZE> {
-    type Target = Io<SIZE>;
+    type Target = Mmio<SIZE>;
 
     fn deref(&self) -> &Self::Target {
         // SAFETY: Safe as by the invariant of `IoMem`.
-        unsafe { Io::from_raw(&self.io) }
+        unsafe { Mmio::from_raw(&self.io) }
     }
 }
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 7fcc5f6022c1..77a8eb39ad32 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -10,7 +10,7 @@
     devres::Devres,
     driver,
     error::{from_result, to_result, Result},
-    io::{Io, IoRaw},
+    io::{Mmio, MmioRaw},
     irq::{self, IrqRequest},
     str::CStr,
     sync::aref::ARef,
@@ -313,7 +313,7 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
 /// memory mapped PCI bar and its size.
 pub struct Bar<const SIZE: usize = 0> {
     pdev: ARef<Device>,
-    io: IoRaw<SIZE>,
+    io: MmioRaw<SIZE>,
     num: i32,
 }
 
@@ -349,7 +349,7 @@ fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
             return Err(ENOMEM);
         }
 
-        let io = match IoRaw::new(ioptr, len as usize) {
+        let io = match MmioRaw::new(ioptr, len as usize) {
             Ok(io) => io,
             Err(err) => {
                 // SAFETY:
@@ -403,11 +403,11 @@ fn drop(&mut self) {
 }
 
 impl<const SIZE: usize> Deref for Bar<SIZE> {
-    type Target = Io<SIZE>;
+    type Target = Mmio<SIZE>;
 
     fn deref(&self) -> &Self::Target {
         // SAFETY: By the type invariant of `Self`, the MMIO range in `self.io` is properly mapped.
-        unsafe { Io::from_raw(&self.io) }
+        unsafe { Mmio::from_raw(&self.io) }
     }
 }
 
-- 
2.47.3

Hi Zhi,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.18-rc2 next-20251020]
[cannot apply to driver-core/driver-core-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhi-Wang/rust-io-factor-common-I-O-helpers-into-Io-trait-and-specialize-Mmio-SIZE/20251017-050553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20251016210250.15932-2-zhiw%40nvidia.com
patch subject: [PATCH v2 1/5] rust/io: factor common I/O helpers into Io trait and specialize Mmio<SIZE>
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20251021/202510210730.qW10Mhd0-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251021/202510210730.qW10Mhd0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510210730.qW10Mhd0-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0432]: unresolved import `kernel::io::IoRaw`
   --> rust/doctests_kernel_generated.rs:5047:74
   |
   5047 | use kernel::{bindings, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}};
   |                                                                          ^^^^^ no `IoRaw` in `io`
--
>> error[E0782]: expected a type, found a trait
   --> rust/doctests_kernel_generated.rs:7075:46
   |
   7075 | fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
   |                                              ^^^^^^^^
   |
   = note: `Io<SIZE>` is dyn-incompatible, otherwise a trait object could be used
   help: use a new generic type parameter, constrained by `Io<SIZE>`
   |
   7075 - fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
   7075 + fn wait_for_hardware<const SIZE: usize, T: Io<SIZE>>(io: &T) -> Result<()> {
   |
   help: you can also use an opaque type, but users won't be able to specify the type parameter when calling the `fn`, having to rely exclusively on type inference
   |
   7075 | fn wait_for_hardware<const SIZE: usize>(io: &impl Io<SIZE>) -> Result<()> {
   |                                              ++++
--
>> error[E0782]: expected a type, found a trait
   --> rust/doctests_kernel_generated.rs:5078:18
   |
   5078 |    type Target = Io<SIZE>;
   |                  ^^^^^^^^
--
>> error[E0782]: expected a type, found a trait
   --> rust/doctests_kernel_generated.rs:5082:18
   |
   5082 |         unsafe { Io::from_raw(&self.0) }
   |                  ^^
   |
   help: you can add the `dyn` keyword if you want a trait object
   |
   5082 |         unsafe { <dyn Io>::from_raw(&self.0) }
   |                  ++++   +

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On 10/16/25 2:02 PM, Zhi Wang wrote:
> The previous Io<SIZE> type combined both the generic I/O access helpers
> and MMIO implementation details in a single struct.
> 
> To establish a cleaner layering between the I/O interface and its concrete
> backends, paving the way for supporting additional I/O mechanisms in the
> future, Io<SIZE> need to be factored.
> 
> Factor the common helpers into a new Io trait, and moves the MMIO-specific
> logic into a dedicated Mmio<SIZE> type implementing that trait. Rename the
> IoRaw to MmioRaw and pdate the bus MMIO implementations to use MmioRaw.
> 
> No functional change intended.
> 
> Cc: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  drivers/gpu/nova-core/regs/macros.rs | 36 +++++------
>  rust/kernel/io.rs                    | 89 +++++++++++++++++-----------
>  rust/kernel/io/mem.rs                | 16 ++---
>  rust/kernel/pci.rs                   | 10 ++--
>  4 files changed, 87 insertions(+), 64 deletions(-)

Hi Zhi,

This fails to build for me. Looking closer, I see that the doctests are
failing, which implies that you must not be building them. 

Can I suggest that you set this, so as to avoid this in the future:

    CONFIG_RUST_KERNEL_DOCTESTS=y

And here is a diff that fixes the build for me, with that config
option set:

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 10a6a1789854..d35cd39e32bf 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -52,11 +52,11 @@ struct Inner<T: Send> {
 /// # Examples
 ///
 /// ```no_run
-/// # use kernel::{bindings, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, device::{Bound, Device}, devres::Devres, io::{Mmio, MmioRaw}};
 /// # use core::ops::Deref;
 ///
 /// // See also [`pci::Bar`] for a real example.
-/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>);
 ///
 /// impl<const SIZE: usize> IoMem<SIZE> {
 ///     /// # Safety
@@ -71,7 +71,7 @@ struct Inner<T: Send> {
 ///             return Err(ENOMEM);
 ///         }
 ///
-///         Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+///         Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?))
 ///     }
 /// }
 ///
@@ -83,11 +83,11 @@ struct Inner<T: Send> {
 /// }
 ///
 /// impl<const SIZE: usize> Deref for IoMem<SIZE> {
-///    type Target = Io<SIZE>;
+///    type Target = Mmio<SIZE>;
 ///
 ///    fn deref(&self) -> &Self::Target {
 ///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
-///         unsafe { Io::from_raw(&self.0) }
+///         unsafe { Mmio::from_raw(&self.0) }
 ///    }
 /// }
 /// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> {
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
index 613eb25047ef..835599085339 100644
--- a/rust/kernel/io/poll.rs
+++ b/rust/kernel/io/poll.rs
@@ -37,12 +37,12 @@
 /// # Examples
 ///
 /// ```no_run
-/// use kernel::io::{Io, poll::read_poll_timeout};
+/// use kernel::io::{Mmio, poll::read_poll_timeout};
 /// 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<()> {
 ///     match read_poll_timeout(
 ///         // The `op` closure reads the value of a specific status register.
 ///         || io.try_read16(0x1000),



thanks,
-- 
John Hubbard

> 
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
> index 8058e1696df9..c2a6547d58cd 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/drivers/gpu/nova-core/regs/macros.rs
> @@ -609,7 +609,7 @@ impl $name {
>              /// Read the register from its address in `io`.
>              #[inline(always)]
>              pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>              {
>                  Self(io.read32($offset))
>              }
> @@ -617,7 +617,7 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
>              /// Write the value contained in `self` to the register address in `io`.
>              #[inline(always)]
>              pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>              {
>                  io.write32(self.0, $offset)
>              }
> @@ -629,7 +629,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
>                  io: &T,
>                  f: F,
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  F: ::core::ops::FnOnce(Self) -> Self,
>              {
>                  let reg = f(Self::read(io));
> @@ -652,7 +652,7 @@ pub(crate) fn read<const SIZE: usize, T, B>(
>                  #[allow(unused_variables)]
>                  base: &B,
>              ) -> Self where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>              {
>                  const OFFSET: usize = $name::OFFSET;
> @@ -673,7 +673,7 @@ pub(crate) fn write<const SIZE: usize, T, B>(
>                  #[allow(unused_variables)]
>                  base: &B,
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>              {
>                  const OFFSET: usize = $name::OFFSET;
> @@ -693,7 +693,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
>                  base: &B,
>                  f: F,
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>                  F: ::core::ops::FnOnce(Self) -> Self,
>              {
> @@ -717,7 +717,7 @@ pub(crate) fn read<const SIZE: usize, T>(
>                  io: &T,
>                  idx: usize,
>              ) -> Self where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>              {
>                  build_assert!(idx < Self::SIZE);
>  
> @@ -734,7 +734,7 @@ pub(crate) fn write<const SIZE: usize, T>(
>                  io: &T,
>                  idx: usize
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>              {
>                  build_assert!(idx < Self::SIZE);
>  
> @@ -751,7 +751,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
>                  idx: usize,
>                  f: F,
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  F: ::core::ops::FnOnce(Self) -> Self,
>              {
>                  let reg = f(Self::read(io, idx));
> @@ -767,7 +767,7 @@ pub(crate) fn try_read<const SIZE: usize, T>(
>                  io: &T,
>                  idx: usize,
>              ) -> ::kernel::error::Result<Self> where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>              {
>                  if idx < Self::SIZE {
>                      Ok(Self::read(io, idx))
> @@ -786,7 +786,7 @@ pub(crate) fn try_write<const SIZE: usize, T>(
>                  io: &T,
>                  idx: usize,
>              ) -> ::kernel::error::Result where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>              {
>                  if idx < Self::SIZE {
>                      Ok(self.write(io, idx))
> @@ -806,7 +806,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, F>(
>                  idx: usize,
>                  f: F,
>              ) -> ::kernel::error::Result where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  F: ::core::ops::FnOnce(Self) -> Self,
>              {
>                  if idx < Self::SIZE {
> @@ -838,7 +838,7 @@ pub(crate) fn read<const SIZE: usize, T, B>(
>                  base: &B,
>                  idx: usize,
>              ) -> Self where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>              {
>                  build_assert!(idx < Self::SIZE);
> @@ -860,7 +860,7 @@ pub(crate) fn write<const SIZE: usize, T, B>(
>                  base: &B,
>                  idx: usize
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>              {
>                  build_assert!(idx < Self::SIZE);
> @@ -881,7 +881,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
>                  idx: usize,
>                  f: F,
>              ) where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>                  F: ::core::ops::FnOnce(Self) -> Self,
>              {
> @@ -900,7 +900,7 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
>                  base: &B,
>                  idx: usize,
>              ) -> ::kernel::error::Result<Self> where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>              {
>                  if idx < Self::SIZE {
> @@ -922,7 +922,7 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
>                  base: &B,
>                  idx: usize,
>              ) -> ::kernel::error::Result where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>              {
>                  if idx < Self::SIZE {
> @@ -945,7 +945,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
>                  idx: usize,
>                  f: F,
>              ) -> ::kernel::error::Result where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,
>                  B: crate::regs::macros::RegisterBase<$base>,
>                  F: ::core::ops::FnOnce(Self) -> Self,
>              {
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index ee182b0b5452..78413dc7ffcc 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -18,16 +18,16 @@
>  /// By itself, the existence of an instance of this structure does not provide any guarantees that
>  /// the represented MMIO region does exist or is properly mapped.
>  ///
> -/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
> -/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
> -/// any guarantees are given.
> -pub struct IoRaw<const SIZE: usize = 0> {
> +/// Instead, the bus specific MMIO implementation must convert this raw representation into an
> +/// `Mmio` instance providing the actual memory accessors. Only by the conversion into an `Mmio`
> +/// structure any guarantees are given.
> +pub struct MmioRaw<const SIZE: usize = 0> {
>      addr: usize,
>      maxsize: usize,
>  }
>  
> -impl<const SIZE: usize> IoRaw<SIZE> {
> -    /// Returns a new `IoRaw` instance on success, an error otherwise.
> +impl<const SIZE: usize> MmioRaw<SIZE> {
> +    /// Returns a new `MmioRaw` instance on success, an error otherwise.
>      pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
>          if maxsize < SIZE {
>              return Err(EINVAL);
> @@ -62,11 +62,11 @@ pub fn maxsize(&self) -> usize {
>  /// # Examples
>  ///
>  /// ```no_run
> -/// # use kernel::{bindings, ffi::c_void, io::{Io, IoRaw}};
> +/// # use kernel::{bindings, ffi::c_void, io::{Mmio, MmioRaw}};
>  /// # use core::ops::Deref;
>  ///
>  /// // See also [`pci::Bar`] for a real example.
> -/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
> +/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>);
>  ///
>  /// impl<const SIZE: usize> IoMem<SIZE> {
>  ///     /// # Safety
> @@ -81,7 +81,7 @@ pub fn maxsize(&self) -> usize {
>  ///             return Err(ENOMEM);
>  ///         }
>  ///
> -///         Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
> +///         Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?))
>  ///     }
>  /// }
>  ///
> @@ -93,11 +93,11 @@ pub fn maxsize(&self) -> usize {
>  /// }
>  ///
>  /// impl<const SIZE: usize> Deref for IoMem<SIZE> {
> -///    type Target = Io<SIZE>;
> +///    type Target = Mmio<SIZE>;
>  ///
>  ///    fn deref(&self) -> &Self::Target {
>  ///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
> -///         unsafe { Io::from_raw(&self.0) }
> +///         unsafe { Mmio::from_raw(&self.0) }
>  ///    }
>  /// }
>  ///
> @@ -111,7 +111,7 @@ pub fn maxsize(&self) -> usize {
>  /// # }
>  /// ```
>  #[repr(transparent)]
> -pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
> +pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
>  
>  macro_rules! define_read {
>      ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident -> $type_name:ty) => {
> @@ -172,32 +172,24 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
>      };
>  }
>  
> -impl<const SIZE: usize> Io<SIZE> {
> -    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
> -    ///
> -    /// # Safety
> -    ///
> -    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
> -    /// `maxsize`.
> -    pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
> -        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
> -        unsafe { &*core::ptr::from_ref(raw).cast() }
> -    }
> -
> +/// 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.
> +///
> +/// Types implementing this trait (e.g. MMIO BARs or PCI config
> +/// regions) can share the same accessors.
> +pub trait Io<const SIZE: usize> {
>      /// Returns the base address of this mapping.
> -    #[inline]
> -    pub fn addr(&self) -> usize {
> -        self.0.addr()
> -    }
> +    fn addr(&self) -> usize;
>  
>      /// Returns the maximum size of this mapping.
> -    #[inline]
> -    pub fn maxsize(&self) -> usize {
> -        self.0.maxsize()
> -    }
> +    fn maxsize(&self) -> usize;
>  
> +    /// Checks whether an access of type `U` at the given `offset`
> +    /// is valid within this region.
>      #[inline]
> -    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
> +    fn offset_valid<U>(offset: usize, size: usize) -> bool {
>          let type_size = core::mem::size_of::<U>();
>          if let Some(end) = offset.checked_add(type_size) {
>              end <= size && offset % type_size == 0
> @@ -206,6 +198,8 @@ const fn offset_valid<U>(offset: usize, size: usize) -> bool {
>          }
>      }
>  
> +    /// Returns the absolute I/O address for a given `offset`.
> +    /// Performs runtime bounds checks using [`offset_valid`]
>      #[inline]
>      fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>          if !Self::offset_valid::<U>(offset, self.maxsize()) {
> @@ -217,12 +211,41 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>          self.addr().checked_add(offset).ok_or(EINVAL)
>      }
>  
> +    /// Returns the absolute I/O address for a given `offset`,
> +    /// performing compile-time bound checks.
>      #[inline]
>      fn io_addr_assert<U>(&self, offset: usize) -> usize {
>          build_assert!(Self::offset_valid::<U>(offset, SIZE));
>  
>          self.addr() + offset
>      }
> +}
> +
> +impl<const SIZE: usize> Io<SIZE> for Mmio<SIZE> {
> +    /// Returns the base address of this mapping.
> +    #[inline]
> +    fn addr(&self) -> usize {
> +        self.0.addr()
> +    }
> +
> +    /// Returns the maximum size of this mapping.
> +    #[inline]
> +    fn maxsize(&self) -> usize {
> +        self.0.maxsize()
> +    }
> +}
> +
> +impl<const SIZE: usize> Mmio<SIZE> {
> +    /// Converts an `MmioRaw` into an `Mmio` instance, providing the accessors to the MMIO mapping.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
> +    /// `maxsize`.
> +    pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self {
> +        // SAFETY: `Mmio` is a transparent wrapper around `MmioRaw`.
> +        unsafe { &*core::ptr::from_ref(raw).cast() }
> +    }
>  
>      define_read!(read8, try_read8, readb -> u8);
>      define_read!(read16, try_read16, readw -> u16);
> diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
> index 6f99510bfc3a..93cad8539b18 100644
> --- a/rust/kernel/io/mem.rs
> +++ b/rust/kernel/io/mem.rs
> @@ -11,8 +11,8 @@
>  use crate::io;
>  use crate::io::resource::Region;
>  use crate::io::resource::Resource;
> -use crate::io::Io;
> -use crate::io::IoRaw;
> +use crate::io::Mmio;
> +use crate::io::MmioRaw;
>  use crate::prelude::*;
>  
>  /// An IO request for a specific device and resource.
> @@ -195,7 +195,7 @@ pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit<Devres<Self>, Error> +
>  }
>  
>  impl<const SIZE: usize> Deref for ExclusiveIoMem<SIZE> {
> -    type Target = Io<SIZE>;
> +    type Target = Mmio<SIZE>;
>  
>      fn deref(&self) -> &Self::Target {
>          &self.iomem
> @@ -209,10 +209,10 @@ fn deref(&self) -> &Self::Target {
>  ///
>  /// # Invariants
>  ///
> -/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the
> +/// [`IoMem`] always holds an [`MmioRaw`] instance that holds a valid pointer to the
>  /// start of the I/O memory mapped region.
>  pub struct IoMem<const SIZE: usize = 0> {
> -    io: IoRaw<SIZE>,
> +    io: MmioRaw<SIZE>,
>  }
>  
>  impl<const SIZE: usize> IoMem<SIZE> {
> @@ -247,7 +247,7 @@ fn ioremap(resource: &Resource) -> Result<Self> {
>              return Err(ENOMEM);
>          }
>  
> -        let io = IoRaw::new(addr as usize, size)?;
> +        let io = MmioRaw::new(addr as usize, size)?;
>          let io = IoMem { io };
>  
>          Ok(io)
> @@ -270,10 +270,10 @@ fn drop(&mut self) {
>  }
>  
>  impl<const SIZE: usize> Deref for IoMem<SIZE> {
> -    type Target = Io<SIZE>;
> +    type Target = Mmio<SIZE>;
>  
>      fn deref(&self) -> &Self::Target {
>          // SAFETY: Safe as by the invariant of `IoMem`.
> -        unsafe { Io::from_raw(&self.io) }
> +        unsafe { Mmio::from_raw(&self.io) }
>      }
>  }
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 7fcc5f6022c1..77a8eb39ad32 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -10,7 +10,7 @@
>      devres::Devres,
>      driver,
>      error::{from_result, to_result, Result},
> -    io::{Io, IoRaw},
> +    io::{Mmio, MmioRaw},
>      irq::{self, IrqRequest},
>      str::CStr,
>      sync::aref::ARef,
> @@ -313,7 +313,7 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>  /// memory mapped PCI bar and its size.
>  pub struct Bar<const SIZE: usize = 0> {
>      pdev: ARef<Device>,
> -    io: IoRaw<SIZE>,
> +    io: MmioRaw<SIZE>,
>      num: i32,
>  }
>  
> @@ -349,7 +349,7 @@ fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
>              return Err(ENOMEM);
>          }
>  
> -        let io = match IoRaw::new(ioptr, len as usize) {
> +        let io = match MmioRaw::new(ioptr, len as usize) {
>              Ok(io) => io,
>              Err(err) => {
>                  // SAFETY:
> @@ -403,11 +403,11 @@ fn drop(&mut self) {
>  }
>  
>  impl<const SIZE: usize> Deref for Bar<SIZE> {
> -    type Target = Io<SIZE>;
> +    type Target = Mmio<SIZE>;
>  
>      fn deref(&self) -> &Self::Target {
>          // SAFETY: By the type invariant of `Self`, the MMIO range in `self.io` is properly mapped.
> -        unsafe { Io::from_raw(&self.io) }
> +        unsafe { Mmio::from_raw(&self.io) }
>      }
>  }
>

On Fri Oct 17, 2025 at 6:02 AM JST, Zhi Wang wrote:
<snip>
> +/// 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.
> +///
> +/// Types implementing this trait (e.g. MMIO BARs or PCI config
> +/// regions) can share the same accessors.
> +pub trait Io<const SIZE: usize> {
>      /// Returns the base address of this mapping.
> -    #[inline]
> -    pub fn addr(&self) -> usize {
> -        self.0.addr()
> -    }
> +    fn addr(&self) -> usize;
>  
>      /// Returns the maximum size of this mapping.
> -    #[inline]
> -    pub fn maxsize(&self) -> usize {
> -        self.0.maxsize()
> -    }
> +    fn maxsize(&self) -> usize;
>  
> +    /// Checks whether an access of type `U` at the given `offset`
> +    /// is valid within this region.
>      #[inline]
> -    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
> +    fn offset_valid<U>(offset: usize, size: usize) -> bool {

Is it ok to lose the `const` attribute here?

Since this method doesn't take `self`, and is not supposed to be
overloaded anyway, I'd suggest moving it out of the trait and turning it
into a private function of this module.

On Thu Oct 16, 2025 at 11:02 PM CEST, Zhi Wang wrote:
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
> index 8058e1696df9..c2a6547d58cd 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/drivers/gpu/nova-core/regs/macros.rs
> @@ -609,7 +609,7 @@ impl $name {
>              /// Read the register from its address in `io`.
>              #[inline(always)]
>              pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
> -                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
> +                T: ::core::ops::Deref<Target = ::kernel::io::Mmio<SIZE>>,

This should be

	T: ::core::ops::Deref<Target = I>,
	I: ::kernel::io::Io<SIZE>,

instead, otherwise register!() only works for MMIO, but it should work for any
I/O backend.

> +impl<const SIZE: usize> Io<SIZE> for Mmio<SIZE> {
> +    /// Returns the base address of this mapping.
> +    #[inline]
> +    fn addr(&self) -> usize {
> +        self.0.addr()
> +    }
> +
> +    /// Returns the maximum size of this mapping.
> +    #[inline]
> +    fn maxsize(&self) -> usize {
> +        self.0.maxsize()
> +    }
> +}

The I/O trait should contain the corresponding read/write accessors, otherwise
we can't easily wire up the register!() macro with arbitrary I/O backends.

I think more specific things, such as relaxed operations can remain MMIO
specific, but all the {try_}{read,write}{8,16,32,64} accessors should be part of
the I/O trait.

On Thu, Oct 16, 2025 at 09:02:46PM +0000, Zhi Wang wrote:
> The previous Io<SIZE> type combined both the generic I/O access helpers
> and MMIO implementation details in a single struct.
> 
> To establish a cleaner layering between the I/O interface and its concrete
> backends, paving the way for supporting additional I/O mechanisms in the
> future, Io<SIZE> need to be factored.
> 
> Factor the common helpers into a new Io trait, and moves the MMIO-specific
> logic into a dedicated Mmio<SIZE> type implementing that trait. Rename the
> IoRaw to MmioRaw and pdate the bus MMIO implementations to use MmioRaw.

s/and moves/and move/ to match "Factor" and "Rename"
s/pdate/update/ ?

> +/// Instead, the bus specific MMIO implementation must convert this raw representation into an
> +/// `Mmio` instance providing the actual memory accessors. Only by the conversion into an `Mmio`
> +/// structure any guarantees are given.

s/any guarantees are given/are any guarantees given/

> +    /// Returns a new `MmioRaw` instance ...
> +/// Provides common helpers ...
> +    /// Checks whether an access ...

There's a Linux trend toward using "imperative mood", e.g., "Return",
for commit logs and comments, but I notice Rust code more often seems
to use the indicative mood: "Returns", "Provides", "Checks", etc.

Maybe that's part of the Rust style; I dunno if that's intentional or
worth commenting on, just something I notice.

Bjorn