Re: [PATCH v6 RESEND 5/7] rust: io: factor out MMIO read/write macros

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Nov 11, 2025 at 5:41 AM JST, Zhi Wang wrote:
> Refactor the existing MMIO accessors to use common call macros
> instead of inlining the bindings calls in each `define_{read,write}!`
> expansion.
>
> This factoring separates the common offset/bounds checks from the
> low-level call pattern, making it easier to add additional I/O accessor
> families.
>
> No functional change intended.
>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/io.rs | 110 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 73 insertions(+), 37 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 4d98d431b523..090d1b11a896 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -124,8 +124,34 @@ pub fn maxsize(&self) -> usize {
>  #[repr(transparent)]
>  pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
>  
> +macro_rules! call_mmio_read {
> +    (infallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        unsafe { bindings::$c_fn($addr as *const c_void) as $type }
> +    };
> +
> +    (fallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {{
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        Ok(unsafe { bindings::$c_fn($addr as *const c_void) as $type })
> +    }};
> +}
> +
> +macro_rules! call_mmio_write {
> +    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        unsafe { bindings::$c_fn($value, $addr as *mut c_void) }
> +    };
> +
> +    (fallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {{
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        unsafe { bindings::$c_fn($value, $addr as *mut c_void) };
> +        Ok(())
> +    }};
> +}

I understand the intent from the commit message, but this is starting to
look like an intricate maze of macro expansion and it might not be as
easy for first-time readers - could you add an explanatory doccomment
for these?

> +
>  macro_rules! define_read {
> -    (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident -> $type_name:ty) => {
> +    (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro:ident, $c_fn:ident ->
> +     $type_name:ty) => {
>          /// Read IO data from a given offset known at compile time.
>          ///
>          /// Bound checks are performed on compile time, hence if the offset is not known at compile
> @@ -135,12 +161,13 @@ macro_rules! define_read {
>          $vis fn $name(&self, offset: usize) -> $type_name {
>              let addr = self.io_addr_assert::<$type_name>(offset);
>  
> -            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> -            unsafe { bindings::$c_fn(addr as *const c_void) }
> +            // SAFETY: By the type invariant `addr` is a valid address for IO operations.
> +            $call_macro!(infallible, $c_fn, self, $type_name, addr)
>          }
>      };

To convey the fact that `$c_fn` is passed to `$call_macro`, how about
changing the syntax to something like 

  `define_read(infallible, $vis $name $call_macro($c_fn) -> $type_name`

?