Message-Id: <919702DB-9992-476C-882B-94443C97C0F0@collabora.com>
Date: Wed, 4 Feb 2026 12:24:34 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Miguel Ojeda <ojeda@...nel.org>,
 Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>,
 Trevor Gross <tmgross@...ch.edu>,
 Bjorn Helgaas <bhelgaas@...gle.com>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>,
 driver-core@...ts.linux.dev,
 rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org,
 Zhi Wang <zhiw@...dia.com>,
 Lyude Paul <lyude@...hat.com>,
 Eliot Courtney <ecourtney@...dia.com>
Subject: Re: [PATCH 5/6] rust: pci: io: remove overloaded Io methods of
 ConfigSpace



> On 2 Feb 2026, at 05:13, Alexandre Courbot <acourbot@...dia.com> wrote:
> 
> Since `ConfigSpace` now has the relevant implementations of `IoCapable`,
> the default methods of `Io` can be used in place of the overloaded ones.
> Remove them as well as the macros generating them.
> 
> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
> ---
> rust/kernel/io.rs     |  3 ---
> rust/kernel/pci/io.rs | 70 ---------------------------------------------------
> 2 files changed, 73 deletions(-)
> 
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index e1094a237ff7..0e8031dfe0f0 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -226,8 +226,6 @@ macro_rules! define_read {
>         }
>     };
> }
> -pub(crate) use define_read;
> -
> macro_rules! define_write {
>     (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro:ident($c_fn:ident) <-
>      $type_name:ty) => {
> @@ -259,7 +257,6 @@ macro_rules! define_write {
>         }
>     };
> }
> -pub(crate) use define_write;
> 
> /// Checks whether an access of type `U` at the given `offset`
> /// is valid within this region.
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index 8c8aab2e3f22..ae78676c927f 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -8,8 +8,6 @@
>     device,
>     devres::Devres,
>     io::{
> -        define_read,
> -        define_write,
>         Io,
>         IoCapable,
>         IoKnownSize,
> @@ -85,63 +83,6 @@ pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> {
>     _marker: PhantomData<S>,
> }
> 
> -/// Internal helper macros used to invoke C PCI configuration space read functions.
> -///
> -/// This macro is intended to be used by higher-level PCI configuration space access macros
> -/// (define_read) and provides a unified expansion for infallible vs. fallible read semantics. It
> -/// emits a direct call into the corresponding C helper and performs the required cast to the Rust
> -/// return type.
> -///
> -/// # Parameters
> -///
> -/// * `$c_fn` – The C function performing the PCI configuration space write.
> -/// * `$self` – The I/O backend object.
> -/// * `$ty` – The type of the value to read.
> -/// * `$addr` – The PCI configuration space offset to read.
> -///
> -/// This macro does not perform any validation; all invariants must be upheld by the higher-level
> -/// abstraction invoking it.
> -macro_rules! call_config_read {
> -    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{
> -        let mut val: $ty = 0;
> -        // SAFETY: By the type invariant `$self.pdev` is a valid address.
> -        // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset
> -        // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits
> -        // within `i32` without truncation or sign change.
> -        // Return value from C function is ignored in infallible accessors.
> -        let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) };
> -        val
> -    }};
> -}
> -
> -/// Internal helper macros used to invoke C PCI configuration space write functions.
> -///
> -/// This macro is intended to be used by higher-level PCI configuration space access macros
> -/// (define_write) and provides a unified expansion for infallible vs. fallible read semantics. It
> -/// emits a direct call into the corresponding C helper and performs the required cast to the Rust
> -/// return type.
> -///
> -/// # Parameters
> -///
> -/// * `$c_fn` – The C function performing the PCI configuration space write.
> -/// * `$self` – The I/O backend object.
> -/// * `$ty` – The type of the written value.
> -/// * `$addr` – The configuration space offset to write.
> -/// * `$value` – The value to write.
> -///
> -/// This macro does not perform any validation; all invariants must be upheld by the higher-level
> -/// abstraction invoking it.
> -macro_rules! call_config_write {
> -    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {
> -        // SAFETY: By the type invariant `$self.pdev` is a valid address.
> -        // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset
> -        // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits
> -        // within `i32` without truncation or sign change.
> -        // Return value from C function is ignored in infallible accessors.
> -        let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, $value) };
> -    };
> -}
> -
> /// Implements [`IoCapable`] on [`ConfigSpace`] for `$ty` using `$read_fn` and `$write_fn`.
> macro_rules! impl_config_space_io_capable {
>     ($ty:ty, $read_fn:ident, $write_fn:ident) => {
> @@ -190,17 +131,6 @@ fn addr(&self) -> usize {
>     fn maxsize(&self) -> usize {
>         self.pdev.cfg_size().into_raw()
>     }
> -
> -    // PCI configuration space does not support fallible operations.
> -    // The default implementations from the Io trait are not used.
> -
> -    define_read!(infallible, read8, call_config_read(pci_read_config_byte) -> u8);
> -    define_read!(infallible, read16, call_config_read(pci_read_config_word) -> u16);
> -    define_read!(infallible, read32, call_config_read(pci_read_config_dword) -> u32);
> -
> -    define_write!(infallible, write8, call_config_write(pci_write_config_byte) <- u8);
> -    define_write!(infallible, write16, call_config_write(pci_write_config_word) <- u16);
> -    define_write!(infallible, write32, call_config_write(pci_write_config_dword) <- u32);
> }
> 
> impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {
> 
> -- 
> 2.52.0
> 
> 


Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>

Powered by blists - more mailing lists