[PATCH v1 0/3] rust: Build PHY device tables by using module_device_table macro

[PATCH v1 0/3] rust: Build PHY device tables by using module_device_table macro

[FUJITA Tomonori]

Build PHY device tables by using module_device_table macro.

The PHY abstractions have been generating their own device tables
manually instead of using the module_device_table macro provided by
the device_id crate. However, the format of device tables occasionally
changes [1] [2], requiring updates to both the device_id crate and the custom
format used by the PHY abstractions, which is cumbersome to maintain.

[1]: https://lore.kernel.org/lkml/20241119235705.1576946-14-masahiroy@kernel.org/
[2]: https://lore.kernel.org/lkml/6e2f70b07a710e761eb68d089d96cee7b27bb2d5.1750511018.git.legion@kernel.org/

FUJITA Tomonori (3):
  rust: device_id: make DRIVER_DATA_OFFSET optional
  rust: net::phy represent DeviceId as transparent wrapper over
    mdio_device_id
  rust: net::phy Change module_phy_driver macro to use
    module_device_table macro

 rust/kernel/auxiliary.rs |   6 ++-
 rust/kernel/device_id.rs |  26 ++++++----
 rust/kernel/net/phy.rs   | 109 ++++++++++++++++++++-------------------
 rust/kernel/of.rs        |   3 +-
 rust/kernel/pci.rs       |   3 +-
 5 files changed, 79 insertions(+), 68 deletions(-)


base-commit: dc35ddcf97e99b18559d0855071030e664aae44d
-- 
2.43.0

[PATCH v1 1/3] rust: device_id: make DRIVER_DATA_OFFSET optional

[FUJITA Tomonori]

Enable support for device ID structures that do not contain
context/data field (usually named `driver_data`), making the trait
usable in a wider range of subsystems and buses.

Several such structures are defined in
include/linux/mod_devicetable.h.

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 |  6 ++++--
 rust/kernel/device_id.rs | 26 +++++++++++++++-----------
 rust/kernel/of.rs        |  3 ++-
 rust/kernel/pci.rs       |  3 ++-
 4 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index d2cfe1eeefb6..7b8798599128 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -147,8 +147,10 @@ pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self {
 unsafe impl RawDeviceId for DeviceId {
     type RawType = bindings::auxiliary_device_id;
 
-    const DRIVER_DATA_OFFSET: usize =
-        core::mem::offset_of!(bindings::auxiliary_device_id, driver_data);
+    const DRIVER_DATA_OFFSET: Option<usize> = Some(core::mem::offset_of!(
+        bindings::auxiliary_device_id,
+        driver_data
+    ));
 
     fn index(&self) -> usize {
         self.0.driver_data
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 3dc72ca8cfc2..b7d00587a0e2 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -25,8 +25,9 @@
 ///     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.
+///   - If [`RawDeviceId::DRIVER_DATA_OFFSET`] is `Some(offset)`, it's 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.
@@ -37,7 +38,7 @@ pub unsafe trait RawDeviceId {
     type RawType: Copy;
 
     /// The offset to the context/data field.
-    const DRIVER_DATA_OFFSET: usize;
+    const DRIVER_DATA_OFFSET: Option<usize> = None;
 
     /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the [`RawDeviceId`] trait.
     fn index(&self) -> usize;
@@ -77,14 +78,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(data_offset) = T::DRIVER_DATA_OFFSET {
+                // 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(data_offset)
+                        .cast::<usize>()
+                        .write(i);
+                }
             }
 
             // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 40d1bd13682c..0ca1692d61f3 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -19,7 +19,8 @@
 unsafe impl RawDeviceId for DeviceId {
     type RawType = bindings::of_device_id;
 
-    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
+    const DRIVER_DATA_OFFSET: Option<usize> =
+        Some(core::mem::offset_of!(bindings::of_device_id, data));
 
     fn index(&self) -> usize {
         self.0.data as usize
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index f6b19764ad17..dea49abe7cd3 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -168,7 +168,8 @@ pub const fn from_class(class: u32, class_mask: u32) -> Self {
 unsafe impl RawDeviceId for DeviceId {
     type RawType = bindings::pci_device_id;
 
-    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
+    const DRIVER_DATA_OFFSET: Option<usize> =
+        Some(core::mem::offset_of!(bindings::pci_device_id, driver_data));
 
     fn index(&self) -> usize {
         self.0.driver_data
-- 
2.43.0

[PATCH v1 2/3] rust: net::phy Represent DeviceId as transparent wrapper over mdio_device_id

[FUJITA Tomonori]

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

[PATCH v1 3/3] rust: net::phy Change module_phy_driver macro to use module_device_table macro

[FUJITA Tomonori]

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 | 56 ++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 940972ffadae..4d797f7b79d2 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,17 @@ 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;
+
+    // This should never be called.
+    fn index(&self) -> usize {
+        0
+    }
+}
+
 enum DeviceMask {
     Exact,
     Model,
@@ -850,19 +861,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([
+///         ::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 +883,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([ $(($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

Re: [PATCH v1 3/3] rust: net::phy Change module_phy_driver macro to use module_device_table macro

[Miguel Ojeda]

On Mon, Jun 23, 2025 at 8:10 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> +// SAFETY: [`DeviceId`] is a `#[repr(transparent)` wrapper of `struct mdio_device_id`

No need for intra-doc link in normal comments.

Also, missing bracket (which apparently comes from another comment, so
probably copy-pasted).

> +    // This should never be called.
> +    fn index(&self) -> usize {
> +        0
> +    }

Hmm... This isn't great. Could this perhaps be designed differently?
e.g. split into two traits, possibly based one on another, or similar?

Worst case, we should at least do a `debug_assert!` or similar.

Cheers,
Miguel

Re: [PATCH v1 3/3] rust: net::phy Change module_phy_driver macro to use module_device_table macro

[FUJITA Tomonori]

On Mon, 23 Jun 2025 11:21:59 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Mon, Jun 23, 2025 at 8:10 AM FUJITA Tomonori
> <fujita.tomonori@gmail.com> wrote:
>>
>> +// SAFETY: [`DeviceId`] is a `#[repr(transparent)` wrapper of `struct mdio_device_id`
> 
> No need for intra-doc link in normal comments.

Oops. I'll fix.

> Also, missing bracket (which apparently comes from another comment, so
> probably copy-pasted).

Yeah, it was copied from the PCI code. The same typo exists in
multiple files, so I’ll send a fix for them later.

>> +    // This should never be called.
>> +    fn index(&self) -> usize {
>> +        0
>> +    }
> 
> Hmm... This isn't great. Could this perhaps be designed differently?
> e.g. split into two traits, possibly based one on another, or similar?

Yeah, I don’t really like it either.

I think we could split the RawDeviceId trait into two: RawDeviceId and
RawDeviceIdIndex. Device ID structures that do not contain
context/data field (like mdio_device for PHY) would implement only
RawDeviceId, while pci and others would implement both.

To avoid duplicate code, the new() function has been split into two as
follows. I'm also fine with simply adding an assert to index().

diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 3dc72ca8cfc2..f9205ea2a05e 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -35,7 +35,9 @@ pub unsafe trait RawDeviceId {
     ///
     /// Id tables created from [`Self`] are going to hold this type in its zero-terminated array.
     type RawType: Copy;
+}
 
+pub unsafe trait RawDeviceIdIndex {
     /// The offset to the context/data field.
     const DRIVER_DATA_OFFSET: usize;
 
@@ -65,10 +67,7 @@ pub struct IdArray<T: RawDeviceId, U, const N: usize> {
 }
 
 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 {
+    const fn init_ids(ids: [(T, U); N]) -> ([MaybeUninit<T::RawType>; N], [MaybeUninit<U>; N]) {
         let mut raw_ids = [const { MaybeUninit::<T::RawType>::uninit() }; N];
         let mut infos = [const { MaybeUninit::uninit() }; N];
 
@@ -77,24 +76,18 @@ 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);
-            }
 
             // SAFETY: this is effectively a move: `infos[i] = ids[i].1`. We make a copy here but
             // later forget `ids`.
             infos[i] = MaybeUninit::new(unsafe { core::ptr::read(&ids[i].1) });
             i += 1;
         }
-
         core::mem::forget(ids);
 
+        (raw_ids, infos)
+    }
+
+    const fn build_ids(raw_ids: [MaybeUninit<T::RawType>; N], infos: [MaybeUninit<U>; N]) -> Self {
         Self {
             raw_ids: RawIdArray {
                 // SAFETY: this is effectively `array_assume_init`, which is unstable, so we use
@@ -109,12 +102,47 @@ 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.
+    // once the specialization feature becomes stable, we can use `new()` name.
+    pub const fn new_without_index(ids: [(T, U); N]) -> Self {
+        let (raw_ids, infos) = Self::init_ids(ids);
+
+        Self::build_ids(raw_ids, infos)
+    }
+
     /// 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 {
+        let (mut raw_ids, infos) = Self::init_ids(ids);
+
+        let mut i = 0usize;
+        while i < N {
+            // 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);
+            }
+            i += 1;
+        }
+
+        Self::build_ids(raw_ids, infos)
+    }
+}
+
 /// A device id table.
 ///
 /// This trait is only implemented by `IdArray`.
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index f6b19764ad17..25ed7e5c56fa 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -8,6 +8,7 @@
     alloc::flags::*,
     bindings, container_of, device,
     device_id::RawDeviceId,
+    device_id::RawDeviceIdIndex,
     devres::Devres,
     driver,
     error::{to_result, Result},
@@ -167,7 +168,9 @@ pub const fn from_class(class: u32, class_mask: u32) -> Self {
 // * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
 unsafe impl RawDeviceId for DeviceId {
     type RawType = bindings::pci_device_id;
+}
 
+unsafe impl RawDeviceIdIndex for DeviceId {
     const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
 
     fn index(&self) -> usize {

Re: [PATCH v1 1/3] rust: device_id: make DRIVER_DATA_OFFSET optional

[Danilo Krummrich]

On 6/23/25 8:09 AM, FUJITA Tomonori wrote:
> Enable support for device ID structures that do not contain
> context/data field (usually named `driver_data`), making the trait
> usable in a wider range of subsystems and buses.
> 
> Several such structures are defined in
> include/linux/mod_devicetable.h.
> 
> This refactoring is a preparation for enabling the PHY abstractions to
> use device_id trait.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Acked-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v1 1/3] rust: device_id: make DRIVER_DATA_OFFSET optional

[FUJITA Tomonori]

On Fri, 4 Jul 2025 00:15:19 +0200
Danilo Krummrich <dakr@kernel.org> wrote:

> On 6/23/25 8:09 AM, FUJITA Tomonori wrote:
>> Enable support for device ID structures that do not contain
>> context/data field (usually named `driver_data`), making the trait
>> usable in a wider range of subsystems and buses.
>> Several such structures are defined in
>> include/linux/mod_devicetable.h.
>> This refactoring is a preparation for enabling the PHY abstractions to
>> use device_id trait.
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> Acked-by: Danilo Krummrich <dakr@kernel.org>

Thanks a lot!

Miguel suggested that splitting the RawDeviceId trait might lead to a
cleaner design, and I also tried that approach [v2]. But just to
confirm ― do you prefer the original v1 approach instead?

https://lore.kernel.org/lkml/CANiq72k0sdUoBxVYghgh50+ZRV2gbDkgVjuZgJLtj=4s9852xg@mail.gmail.com/

[v2]: https://lore.kernel.org/rust-for-linux/20250701141252.600113-1-fujita.tomonori@gmail.com/ 

Either way works for me.

Sorry Miguel, I forgot to fix the comment typo you pointed out in v1. I'll correct it in v3.

Re: [PATCH v1 1/3] rust: device_id: make DRIVER_DATA_OFFSET optional

[Danilo Krummrich]

On Fri, Jul 04, 2025 at 08:41:59AM +0900, FUJITA Tomonori wrote:
> On Fri, 4 Jul 2025 00:15:19 +0200
> Danilo Krummrich <dakr@kernel.org> wrote:
> 
> > On 6/23/25 8:09 AM, FUJITA Tomonori wrote:
> >> Enable support for device ID structures that do not contain
> >> context/data field (usually named `driver_data`), making the trait
> >> usable in a wider range of subsystems and buses.
> >> Several such structures are defined in
> >> include/linux/mod_devicetable.h.
> >> This refactoring is a preparation for enabling the PHY abstractions to
> >> use device_id trait.
> >> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > 
> > Acked-by: Danilo Krummrich <dakr@kernel.org>
> 
> Thanks a lot!
> 
> Miguel suggested that splitting the RawDeviceId trait might lead to a
> cleaner design, and I also tried that approach [v2]. But just to
> confirm ― do you prefer the original v1 approach instead?

Sorry, I didn't catch v2.

From only looking at patch 1, I indeed prefer v1.

> https://lore.kernel.org/lkml/CANiq72k0sdUoBxVYghgh50+ZRV2gbDkgVjuZgJLtj=4s9852xg@mail.gmail.com/

Looking at this reply to patch 3, I can see why Miguel suggested it though.
Given that, I think splitting the trait is the correct thing to do. I'll reply
to v2.

> [v2]: https://lore.kernel.org/rust-for-linux/20250701141252.600113-1-fujita.tomonori@gmail.com/ 
> 
> Either way works for me.
> 
> Sorry Miguel, I forgot to fix the comment typo you pointed out in v1. I'll correct it in v3.