From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/rockchip: vop2: Fix wrong wait target in layer cfg done check Date: Tue, 05 May 2026 08:08:45 +1000 Message-ID: In-Reply-To: <20260504-vop2-layer-cfg-tmout-v1-1-730226a7331e@collabora.com> References: <20260504-vop2-layer-cfg-tmout-v1-0-730226a7331e@collabora.com> <20260504-vop2-layer-cfg-tmout-v1-1-730226a7331e@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 **Verdict: Good fix, correct.** The bug is clear: before this patch, the original code did: 1. `old_layer_sel =3D vop2->old_layer_sel` (capture old value into local) 2. `vop2->old_layer_sel =3D layer_sel` (overwrite struct member with new va= lue) 3. `rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel)` =E2=80= =94 passes the **already-overwritten** value The fix changes the call to use the local `old_layer_sel` which still holds= the captured previous value: ```diff - rk3568_vop2_wait_for_layer_cfg_done(vop2, vop2->old_layer_sel); + rk3568_vop2_wait_for_layer_cfg_done(vop2, old_layer_sel); ``` This is a clear correctness fix. The wait was polling for the hardware regi= ster to match a value that hadn't even been written to the shadow register = yet, guaranteeing a timeout whenever the old and new values differed. The Fixes tag and commit message are accurate. No issues. --- --- Generated by Claude Code Patch Reviewer