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/v3d: Allocate all resources before enabling the clock
Date: Fri, 13 Mar 2026 13:47:07 +1000
Message-ID: <review-patch4-20260312-v3d-power-management-v7-4-9f006a1d4c55@igalia.com>
In-Reply-To: <20260312-v3d-power-management-v7-4-9f006a1d4c55@igalia.com>
References: <20260312-v3d-power-management-v7-0-9f006a1d4c55@igalia.com> <20260312-v3d-power-management-v7-4-9f006a1d4c55@igalia.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

This is the key preparation patch that reorders probe to do all resource al=
location before enabling the clock. The reordering is correct: `map_regs`, =
`devm_reset_control_get_optional_exclusive`, `devm_clk_get_optional`, `v3d_=
irq_init`, `v3d_perfmon_init`, `dma_alloc_wc`, and `v3d_gem_init` all happe=
n before `clk_prepare_enable`.

A few notes:

1. **Error path ordering concern**: The error cleanup labels are:
```c
clk_disable:
	v3d_power_off_sms(v3d);
	clk_disable_unprepare(v3d->clk);
gem_destroy:
	v3d_gem_destroy(drm);
dma_free:
	dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
```
The labels fall through, so `clk_disable` falls through to `gem_destroy` an=
d `dma_free`. This is correct =E2=80=94 if the clock was enabled and someth=
ing later fails, all three cleanups happen. But `gem_destroy` and `dma_free=
` are only reached by `goto` from before the clock is enabled, so `clk_disa=
ble` cleanup is never incorrectly reached from those paths. Good.

2. The `v3d_init_hw_state` and `v3d_mmu_set_page_table` calls are correctly=
 moved out of `v3d_gem_init` and into the post-clock-enable section. The `v=
3d_irq_enable` call is likewise moved out of `v3d_irq_init` and into the po=
st-clock-enable section. This is a clean separation.

3. **Extra blank line** in `v3d_gem.c` after `v3d_init_hw_state()`:
```c
+void
+v3d_init_hw_state(struct v3d_dev *v3d)
+{
+	v3d_init_core(v3d, 0);
+}
+
+
 static void
```
There's a double blank line =E2=80=94 minor style nit.

---
Generated by Claude Code Patch Reviewer