Claude review: drm: nova: demonstrate interaction with nova-core

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

Patch Review

This is explicitly labeled as a POC that won't be merged. Even so, there are several issues worth noting for when the real implementation is built:

**1. Self-referential struct field ordering change:**

The patch reorders `NovaCore`'s fields to put `_reg` first:

```rust
pub struct NovaCore<'bound> {
    _reg: auxiliary::Registration<'bound, ForLt!(AuxData<'_>)>,
    #[pin]
    pub(crate) gpu: Gpu<'bound>,
    bar: pci::Bar<'bound, BAR0_SIZE>,
}
```

The comment says "Fields are dropped in declaration order: unregister the auxiliary device before dropping `gpu`". This is correct for Rust's drop order (fields are dropped in declaration order). However, this conflicts with the current upstream code which uses `Devres<auxiliary::Registration>` — the `Devres` wrapper handles device-managed lifetime differently. The POC has regressed to a non-`Devres` approach (no `#[pin]` on `_reg`, direct `Registration` instead of `Devres<Registration>`), which may not be the right pattern for the final implementation.

**2. Unsafe self-referential pointer construction:**

```rust
gpu: &*core::ptr::from_ref(&this.as_ref().gpu),
```

The `SAFETY` comment says "`this.gpu` is initialized before this expression is evaluated", but this relies on `try_pin_init!(&this in ...)` guaranteeing field initialization order. This is correct for `pin_init`'s current semantics but is subtle. The comment should also note that `AuxData` must not outlive `NovaCore` (which it can't since `_reg` is dropped first).

**3. Changed struct to non-`#[pin]` bar field:**

The upstream code uses `Arc<Devres<Bar0>>` for the bar (shared ownership with device-managed cleanup), but the POC switches to `pci::Bar<'bound, BAR0_SIZE>` (borrowed, lifetime-bound). This is a significant API difference that makes the POC not representative of the final design.

**4. Public visibility changes:**

Making `driver` and `gpu` modules `pub` and exposing `Spec.chipset` and `Gpu.spec` as `pub(crate)` is reasonable for the cross-crate use case. The `AuxData` wrapper providing a `chipset()` method is a clean API.

**No blocking issues** (this is a POC and labeled as such).

---
Generated by Claude Code Patch Reviewer