Re: [PATCH 2/8] rust: device: introduce Device::drvdata()

Danilo Krummrich
2025-10-29T17:02:50Z

On Wed Oct 29, 2025 at 4:30 PM CET, Danilo Krummrich wrote:
> On Wed Oct 29, 2025 at 1:59 PM CET, Alice Ryhl wrote:
>> Are you going to open that docs PR to the Rust compiler about the size
>> of TypeID that we talked about? :)
>
> Yes, I will -- thanks for the reminder.
>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>
>>> +// Compile-time checks.
>>> +const _: () = {
>>> + // Assert that we can `read()` / `write()` a `TypeId` instance from / into `struct driver_type`.
>>> + static_assert!(core::mem::size_of::<bindings::driver_type>() == core::mem::size_of::<TypeId>());
>>> +};
>>
>> You don't need the "const _: ()" part. See the definition of
>> static_assert! to see why.
>
> Indeed, good catch -- same for the suggestions below.
>
>> Also, I would not require equality. The Rust team did not think that it
>> would ever increase in size, but it may decrease.
>>
>>> /// The core representation of a device in the kernel's driver model.
>>> ///
>>> /// This structure represents the Rust abstraction for a C `struct device`. A [`Device`] can either
>>> @@ -198,12 +204,29 @@ pub unsafe fn as_bound(&self) -> &Device<Bound> {
>>> }
>>>
>>> impl Device<CoreInternal> {
>>> + fn type_id_store<T: 'static>(&self) {
>>
>> This name isn't great. How about "set_type_id()" instead?

Here's the diff, including a missing check in case someone tries to call
Device::drvdata() from probe().

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 36c6eec0ceab..1a307be953c2 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -17,11 +17,8 @@

pub mod property;

-// Compile-time checks.
-const _: () = {
- // Assert that we can `read()` / `write()` a `TypeId` instance from / into `struct driver_type`.
- static_assert!(core::mem::size_of::<bindings::driver_type>() == core::mem::size_of::<TypeId>());
-};
+// Assert that we can `read()` / `write()` a `TypeId` instance from / into `struct driver_type`.
+static_assert!(core::mem::size_of::<bindings::driver_type>() >= core::mem::size_of::<TypeId>());

/// The core representation of a device in the kernel's driver model.
///
@@ -204,7 +201,7 @@ pub unsafe fn as_bound(&self) -> &Device<Bound> {
}

impl Device<CoreInternal> {
- fn type_id_store<T: 'static>(&self) {
+ fn set_type_id<T: 'static>(&self) {
// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
let private = unsafe { (*self.as_raw()).p };

@@ -226,7 +223,7 @@ pub fn set_drvdata<T: 'static>(&self, data: impl PinInit<T, Error>) -> Result {

// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
unsafe { bindings::dev_set_drvdata(self.as_raw(), data.into_foreign().cast()) };
- self.type_id_store::<T>();
+ self.set_type_id::<T>();

Ok(())
}
@@ -242,6 +239,9 @@ pub unsafe fn drvdata_obtain<T: 'static>(&self) -> Pin<KBox<T>> {
// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };

+ // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+ unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
+
// SAFETY:
// - By the safety requirements of this function, `ptr` comes from a previous call to
// `into_foreign()`.
@@ -286,7 +286,7 @@ unsafe fn drvdata_unchecked<T: 'static>(&self) -> Pin<&T> {
unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) }
}

- fn type_id_match<T: 'static>(&self) -> Result {
+ fn match_type_id<T: 'static>(&self) -> Result {
// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
let private = unsafe { (*self.as_raw()).p };

@@ -311,11 +311,16 @@ fn type_id_match<T: 'static>(&self) -> Result {
/// Returns a pinned reference to the driver's private data or [`EINVAL`] if it doesn't match
/// the asserted type `T`.
pub fn drvdata<T: 'static>(&self) -> Result<Pin<&T>> {
- self.type_id_match::<T>()?;
+ // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+ if unsafe { bindings::dev_get_drvdata(self.as_raw()) }.is_null() {
+ return Err(ENOENT);
+ }
+
+ self.match_type_id::<T>()?;

// SAFETY:
- // - The `Bound` device context guarantees that this is only ever call after a call
- // to `set_drvdata()` and before `drvdata_obtain()`.
+ // - The above check of `dev_get_drvdata()` guarantees that we are called after
+ // `set_drvdata()` and before `drvdata_obtain()`.
// - We've just checked that the type of the driver's private data is in fact `T`.
Ok(unsafe { self.drvdata_unchecked() })
}