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/imagination: Access FW initialised state with READ/WRITE_ONCE Date: Sat, 16 May 2026 14:11:26 +1000 Message-ID: In-Reply-To: <20260512-b4-context_reset-v1-4-439bee96ed83@imgtec.com> References: <20260512-b4-context_reset-v1-0-439bee96ed83@imgtec.com> <20260512-b4-context_reset-v1-4-439bee96ed83@imgtec.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: Reasonable, minor question** Using `READ_ONCE`/`WRITE_ONCE` for `fw_dev->initialised` is appropriate sin= ce this bool is accessed from multiple contexts (IRQ handler, watchdog work= er, suspend/resume, init/fini) without a common lock. This prevents compile= r optimizations like repeated loads or store tearing. The changes are straightforward and mechanical. **Minor note:** `READ_ONCE`/`WRITE_ONCE` alone do not guarantee ordering wi= th respect to the operations that the `initialised` flag guards. For exampl= e, in `pvr_fw_init()`: ```c WRITE_ONCE(fw_dev->initialised, true); ``` There's no barrier ensuring that the FW initialization steps are visible to= other CPUs before `initialised` becomes `true`. If ordering matters (i.e.,= if another CPU could check `initialised` and then access FW structures), a= `smp_store_release()`/`smp_load_acquire()` pair would be more appropriate.= The commit message says "ensure atomicity of operation" =E2=80=94 but `REA= D_ONCE`/`WRITE_ONCE` don't provide atomicity guarantees in the hardware sen= se; they just prevent compiler issues. Worth clarifying the intent. --- Generated by Claude Code Patch Reviewer