Claude review: rust: auxiliary: add registration data to auxiliary devices

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

Patch Review

**C header change** (`include/linux/auxiliary_bus.h`):

The `void *registration_data_rust` field added to `struct auxiliary_device` is well-placed. The `_rust` suffix is an appropriate convention making clear that C code should not touch this field. However, this grows every `struct auxiliary_device` by a pointer even when no Rust driver is involved:

```c
void *registration_data_rust;
```

Minor nit: the kernel community may push back on adding a Rust-specific field to a core C structure that affects all users. It might be worth noting in the commit message or cover letter that this is a single pointer and the auxiliary_device is already not a high-frequency allocation.

**`RegistrationData<T>` wrapper** (`rust/kernel/auxiliary.rs`):

The `#[repr(C)]` layout with `type_id` at offset 0 enables the safe type-checking pattern in `registration_data()`:

```rust
#[repr(C)]
#[pin_data]
struct RegistrationData<T> {
    type_id: TypeId,
    #[pin]
    data: T,
}
```

This is sound: `#[repr(C)]` guarantees `type_id` is at offset 0 regardless of `T`, so the `ptr.cast::<TypeId>().read()` in `registration_data()` is valid even before the actual type `T` is confirmed. The alignment is also guaranteed since `RegistrationData<T>`'s alignment is at least `align_of::<TypeId>()`.

**`Registration::new` API change**:

The shift from returning `impl PinInit<Devres<Self>>` (used with `<-` in `pin_init!`) to returning `Result<Devres<Self>>` (used with `=` and `?`) simplifies the call sites. This works because `Devres::new` already accepts `impl PinInit<T, E>`, and a value of type `Self` satisfies `PinInit<Self, Infallible>`.

**Error path in `Registration::new`**:

```rust
if ret != 0 {
    let _ = unsafe {
        Pin::<KBox<RegistrationData<T>>>::from_foreign((*adev).registration_data_rust)
    };
    unsafe { bindings::auxiliary_device_uninit(adev) };
    return Err(Error::from_errno(ret));
}
```

Correct: registration data is reclaimed before `auxiliary_device_uninit`, which will eventually trigger `Device::release` to free the device memory. The data cleanup doesn't depend on device validity, just on the pointer stored in the field.

**`Registration::drop` ordering**:

```rust
unsafe { bindings::auxiliary_device_delete(self.adev.as_ptr()) };
// then free registration data
// then auxiliary_device_uninit
```

This is the critical ordering and it's correct. `auxiliary_device_delete()` is synchronous — it unbinds the child driver and waits for `remove_callback` to complete. Only after that is the registration data freed. Since `registration_data()` is only available on `Device<Bound>`, and the child is no longer bound after `auxiliary_device_delete()`, there's no race.

**Invariant comment nit**: The invariant says "`registration_data` points to a valid `Pin<KBox<RegistrationData<T>>>`", but there is no field named `registration_data` in the struct — it refers to the `registration_data_rust` field of the pointed-to C struct. Consider: "the `registration_data_rust` field of the `auxiliary_device` pointed to by `self.adev`..."

**`Sync` bound**:

```rust
unsafe impl<T: Send> Sync for Registration<T> {}
```

This requires `T: Send` (not `T: Sync`) for `Registration<T>: Sync`. This is appropriate because `Registration` doesn't hand out `&T` through `&Registration<T>` — the data is accessed via a separate path through the device.

**Nova-core update** (`drivers/gpu/nova-core/driver.rs`):

The change from `#[pin]` + `<-` init to non-pinned `=` init with `?` is clean. The `()` registration data is the right choice when no inter-driver communication is needed. If `_reg1` creation fails, Rust's drop semantics will correctly clean up the already-constructed `_reg0`.

**Sample driver update** (`samples/rust/rust_driver_auxiliary.rs`):

Good demonstration of the new API. The `Data { index: u32 }` struct makes the sample more meaningful than the old `TypeId::of::<Self>()` approach. The `#[pin_data]` attribute is correctly removed from `ParentDriver` since no fields need pin-initialization anymore.

---
Generated by Claude Code Patch Reviewer