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: drm/panthor: Use the drm_dev_access guard
Date: Sat, 16 May 2026 11:40:46 +1000
Message-ID: <review-patch5-20260513-panthor-guard-refactor-v1-5-f2d8c15a97ce@collabora.com>
In-Reply-To: <20260513-panthor-guard-refactor-v1-5-f2d8c15a97ce@collabora.com>
References: <20260513-panthor-guard-refactor-v1-0-f2d8c15a97ce@collabora.com> <20260513-panthor-guard-refactor-v1-5-f2d8c15a97ce@collabora.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

Clean application of the drm_dev_access guard from patch 3. Several pattern=
s emerge:

**IOCTL paths using `ACQUIRE`/`ACQUIRE_ERR`:**
```c
ACQUIRE(drm_dev_access, dev_guard)(ddev);
if (ACQUIRE_ERR(drm_dev_access, &dev_guard))
    return -ENODEV;
```
This is verbose compared to the old `if (!drm_dev_enter(ddev, &cookie)) ret=
urn -ENODEV;` =E2=80=94 it's 2 lines vs 2 lines but with longer names. The =
payoff is removing the `drm_dev_exit(cookie)` and any gotos to it.

**`panthor_device_resume()` =E2=80=94 `scoped_cond_guard` with `ret =3D 0` =
fallthrough:**
```c
scoped_cond_guard(drm_dev_access, ret =3D 0, &ptdev->base) {
```
When `drm_dev_enter` fails (device unplugged), `ret` is set to 0 and the bl=
ock is skipped. This is correct =E2=80=94 the original code also just skipp=
ed the block with `drm_dev_enter()` returning false.

**`panthor_device_suspend()` =E2=80=94 `scoped_guard` without error action:=
**
```c
scoped_guard(drm_dev_access, &ptdev->base) {
```
Using `scoped_guard` (not `scoped_cond_guard`) means if `drm_dev_enter` fai=
ls, the block is silently skipped. The original code had the same behavior =
(`if (...drm_dev_enter(...))`). This is correct for suspend =E2=80=94 if th=
e device is already unplugged, skipping teardown is fine.

**`panthor_vm_active()` simplification:**
The conversion from:
```c
scoped_guard(mutex, &vm->op_lock) {
    guard(mutex)(&ptdev->mmu->as.slots_lock);
    ret =3D panthor_vm_active_locked(vm);
}
```
to:
```c
guard(mutex)(&vm->op_lock);
guard(mutex)(&ptdev->mmu->as.slots_lock);
return panthor_vm_active_locked(vm);
```
This is functionally equivalent since we're at the end of the function eith=
er way. Cleaner.

---
Generated by Claude Code Patch Reviewer