| Message ID | 20241210224947.23804-14-dakr@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Device / Driver PCI / Platform Rust abstractions | expand |
Hi Danilo, On Tue, Dec 10, 2024 at 2:51 PM Danilo Krummrich <dakr@kernel.org> wrote: > > In order to not duplicate code in bus specific implementations (e.g. > platform), implement a generic `driver::Adapter` to represent the > connection of matched drivers and devices. > > Bus specific `Adapter` implementations can simply implement this trait > to inherit generic functionality, such as matching OF or ACPI device IDs > and ID table entries. > > Suggested-by: Rob Herring (Arm) <robh@kernel.org> > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > --- > rust/kernel/driver.rs | 59 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 58 insertions(+), 1 deletion(-) > > diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs > index ab0bb46fe2cc..d169899a5da1 100644 > --- a/rust/kernel/driver.rs > +++ b/rust/kernel/driver.rs > @@ -6,7 +6,9 @@ > //! register using the [`Registration`] class. > > use crate::error::{Error, Result}; > -use crate::{init::PinInit, str::CStr, try_pin_init, types::Opaque, ThisModule}; > +use crate::{ > + device, device_id, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule, > +}; > use core::pin::Pin; > use macros::{pin_data, pinned_drop}; > > @@ -114,3 +116,58 @@ macro_rules! module_driver { > } > } > } > + > +/// The bus independent adapter to match a drivers and a devices. > +/// > +/// This trait should be implemented by the bus specific adapter, which represents the connection > +/// of a device and a driver. > +/// > +/// It provides bus independent functions for device / driver interactions. > +pub trait Adapter { > + /// The type holding driver private data about each device id supported by the driver. > + type IdInfo: 'static; > + > + /// The [`of::IdTable`] of the corresponding driver. > + fn of_id_table() -> of::IdTable<Self::IdInfo>; I think we may want this to return an Option<of::IdTable<Self::IdInfo>> instead. I don't think we want to force every bus abstraction to have to implement every possible IdTable that this adapter will support. For instance if your driver only supports ACPI, it will still be required to provide an empty OF table because the bus abstraction needs it for implementing this trait. > + > + /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any. > + /// > + /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`]. > + #[cfg(CONFIG_OF)] > + fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { > + let table = Self::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(), dev.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 device_id::RawDeviceId>::index(id))) > + } > + } > + > + #[cfg(not(CONFIG_OF))] > + fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> { > + None > + } > + > + /// Returns the driver's private data from the matching entry of any of the ID tables, if any. > + /// > + /// If this returns `None`, it means that there is no match in any of the ID tables directly > + /// associated with a [`device::Device`]. > + fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { > + let id = Self::of_id_info(dev); > + if id.is_some() { > + return id; > + } > + > + None > + } > +} > -- > 2.47.0 >
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs index ab0bb46fe2cc..d169899a5da1 100644 --- a/rust/kernel/driver.rs +++ b/rust/kernel/driver.rs @@ -6,7 +6,9 @@ //! register using the [`Registration`] class. use crate::error::{Error, Result}; -use crate::{init::PinInit, str::CStr, try_pin_init, types::Opaque, ThisModule}; +use crate::{ + device, device_id, init::PinInit, of, str::CStr, try_pin_init, types::Opaque, ThisModule, +}; use core::pin::Pin; use macros::{pin_data, pinned_drop}; @@ -114,3 +116,58 @@ macro_rules! module_driver { } } } + +/// The bus independent adapter to match a drivers and a devices. +/// +/// This trait should be implemented by the bus specific adapter, which represents the connection +/// of a device and a driver. +/// +/// It provides bus independent functions for device / driver interactions. +pub trait Adapter { + /// The type holding driver private data about each device id supported by the driver. + type IdInfo: 'static; + + /// The [`of::IdTable`] of the corresponding driver. + fn of_id_table() -> of::IdTable<Self::IdInfo>; + + /// Returns the driver's private data from the matching entry in the [`of::IdTable`], if any. + /// + /// If this returns `None`, it means there is no match with an entry in the [`of::IdTable`]. + #[cfg(CONFIG_OF)] + fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { + let table = Self::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(), dev.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 device_id::RawDeviceId>::index(id))) + } + } + + #[cfg(not(CONFIG_OF))] + fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> { + None + } + + /// Returns the driver's private data from the matching entry of any of the ID tables, if any. + /// + /// If this returns `None`, it means that there is no match in any of the ID tables directly + /// associated with a [`device::Device`]. + fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> { + let id = Self::of_id_info(dev); + if id.is_some() { + return id; + } + + None + } +}
In order to not duplicate code in bus specific implementations (e.g. platform), implement a generic `driver::Adapter` to represent the connection of matched drivers and devices. Bus specific `Adapter` implementations can simply implement this trait to inherit generic functionality, such as matching OF or ACPI device IDs and ID table entries. Suggested-by: Rob Herring (Arm) <robh@kernel.org> Signed-off-by: Danilo Krummrich <dakr@kernel.org> --- rust/kernel/driver.rs | 59 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)