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/msm/a6xx: Append SEL regs to dyn pwrup reglist Date: Tue, 05 May 2026 08:06:04 +1000 Message-ID: In-Reply-To: <20260504190751.61052-16-robin.clark@oss.qualcomm.com> References: <20260504190751.61052-1-robin.clark@oss.qualcomm.com> <20260504190751.61052-16-robin.clark@oss.qualcomm.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 **Bug: `nr_regs` subtraction is backwards.** ```c unsigned nr_regs =3D (reglist_sel_start - reglist) / 3; ``` At this point, `reglist` has been advanced past the SEL entries, so `reglis= t > reglist_sel_start`. The subtraction `reglist_sel_start - reglist` would= be negative (wrapping to a huge unsigned value). This should be: ```c unsigned nr_regs =3D (reglist - reglist_sel_start) / 3; ``` The division by 3 is correct =E2=80=94 each dynamic reglist entry is 3 uint= 32_t values (pipe/aperture, register, value). **Good: Barrier placement.** The two `wmb()` barriers are correctly placed: 1. Before modifying SEL entries: ensures CP sees the reduced `dynamic_list_= len` before entries are modified. 2. Before increasing `dynamic_list_len`: ensures CP sees the new entries be= fore the new length. The comment about skipping the shared-memory spinlock dance with SQE (as kg= sl does) is well-reasoned =E2=80=94 the ringbuffer programming will correct= any race with IFPC exit. --- Generated by Claude Code Patch Reviewer