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: Store register addresses in struct ast_dramstruct Date: Wed, 25 Mar 2026 07:46:41 +1000 Message-ID: In-Reply-To: <20260323160407.245773-10-tzimmermann@suse.de> References: <20260323160407.245773-1-tzimmermann@suse.de> <20260323160407.245773-10-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 This is a substantive change - widening `ast_dramstruct.index` from `u16` to `u32` to hold full register addresses instead of segment offsets. The approach is sound: it makes the DRAM tables self-contained with full addresses, removing the fragile dependency on the correct P2A segment being pre-selected. The new `AST_DRAMSTRUCT_REG()` macro and `AST_DRAMSTRUCT_IS_REG()` are clean replacements. The removal of `__AST_DRAMSTRUCT_DRAM_TYPE` in favor of direct `AST_REG_MCR04` comparison is good. Minor concern: the control commands `__AST_DRAMSTRUCT_UDELAY` (0xff00) and `__AST_DRAMSTRUCT_INVALID` (0xffff) are now in the same u32 namespace as real register addresses. These values don't collide with any AST register address, but a comment noting this invariant might be helpful for future maintenance. There's no realistic collision risk though. Note: `// DRAM type` comments in the table use C++ style (`//`) rather than C style (`/* */`). The kernel coding style prefers `/* */`. --- Generated by Claude Code Patch Reviewer