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/ast: Move 32-bit register-access helpers to ast_drv.{c, h} Date: Wed, 25 Mar 2026 07:46:39 +1000 Message-ID: In-Reply-To: <20260323160407.245773-3-tzimmermann@suse.de> References: <20260323160407.245773-1-tzimmermann@suse.de> <20260323160407.245773-3-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Good refactoring. The reimplementation of `__ast_mindwm`/`__ast_moutdwm` with properly named sub-helpers (`__ast_selseg`, `__ast_rdseg32`, `__ast_wrseg32`) is cleaner and adds `cpu_relax()` to the busy-wait loop, which the original lacked. One minor observation: the `__ast_selseg` helper calls `cpu_relax()` before the first read in the polling loop, whereas the original code did not relax at all. This is strictly better but moves from a tight spin to a relaxed spin. The P2A register constants are well-defined: ```c #define AST_REG_P2A00 AST_REG_P2A(0xf000) #define AST_REG_P2A04 AST_REG_P2A(0xf004) ``` Note: The naming `P2A00` for offset `0xf000` is a bit confusing - the "00" suffix doesn't correspond to the offset. This follows the hardware register naming convention (P2A register 00), but a comment might help. --- Generated by Claude Code Patch Reviewer