[PATCH v2 3/6] rust: device: Support testing devices for equality

Matthew Maurer posted 6 patches 5 days, 3 hours ago
[PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Matthew Maurer 5 days, 3 hours ago
This allows device drivers to check if, for example, an auxiliary
devices is one of its children by comparing the parent field, or
checking if a device parameter is its own device.

Also convert existing `.as_raw() != .as_raw()` to  use this new
implementation.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/kernel/device.rs     | 8 ++++++++
 rust/kernel/devres.rs     | 2 +-
 rust/kernel/drm/driver.rs | 2 +-
 rust/kernel/pwm.rs        | 2 +-
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 94e0548e76871d8b7de309c1f1c7b77bb49738ed..aa10359d3ebdd1c99cc567a35b89f52ddb2ee050 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -516,6 +516,14 @@ unsafe impl Send for Device {}
 // synchronization in `struct device`.
 unsafe impl Sync for Device {}
 
+impl<Ctx: DeviceContext, Ctx2: DeviceContext> PartialEq<Device<Ctx2>> for Device<Ctx> {
+    fn eq(&self, other: &Device<Ctx2>) -> bool {
+        self.as_raw() == other.as_raw()
+    }
+}
+
+impl<Ctx: DeviceContext> Eq for Device<Ctx> {}
+
 /// Marker trait for the context or scope of a bus specific device.
 ///
 /// [`DeviceContext`] is a marker trait for types representing the context of a bus specific
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index cdc49677022a6b466e771d9d8cf3818ab9b9112d..20126daad193370868661b9412937937eda6d3c4 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -281,7 +281,7 @@ pub fn device(&self) -> &Device {
     /// }
     /// ```
     pub fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Result<&'a T> {
-        if self.dev.as_raw() != dev.as_raw() {
+        if self.dev.as_ref() != dev {
             return Err(EINVAL);
         }
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index f30ee4c6245cda72ac72852bf9362736d8fe992f..497ef46028d560bc9649dbbdf69316ce4fce8199 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -139,7 +139,7 @@ pub fn new_foreign_owned(
     where
         T: 'static,
     {
-        if drm.as_ref().as_raw() != dev.as_raw() {
+        if drm.as_ref() != dev {
             return Err(EINVAL);
         }
 
diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index cb00f8a8765c8ec58ed78a73275b022b02bf7aa3..033f778909a2633acbc25d5a21a1c8a7b8e41a70 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -680,7 +680,7 @@ impl<T: 'static + PwmOps + Send + Sync> Registration<T> {
     /// calling `pwmchip_remove`. This function should be called from the driver's `probe`.
     pub fn register(dev: &device::Device<Bound>, chip: ARef<Chip<T>>) -> Result {
         let chip_parent = chip.device().parent().ok_or(EINVAL)?;
-        if dev.as_raw() != chip_parent.as_raw() {
+        if dev != chip_parent {
             return Err(EINVAL);
         }
 

-- 
2.53.0.rc2.204.g2597b5adb4-goog
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Greg Kroah-Hartman 5 days, 3 hours ago
On Tue, Feb 03, 2026 at 03:46:32PM +0000, Matthew Maurer wrote:
> This allows device drivers to check if, for example, an auxiliary
> devices is one of its children by comparing the parent field, or
> checking if a device parameter is its own device.
> 
> Also convert existing `.as_raw() != .as_raw()` to  use this new
> implementation.

Ah, ok, I can see how getting rid of the as_raw() calls here is a "good
thing", but overall it's just the same code paths :)

And I don't see what patch in this series uses this, am I missing it?

thanks,

greg k-h
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Danilo Krummrich 5 days, 2 hours ago
On Tue Feb 3, 2026 at 5:17 PM CET, Greg Kroah-Hartman wrote:
> And I don't see what patch in this series uses this, am I missing it?

	impl Smem {
	    pub(crate) fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Option<&'a Mmio> {
	        if *dev != *self.dev {
	            return None;
	        }
	
	        // SAFETY: By our invariant, this was a subrange of what was returned by smem_aux_get, for
	        // self.dev, and by our above check, that auxdev is still available.
	        Some(unsafe { Mmio::from_raw(&self.raw) })
	    }
	}

It's used to ensure that the Smem provided by the auxiliary parent can only be
accessed as long as the auxiliary parent device is bound.
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Greg Kroah-Hartman 5 days, 2 hours ago
On Tue, Feb 03, 2026 at 05:29:16PM +0100, Danilo Krummrich wrote:
> On Tue Feb 3, 2026 at 5:17 PM CET, Greg Kroah-Hartman wrote:
> > And I don't see what patch in this series uses this, am I missing it?
> 
> 	impl Smem {
> 	    pub(crate) fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Option<&'a Mmio> {
> 	        if *dev != *self.dev {
> 	            return None;
> 	        }
> 	
> 	        // SAFETY: By our invariant, this was a subrange of what was returned by smem_aux_get, for
> 	        // self.dev, and by our above check, that auxdev is still available.
> 	        Some(unsafe { Mmio::from_raw(&self.raw) })
> 	    }
> 	}
> 
> It's used to ensure that the Smem provided by the auxiliary parent can only be
> accessed as long as the auxiliary parent device is bound.

But how can a parent device ever bevome "unbound"?  A child can always
rely on its parent being present (some odd scsi devices are the one
exception, that mess is hell at times...)

thanks,

greg k-h
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Danilo Krummrich 5 days, 2 hours ago
On Tue Feb 3, 2026 at 5:40 PM CET, Greg Kroah-Hartman wrote:
> On Tue, Feb 03, 2026 at 05:29:16PM +0100, Danilo Krummrich wrote:
>> On Tue Feb 3, 2026 at 5:17 PM CET, Greg Kroah-Hartman wrote:
>> > And I don't see what patch in this series uses this, am I missing it?
>> 
>> 	impl Smem {
>> 	    pub(crate) fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Option<&'a Mmio> {
>> 	        if *dev != *self.dev {
>> 	            return None;
>> 	        }
>> 	
>> 	        // SAFETY: By our invariant, this was a subrange of what was returned by smem_aux_get, for
>> 	        // self.dev, and by our above check, that auxdev is still available.
>> 	        Some(unsafe { Mmio::from_raw(&self.raw) })
>> 	    }
>> 	}
>> 
>> It's used to ensure that the Smem provided by the auxiliary parent can only be
>> accessed as long as the auxiliary parent device is bound.
>
> But how can a parent device ever bevome "unbound"?

It can't, that's why auxiliary::Device::parent() returns a &Device<Bound>, i.e.
as long as the child is bound the parent is guaranteed to be bound as well.

The point in this implementation is that we need to prove to the resource
container (Smem) that we are allowed to access the resource, since it is only
valid to access for the duration the parent is bound.

In the end this is equivalent to Devres::access(), which bypasses the Revocable
if we can prove that we are in a &Device<Bound> scope.

Having that said, the code should probably just use Devres instead. :)
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Matthew Maurer 5 days, 2 hours ago
On Tue, Feb 3, 2026 at 8:46 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Feb 3, 2026 at 5:40 PM CET, Greg Kroah-Hartman wrote:
> > On Tue, Feb 03, 2026 at 05:29:16PM +0100, Danilo Krummrich wrote:
> >> On Tue Feb 3, 2026 at 5:17 PM CET, Greg Kroah-Hartman wrote:
> >> > And I don't see what patch in this series uses this, am I missing it?
> >>
> >>      impl Smem {
> >>          pub(crate) fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Option<&'a Mmio> {
> >>              if *dev != *self.dev {
> >>                  return None;
> >>              }
> >>
> >>              // SAFETY: By our invariant, this was a subrange of what was returned by smem_aux_get, for
> >>              // self.dev, and by our above check, that auxdev is still available.
> >>              Some(unsafe { Mmio::from_raw(&self.raw) })
> >>          }
> >>      }
> >>
> >> It's used to ensure that the Smem provided by the auxiliary parent can only be
> >> accessed as long as the auxiliary parent device is bound.
> >
> > But how can a parent device ever bevome "unbound"?
>
> It can't, that's why auxiliary::Device::parent() returns a &Device<Bound>, i.e.
> as long as the child is bound the parent is guaranteed to be bound as well.
>
> The point in this implementation is that we need to prove to the resource
> container (Smem) that we are allowed to access the resource, since it is only
> valid to access for the duration the parent is bound.
>
> In the end this is equivalent to Devres::access(), which bypasses the Revocable
> if we can prove that we are in a &Device<Bound> scope.
>
> Having that said, the code should probably just use Devres instead. :)

Not using Devres was intentional because:
1. Implementing `subrange` would require more unsafe / invariant logic
with Devres. I would need to either wrap `MmioRaw` in a wrapper that
maintained that it would always be inside a Devres that owned it, or
wrap the Devres<MmioRaw> in a wrapper that encoded that invariant.
Then, I would need to propagate the `dev` from the source `Devres` to
the derived `Devres`.
2. Devres is PinInit, so I can't put it in a vector.
3. I don't need the destructor feature of Devres that necessitates the
extra complexity, because this is morally a checked borrow, not an
owned value.

I considered adding a more lightweight `Devref` to the kernel crate
which only supports types which do not implement `Drop` and access
through `access`, and so does not need to be `PinInit`. However, since
that would still require the manual dev propagation wrapper, it seemed
like it might be overkill.
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Greg Kroah-Hartman 5 days, 3 hours ago
On Tue, Feb 03, 2026 at 03:46:32PM +0000, Matthew Maurer wrote:
> This allows device drivers to check if, for example, an auxiliary
> devices is one of its children by comparing the parent field, or
> checking if a device parameter is its own device.

Ok, but why?  This says what it does, but I have no context as to why
this would be needed.

What driver wants to know this?

thanks,

greg k-h
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Gary Guo 5 days, 3 hours ago
On Tue Feb 3, 2026 at 3:46 PM GMT, Matthew Maurer wrote:
> This allows device drivers to check if, for example, an auxiliary
> devices is one of its children by comparing the parent field, or
> checking if a device parameter is its own device.
> 
> Also convert existing `.as_raw() != .as_raw()` to  use this new
> implementation.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/device.rs     | 8 ++++++++
>  rust/kernel/devres.rs     | 2 +-
>  rust/kernel/drm/driver.rs | 2 +-
>  rust/kernel/pwm.rs        | 2 +-
>  4 files changed, 11 insertions(+), 3 deletions(-)
Re: [PATCH v2 3/6] rust: device: Support testing devices for equality
Posted by Danilo Krummrich 5 days, 3 hours ago
On Tue Feb 3, 2026 at 4:46 PM CET, Matthew Maurer wrote:
> This allows device drivers to check if, for example, an auxiliary
> devices is one of its children by comparing the parent field, or
> checking if a device parameter is its own device.
>
> Also convert existing `.as_raw() != .as_raw()` to  use this new
> implementation.
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>

In case this should eventually go through another tree:

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

But please let me know if you want me to pick it up.