From 0242623384c767b1156b61b67894b4ecf6682b8b Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 16 Oct 2025 14:55:28 +0200 Subject: rust: driver: let probe() return impl PinInit The driver model defines the lifetime of the private data stored in (and owned by) a bus device to be valid from when the driver is bound to a device (i.e. from successful probe()) until the driver is unbound from the device. This is already taken care of by the Rust implementation of the driver model. However, we still ask drivers to return a Result>> from probe(). Unlike in C, where we do not have the concept of initializers, but rather deal with uninitialized memory, drivers can just return an impl PinInit instead. This contributes to more clarity to the fact that a driver returns it's device private data in probe() and the Rust driver model owns the data, manages the lifetime and - considering the lifetime - provides (safe) accessors for the driver. Hence, let probe() functions return an impl PinInit instead of Result>>. Reviewed-by: Alice Ryhl Acked-by: Viresh Kumar Reviewed-by: Alexandre Courbot Acked-by: Greg Kroah-Hartman Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index e11848bbf206..4163129b4103 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -68,9 +68,9 @@ impl Adapter { let info = T::ID_TABLE.info(id.index()); from_result(|| { - let data = T::probe(adev, info)?; + let data = T::probe(adev, info); - adev.as_ref().set_drvdata(data); + adev.as_ref().set_drvdata(data)?; Ok(0) }) } @@ -184,7 +184,7 @@ pub trait Driver { /// Auxiliary driver probe. /// /// Called when an auxiliary device is matches a corresponding driver. - fn probe(dev: &Device, id_info: &Self::IdInfo) -> Result>>; + fn probe(dev: &Device, id_info: &Self::IdInfo) -> impl PinInit; } /// The auxiliary device representation. -- cgit From 6bbaa93912bfdfd5ffdc804275cc6a444c9400af Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 21 Oct 2025 00:34:23 +0200 Subject: rust: device: narrow the generic of drvdata_obtain() Let T be the actual private driver data type without the surrounding box, as it leaves less room for potential bugs. Reviewed-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index e12f78734606..a6a2b23befce 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -85,7 +85,7 @@ impl Adapter { // SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called // and stored a `Pin>`. - drop(unsafe { adev.as_ref().drvdata_obtain::>>() }); + drop(unsafe { adev.as_ref().drvdata_obtain::() }); } } -- cgit From 589b061975db3c7e87b819cc9a8006eb99ac4b5f Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 21 Oct 2025 00:34:25 +0200 Subject: rust: auxiliary: consider auxiliary devices always have a parent An auxiliary device is guaranteed to always have a parent device (both in C and Rust), hence don't return an Option<&auxiliary::Device> in auxiliary::Device::parent(). Reviewed-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index a6a2b23befce..e5bddb738d58 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -215,9 +215,10 @@ impl Device { unsafe { (*self.as_raw()).id } } - /// Returns a reference to the parent [`device::Device`], if any. - pub fn parent(&self) -> Option<&device::Device> { - self.as_ref().parent() + /// Returns a reference to the parent [`device::Device`]. + pub fn parent(&self) -> &device::Device { + // SAFETY: A `struct auxiliary_device` always has a parent. + unsafe { self.as_ref().parent().unwrap_unchecked() } } } -- cgit From e4e679c8608e5c747081cc6ce63ee0b0e524c68d Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 21 Oct 2025 00:34:26 +0200 Subject: rust: auxiliary: unregister on parent device unbind Guarantee that an auxiliary driver will be unbound before its parent is unbound; there is no point in operating an auxiliary device whose parent has been unbound. In practice, this guarantee allows us to assume that for a bound auxiliary device, also the parent device is bound. This is useful when an auxiliary driver calls into its parent, since it allows the parent to directly access device resources and its device private data due to the guaranteed bound device context. Reviewed-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 89 +++++++++++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 38 deletions(-) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index e5bddb738d58..8c0a2472c26a 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -7,6 +7,7 @@ use crate::{ bindings, container_of, device, device_id::{RawDeviceId, RawDeviceIdIndex}, + devres::Devres, driver, error::{from_result, to_result, Result}, prelude::*, @@ -279,8 +280,8 @@ unsafe impl Sync for Device {} /// The registration of an auxiliary device. /// -/// This type represents the registration of a [`struct auxiliary_device`]. When an instance of this -/// type is dropped, its respective auxiliary device will be unregistered from the system. +/// This type represents the registration of a [`struct auxiliary_device`]. When its parent device +/// is unbound, the corresponding auxiliary device will be unregistered from the system. /// /// # Invariants /// @@ -290,44 +291,56 @@ pub struct Registration(NonNull); impl Registration { /// Create and register a new auxiliary device. - pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result { - let boxed = KBox::new(Opaque::::zeroed(), GFP_KERNEL)?; - let adev = boxed.get(); + pub fn new<'a>( + parent: &'a device::Device, + name: &'a CStr, + id: u32, + modname: &'a CStr, + ) -> impl PinInit, Error> + 'a { + pin_init::pin_init_scope(move || { + let boxed = KBox::new(Opaque::::zeroed(), GFP_KERNEL)?; + let adev = boxed.get(); + + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. + unsafe { + (*adev).dev.parent = parent.as_raw(); + (*adev).dev.release = Some(Device::release); + (*adev).name = name.as_char_ptr(); + (*adev).id = id; + } - // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. - unsafe { - (*adev).dev.parent = parent.as_raw(); - (*adev).dev.release = Some(Device::release); - (*adev).name = name.as_char_ptr(); - (*adev).id = id; - } - - // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, - // which has not been initialized yet. - unsafe { bindings::auxiliary_device_init(adev) }; - - // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed - // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. - let _ = KBox::into_raw(boxed); - - // SAFETY: - // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has - // been initialialized, - // - `modname.as_char_ptr()` is a NULL terminated string. - let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; - if ret != 0 { // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, - // which has been initialialized. - unsafe { bindings::auxiliary_device_uninit(adev) }; - - return Err(Error::from_errno(ret)); - } - - // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated successfully. - // - // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is called, - // which happens in `Self::drop()`. - Ok(Self(unsafe { NonNull::new_unchecked(adev) })) + // which has not been initialized yet. + unsafe { bindings::auxiliary_device_init(adev) }; + + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be + // freed by `Device::release` when the last reference to the `struct auxiliary_device` + // is dropped. + let _ = KBox::into_raw(boxed); + + // SAFETY: + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which + // has been initialized, + // - `modname.as_char_ptr()` is a NULL terminated string. + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; + if ret != 0 { + // SAFETY: `adev` is guaranteed to be a valid pointer to a + // `struct auxiliary_device`, which has been initialized. + unsafe { bindings::auxiliary_device_uninit(adev) }; + + return Err(Error::from_errno(ret)); + } + + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated + // successfully. + // + // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is + // called, which happens in `Self::drop()`. + Ok(Devres::new( + parent, + Self(unsafe { NonNull::new_unchecked(adev) }), + )) + }) } } -- cgit From b69165a09727b653993934d700a02d32a8961327 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 21 Oct 2025 00:34:27 +0200 Subject: rust: auxiliary: move parent() to impl Device Currently, the parent method is implemented for any Device, i.e. any device context and returns a &device::Device. However, a subsequent patch will introduce impl Device { pub fn parent() -> device::Device { ... } } which takes advantage of the fact that if the auxiliary device is bound the parent is guaranteed to be bound as well. I.e. the behavior we want is that all device contexts that dereference to Bound, will use the implementation above, whereas the old implementation should only be implemented for Device. Hence, move the current implementation. Reviewed-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index 8c0a2472c26a..497601f7473b 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -215,15 +215,15 @@ impl Device { // `struct auxiliary_device`. unsafe { (*self.as_raw()).id } } +} +impl Device { /// Returns a reference to the parent [`device::Device`]. pub fn parent(&self) -> &device::Device { // SAFETY: A `struct auxiliary_device` always has a parent. unsafe { self.as_ref().parent().unwrap_unchecked() } } -} -impl Device { extern "C" fn release(dev: *mut bindings::device) { // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` // embedded in `struct auxiliary_device`. -- cgit From 675e514edd659b5cfc15eb70bd8abf53568947cc Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 21 Oct 2025 00:34:28 +0200 Subject: rust: auxiliary: implement parent() for Device Take advantage of the fact that if the auxiliary device is bound the parent is guaranteed to be bound as well and implement a separate parent() method for auxiliary::Device. Reviewed-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index 497601f7473b..cc67fa5ddde3 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -217,6 +217,16 @@ impl Device { } } +impl Device { + /// Returns a bound reference to the parent [`device::Device`]. + pub fn parent(&self) -> &device::Device { + let parent = (**self).parent(); + + // SAFETY: A bound auxiliary device always has a bound parent device. + unsafe { parent.as_bound() } + } +} + impl Device { /// Returns a reference to the parent [`device::Device`]. pub fn parent(&self) -> &device::Device { -- cgit From 1bf5b90cd2f984e5d6ff6fd30d5d85f9f579b6f0 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 3 Nov 2025 21:39:18 +0100 Subject: rust: auxiliary: fix false positive warning for missing a safety comment Some older (yet supported) versions of clippy throw a false positive warning for missing a safety comment when the safety comment is on a multiline statement. warning: unsafe block missing a safety comment --> rust/kernel/auxiliary.rs:351:22 | 351 | Self(unsafe { NonNull::new_unchecked(adev) }), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider adding a safety comment on the preceding line = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks = note: requested on the command line with `-W clippy::undocumented-unsafe-blocks` warning: 1 warning emitted Fix this by placing the safety comment right on top of the same line introducing the unsafe block. Fixes: e4e679c8608e ("rust: auxiliary: unregister on parent device unbind") Reviewed-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Link: https://patch.msgid.link/20251103203932.2361660-1-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index cc67fa5ddde3..618eeeec2bd0 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -341,13 +341,12 @@ impl Registration { return Err(Error::from_errno(ret)); } - // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated - // successfully. - // // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is // called, which happens in `Self::drop()`. Ok(Devres::new( parent, + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated + // successfully. Self(unsafe { NonNull::new_unchecked(adev) }), )) }) -- cgit From e4addc7cc2dfcc19f1c8c8e47f3834b22cb21559 Mon Sep 17 00:00:00 2001 From: Markus Probst Date: Mon, 27 Oct 2025 20:06:03 +0000 Subject: rust: Add trait to convert a device reference to a bus device reference Implement the `AsBusDevice` trait for converting a `Device` reference to a bus device reference for all bus devices. The `AsBusDevice` trait allows abstractions to provide the bus device in class device callbacks. It must not be used by drivers and is intended for bus and class device abstractions only. Signed-off-by: Markus Probst Link: https://patch.msgid.link/20251027200547.1038967-2-markus.probst@posteo.de [ * Remove unused import. * Change visibility of AsBusDevice to public. * Fix build for USB. * Add impl for I2cClient. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'rust/kernel/auxiliary.rs') diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index 618eeeec2bd0..56f3c180e8f6 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -16,6 +16,7 @@ use crate::{ }; use core::{ marker::PhantomData, + mem::offset_of, ptr::{addr_of_mut, NonNull}, }; @@ -245,6 +246,12 @@ impl Device { } } +// SAFETY: `auxiliary::Device` is a transparent wrapper of `struct auxiliary_device`. +// The offset is guaranteed to point to a valid device field inside `auxiliary::Device`. +unsafe impl device::AsBusDevice for Device { + const OFFSET: usize = offset_of!(bindings::auxiliary_device, dev); +} + // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); -- cgit