Claude review: rust: drm: Add RegistrationData to drm::Driver

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This is the most complex and safety-critical patch. Several observations:

**`registration_data` field in `Device`**:
```rust
pub(super) registration_data: UnsafeCell<NonNull<<T::RegistrationData as ForLt>::Of<'static>>>,
```
Using `UnsafeCell<NonNull<...>>` is the right choice — it's interior-mutable and non-null. The `'static` lifetime here is the erased form that gets shortened via `ForLt::cast_ref_unchecked`.

**Initialization with `NonNull::dangling()`**:
```rust
unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
```
This is set in `UnregisteredDevice::new()` before the device is registered. It's later overwritten in `Registration::new_with_lt()` before `drm_dev_register()`. The dangling pointer is never dereferenced because `UnbindGuard` is only available on `Device<T, Registered>`, and registration only succeeds after the real pointer is written. This is correct.

**`Registration::new_with_lt()` — the `mem::transmute`**:
```rust
let ptr: NonNull<<T::RegistrationData as ForLt>::Of<'static>> =
    unsafe { mem::transmute(NonNull::from(Pin::get_ref(reg_data.as_ref()))) };
```
The safety comment says "Lifetimes do not affect layout, so the pointer cast is sound." This is true — `NonNull<T<'a>>` and `NonNull<T<'static>>` have identical layout. The transmute is used to erase the lifetime for storage. The data remains valid because `_reg_data` is owned by `Registration` and only dropped after `drm_dev_unplug()`.

**Error path in `new_with_lt()`**:
```rust
if let Err(e) = to_result(ret) {
    unsafe { *drm.registration_data.get() = NonNull::dangling() };
    return Err(e);
}
```
Good — on `drm_dev_register()` failure, the pointer is reset to dangling. Since registration failed, no ioctls can arrive, so the dangling pointer is safe.

**`registration_data_unchecked()`**:
```rust
unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
```
The returned lifetime is `'_` (elided to the borrow of `self`), but the actual data lives in `Registration`. The caller must ensure the parent device is bound (via UnbindGuard), which transitively guarantees `Registration` hasn't been dropped yet. The HRTB requirement prevents concrete lifetime selection. This is sound.

**`registration_data_with()` on `UnbindGuard`**:
```rust
pub fn registration_data_with<R, F>(&self, f: F) -> R
where
    F: for<'a> FnOnce(
        &'a T::ParentDevice<device::Bound>,
        &'a <T::RegistrationData as ForLt>::Of<'a>,
    ) -> R,
```
The `for<'a>` HRTB is the key safety mechanism. The closure cannot choose a concrete lifetime, so it cannot smuggle out a reference with a chosen lifetime. The `'a` tying `ParentDevice` and `RegistrationData::Of<'a>` together means both live exactly as long as the closure scope. This is the correct pattern for invariant-over-lifetime types.

**Potential concern — `dev` vs `dev.as_ref()` check in `new_with_lt()`**:
```rust
if drm.as_ref().as_raw() != dev.as_raw() {
```
This checks that the DRM device's parent device matches `dev`. The `drm.as_ref()` gets the `device::Device` from the DRM device (via `AsRef`), and compares raw pointers. This is correct — it prevents registering a DRM device with the wrong parent.

**`Registration` no longer uses `devres`** — the old `new_foreign_owned()` used `devres::register()` for device-resource-managed lifetime. The new approach has `Registration` owned directly by the driver's `Data<'bound>`, which ties it to the bus binding scope. This is a deliberate API change and is fine.

**Send/Sync bounds**:
```rust
unsafe impl<T: Driver> Sync for Registration<'_, T> where
    for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
```
The `for<'a> ... Send` bound is correct — since the data can be accessed from any thread (via ioctl dispatch), the `Of<'a>` must be `Send` for all lifetimes.

---
Generated by Claude Code Patch Reviewer