Device / Driver PCI / Platform Rust abstractions

Implement the generic Registration type and the DriverOps trait.

The Registration structure is the common type that represents a driver registration and is typically bound to the lifetime of a module. However, it doesn't implement actual calls to the kernel's driver core to register drivers itself.

Instead the DriverOps trait is provided to subsystems, which have to implement DriverOps::register and DrvierOps::unregister. Subsystems

have to provide an implementation for both of those methods where the subsystem specific variants to register / unregister a driver have to implemented.

For instance, the PCI subsystem would call __pci_register_driver() from DriverOps::register and pci_unregister_driver() from DrvierOps::unregister.

Co-developed-by Wedson Almeida Filho <[email protected]>

Signed-off-by Wedson Almeida Filho <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines · Danilo Krummrich

Implement the generic Registration type and the DriverOps trait.

The Registration structure is the common type that represents a driver registration and is typically bound to the lifetime of a module. However, it doesn't implement actual calls to the kernel's driver core to register drivers itself.

Instead the DriverOps trait is provided to subsystems, which have to implement DriverOps::register and DrvierOps::unregister. Subsystems

typo

Quoted 11 lines

have to provide an implementation for both of those methods where the subsystem specific variants to register / unregister a driver have to implemented.

For instance, the PCI subsystem would call __pci_register_driver() from DriverOps::register and pci_unregister_driver() from DrvierOps::unregister.

Co-developed-by Wedson Almeida Filho <[email protected]>

Signed-off-by Wedson Almeida Filho <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

[...]

rust/kernel/driver.rs line 23

Quoted 11 lines

rust/kernel/driver.rs new file +11 -0

+/// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
+/// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
+/// unregister a driver of the particular type (`RegType`).
+///
+/// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call
+/// `bindings::__pci_register_driver` from `RegistrationOps::register` and
+/// `bindings::pci_unregister_driver` from `RegistrationOps::unregister`.
+pub trait RegistrationOps {
+    /// The type that holds information about the registration. This is typically a struct defined
+    /// by the C portion of the kernel.
+    type RegType: Default;

This Default implementation doesn't seem useful. You initialize it and

then register calls a C function to initialize it. Having register return an impl PinInit seems like it would work better here.

rust/kernel/driver.rs line 30

Quoted 7 lines

rust/kernel/driver.rs new file +6 -0

+    /// Registers a driver.
+    ///
+    /// On success, `reg` must remain pinned and valid until the matching call to
+    /// [`RegistrationOps::unregister`].
+    fn register(
+        reg: &mut Self::RegType,

If the intent is that RegType is going to be the raw bindings:: type, then this isn't going to work because you're creating &mut references to the raw type without a Opaque wrapper in between.

rust/kernel/driver.rs line 36

Quoted 7 lines

rust/kernel/driver.rs new file +6 -0

+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result;
+
+    /// Unregisters a driver previously registered with [`RegistrationOps::register`].
+    fn unregister(reg: &mut Self::RegType);

I believe this handles pinning incorrectly. You can't hand out &mut references to pinned values.

Alice

rust/kernel/device_id.rs new file +5 -0

+        ]
+        static $module_table_name: [core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
+            unsafe { core::mem::transmute_copy($table_name.raw_ids()) };
+    };
+}

Hi Danilo,

rust/kernel/device_id.rs line 161

Quoted 11 lines

rust/kernel/device_id.rs new file +11 -0

+/// Create device table alias for modpost.
+#[macro_export]
+macro_rules! module_device_table {
+    ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
+        #[rustfmt::skip]
+        #[export_name =
+            concat!("__mod_", $table_type,
+                    "__", module_path!(),
+                    "_", line!(),
+                    "_", stringify!($table_name),
+                    "_device_table")

This doesn't work on top of v6.13-rc1. The alias symbol name has been renamed by commit 054a9cd395a7 (modpost: rename alias symbol for MODULE_DEVICE_TABLE())

I applied the following change to make it work again:

Quoted 7 lines

rust/kernel/device_id.rs new file +5 -0

+        ]
+        static $module_table_name: [core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
+            unsafe { core::mem::transmute_copy($table_name.raw_ids()) };
+    };
+}

From: Wedson Almeida Filho <[email protected]>

Revocable allows access to objects to be safely revoked at run time.

This is useful, for example, for resources allocated during device probe; when the device is removed, the driver should stop accessing the device resources even if another state is kept in memory due to existing references (i.e., device context data is ref-counted and has a non-zero refcount after removal of the device).

Signed-off-by Wedson Almeida Filho <[email protected]>

Co-developed-by Danilo Krummrich <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

rust/kernel/revocable.rs new file +34 -0

+            },
+        })
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a guard that gives access to the object otherwise; the object is guaranteed to
+    /// remain accessible while the guard is alive. In such cases, callers are not allowed to sleep
+    /// because another CPU may be waiting to complete the revocation of this object.
+    pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
+        let guard = rcu::read_lock();
+        if self.is_available.load(Ordering::Relaxed) {
+            // Since `self.is_available` is true, data is initialised and has to remain valid
+            // because the RCU read side lock prevents it from being dropped.
+            Some(RevocableGuard::new(self.data.get(), guard))
+        } else {
+            None
+        }
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a shared reference to the object otherwise; the object is guaranteed to
+    /// remain accessible while the rcu read side guard is alive. In such cases, callers are not
+    /// allowed to sleep because another CPU may be waiting to complete the revocation of this
+    /// object.
+    pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
+        if self.is_available.load(Ordering::Relaxed) {
+            // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
+            // valid because the RCU read side lock prevents it from being dropped.

Quoted 11 lines · Danilo Krummrich

From: Wedson Almeida Filho <[email protected]>

Revocable allows access to objects to be safely revoked at run time.

This is useful, for example, for resources allocated during device probe; when the device is removed, the driver should stop accessing the device resources even if another state is kept in memory due to existing references (i.e., device context data is ref-counted and has a non-zero refcount after removal of the device).

Quoted 3 lines

Signed-off-by Wedson Almeida Filho <[email protected]>

Co-developed-by Danilo Krummrich <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Overall looks reasonable, but some comments below.

rust/kernel/revocable.rs line 97

Quoted 11 lines

rust/kernel/revocable.rs new file +11 -0

+impl<T> Revocable<T> {
+    /// Creates a new revocable instance of the given data.
+    pub fn new(data: impl PinInit<T>) -> impl PinInit<Self> {
+        pin_init!(Self {
+            is_available: AtomicBool::new(true),
+            // SAFETY: The closure only returns `Ok(())` if `ptr` is fully initialized; on error
+            // `ptr` is not partially initialized and does not need to be dropped.
+            data <- unsafe {
+                Opaque::try_ffi_init(|ptr: *mut T| {
+                    init::PinInit::<T, core::convert::Infallible>::__pinned_init(data, ptr)
+                })

This is pretty awkward ... could we have an Opaque::pin_init that takes an `impl PinInit instead of using fii_init?

Quoted 40 lines

rust/kernel/revocable.rs new file +39 -0

+            },
+        })
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a guard that gives access to the object otherwise; the object is guaranteed to
+    /// remain accessible while the guard is alive. In such cases, callers are not allowed to sleep
+    /// because another CPU may be waiting to complete the revocation of this object.
+    pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
+        let guard = rcu::read_lock();
+        if self.is_available.load(Ordering::Relaxed) {
+            // Since `self.is_available` is true, data is initialised and has to remain valid
+            // because the RCU read side lock prevents it from being dropped.
+            Some(RevocableGuard::new(self.data.get(), guard))
+        } else {
+            None
+        }
+    }
+
+    /// Tries to access the revocable wrapped object.
+    ///
+    /// Returns `None` if the object has been revoked and is therefore no longer accessible.
+    ///
+    /// Returns a shared reference to the object otherwise; the object is guaranteed to
+    /// remain accessible while the rcu read side guard is alive. In such cases, callers are not
+    /// allowed to sleep because another CPU may be waiting to complete the revocation of this
+    /// object.
+    pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
+        if self.is_available.load(Ordering::Relaxed) {
+            // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
+            // valid because the RCU read side lock prevents it from being dropped.
+            Some(unsafe { &*self.data.get() })
+        } else {
+            None
+        }
+    }

rust/kernel/revocable.rs line 141

Quoted 5 lines

rust/kernel/revocable.rs new file +5 -0

+
+    /// # Safety
+    ///
+    /// Callers must ensure that there are no more concurrent users of the revocable object.
+    unsafe fn revoke_internal(&self, sync: bool) {

This boolean could be a const generic to enforce that it must be a compile-time value.

Alice

I/O memory is typically either mapped through direct calls to ioremap() or subsystem / bus specific ones such as pci_iomap().

Even though subsystem / bus specific functions to map I/O memory are based on ioremap() / iounmap() it is not desirable to re-implement them in Rust.

Instead, implement a base type for I/O mapped memory, which generically provides the corresponding accessors, such as Io::readb or Io:try_readb.

Io supports an optional const generic, such that a driver can indicate the minimal expected and required size of the mapping at compile time. Correspondingly, calls to the 'non-try' accessors, support compile time checks of the I/O memory offset to read / write, while the 'try' accessors, provide boundary checks on runtime.

IoRaw is meant to be embedded into a structure (e.g. pci::Bar or io::IoMem) which creates the actual I/O memory mapping and initializes IoRaw accordingly.

To ensure that I/O mapped memory can't out-live the device it may be bound to, subsystems must embed the corresponding I/O memory type (e.g. pci::Bar) into a Devres container, such that it gets revoked once the device is unbound.

Signed-off-by Danilo Krummrich <[email protected]>

I/O memory is typically either mapped through direct calls to ioremap() or subsystem / bus specific ones such as pci_iomap().

Even though subsystem / bus specific functions to map I/O memory are based on ioremap() / iounmap() it is not desirable to re-implement them in Rust.

Instead, implement a base type for I/O mapped memory, which generically provides the corresponding accessors, such as Io::readb or Io:try_readb.

Io supports an optional const generic, such that a driver can indicate the minimal expected and required size of the mapping at compile time. Correspondingly, calls to the 'non-try' accessors, support compile time checks of the I/O memory offset to read / write, while the 'try' accessors, provide boundary checks on runtime.

IoRaw is meant to be embedded into a structure (e.g. pci::Bar or io::IoMem) which creates the actual I/O memory mapping and initializes

IoRaw accordingly.

To ensure that I/O mapped memory can't out-live the device it may be bound to, subsystems must embed the corresponding I/O memory type (e.g. pci::Bar) into a Devres container, such that it gets revoked once the device is unbound.

Signed-off-by Danilo Krummrich <[email protected]>

---

rust/helpers/helpers.c |   1 +
rust/helpers/io.c      | 101 ++++++++++++++++
rust/kernel/io.rs      | 260 +++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs     |   1 +
4 files changed, 363 insertions(+)

create mode 100644 rust/helpers/io.c
create mode 100644 rust/kernel/io.rs

rust/helpers/helpers.c +1 -0

index 060750af6524..63f9b1da179f 100644
@@ -14,6 +14,7 @@
 #include "cred.c"
 #include "err.c"
 #include "fs.c"
+#include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
 #include "mutex.c"

rust/helpers/io.c new file +101 -0

new file mode 100644
index 000000000000..1dde6374c0e2
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/io.h>
+
+void __iomem *rust_helper_ioremap(phys_addr_t offset, size_t size)
+{
+ return ioremap(offset, size);
+}
+
+void rust_helper_iounmap(volatile void __iomem *addr)
+{
+ return iounmap(addr);
+}
+
+u8 rust_helper_readb(const volatile void __iomem *addr)
+{
+ return readb(addr);
+}
+
+u16 rust_helper_readw(const volatile void __iomem *addr)
+{
+ return readw(addr);
+}
+
+u32 rust_helper_readl(const volatile void __iomem *addr)
+{
+ return readl(addr);
+}
+
+#ifdef CONFIG_64BIT
+u64 rust_helper_readq(const volatile void __iomem *addr)
+{
+ return readq(addr);
+}
+#endif
+
+void rust_helper_writeb(u8 value, volatile void __iomem *addr)
+{
+ writeb(value, addr);
+}
+
+void rust_helper_writew(u16 value, volatile void __iomem *addr)
+{
+ writew(value, addr);
+}
+
+void rust_helper_writel(u32 value, volatile void __iomem *addr)
+{
+ writel(value, addr);
+}
+
+#ifdef CONFIG_64BIT
+void rust_helper_writeq(u64 value, volatile void __iomem *addr)
+{
+ writeq(value, addr);
+}
+#endif
+
+u8 rust_helper_readb_relaxed(const volatile void __iomem *addr)
+{
+ return readb_relaxed(addr);
+}
+
+u16 rust_helper_readw_relaxed(const volatile void __iomem *addr)
+{
+ return readw_relaxed(addr);
+}
+
+u32 rust_helper_readl_relaxed(const volatile void __iomem *addr)
+{
+ return readl_relaxed(addr);
+}
+
+#ifdef CONFIG_64BIT
+u64 rust_helper_readq_relaxed(const volatile void __iomem *addr)
+{
+ return readq_relaxed(addr);
+}
+#endif
+
+void rust_helper_writeb_relaxed(u8 value, volatile void __iomem *addr)
+{
+ writeb_relaxed(value, addr);
+}
+
+void rust_helper_writew_relaxed(u16 value, volatile void __iomem *addr)
+{
+ writew_relaxed(value, addr);
+}
+
+void rust_helper_writel_relaxed(u32 value, volatile void __iomem *addr)
+{
+ writel_relaxed(value, addr);
+}
+
+#ifdef CONFIG_64BIT
+void rust_helper_writeq_relaxed(u64 value, volatile void __iomem *addr)
+{
+ writeq_relaxed(value, addr);
+}
+#endif

rust/kernel/io.rs new file +260 -0

new file mode 100644
index 000000000000..36aa2ec1bcc8
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Memory-mapped IO.
+//!
+//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
+
+use crate::error::{code::EINVAL, Result};
+use crate::{bindings, build_assert};
+
+/// Raw representation of an MMIO region.
+///
+/// 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> {
+    addr: usize,
+    maxsize: usize,
+}
+
+impl<const SIZE: usize> IoRaw<SIZE> {
+    /// Returns a new `IoRaw` instance on success, an error otherwise.
+    pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
+        if maxsize < SIZE {
+            return Err(EINVAL);
+        }
+
+        Ok(Self { addr, maxsize })
+    }
+
+    /// Returns the base address of the MMIO region.
+    #[inline]
+    pub fn addr(&self) -> usize {
+        self.addr
+    }
+
+    /// Returns the maximum size of the MMIO region.
+    #[inline]
+    pub fn maxsize(&self) -> usize {
+        self.maxsize
+    }
+}
+
+/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
+///
+/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
+/// mapping, performing an additional region request etc.
+///
+/// # Invariant
+///
+/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
+/// `maxsize`.
+///
+/// # Examples
+///
+/// ```no_run
+/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use core::ops::Deref;
+///
+/// // See also [`pci::Bar`] for a real example.
+/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+///
+/// impl<const SIZE: usize> IoMem<SIZE> {
+///     /// # Safety
+///     ///
+///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
+///     /// virtual address space.
+///     unsafe fn new(paddr: usize) -> Result<Self>{
+///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
+///         // valid for `ioremap`.
+///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE.try_into().unwrap()) };
+///         if addr.is_null() {
+///             return Err(ENOMEM);
+///         }
+///
+///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
+///     fn drop(&mut self) {
+///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
+///         unsafe { bindings::iounmap(self.0.addr() as _); };
+///     }
+/// }
+///
+/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
+///    type Target = Io<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) }
+///    }
+/// }
+///
+///# fn no_run() -> Result<(), Error> {
+/// // SAFETY: Invalid usage for example purposes.
+/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// iomem.writel(0x42, 0x0);
+/// assert!(iomem.try_writel(0x42, 0x0).is_ok());
+/// assert!(iomem.try_writel(0x42, 0x4).is_err());
+/// # Ok(())
+/// # }
+/// ```
+#[repr(transparent)]
+pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
+
+macro_rules! define_read {
+    ($(#[$attr:meta])* $name:ident, $try_name: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
+        /// time, the build will fail.
+        $(#[$attr])*
+        #[inline]
+        pub 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::$name(addr as _) }
+        }
+
+        /// Read IO data from a given offset.
+        ///
+        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+        /// out of bounds.
+        $(#[$attr])*
+        pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
+            let addr = self.io_addr::<$type_name>(offset)?;
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            Ok(unsafe { bindings::$name(addr as _) })
+        }
+    };
+}
+
+macro_rules! define_write {
+    ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => {
+        /// Write 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
+        /// time, the build will fail.
+        $(#[$attr])*
+        #[inline]
+        pub fn $name(&self, value: $type_name, offset: usize) {
+            let addr = self.io_addr_assert::<$type_name>(offset);
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(value, addr as _, ) }
+        }
+
+        /// Write IO data from a given offset.
+        ///
+        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+        /// out of bounds.
+        $(#[$attr])*
+        pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
+            let addr = self.io_addr::<$type_name>(offset)?;
+
+            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
+            unsafe { bindings::$name(value, addr as _) }
+            Ok(())
+        }
+    };
+}
+
+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<'a>(raw: &IoRaw<SIZE>) -> &'a Self {
+        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
+    }
+
+    /// Returns the base address of this mapping.
+    #[inline]
+    pub fn addr(&self) -> usize {
+        self.0.addr()
+    }
+
+    /// Returns the maximum size of this mapping.
+    #[inline]
+    pub fn maxsize(&self) -> usize {
+        self.0.maxsize()
+    }
+
+    #[inline]
+    const 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
+        } else {
+            false
+        }
+    }
+
+    #[inline]
+    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
+        if !Self::offset_valid::<U>(offset, self.maxsize()) {
+            return Err(EINVAL);
+        }
+
+        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
+        // this can't overflow.
+        self.addr().checked_add(offset).ok_or(EINVAL)
+    }
+
+    #[inline]
+    fn io_addr_assert<U>(&self, offset: usize) -> usize {
+        build_assert!(Self::offset_valid::<U>(offset, SIZE));
+
+        self.addr() + offset
+    }
+
+    define_read!(readb, try_readb, u8);
+    define_read!(readw, try_readw, u16);
+    define_read!(readl, try_readl, u32);
+    define_read!(
+        #[cfg(CONFIG_64BIT)]
+        readq,
+        try_readq,
+        u64
+    );
+
+    define_read!(readb_relaxed, try_readb_relaxed, u8);
+    define_read!(readw_relaxed, try_readw_relaxed, u16);
+    define_read!(readl_relaxed, try_readl_relaxed, u32);
+    define_read!(
+        #[cfg(CONFIG_64BIT)]
+        readq_relaxed,
+        try_readq_relaxed,
+        u64
+    );
+
+    define_write!(writeb, try_writeb, u8);
+    define_write!(writew, try_writew, u16);
+    define_write!(writel, try_writel, u32);
+    define_write!(
+        #[cfg(CONFIG_64BIT)]
+        writeq,
+        try_writeq,
+        u64
+    );
+
+    define_write!(writeb_relaxed, try_writeb_relaxed, u8);
+    define_write!(writew_relaxed, try_writew_relaxed, u16);
+    define_write!(writel_relaxed, try_writel_relaxed, u32);
+    define_write!(
+        #[cfg(CONFIG_64BIT)]
+        writeq_relaxed,
+        try_writeq_relaxed,
+        u64
+    );
+}

rust/kernel/lib.rs +1 -0

index 200c5f99a805..d1f30b1a05fb 100644
@@ -79,6 +79,7 @@
 
 #[doc(hidden)]
 pub use bindings;
+pub mod io;
 pub use macros;
 pub use uapi;
 

Signature · 1 line

2.47.0

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

rust/kernel/pci.rs new file +40 -1

+impl Device {
+    /// Create a PCI Device instance from an existing `device::Device`.
+    ///
+    /// # Safety
+    ///
+    /// `dev` must be an `ARef<device::Device>` whose underlying `bindings::device` is a member of
+    /// a `bindings::pci_dev`.
+    pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
+        Self(dev)
+    }
+
+    fn as_raw(&self) -> *mut bindings::pci_dev {
+        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
+        // embedded in `struct pci_dev`.
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+    }
+
+    /// Enable memory resources for this device.
+    pub fn enable_device_mem(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Enable bus-mastering for this device.
+    pub fn set_master(&self) {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        unsafe { bindings::pci_set_master(self.as_raw()) };
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        &self.0
+    }
+}
--

2.47.0

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

rust/kernel/pci.rs line 243

Quoted 6 lines

rust/kernel/pci.rs new file +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Alice

Quoted 43 lines

rust/kernel/pci.rs new file +40 -1

+impl Device {
+    /// Create a PCI Device instance from an existing `device::Device`.
+    ///
+    /// # Safety
+    ///
+    /// `dev` must be an `ARef<device::Device>` whose underlying `bindings::device` is a member of
+    /// a `bindings::pci_dev`.
+    pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
+        Self(dev)
+    }
+
+    fn as_raw(&self) -> *mut bindings::pci_dev {
+        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
+        // embedded in `struct pci_dev`.
+        unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+    }
+
+    /// Enable memory resources for this device.
+    pub fn enable_device_mem(&self) -> Result {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Enable bus-mastering for this device.
+    pub fn set_master(&self) {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        unsafe { bindings::pci_set_master(self.as_raw()) };
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        &self.0
+    }
+}
--

2.47.0

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

rust/kernel/pci.rs line 243

Quoted 6 lines

rust/kernel/pci.rs new file +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

Quoted 38 lines · Danilo Krummrich

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

rust/kernel/pci.rs line 243

Quoted 6 lines

rust/kernel/pci.rs new file +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

Quoted 4 lines

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

We could perhaps write a derive macro for AlwaysRefCounted that delegates to the inner type? That way, we can have the best of both worlds.

Alice

Quoted 44 lines · Alice Ryhl

Quoted 42 lines · Danilo Krummrich

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

rust/kernel/pci.rs line 243

Quoted 6 lines

rust/kernel/pci.rs new file +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

Quoted 3 lines

We could perhaps write a derive macro for AlwaysRefCounted that delegates to the inner type? That way, we can have the best of both worlds.

Sounds interesting, how exactly would this work?

(I'll already send out a v5, but let's keep discussing this.)

Quoted 50 lines · Danilo Krummrich

Quoted 47 lines · Alice Ryhl

Quoted 35 lines · Danilo Krummrich

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

rust/kernel/pci.rs line 243

Quoted 6 lines

rust/kernel/pci.rs new file +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Quoted 7 lines

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

We could perhaps write a derive macro for AlwaysRefCounted that delegates to the inner type? That way, we can have the best of both worlds.

Quoted 3 lines

Sounds interesting, how exactly would this work?

(I'll already send out a v5, but let's keep discussing this.)

Well, the derive macro could assume that the refcount is manipulated in the same way as the inner type does it. I admit that the idea is not fully formed, but if we can avoid wrapping ARef, that would be ideal.

It sounds like the only reason you don't do that is that it's more unsafe, which the macro could reduce.

Alice

Quoted 55 lines · Alice Ryhl

Quoted 53 lines · Danilo Krummrich

Quoted 39 lines · Alice Ryhl

Quoted 38 lines · Danilo Krummrich

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

Quoted 8 lines

Quoted 4 lines

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

We could perhaps write a derive macro for AlwaysRefCounted that delegates to the inner type? That way, we can have the best of both worlds.

Sounds interesting, how exactly would this work?

(I'll already send out a v5, but let's keep discussing this.)

Quoted 4 lines

Well, the derive macro could assume that the refcount is manipulated in the same way as the inner type does it. I admit that the idea is not fully formed, but if we can avoid wrapping ARef, that would be ideal.

If we can get this to work, I agree it's a good solution.

What do you think about making this a follow up of this series?

Quoted 61 lines · Danilo Krummrich

Quoted 59 lines · Alice Ryhl

Quoted 46 lines · Danilo Krummrich

Quoted 44 lines · Alice Ryhl

Quoted 42 lines · Danilo Krummrich

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

Quoted 7 lines

Quoted 3 lines

We could perhaps write a derive macro for AlwaysRefCounted that delegates to the inner type? That way, we can have the best of both worlds.

Sounds interesting, how exactly would this work?

(I'll already send out a v5, but let's keep discussing this.)

Well, the derive macro could assume that the refcount is manipulated in the same way as the inner type does it. I admit that the idea is not fully formed, but if we can avoid wrapping ARef, that would be ideal.

Quoted 3 lines

If we can get this to work, I agree it's a good solution.

What do you think about making this a follow up of this series?

That's fine, if you remove the patch that adds the module name to the function as ideally that's what we are trying to remove here :)

Quoted 55 lines · Alice Ryhl

Quoted 53 lines · Danilo Krummrich

Quoted 39 lines · Alice Ryhl

Quoted 38 lines · Danilo Krummrich

Quoted 22 lines · Alice Ryhl

Quoted 20 lines · Danilo Krummrich

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Quoted 10 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+/// The PCI device representation.
+///
+/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI
+/// device, hence, also increments the base device' reference count.
+#[derive(Clone)]
+pub struct Device(ARef<device::Device>);

It seems more natural for this to be a wrapper around Opaque<bindings::pci_dev>. Then you can have both &Device and ARef<Device> depending on whether you want to hold a refcount or not.

Yeah, but then every bus device has to re-implement the refcount dance we already have in device::Device for the underlying base struct device.

Quoted 8 lines

Quoted 4 lines

I forgot to mention this in my previous reply to Boqun, but we even documented it this way in device::Device [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28

We could perhaps write a derive macro for AlwaysRefCounted that delegates to the inner type? That way, we can have the best of both worlds.

Sounds interesting, how exactly would this work?

(I'll already send out a v5, but let's keep discussing this.)

Quoted 4 lines

Well, the derive macro could assume that the refcount is manipulated in the same way as the inner type does it. I admit that the idea is not fully formed, but if we can avoid wrapping ARef, that would be ideal.

If we can get this to work, I agree it's a good solution.

What do you think about making this a follow up of this series?

Implement the basic PCI abstractions required to write a basic PCI driver. This includes the following data structures:

The pci::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The pci::Device abstraction represents a struct pci_dev and provides abstractions for common functions, such as pci::Device::set_master.

In order to provide the PCI specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by pci::Adapter.

pci::DeviceId implements PCI device IDs based on the generic device_id::RawDevceId abstraction.

Co-developed-by FUJITA Tomonori <[email protected]>

Signed-off-by FUJITA Tomonori <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

Implement pci::Bar, pci::Device::iomap_region and pci::Device::iomap_region_sized to allow for I/O mappings of PCI BARs.

To ensure that a pci::Bar, and hence the I/O memory mapping, can't out-live the PCI device, the pci::Bar type is always embedded into a
Devres container, such that the pci::Bar is revoked once the device
is unbound and hence the I/O mapped memory is unmapped.

A pci::Bar can be requested with (pci::Device::iomap_region_sized) or
without (pci::Device::iomap_region) a const generic representing the
minimal requested size of the I/O mapped memory region. In case of the
latter only runtime checked I/O reads / writes are possible.

Co-developed-by Philipp Stanner <[email protected]>

Signed-off-by Philipp Stanner <[email protected]>

Signed-off-by Danilo Krummrich <[email protected]>

---
 rust/kernel/pci.rs | 145
+++++++++++++++++++++++++++++++++++++++++++++

 1 file changed, 145 insertions(+)

rust/kernel/pci.rs +0 -0

index 748912c1c994..0a32a5935c9c 100644
@@ -5,10 +5,14 @@
  //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h)

 use crate::{
+    alloc::flags::*,
     bindings, container_of, device,
     device_id::RawDeviceId,
+    devres::Devres,
     driver,
     error::{to_result, Result},
+    io::Io,
+    io::IoRaw,
     str::CStr,
     types::{ARef, ForeignOwnable},
     ThisModule,

rust/kernel/pci.rs +0 -0

@@ -239,9 +243,115 @@ pub trait Driver {
  ///
  /// A PCI device is based on an always reference counted

device:Device instance. Cloning a PCI
 /// device, hence, also increments the base device' reference count.

rust/kernel/pci.rs +4 -0

+///
+/// # Invariants
+///
+/// `Device` hold a valid reference of `ARef<device::Device>` whose

underlying struct device is a
+/// member of a struct pci_dev.
 #[derive(Clone)]
 pub struct Device(ARef<device::Device>);

rust/kernel/pci.rs +5 -0

+/// A PCI BAR to perform I/O-Operations on.
+///
+/// # Invariants
+///
+/// `Bar` always holds an `IoRaw` inststance that holds a valid

pointer to the start of the I/O

rust/kernel/pci.rs +15 -0

+/// memory mapped PCI bar and its size.
+pub struct Bar<const SIZE: usize = 0> {
+    pdev: Device,
+    io: IoRaw<SIZE>,
+    num: i32,
+}
+
+impl<const SIZE: usize> Bar<SIZE> {
+    fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> {
+        let len = pdev.resource_len(num)?;
+        if len == 0 {
+            return Err(ENOMEM);
+        }
+
+        // Convert to `i32`, since that's what all the C bindings

use.
+        let num = i32::try_from(num)?;
+
+        // SAFETY:
+        // pdev is valid by the invariants of Device. +        // num is checked for validity by a previous call to Device::resource_len.
+        // name is always valid.
+        let ret = unsafe { bindings::pci_request_region(pdev.as_raw(), num, name.as_char_ptr()) };
+        if ret != 0 {
+            return Err(EBUSY);
+        }
+
+        // SAFETY:
+        // pdev is valid by the invariants of Device. +        // num is checked for validity by a previous call to Device::resource_len.
+        // name is always valid.
+        let ioptr: usize = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) } as usize;
+        if ioptr == 0 {
+            // SAFETY:
+            // pdev valid by the invariants of Device. +            // num is checked for validity by a previous call to Device::resource_len.
+            unsafe { bindings::pci_release_region(pdev.as_raw(), num) };
+            return Err(ENOMEM);
+        }
+
+        let io = match IoRaw::new(ioptr, len as usize) { +            Ok(io) => io,
+            Err(err) => {
+                // SAFETY:
+                // pdev is valid by the invariants of Device. +                // ioptr is guaranteed to be the start of a valid I/O mapped memory region.
+                // num is checked for validity by a previous call to Device::resource_len.
+                unsafe { Self::do_release(&pdev, ioptr, num) }; +                return Err(err);
+            }
+        };

rust/kernel/pci.rs +7 -0

+
+        Ok(Bar { pdev, io, num })
+    }
+
+    /// # Safety
+    ///
+    /// `ioptr` must be a valid pointer to the memory mapped PCI bar

number num.
+    unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) { +        // SAFETY:
+        // pdev is valid by the invariants of Device. +        // ioptr is valid by the safety requirements. +        // num is valid by the safety requirements. +        unsafe {
+            bindings::pci_iounmap(pdev.as_raw(), ioptr as _); +            bindings::pci_release_region(pdev.as_raw(), num); +        }
+    }
+
+    fn release(&self) {
+        // SAFETY: The safety requirements are guaranteed by the type invariant of self.pdev.
+        unsafe { Self::do_release(&self.pdev, self.io.addr(), self.num) };
+    }

rust/kernel/pci.rs +5 -0

+}
+
+impl Bar {
+    fn index_is_valid(index: u32) -> bool {
+        // A `struct pci_dev` owns an array of resources with at

most PCI_NUM_RESOURCES entries.
+        index < bindings::PCI_NUM_RESOURCES
+    }

rust/kernel/pci.rs +13 -0

+}
+
+impl<const SIZE: usize> Drop for Bar<SIZE> {
+    fn drop(&mut self) {
+        self.release();
+    }
+}
+
+impl<const SIZE: usize> Deref for Bar<SIZE> {
+    type Target = Io<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) }
+    }

rust/kernel/pci.rs +2 -0

+}
+

 impl Device {
     /// Create a PCI Device instance from an existing device::Device.
     ///

rust/kernel/pci.rs +0 -0

@@ -275,6 +385,41 @@ pub fn set_master(&self) {
          // SAFETY: `self.as_raw` is guaranteed to be a pointer to a

valid struct pci_dev.
         unsafe { bindings::pci_set_master(self.as_raw()) }; }
+
+    /// Returns the size of the given PCI bar resource.
+    pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
+        if !Bar::index_is_valid(bar) {
+            return Err(EINVAL);
+        }
+
+        // SAFETY:
+        // - bar is a valid bar number, as guaranteed by the above call to Bar::index_is_valid,
+        // - by its type invariant self.as_raw is always a valid pointer to a struct pci_dev.
+        Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) })
+    }
+
+    /// Mapps an entire PCI-BAR after performing a region-request on it. I/O operation bound checks
+    /// can be performed on compile time for offsets (plus the requested type size) < SIZE.
+    pub fn iomap_region_sized<const SIZE: usize>(
+        &self,
+        bar: u32,
+        name: &CStr,
+    ) -> Result<Devres<Bar<SIZE>>> {
+        let bar = Bar::<SIZE>::new(self.clone(), bar, name)?; +        let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?;

rust/kernel/pci.rs +5 -0

+
+        Ok(devres)
+    }
+
+    /// Mapps an entire PCI-BAR after performing a region-request on

it.
+    pub fn iomap_region(&self, bar: u32, name: &CStr) -> Result<Devres<Bar>> {
+        self.iomap_region_sized::<0>(bar, name)
+    }
+
+    /// Returns a new ARef of the base device::Device. +    pub fn as_dev(&self) -> ARef<device::Device> { +        self.0.clone()
+    }
 }

 impl AsRef<device::Device> for Device {

of::DeviceId is an abstraction around struct of_device_id.

This is used by subsequent patches, in particular the platform bus abstractions, to create OF device ID tables.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS        |  1 +
 rust/kernel/lib.rs |  1 +
 rust/kernel/of.rs  | 57 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 rust/kernel/of.rs

Implement the basic platform bus abstractions required to write a basic platform driver. This includes the following data structures:

The platform::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The platform::Device abstraction represents a struct platform_device.

In order to provide the platform bus specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by platform::Adapter.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/platform.c         |  13 ++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 222 ++++++++++++++++++++++++++++++++
 6 files changed, 240 insertions(+)
 create mode 100644 rust/helpers/platform.c
 create mode 100644 rust/kernel/platform.rs

MAINTAINERS +1 -0

index 7d6bb4b15d2c..365fc48b7041 100644
@@ -7034,6 +7034,7 @@ F:	rust/kernel/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
+F:	rust/kernel/platform.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <[email protected]>

rust/bindings/bindings_helper.h +2 -0

index 6d7a68e2ecb7..e9fdceb568b8 100644
@@ -20,9 +20,11 @@
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
 #include <linux/miscdevice.h>
+#include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/phy.h>
 #include <linux/pid_namespace.h>
+#include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>

rust/helpers/helpers.c +1 -0

index 3fda33cd42d4..0640b7e115be 100644
@@ -20,6 +20,7 @@
 #include "kunit.c"
 #include "mutex.c"
 #include "page.c"
+#include "platform.c"
 #include "pci.c"
 #include "pid_namespace.c"
 #include "rbtree.c"

rust/helpers/platform.c new file +13 -0

new file mode 100644
index 000000000000..ab9b9f317301
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/platform_device.h>
+
+void *rust_helper_platform_get_drvdata(const struct platform_device *pdev)
+{
+	return platform_get_drvdata(pdev);
+}
+
+void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data)
+{
+	platform_set_drvdata(pdev, data);
+}

rust/kernel/lib.rs +1 -0

index 7a0e4c82ad0c..cc8f48aa162b 100644
@@ -59,6 +59,7 @@
 pub mod of;
 pub mod page;
 pub mod pid_namespace;
+pub mod platform;
 pub mod prelude;
 pub mod print;
 pub mod rbtree;

rust/kernel/platform.rs new file +71 -0

new file mode 100644
index 000000000000..868cfddb75a2
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the platform bus.
+//!
+//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
+
+use crate::{
+    bindings, container_of, device, driver,
+    error::{to_result, Result},
+    of,
+    prelude::*,
+    str::CStr,
+    types::{ARef, ForeignOwnable},
+    ThisModule,
+};
+
+/// An adapter for the registration of platform drivers.
+pub struct Adapter<T: Driver>(T);
+
+impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::platform_driver;
+
+    fn register(
+        pdrv: &mut Self::RegType,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        pdrv.driver.name = name.as_char_ptr();
+        pdrv.probe = Some(Self::probe_callback);
+
+        // Both members of this union are identical in data layout and semantics.
+        pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback);
+        pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr();
+
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) })
+    }
+
+    fn unregister(pdrv: &mut Self::RegType) {
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::platform_driver_unregister(pdrv) };
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    #[cfg(CONFIG_OF)]
+    fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let table = T::OF_ID_TABLE;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
+
+            Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+        }
+    }
+
+    #[cfg(not(CONFIG_OF))]
+    fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> {
+        None
+    }
+
+    // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular

Quoted 178 lines · Danilo Krummrich

Implement the basic platform bus abstractions required to write a basic platform driver. This includes the following data structures:

The platform::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The platform::Device abstraction represents a struct platform_device.

In order to provide the platform bus specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by platform::Adapter.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/platform.c         |  13 ++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 222 ++++++++++++++++++++++++++++++++
 6 files changed, 240 insertions(+)
 create mode 100644 rust/helpers/platform.c
 create mode 100644 rust/kernel/platform.rs

MAINTAINERS +1 -0

index 7d6bb4b15d2c..365fc48b7041 100644
@@ -7034,6 +7034,7 @@ F:	rust/kernel/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
+F:	rust/kernel/platform.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <[email protected]>

rust/bindings/bindings_helper.h +2 -0

index 6d7a68e2ecb7..e9fdceb568b8 100644
@@ -20,9 +20,11 @@
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
 #include <linux/miscdevice.h>
+#include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/phy.h>
 #include <linux/pid_namespace.h>
+#include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>

rust/helpers/helpers.c +1 -0

index 3fda33cd42d4..0640b7e115be 100644
@@ -20,6 +20,7 @@
 #include "kunit.c"
 #include "mutex.c"
 #include "page.c"
+#include "platform.c"
 #include "pci.c"
 #include "pid_namespace.c"
 #include "rbtree.c"

rust/helpers/platform.c new file +13 -0

new file mode 100644
index 000000000000..ab9b9f317301
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/platform_device.h>
+
+void *rust_helper_platform_get_drvdata(const struct platform_device *pdev)
+{
+	return platform_get_drvdata(pdev);
+}
+
+void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data)
+{
+	platform_set_drvdata(pdev, data);
+}

rust/kernel/lib.rs +1 -0

index 7a0e4c82ad0c..cc8f48aa162b 100644
@@ -59,6 +59,7 @@
 pub mod of;
 pub mod page;
 pub mod pid_namespace;
+pub mod platform;
 pub mod prelude;
 pub mod print;
 pub mod rbtree;

rust/kernel/platform.rs new file +77 -0

new file mode 100644
index 000000000000..868cfddb75a2
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the platform bus.
+//!
+//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
+
+use crate::{
+    bindings, container_of, device, driver,
+    error::{to_result, Result},
+    of,
+    prelude::*,
+    str::CStr,
+    types::{ARef, ForeignOwnable},
+    ThisModule,
+};
+
+/// An adapter for the registration of platform drivers.
+pub struct Adapter<T: Driver>(T);
+
+impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::platform_driver;
+
+    fn register(
+        pdrv: &mut Self::RegType,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        pdrv.driver.name = name.as_char_ptr();
+        pdrv.probe = Some(Self::probe_callback);
+
+        // Both members of this union are identical in data layout and semantics.
+        pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback);
+        pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr();
+
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) })
+    }
+
+    fn unregister(pdrv: &mut Self::RegType) {
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::platform_driver_unregister(pdrv) };
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    #[cfg(CONFIG_OF)]
+    fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let table = T::OF_ID_TABLE;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
+
+            Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+        }
+    }
+
+    #[cfg(not(CONFIG_OF))]
+    fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> {
+        None
+    }
+
+    // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular
+    // table, it means we don't have a match in there. If we don't match any of the ID tables, it
+    // means we were matched by name.
+    fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let id = Self::of_id_info(pdev);
+        if id.is_some() {
+            return id;

rust/kernel/platform.rs line 63

Quoted 4 lines

rust/kernel/platform.rs new file +4 -0

+        }
+
+        None
+    }

These methods are going to have to be duplicated by every bus type which can do DT matching (and later ACPI). Can't this be moved to be part of the common Driver trait.

I'll say it again for Greg to comment (doubtful he will look at v3 again). Really, I think we should also align the probe method interface across bus types. That means getting rid of the 'id' in the PCI probe (or add it for everyone). Most drivers never need it. The typical case is needing nothing or the matched data. In a quick scan[1], there's only a handful of cases. So I think probe should match the common scenario and make retrieving the id match explicit if needed.

Rob

[1] git grep -W 'const struct pci_device_id \*' drivers/ | grep -P 'id->(?!driver_data)'

Quoted 172 lines · Danilo Krummrich

Implement the basic platform bus abstractions required to write a basic platform driver. This includes the following data structures:

The platform::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The platform::Device abstraction represents a struct platform_device.

In order to provide the platform bus specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by platform::Adapter.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/platform.c         |  13 ++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 222 ++++++++++++++++++++++++++++++++
 6 files changed, 240 insertions(+)
 create mode 100644 rust/helpers/platform.c
 create mode 100644 rust/kernel/platform.rs

MAINTAINERS +1 -0

index 7d6bb4b15d2c..365fc48b7041 100644
@@ -7034,6 +7034,7 @@ F:	rust/kernel/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
+F:	rust/kernel/platform.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <[email protected]>

rust/bindings/bindings_helper.h +2 -0

index 6d7a68e2ecb7..e9fdceb568b8 100644
@@ -20,9 +20,11 @@
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
 #include <linux/miscdevice.h>
+#include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/phy.h>
 #include <linux/pid_namespace.h>
+#include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>

rust/helpers/helpers.c +1 -0

index 3fda33cd42d4..0640b7e115be 100644
@@ -20,6 +20,7 @@
 #include "kunit.c"
 #include "mutex.c"
 #include "page.c"
+#include "platform.c"
 #include "pci.c"
 #include "pid_namespace.c"
 #include "rbtree.c"

rust/helpers/platform.c new file +13 -0

new file mode 100644
index 000000000000..ab9b9f317301
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/platform_device.h>
+
+void *rust_helper_platform_get_drvdata(const struct platform_device *pdev)
+{
+	return platform_get_drvdata(pdev);
+}
+
+void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data)
+{
+	platform_set_drvdata(pdev, data);
+}

rust/kernel/lib.rs +1 -0

index 7a0e4c82ad0c..cc8f48aa162b 100644
@@ -59,6 +59,7 @@
 pub mod of;
 pub mod page;
 pub mod pid_namespace;
+pub mod platform;
 pub mod prelude;
 pub mod print;
 pub mod rbtree;

rust/kernel/platform.rs new file +71 -0

new file mode 100644
index 000000000000..868cfddb75a2
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the platform bus.
+//!
+//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
+
+use crate::{
+    bindings, container_of, device, driver,
+    error::{to_result, Result},
+    of,
+    prelude::*,
+    str::CStr,
+    types::{ARef, ForeignOwnable},
+    ThisModule,
+};
+
+/// An adapter for the registration of platform drivers.
+pub struct Adapter<T: Driver>(T);
+
+impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::platform_driver;
+
+    fn register(
+        pdrv: &mut Self::RegType,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        pdrv.driver.name = name.as_char_ptr();
+        pdrv.probe = Some(Self::probe_callback);
+
+        // Both members of this union are identical in data layout and semantics.
+        pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback);
+        pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr();
+
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) })
+    }
+
+    fn unregister(pdrv: &mut Self::RegType) {
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::platform_driver_unregister(pdrv) };
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    #[cfg(CONFIG_OF)]
+    fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let table = T::OF_ID_TABLE;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
+
+            Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+        }
+    }
+
+    #[cfg(not(CONFIG_OF))]
+    fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> {
+        None
+    }
+
+    // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular

rust/kernel/platform.rs line 81

Quoted 10 lines

rust/kernel/platform.rs new file +10 -0

+    // table, it means we don't have a match in there. If we don't match any of the ID tables, it
+    // means we were matched by name.
+    fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let id = Self::of_id_info(pdev);
+        if id.is_some() {
+            return id;
+        }
+
+        None
+    }

These methods are going to have to be duplicated by every bus type which can do DT matching (and later ACPI). Can't this be moved to be part of the common Driver trait.

I'll say it again for Greg to comment (doubtful he will look at v3 again). Really, I think we should also align the probe method interface across bus types. That means getting rid of the 'id' in the PCI probe (or add it for everyone). Most drivers never need it. The typical case is needing nothing or the matched data. In a quick scan[1], there's only a handful of cases. So I think probe should match the common scenario and make retrieving the id match explicit if needed.

Quoted 189 lines · Rob Herring

Quoted 172 lines · Danilo Krummrich

Implement the basic platform bus abstractions required to write a basic platform driver. This includes the following data structures:

The platform::Driver trait represents the interface to the driver and provides pci::Driver::probe for the driver to implement.

The platform::Device abstraction represents a struct platform_device.

In order to provide the platform bus specific parts to a generic driver::Registration the driver::RegistrationOps trait is implemented by platform::Adapter.

Signed-off-by Danilo Krummrich <[email protected]>

---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/platform.c         |  13 ++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 222 ++++++++++++++++++++++++++++++++
 6 files changed, 240 insertions(+)
 create mode 100644 rust/helpers/platform.c
 create mode 100644 rust/kernel/platform.rs

MAINTAINERS +1 -0

index 7d6bb4b15d2c..365fc48b7041 100644
@@ -7034,6 +7034,7 @@ F:	rust/kernel/device.rs
 F:	rust/kernel/device_id.rs
 F:	rust/kernel/devres.rs
 F:	rust/kernel/driver.rs
+F:	rust/kernel/platform.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <[email protected]>

rust/bindings/bindings_helper.h +2 -0

index 6d7a68e2ecb7..e9fdceb568b8 100644
@@ -20,9 +20,11 @@
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
 #include <linux/miscdevice.h>
+#include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/phy.h>
 #include <linux/pid_namespace.h>
+#include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>

rust/helpers/helpers.c +1 -0

index 3fda33cd42d4..0640b7e115be 100644
@@ -20,6 +20,7 @@
 #include "kunit.c"
 #include "mutex.c"
 #include "page.c"
+#include "platform.c"
 #include "pci.c"
 #include "pid_namespace.c"
 #include "rbtree.c"

rust/helpers/platform.c new file +13 -0

new file mode 100644
index 000000000000..ab9b9f317301
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/platform_device.h>
+
+void *rust_helper_platform_get_drvdata(const struct platform_device *pdev)
+{
+	return platform_get_drvdata(pdev);
+}
+
+void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data)
+{
+	platform_set_drvdata(pdev, data);
+}

rust/kernel/lib.rs +1 -0

index 7a0e4c82ad0c..cc8f48aa162b 100644
@@ -59,6 +59,7 @@
 pub mod of;
 pub mod page;
 pub mod pid_namespace;
+pub mod platform;
 pub mod prelude;
 pub mod print;
 pub mod rbtree;

rust/kernel/platform.rs new file +71 -0

new file mode 100644
index 000000000000..868cfddb75a2
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for the platform bus.
+//!
+//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
+
+use crate::{
+    bindings, container_of, device, driver,
+    error::{to_result, Result},
+    of,
+    prelude::*,
+    str::CStr,
+    types::{ARef, ForeignOwnable},
+    ThisModule,
+};
+
+/// An adapter for the registration of platform drivers.
+pub struct Adapter<T: Driver>(T);
+
+impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::platform_driver;
+
+    fn register(
+        pdrv: &mut Self::RegType,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        pdrv.driver.name = name.as_char_ptr();
+        pdrv.probe = Some(Self::probe_callback);
+
+        // Both members of this union are identical in data layout and semantics.
+        pdrv.__bindgen_anon_1.remove = Some(Self::remove_callback);
+        pdrv.driver.of_match_table = T::OF_ID_TABLE.as_ptr();
+
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::__platform_driver_register(pdrv, module.0) })
+    }
+
+    fn unregister(pdrv: &mut Self::RegType) {
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::platform_driver_unregister(pdrv) };
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    #[cfg(CONFIG_OF)]
+    fn of_id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let table = T::OF_ID_TABLE;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::of_match_device(table.as_ptr(), pdev.as_ref().as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<of::DeviceId>() };
+
+            Some(table.info(<of::DeviceId as crate::device_id::RawDeviceId>::index(id)))
+        }
+    }
+
+    #[cfg(not(CONFIG_OF))]
+    fn of_id_info(_pdev: &Device) -> Option<&'static T::IdInfo> {
+        None
+    }
+
+    // Try to retrieve an `IdInfo` from any of the ID tables; if we can't find one for a particular

rust/kernel/platform.rs line 81

Quoted 10 lines

rust/kernel/platform.rs new file +10 -0

+    // table, it means we don't have a match in there. If we don't match any of the ID tables, it
+    // means we were matched by name.
+    fn id_info(pdev: &Device) -> Option<&'static T::IdInfo> {
+        let id = Self::of_id_info(pdev);
+        if id.is_some() {
+            return id;
+        }
+
+        None
+    }

These methods are going to have to be duplicated by every bus type which can do DT matching (and later ACPI). Can't this be moved to be part of the common Driver trait.

Quoted 7 lines

I'll say it again for Greg to comment (doubtful he will look at v3 again). Really, I think we should also align the probe method interface across bus types. That means getting rid of the 'id' in the PCI probe (or add it for everyone). Most drivers never need it. The typical case is needing nothing or the matched data. In a quick scan[1], there's only a handful of cases. So I think probe should match the common scenario and make retrieving the id match explicit if needed.