* [PATCH v2 1/3] rust: device_id: split out index support into a separate trait
2025-07-01 14:12 [PATCH v2 0/3] rust: Build PHY device tables by using module_device_table macro FUJITA Tomonori
@ 2025-07-01 14:12 ` FUJITA Tomonori
2025-07-04 0:44 ` Danilo Krummrich
2025-07-01 14:12 ` [PATCH v2 2/3] rust: net::phy represent DeviceId as transparent wrapper over mdio_device_id FUJITA Tomonori
2025-07-01 14:12 ` [PATCH v2 3/3] rust: net::phy Change module_phy_driver macro to use module_device_table macro FUJITA Tomonori
2 siblings, 1 reply; 6+ messages in thread
From: FUJITA Tomonori @ 2025-07-01 14:12 UTC (permalink / raw)
To: alex.gaynor, dakr, gregkh, ojeda, rafael, robh, saravanak
Cc: a.hindborg, aliceryhl, bhelgaas, bjorn3_gh, boqun.feng,
david.m.ertman, devicetree, gary, ira.weiny, kwilczynski, leon,
linux-kernel, linux-pci, lossin, netdev, rust-for-linux, tmgross
Introduce a new trait `RawDeviceIdIndex`, which extends `RawDeviceId`
to provide support for device ID types that include an index or
context field (e.g., `driver_data`). This separates the concerns of
layout compatibility and index-based data embedding, and allows
`RawDeviceId` to be implemented for types that do not contain a
`driver_data` field. Several such structures are defined in
include/linux/mod_devicetable.h.
Refactor `IdArray::new()` into a generic `build()` function, which
takes an optional offset. Based on the presence of `RawDeviceIdIndex`,
index writing is conditionally enabled. A new `new_without_index()`
constructor is also provided for use cases where no index should be
written.
This refactoring is a preparation for enabling the PHY abstractions to
use device_id trait.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/auxiliary.rs | 7 +++-
rust/kernel/device_id.rs | 80 +++++++++++++++++++++++++++++++---------
rust/kernel/of.rs | 11 +++++-
rust/kernel/pci.rs | 7 +++-
4 files changed, 81 insertions(+), 24 deletions(-)
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index d2cfe1eeefb6..655ed61247d0 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -6,7 +6,7 @@
use crate::{
bindings, container_of, device,
- device_id::RawDeviceId,
+ device_id::{RawDeviceId, RawDeviceIdIndex},
driver,
error::{to_result, Result},
prelude::*,
@@ -143,10 +143,13 @@ pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self {
// SAFETY:
// * `DeviceId` is a `#[repr(transparent)`] wrapper of `auxiliary_device_id` and does not add
// additional invariants, so it's safe to transmute to `RawType`.
-// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
unsafe impl RawDeviceId for DeviceId {
type RawType = bindings::auxiliary_device_id;
+}
+// SAFETY:
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceIdIndex for DeviceId {
const DRIVER_DATA_OFFSET: usize =
core::mem::offset_of!(bindings::auxiliary_device_id, driver_data);
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 3dc72ca8cfc2..3bcab2310087 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -24,22 +24,36 @@
/// Ideally, this should be achieved using a const function that does conversion instead of
/// transmute; however, const trait functions relies on `const_trait_impl` unstable feature,
/// which is broken/gone in Rust 1.73.
-///
-/// - `DRIVER_DATA_OFFSET` is the offset of context/data field of the device ID (usually named
-/// `driver_data`) of the device ID, the field is suitable sized to write a `usize` value.
-///
-/// Similar to the previous requirement, the data should ideally be added during `Self` to
-/// `RawType` conversion, but there's currently no way to do it when using traits in const.
pub unsafe trait RawDeviceId {
/// The raw type that holds the device id.
///
/// Id tables created from [`Self`] are going to hold this type in its zero-terminated array.
type RawType: Copy;
+}
- /// The offset to the context/data field.
+/// Extension trait for [`RawDeviceId`] for devices that embed an index or context value.
+///
+/// This is typically used when the device ID struct includes a field like `driver_data`
+/// that is used to store a pointer-sized value (e.g., an index or context pointer).
+///
+/// # Safety
+///
+/// Implementers must ensure that:
+/// - `DRIVER_DATA_OFFSET` is the correct offset (in bytes) to the context/data field (e.g., the
+/// `driver_data` field) within the raw device ID structure. This field must be correctly sized
+/// to hold a `usize`.
+///
+/// Ideally, the data should ideally be added during `Self` to `RawType` conversion,
+/// but there's currently no way to do it when using traits in const.
+///
+/// - The `index` method must return the value stored at the location specified
+/// by `DRIVER_DATA_OFFSET`, assuming `self` is layout-compatible with `RawType`.
+pub unsafe trait RawDeviceIdIndex: RawDeviceId {
+ /// The offset (in bytes) to the context/data field in the raw device ID.
const DRIVER_DATA_OFFSET: usize;
- /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceId`] trait.
+ /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceIdIndex`]
+ /// trait.
fn index(&self) -> usize;
}
@@ -68,7 +82,14 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
/// Creates a new instance of the array.
///
/// The contents are derived from the given identifiers and context information.
- pub const fn new(ids: [(T, U); N]) -> Self {
+ ///
+ /// # Safety
+ ///
+ /// If `offset` is `Some(offset)`, then:
+ /// - `offset` must be the correct offset (in bytes) to the context/data field
+ /// (e.g., the `driver_data` field) within the raw device ID structure.
+ /// - The field at `offset` must be correctly sized to hold a `usize`.
+ const unsafe fn build(ids: [(T, U); N], offset: Option<usize>) -> Self {
let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
let mut infos = [const { MaybeUninit::uninit() }; N];
@@ -77,14 +98,17 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
// SAFETY: by the safety requirement of `RawDeviceId`, we're guaranteed that `T` is
// layout-wise compatible with `RawType`.
raw_ids[i] = unsafe { core::mem::transmute_copy(&ids[i].0) };
- // SAFETY: by the safety requirement of `RawDeviceId`, this would be effectively
- // `raw_ids[i].driver_data = i;`.
- unsafe {
- raw_ids[i]
- .as_mut_ptr()
- .byte_add(T::DRIVER_DATA_OFFSET)
- .cast::<usize>()
- .write(i);
+
+ if let Some(offset) = offset {
+ // SAFETY: by the safety requirement of this function, this would be effectively
+ // `raw_ids[i].driver_data = i;`.
+ unsafe {
+ raw_ids[i]
+ .as_mut_ptr()
+ .byte_add(offset)
+ .cast::<usize>()
+ .write(i);
+ }
}
// SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
@@ -92,7 +116,6 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
infos[i] = MaybeUninit::new(unsafe { core::ptr::read(&ids[i].1) });
i += 1;
}
-
core::mem::forget(ids);
Self {
@@ -109,12 +132,33 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
}
}
+ /// Creates a new instance of the array without writing index values.
+ ///
+ /// The contents are derived from the given identifiers and context information.
+ pub const fn new_without_index(ids: [(T, U); N]) -> Self {
+ // SAFETY: Calling `Self::build` with `offset = None` is always safe,
+ // because no raw memory writes are performed in this case.
+ unsafe { Self::build(ids, None) }
+ }
+
/// Reference to the contained [`RawIdArray`].
pub const fn raw_ids(&self) -> &RawIdArray<T, N> {
&self.raw_ids
}
}
+impl<T: RawDeviceId + RawDeviceIdIndex, U, const N: usize> IdArray<T, U, N> {
+ /// Creates a new instance of the array.
+ ///
+ /// The contents are derived from the given identifiers and context information.
+ pub const fn new(ids: [(T, U); N]) -> Self {
+ // SAFETY: by the safety requirement of `RawDeviceIdIndex`,
+ // `T::DRIVER_DATA_OFFSET` is guaranteed to be the correct offset (in bytes) to
+ // a field within `T::RawType`.
+ unsafe { Self::build(ids, Some(T::DRIVER_DATA_OFFSET)) }
+ }
+}
+
/// A device id table.
///
/// This trait is only implemented by `IdArray`.
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 40d1bd13682c..c690bceb0d73 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -2,7 +2,11 @@
//! Device Tree / Open Firmware abstractions.
-use crate::{bindings, device_id::RawDeviceId, prelude::*};
+use crate::{
+ bindings,
+ device_id::{RawDeviceId, RawDeviceIdIndex},
+ prelude::*,
+};
/// IdTable type for OF drivers.
pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
@@ -15,10 +19,13 @@
// SAFETY:
// * `DeviceId` is a `#[repr(transparent)` wrapper of `struct of_device_id` and does not add
// additional invariants, so it's safe to transmute to `RawType`.
-// * `DRIVER_DATA_OFFSET` is the offset to the `data` field.
unsafe impl RawDeviceId for DeviceId {
type RawType = bindings::of_device_id;
+}
+// SAFETY:
+// * `DRIVER_DATA_OFFSET` is the offset to the `data` field.
+unsafe impl RawDeviceIdIndex for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 6b94fd7a3ce9..96d6275c7afb 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -7,7 +7,7 @@
use crate::{
alloc::flags::*,
bindings, container_of, device,
- device_id::RawDeviceId,
+ device_id::{RawDeviceId, RawDeviceIdIndex},
devres::Devres,
driver,
error::{to_result, Result},
@@ -164,10 +164,13 @@ pub const fn from_class(class: u32, class_mask: u32) -> Self {
// SAFETY:
// * `DeviceId` is a `#[repr(transparent)` wrapper of `pci_device_id` and does not add
// additional invariants, so it's safe to transmute to `RawType`.
-// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
unsafe impl RawDeviceId for DeviceId {
type RawType = bindings::pci_device_id;
+}
+// SAFETY:
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceIdIndex for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
fn index(&self) -> usize {
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] rust: net::phy represent DeviceId as transparent wrapper over mdio_device_id
2025-07-01 14:12 [PATCH v2 0/3] rust: Build PHY device tables by using module_device_table macro FUJITA Tomonori
2025-07-01 14:12 ` [PATCH v2 1/3] rust: device_id: split out index support into a separate trait FUJITA Tomonori
@ 2025-07-01 14:12 ` FUJITA Tomonori
2025-07-01 14:12 ` [PATCH v2 3/3] rust: net::phy Change module_phy_driver macro to use module_device_table macro FUJITA Tomonori
2 siblings, 0 replies; 6+ messages in thread
From: FUJITA Tomonori @ 2025-07-01 14:12 UTC (permalink / raw)
To: alex.gaynor, dakr, gregkh, ojeda, rafael, robh, saravanak
Cc: a.hindborg, aliceryhl, bhelgaas, bjorn3_gh, boqun.feng,
david.m.ertman, devicetree, gary, ira.weiny, kwilczynski, leon,
linux-kernel, linux-pci, lossin, netdev, rust-for-linux, tmgross
Refactor the DeviceId struct to be a #[repr(transparent)] wrapper
around the C struct bindings::mdio_device_id.
This refactoring is a preparation for enabling the PHY abstractions to
use device_id trait.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/net/phy.rs | 53 +++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 65ac4d59ad77..940972ffadae 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -507,7 +507,7 @@ pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
DriverVTable(Opaque::new(bindings::phy_driver {
name: T::NAME.as_char_ptr().cast_mut(),
flags: T::FLAGS,
- phy_id: T::PHY_DEVICE_ID.id,
+ phy_id: T::PHY_DEVICE_ID.id(),
phy_id_mask: T::PHY_DEVICE_ID.mask_as_int(),
soft_reset: if T::HAS_SOFT_RESET {
Some(Adapter::<T>::soft_reset_callback)
@@ -691,42 +691,41 @@ fn drop(&mut self) {
///
/// Represents the kernel's `struct mdio_device_id`. This is used to find an appropriate
/// PHY driver.
-pub struct DeviceId {
- id: u32,
- mask: DeviceMask,
-}
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::mdio_device_id);
impl DeviceId {
/// Creates a new instance with the exact match mask.
pub const fn new_with_exact_mask(id: u32) -> Self {
- DeviceId {
- id,
- mask: DeviceMask::Exact,
- }
+ Self(bindings::mdio_device_id {
+ phy_id: id,
+ phy_id_mask: DeviceMask::Exact.as_int(),
+ })
}
/// Creates a new instance with the model match mask.
pub const fn new_with_model_mask(id: u32) -> Self {
- DeviceId {
- id,
- mask: DeviceMask::Model,
- }
+ Self(bindings::mdio_device_id {
+ phy_id: id,
+ phy_id_mask: DeviceMask::Model.as_int(),
+ })
}
/// Creates a new instance with the vendor match mask.
pub const fn new_with_vendor_mask(id: u32) -> Self {
- DeviceId {
- id,
- mask: DeviceMask::Vendor,
- }
+ Self(bindings::mdio_device_id {
+ phy_id: id,
+ phy_id_mask: DeviceMask::Vendor.as_int(),
+ })
}
/// Creates a new instance with a custom match mask.
pub const fn new_with_custom_mask(id: u32, mask: u32) -> Self {
- DeviceId {
- id,
- mask: DeviceMask::Custom(mask),
- }
+ Self(bindings::mdio_device_id {
+ phy_id: id,
+ phy_id_mask: DeviceMask::Custom(mask).as_int(),
+ })
}
/// Creates a new instance from [`Driver`].
@@ -734,18 +733,20 @@ pub const fn new_with_driver<T: Driver>() -> Self {
T::PHY_DEVICE_ID
}
+ /// Get a `phy_id` as u32.
+ pub const fn id(&self) -> u32 {
+ self.0.phy_id
+ }
+
/// Get a `mask` as u32.
pub const fn mask_as_int(&self) -> u32 {
- self.mask.as_int()
+ self.0.phy_id_mask
}
// macro use only
#[doc(hidden)]
pub const fn mdio_device_id(&self) -> bindings::mdio_device_id {
- bindings::mdio_device_id {
- phy_id: self.id,
- phy_id_mask: self.mask.as_int(),
- }
+ self.0
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] rust: net::phy Change module_phy_driver macro to use module_device_table macro
2025-07-01 14:12 [PATCH v2 0/3] rust: Build PHY device tables by using module_device_table macro FUJITA Tomonori
2025-07-01 14:12 ` [PATCH v2 1/3] rust: device_id: split out index support into a separate trait FUJITA Tomonori
2025-07-01 14:12 ` [PATCH v2 2/3] rust: net::phy represent DeviceId as transparent wrapper over mdio_device_id FUJITA Tomonori
@ 2025-07-01 14:12 ` FUJITA Tomonori
2 siblings, 0 replies; 6+ messages in thread
From: FUJITA Tomonori @ 2025-07-01 14:12 UTC (permalink / raw)
To: alex.gaynor, dakr, gregkh, ojeda, rafael, robh, saravanak
Cc: a.hindborg, aliceryhl, bhelgaas, bjorn3_gh, boqun.feng,
david.m.ertman, devicetree, gary, ira.weiny, kwilczynski, leon,
linux-kernel, linux-pci, lossin, netdev, rust-for-linux, tmgross
Change module_phy_driver macro to build device tables which are
exported to userspace by using module_device_table macro.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/net/phy.rs | 51 ++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 940972ffadae..8bb362aefe27 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -6,7 +6,7 @@
//!
//! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h).
-use crate::{error::*, prelude::*, types::Opaque};
+use crate::{device_id::RawDeviceId, error::*, prelude::*, types::Opaque};
use core::{marker::PhantomData, ptr::addr_of_mut};
pub mod reg;
@@ -750,6 +750,12 @@ pub const fn mdio_device_id(&self) -> bindings::mdio_device_id {
}
}
+// SAFETY: [`DeviceId`] is a `#[repr(transparent)` wrapper of `struct mdio_device_id`
+// and does not add additional invariants, so it's safe to transmute to `RawType`.
+unsafe impl RawDeviceId for DeviceId {
+ type RawType = bindings::mdio_device_id;
+}
+
enum DeviceMask {
Exact,
Model,
@@ -850,19 +856,18 @@ const fn as_int(&self) -> u32 {
/// }
/// };
///
-/// const _DEVICE_TABLE: [::kernel::bindings::mdio_device_id; 2] = [
-/// ::kernel::bindings::mdio_device_id {
-/// phy_id: 0x00000001,
-/// phy_id_mask: 0xffffffff,
-/// },
-/// ::kernel::bindings::mdio_device_id {
-/// phy_id: 0,
-/// phy_id_mask: 0,
-/// },
-/// ];
-/// #[cfg(MODULE)]
-/// #[no_mangle]
-/// static __mod_device_table__mdio__phydev: [::kernel::bindings::mdio_device_id; 2] = _DEVICE_TABLE;
+/// const N: usize = 1;
+///
+/// const TABLE: ::kernel::device_id::IdArray<::kernel::net::phy::DeviceId, (), N> =
+/// ::kernel::device_id::IdArray::new_without_index([
+/// ::kernel::net::phy::DeviceId(
+/// ::kernel::bindings::mdio_device_id {
+/// phy_id: 0x00000001,
+/// phy_id_mask: 0xffffffff,
+/// }),
+/// ]);
+///
+/// ::kernel::module_device_table!("mdio", phydev, TABLE);
/// ```
#[macro_export]
macro_rules! module_phy_driver {
@@ -873,20 +878,12 @@ macro_rules! module_phy_driver {
};
(@device_table [$($dev:expr),+]) => {
- // SAFETY: C will not read off the end of this constant since the last element is zero.
- const _DEVICE_TABLE: [$crate::bindings::mdio_device_id;
- $crate::module_phy_driver!(@count_devices $($dev),+) + 1] = [
- $($dev.mdio_device_id()),+,
- $crate::bindings::mdio_device_id {
- phy_id: 0,
- phy_id_mask: 0
- }
- ];
+ const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+);
+
+ const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> =
+ $crate::device_id::IdArray::new_without_index([ $(($dev,())),+, ]);
- #[cfg(MODULE)]
- #[no_mangle]
- static __mod_device_table__mdio__phydev: [$crate::bindings::mdio_device_id;
- $crate::module_phy_driver!(@count_devices $($dev),+) + 1] = _DEVICE_TABLE;
+ $crate::module_device_table!("mdio", phydev, TABLE);
};
(drivers: [$($driver:ident),+ $(,)?], device_table: [$($dev:expr),+ $(,)?], $($f:tt)*) => {
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread