register_foreign_release() is useful when a device resource has
associated data, but does not require the capability of accessing it or
manually releasing it.

If we would want to be able to access the device resource and release the
device resource manually before the device is unbound, but still keep
access to the associated data, we could implement it as follows.

	struct Registration<T> {
	   inner: Devres<RegistrationInner>,
	   data: T,
	}

However, if we never need to access the resource or release it manually,
register_foreign_release() is great optimization for the above, since it
does not require the synchronization of the Devres type.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/devres.rs | 80 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 4ee9037f4ad4..495dca6240fc 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -16,6 +16,7 @@
     sync::{rcu, Completion},
     types::{ARef, ForeignOwnable},
 };
+use core::ops::Deref;
 
 /// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to
 /// manage their lifetime.
@@ -303,3 +304,82 @@ pub fn register_foreign_boxed<T, E>(
 
     register_foreign(dev, data)
 }
+
+/// To be implemented by an object passed to [`register_foreign_release`].
+pub trait Release {
+    /// Called once the [`Device`] given to [`register_foreign_release`] is unbound.
+    fn release(&self);
+}
+
+impl<T: Release> Release for crate::sync::ArcBorrow<'_, T> {
+    fn release(&self) {
+        self.deref().release();
+    }
+}
+
+impl<T: Release> Release for Pin<&'_ T> {
+    fn release(&self) {
+        self.deref().release();
+    }
+}
+
+/// Consume the `data`, [`Release::release`] and [`Drop::drop`] `data` once `dev` is unbound.
+///
+/// # Examples
+///
+/// ```no_run
+/// use kernel::{device::{Bound, Device}, devres, devres::Release, sync::Arc};
+///
+/// struct Registration<T> {
+///     data: T,
+/// }
+///
+/// impl<T> Registration<T> {
+///     fn new(data: T) -> Result<Arc<Self>> {
+///         // register (e.g. class device, IRQ, etc.)
+///
+///         Ok(Arc::new(Self { data }, GFP_KERNEL)?)
+///     }
+/// }
+///
+/// impl<T> Release for Registration<T> {
+///     fn release(&self) {
+///        // unregister
+///     }
+/// }
+///
+/// fn from_bound_context(dev: &Device<Bound>) -> Result {
+///     let reg = Registration::new(0x42)?;
+///
+///     devres::register_foreign_release(dev, reg.clone())
+/// }
+/// ```
+pub fn register_foreign_release<P>(dev: &Device<Bound>, data: P) -> Result
+where
+    P: ForeignOwnable,
+    for<'a> P::Borrowed<'a>: Release,
+{
+    let ptr = data.into_foreign();
+
+    #[allow(clippy::missing_safety_doc)]
+    unsafe extern "C" fn callback<P>(ptr: *mut kernel::ffi::c_void)
+    where
+        P: ForeignOwnable,
+        for<'a> P::Borrowed<'a>: Release,
+    {
+        // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid.
+        unsafe { P::borrow(ptr.cast()) }.release();
+
+        // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid.
+        let _ = unsafe { P::from_foreign(ptr.cast()) };
+    }
+
+    // SAFETY:
+    // - `dev.as_raw()` is a pointer to a valid and bound device.
+    // - `ptr` is a valid pointer the `ForeignOwnable` devres takes ownership of.
+    to_result(unsafe {
+        // `devm_add_action_or_reset()` also calls `callback` on failure, such that the
+        // `ForeignOwnable` is released eventually.
+        bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::<P>), ptr.cast())
+    })
+}
-- 
2.49.0

On Thu Jun 12, 2025 at 4:51 PM CEST, Danilo Krummrich wrote:
> register_foreign_release() is useful when a device resource has
> associated data, but does not require the capability of accessing it or
> manually releasing it.
>
> If we would want to be able to access the device resource and release the
> device resource manually before the device is unbound, but still keep
> access to the associated data, we could implement it as follows.
>
> 	struct Registration<T> {
> 	   inner: Devres<RegistrationInner>,
> 	   data: T,
> 	}
>
> However, if we never need to access the resource or release it manually,
> register_foreign_release() is great optimization for the above, since it
> does not require the synchronization of the Devres type.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/devres.rs | 80 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)

Idea & implementation look good, I'm just a little unsatisfied with the
names & docs...

> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 4ee9037f4ad4..495dca6240fc 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -16,6 +16,7 @@
>      sync::{rcu, Completion},
>      types::{ARef, ForeignOwnable},
>  };
> +use core::ops::Deref;
>  
>  /// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to
>  /// manage their lifetime.
> @@ -303,3 +304,82 @@ pub fn register_foreign_boxed<T, E>(
>  
>      register_foreign(dev, data)
>  }
> +
> +/// To be implemented by an object passed to [`register_foreign_release`].

    /// [`Devres`]-releaseable resource.
    ///
    /// Register an object implementing this trait with [`register_foreign_release`]. It's `release`
    /// function will be called once the device unbinds.

> +pub trait Release {
> +    /// Called once the [`Device`] given to [`register_foreign_release`] is unbound.
> +    fn release(&self);

Would it make sense to also supply the `Device` that this is attached
to? In case you have one object in multiple `register_foreign_release`
calls with different devices, or is that something that doesn't happen?

> +}
> +
> +impl<T: Release> Release for crate::sync::ArcBorrow<'_, T> {
> +    fn release(&self) {
> +        self.deref().release();
> +    }
> +}
> +
> +impl<T: Release> Release for Pin<&'_ T> {
> +    fn release(&self) {
> +        self.deref().release();
> +    }
> +}

We should also implement it for `&T`, since that is `Box`'s `Borrowed`.

> +
> +/// Consume the `data`, [`Release::release`] and [`Drop::drop`] `data` once `dev` is unbound.
> +///
> +/// # Examples
> +///
> +/// ```no_run
> +/// use kernel::{device::{Bound, Device}, devres, devres::Release, sync::Arc};
> +///
> +/// struct Registration<T> {

Maybe add some explanation above/below this example. It looks like a new
bus registration?

---
Cheers,
Benno

> +///     data: T,
> +/// }
> +///
> +/// impl<T> Registration<T> {
> +///     fn new(data: T) -> Result<Arc<Self>> {
> +///         // register (e.g. class device, IRQ, etc.)
> +///
> +///         Ok(Arc::new(Self { data }, GFP_KERNEL)?)
> +///     }
> +/// }
> +///
> +/// impl<T> Release for Registration<T> {
> +///     fn release(&self) {
> +///        // unregister
> +///     }
> +/// }
> +///
> +/// fn from_bound_context(dev: &Device<Bound>) -> Result {
> +///     let reg = Registration::new(0x42)?;
> +///
> +///     devres::register_foreign_release(dev, reg.clone())
> +/// }
> +/// ```
> +pub fn register_foreign_release<P>(dev: &Device<Bound>, data: P) -> Result
> +where
> +    P: ForeignOwnable,
> +    for<'a> P::Borrowed<'a>: Release,
> +{
> +    let ptr = data.into_foreign();
> +
> +    #[allow(clippy::missing_safety_doc)]
> +    unsafe extern "C" fn callback<P>(ptr: *mut kernel::ffi::c_void)
> +    where
> +        P: ForeignOwnable,
> +        for<'a> P::Borrowed<'a>: Release,
> +    {
> +        // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid.
> +        unsafe { P::borrow(ptr.cast()) }.release();
> +
> +        // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid.
> +        let _ = unsafe { P::from_foreign(ptr.cast()) };
> +    }
> +
> +    // SAFETY:
> +    // - `dev.as_raw()` is a pointer to a valid and bound device.
> +    // - `ptr` is a valid pointer the `ForeignOwnable` devres takes ownership of.
> +    to_result(unsafe {
> +        // `devm_add_action_or_reset()` also calls `callback` on failure, such that the
> +        // `ForeignOwnable` is released eventually.
> +        bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::<P>), ptr.cast())
> +    })
> +}

On Sun, Jun 22, 2025 at 09:26:33AM +0200, Benno Lossin wrote:
> On Thu Jun 12, 2025 at 4:51 PM CEST, Danilo Krummrich wrote:
> > +/// To be implemented by an object passed to [`register_foreign_release`].
> 
>     /// [`Devres`]-releaseable resource.
>     ///
>     /// Register an object implementing this trait with [`register_foreign_release`]. It's `release`
>     /// function will be called once the device unbinds.

Sounds good, thanks!

> > +pub trait Release {
> > +    /// Called once the [`Device`] given to [`register_foreign_release`] is unbound.
> > +    fn release(&self);
> 
> Would it make sense to also supply the `Device` that this is attached
> to? In case you have one object in multiple `register_foreign_release`
> calls with different devices, or is that something that doesn't happen?

No, doing that wouldn't make any sense. A resource should only be bound to the
lifetime of a single device.

> > +}
> > +
> > +impl<T: Release> Release for crate::sync::ArcBorrow<'_, T> {
> > +    fn release(&self) {
> > +        self.deref().release();
> > +    }
> > +}
> > +
> > +impl<T: Release> Release for Pin<&'_ T> {
> > +    fn release(&self) {
> > +        self.deref().release();
> > +    }
> > +}
> 
> We should also implement it for `&T`, since that is `Box`'s `Borrowed`.

That should implicitly be the case when T: Release, where T is P<T>.

> > +
> > +/// Consume the `data`, [`Release::release`] and [`Drop::drop`] `data` once `dev` is unbound.
> > +///
> > +/// # Examples
> > +///
> > +/// ```no_run
> > +/// use kernel::{device::{Bound, Device}, devres, devres::Release, sync::Arc};
> > +///
> > +/// struct Registration<T> {
> 
> Maybe add some explanation above/below this example. It looks like a new
> bus registration?

*class device registration, see Registration::new() below. But I can also add a
brief comment to the struct. It's indeed a bit subtly this way. :)

> > +///     data: T,
> > +/// }
> > +///
> > +/// impl<T> Registration<T> {
> > +///     fn new(data: T) -> Result<Arc<Self>> {
> > +///         // register (e.g. class device, IRQ, etc.)
> > +///
> > +///         Ok(Arc::new(Self { data }, GFP_KERNEL)?)
> > +///     }
> > +/// }
> > +///
> > +/// impl<T> Release for Registration<T> {
> > +///     fn release(&self) {
> > +///        // unregister
> > +///     }
> > +/// }
> > +///
> > +/// fn from_bound_context(dev: &Device<Bound>) -> Result {
> > +///     let reg = Registration::new(0x42)?;
> > +///
> > +///     devres::register_foreign_release(dev, reg.clone())
> > +/// }
> > +/// ```
> > +pub fn register_foreign_release<P>(dev: &Device<Bound>, data: P) -> Result
> > +where
> > +    P: ForeignOwnable,
> > +    for<'a> P::Borrowed<'a>: Release,
> > +{
> > +    let ptr = data.into_foreign();
> > +
> > +    #[allow(clippy::missing_safety_doc)]
> > +    unsafe extern "C" fn callback<P>(ptr: *mut kernel::ffi::c_void)
> > +    where
> > +        P: ForeignOwnable,
> > +        for<'a> P::Borrowed<'a>: Release,
> > +    {
> > +        // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid.
> > +        unsafe { P::borrow(ptr.cast()) }.release();
> > +
> > +        // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked above and hence valid.
> > +        let _ = unsafe { P::from_foreign(ptr.cast()) };
> > +    }
> > +
> > +    // SAFETY:
> > +    // - `dev.as_raw()` is a pointer to a valid and bound device.
> > +    // - `ptr` is a valid pointer the `ForeignOwnable` devres takes ownership of.
> > +    to_result(unsafe {
> > +        // `devm_add_action_or_reset()` also calls `callback` on failure, such that the
> > +        // `ForeignOwnable` is released eventually.
> > +        bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::<P>), ptr.cast())
> > +    })
> > +}
>

On Sun Jun 22, 2025 at 2:46 PM CEST, Danilo Krummrich wrote:
> On Sun, Jun 22, 2025 at 09:26:33AM +0200, Benno Lossin wrote:
>> On Thu Jun 12, 2025 at 4:51 PM CEST, Danilo Krummrich wrote:
>> > +pub trait Release {
>> > +    /// Called once the [`Device`] given to [`register_foreign_release`] is unbound.
>> > +    fn release(&self);
>> 
>> Would it make sense to also supply the `Device` that this is attached
>> to? In case you have one object in multiple `register_foreign_release`
>> calls with different devices, or is that something that doesn't happen?
>
> No, doing that wouldn't make any sense. A resource should only be bound to the
> lifetime of a single device.

But the API doesn't prevent it... Does it make more sense to instead ask
the user to provide a closure?

>> > +}
>> > +
>> > +impl<T: Release> Release for crate::sync::ArcBorrow<'_, T> {
>> > +    fn release(&self) {
>> > +        self.deref().release();
>> > +    }
>> > +}
>> > +
>> > +impl<T: Release> Release for Pin<&'_ T> {
>> > +    fn release(&self) {
>> > +        self.deref().release();
>> > +    }
>> > +}
>> 
>> We should also implement it for `&T`, since that is `Box`'s `Borrowed`.
>
> That should implicitly be the case when T: Release, where T is P<T>.

I don't understand? When `P` is set to `Box<T>` in the
`register_foreign_release` below, then `P::Borrow<'_> == &'_ T` and the
bound of `&T: Release` is not satisfied.

>> > +
>> > +/// Consume the `data`, [`Release::release`] and [`Drop::drop`] `data` once `dev` is unbound.
>> > +///
>> > +/// # Examples
>> > +///
>> > +/// ```no_run
>> > +/// use kernel::{device::{Bound, Device}, devres, devres::Release, sync::Arc};
>> > +///
>> > +/// struct Registration<T> {
>> 
>> Maybe add some explanation above/below this example. It looks like a new
>> bus registration?
>
> *class device registration, see Registration::new() below. But I can also add a
> brief comment to the struct. It's indeed a bit subtly this way. :)

I'd just add a small paragraph explaining what it does :)

---
Cheers,
Benno

On Sun, Jun 22, 2025 at 10:14:00PM +0200, Benno Lossin wrote:
> I don't understand? When `P` is set to `Box<T>` in the
> `register_foreign_release` below, then `P::Borrow<'_> == &'_ T` and the
> bound of `&T: Release` is not satisfied.

Yeah, you're right, we have to add `&T` too.