From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: rust: drm: Pass bound parent device to ioctl handlers
Date: Thu, 07 May 2026 12:45:10 +1000
Message-ID: <review-patch6-20260506221027.858481-7-dakr@kernel.org>
In-Reply-To: <20260506221027.858481-7-dakr@kernel.org>
References: <20260506221027.858481-1-dakr@kernel.org> <20260506221027.858481-7-dakr@kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

The final ioctl call becomes:
```rust
match $func(dev, &*guard, guard.registration_data(), data, file) {
```

**`&*guard` usage**: This dereferences the `UnbindGuard` through its `Deref=
` impl to get `&T::ParentDevice<Bound>`, then re-borrows it. This is correc=
t.

**Borrow concern**: Both `&*guard` and `guard.registration_data()` borrow t=
he guard simultaneously =E2=80=94 the first immutably through `Deref`, the =
second through a method call. Since both are immutable borrows, this is fin=
e from a borrow-checking perspective. However, `registration_data()` intern=
ally calls `self.dev.raw_registration_data()`, which goes through `UnsafeCe=
ll::get()`. The safety here relies on the fact that no mutable access to `r=
egistration_data` occurs during the SRCU critical section, which is guarant=
eed because `Registration::drop` calls `drm_dev_unplug()` (SRCU barrier) be=
fore modifying anything.

**Driver-side changes**: Nova and tyr add the parent device parameter (`_ad=
ev: &auxiliary::Device<Bound>` and `_pdev: &platform::Device<device::Bound>=
` respectively). These are unused with `_` prefix, which is expected for th=
is infrastructure patch.

**Argument order**: The final ioctl handler signature is:
```
fn foo(dev, parent_device, reg_data, ioctl_data, file) -> Result<u32>
```
This is a 5-argument function. While not a problem per se, it's worth consi=
dering if a context struct would be cleaner for future evolution. But for n=
ow, this is fine.

**Minor nit**: The imports in `nova/file.rs` add `auxiliary` and `device::B=
ound` =E2=80=94 straightforward.

No blocking issues.

### Summary

The series is well-designed with careful safety arguments. The main archite=
ctural dependency is on the `ForLt` trait and `DeviceContext` work that isn=
't in drm-next yet. The `drm_dev_unregister` =E2=86=92 `drm_dev_unplug` swi=
tch is the key safety enabler and is correct. The lifetime erasure/recovery=
 pattern via `mem::transmute` is the most subtle part but is justified by `=
ForLt`'s covariance guarantee. The incremental ioctl handler signature evol=
ution across patches 4-6 is clean. No blocking issues found.

---
Generated by Claude Code Patch Reviewer