Claude review: gpu: nova-core: run Booter Unloader and FWSEC-SB upon unbinding

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

Patch Review

This is the culmination of the series. Key points:

**UnloadBundle trait:**
```rust
pub(super) trait UnloadBundle: Send {
    fn run(&self, dev: ..., bar: ..., gsp_falcon: ..., sec2_falcon: ...) -> Result;
}
```

The `Send` bound is important since the bundle is stored in the `Gsp` struct and may be accessed from different contexts.

**FwsecUnloadFirmware enum** — abstracts over with/without bootloader variants. Clean approach:
```rust
enum FwsecUnloadFirmware {
    WithoutBl(FwsecFirmware),
    WithBl(FwsecFirmwareWithBl),
}
```

**Sec2UnloadBundle::run() implementation:**
```rust
    fn run(&self, ...) -> Result<()> {
        self.fwsec_sb.run(dev, bar, gsp_falcon)?;
        let wpr2_hi = bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI);
        if wpr2_hi.is_wpr2_set() {
            sec2_falcon.reset(bar)?;
            sec2_falcon.load(dev, bar, &self.booter_unloader)?;
            const MAILBOX_SENTINEL: u32 = 0xff;
            let (mbox0, _) = sec2_falcon.boot(bar, Some(MAILBOX_SENTINEL), Some(MAILBOX_SENTINEL))?;
            ...
        }
        Ok(())
    }
```

The sequence is: (1) run FWSEC-SB to reset GSP falcon state, (2) run Booter Unloader via SEC2 to remove WPR2. The sentinel value `0xff` in both mailboxes aligns with the v4 changelog noting alignment with OpenRM.

**Checking WPR2 removal after Booter Unloader:**
```rust
            let wpr2_hi = bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI);
            if wpr2_hi.is_wpr2_set() {
                dev_err!(dev, "WPR2 region still set after Booter Unloader returned\n");
                return Err(EBUSY);
            }
```
Good defensive check.

**Non-fatal unload bundle preparation:**
```rust
        let unload_bundle = Sec2UnloadBundle::build(...)
            .inspect_err(|e| { dev_warn!(...); })
            .ok();
```
This converts errors to `None`, meaning the driver loads successfully even if unload firmware can't be prepared. The three `dev_warn!` messages clearly communicate the degraded state. This is a reasonable tradeoff — failing to load entirely because the unload firmware is unavailable would be too aggressive.

**Pin projection concern:** In `boot.rs`:
```rust
+        *self.as_mut().project().unload = unload_bundle;
```
This requires the `#[pin_data]` macro to generate a `project()` method and `unload` must not be a pinned field. Since `unload` is `Option<KBox<dyn hal::UnloadBundle>>` and is NOT marked `#[pin]` in the struct definition, this should work correctly — `project()` will give direct `&mut` access to non-pinned fields.

**`is_wpr2_set()` helper** added to `regs.rs`:
```rust
    pub(crate) fn is_wpr2_set(self) -> bool {
        self.hi_val() != 0
    }
```
This duplicates the logic already present in `higher_bound()` (which returns 0 when not set), but is more semantically clear. Fine addition.

**The `#[expect(unused)]` and `#[expect(dead_code)]` removals** from `BooterKind::Unloader` and `FwsecCommand::Sb` are correct — these variants are now used.

**One observation:** The unload path in `boot.rs` calls `shutdown_gsp()` then `unload_bundle.run()`. If `shutdown_gsp` succeeds but `unload_bundle.run()` fails (e.g., Booter Unloader errors), the GSP is stopped but WPR2 is not removed. The GPU would then need a PCI reset before rebinding. This seems unavoidable given the sequential nature of the unload, and the error is propagated to the caller.

No blocking issues. The implementation is solid and well-considered.

---
Generated by Claude Code Patch Reviewer