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: Gen1: Fix open-coded register access Date: Wed, 25 Mar 2026 07:46:41 +1000 Message-ID: In-Reply-To: <20260323160407.245773-11-tzimmermann@suse.de> References: <20260323160407.245773-1-tzimmermann@suse.de> <20260323160407.245773-11-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 cleanup. The new `ast_moutdwm_poll()` helper is a useful addition: ```c void ast_moutdwm_poll(struct ast_device *ast, u32 r, u32 v, u32 res) ``` This replaces the pattern of writing then busy-polling until the register reaches a target value. The `cpu_relax()` call in the poll loop is a nice improvement over the original tight `do { ; } while()` loops. **Potential concern**: The original Gen1 code did: ```c ast_write32(ast, 0xf004, AST_REG_MCR00); ast_write32(ast, 0xf000, 0x1); ast_write32(ast, 0x10100, 0xa8); do { ; } while (ast_read32(ast, 0x10100) != 0xa8); ``` This writes directly to the P2A mapped region at offset 0x10100 (which is MCR100 after segment setup), and polls the same. The replacement uses `ast_moutdwm_poll(ast, AST_REG_MCR100, 0xa8, 0xa8)`, which will do a segment select + write + poll. This is functionally equivalent but the segment was previously set to MCR by the preceding `ast_write32(ast, 0xf004, ...)` call that is now removed. The `ast_moutdwm_poll` handles segment selection internally, so this is correct. --- Generated by Claude Code Patch Reviewer