Claude review: gpu: nova-core: run unload sequence upon unbinding

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

Overall Series Review

Subject: gpu: nova-core: run unload sequence upon unbinding
Author: Alexandre Courbot <acourbot@nvidia.com>
Patches: 9
Reviewed: 2026-05-25T20:06:41.048243

---

This is a well-structured 7-patch series that implements proper GSP firmware unloading for the nova-core driver. The series progresses logically: first cleanup patches (1-2), then adding the GSP shutdown command (3), refactoring boot code for reuse (4-5), introducing a HAL abstraction for chipset-specific boot paths (6), and finally the full unload sequence (7).

The architecture is sound: firmware needed for unloading is pre-loaded at boot time (since the filesystem may not be available at unbind), stored as an opaque "unload bundle," and consumed at unbind. The HAL trait cleanly separates Turing/Ampere/Ada (SEC2-based) from Hopper/Blackwell (FSP-based, stubbed out). Error handling during unload continues through partial failures rather than short-circuiting, which is the right approach for teardown paths.

**Key concern**: Patch 7 introduces `Cell<Option<gsp::UnloadBundle>>` in `NovaCore`. `Cell` is `!Sync`, which means `NovaCore` becomes `!Sync`. This is safe for a pinned driver data structure only if the framework doesn't require `Sync` for `pci::Driver::Data`. The `unbind()` callback receives `Pin<&Self::Data>` (shared ref), so the `Cell::take()` is the mechanism to extract the value — but `Cell` on a shared reference is only sound in single-threaded contexts. If the kernel guarantees that `probe` and `unbind` are never called concurrently on the same device, this is fine, but it deserves a safety comment.

**No blocking issues found.** The series is clean, well-documented, and follows good kernel driver patterns. A few minor observations follow per-patch.

---

---
Generated by Claude Code Patch Reviewer