From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: rust: driver core: drop drvdata before devres release
Date: Mon, 25 May 2026 19:29:13 +1000
Message-ID: <review-patch4-20260521233501.1191842-5-dakr@kernel.org>
In-Reply-To: <20260521233501.1191842-5-dakr@kernel.org>
References: <20260521233501.1191842-1-dakr@kernel.org> <20260521233501.1191842-5-dakr@kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**This is the most critical patch in the series.** It reorders `device_unbi=
nd_cleanup()` in C code:

```c
// Before:
devres_release_all(dev);
if (dev->driver->p_cb.post_unbind_rust)
    dev->driver->p_cb.post_unbind_rust(dev);

// After:
if (dev->driver->p_cb.post_unbind_rust)
    dev->driver->p_cb.post_unbind_rust(dev);
devres_release_all(dev);
```

The commit message argues this is safe because "with drvdata() removed, the=
 driver's bus device private data is only accessible by the owning driver i=
tself." However, this changes behavior for **all** existing Rust drivers, n=
ot just HRT ones. If any existing Rust driver's Drop implementation (invoke=
d by `post_unbind_rust`) currently relies on devres resources already being=
 released (e.g., to avoid double-free), this reorder could break it.

That said, the opposite direction (drvdata depending on devres being alive)=
 is the more natural dependency direction, so this reorder should be safe i=
n practice. The comment update in `driver.rs` is also correct: devres resou=
rces are now "still valid" when drvdata is dropped.

**Question for the author:** Is there any existing Rust driver or use-case =
where the Drop of drvdata would conflict with devres resources still being =
alive? The `try_access()` path in Devres could now succeed in Drop where it=
 previously would not =E2=80=94 is that a concern for any current user?

---

### PATCHES 5-9/27: implement Sync for Device\<Bound\>

Five small patches adding `unsafe impl Sync for Device<Bound>` across PCI, =
platform, auxiliary, USB, and the base device type. Required so that `&'bou=
nd Device<Bound>` can be stored in driver data (which must be `Send + Sync`=
).

The safety argument is consistent across all: the underlying C struct is th=
e same as `Device<Normal>` (which already has `Sync`), and `Bound` is a zer=
o-sized type-state marker.

No issues.

---

---
Generated by Claude Code Patch Reviewer