Re: [PATCH v5 04/24] rust: driver: decouple driver private data from driver type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mismatch between commit message and implementation regarding Data<'bound> associated type
- [Low] Incomplete updates to sample drivers and doc comments to reflect the new `probe` return type
--

commit 0e6d93fa6eec846c359654d90bf3423215c84f3d
Author: Danilo Krummrich <dakr@kernel.org>

rust: driver: decouple driver private data from driver type

This commit adds a Data associated type to all bus driver traits, decoupling
the driver's bus device private data type from the driver struct itself. It
also updates existing drivers and documentation examples to set Data = Self
to preserve existing behavior.

> Add a type Data<'bound> associated type to all bus driver traits,
> decoupling the driver's bus device private data type from the driver
> struct itself.
> 
> In the context of adding a 'bound lifetime, making this an associated
> type has the advantage that it allows us to avoid a driver trait global
> lifetime and it avoids the need for ForLt for bus device private data;
> both of which make the subsequent implementation by buses much simpler.

[Severity: Medium]
Is this commit message accurate for the current implementation?

The commit message explicitly claims to add a Data<'bound> associated type
to all bus driver traits to avoid the need for ForLt. However, looking at
the code changes in this patch, the user-facing bus traits only add
"type Data: Send;" without any lifetime parameter.

Could this be an outdated description from a previous iteration of the
design?

> diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
> index 129bb4b39c044..e583c6b8390ad 100644
> --- a/samples/rust/rust_dma.rs
> +++ b/samples/rust/rust_dma.rs
> @@ -58,6 +58,7 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}
>  
>  impl pci::Driver for DmaSampleDriver {
>      type IdInfo = ();
> +    type Data = Self;
>      const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
>  
>      fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {

[Severity: Low]
Should the return type in these sample drivers and documentation examples be
updated to use impl PinInit<Self::Data, Error> instead of Self?

While this compiles correctly because the examples define type Data = Self,
it doesn't fully demonstrate the newly decoupled API. If someone uses this
code as a template and changes the Data type to a distinct structure, they
will likely encounter a confusing type mismatch error. There are several
other examples in this patch (like rust_driver_platform.rs and io/mem.rs)
that also still return impl PinInit<Self, Error>.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525202921.124698-1-dakr@kernel.org?part=4