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/tyr: firmware loading and MCU boot support Date: Tue, 03 Mar 2026 12:48:17 +1000 Message-ID: In-Reply-To: <20260302232500.244489-1-deborah.brouwer@collabora.com> References: <20260302232500.244489-1-deborah.brouwer@collabora.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: drm/tyr: firmware loading and MCU boot support Author: Deborah Brouwer Patches: 13 Reviewed: 2026-03-03T12:48:17.996827 --- This is a well-structured v2 series that adds firmware loading and MCU boot support to the Tyr Rust DRM driver for ARM Mali CSF GPUs. The series progresses logically: Kconfig fixes, small cleanups/renames, register definitions, then the major new subsystems (slot manager, MMU, VM, GEM kernel BOs, firmware parser, firmware loading). The code quality is generally high with good documentation and idiomatic Rust patterns. The LockedBy pattern for slot/seat synchronization is well thought out. Several patches already carry Reviewed-by tags from Alice Ryhl, Boris Brezillon, and Daniel Almeida, indicating significant prior review. Key concerns: 1. The `AsRegister::new()` never validates `as_nr`, making it possible to compute an out-of-bounds MMIO offset. Validation only happens later in `validate_as_slot()`, but some call paths may skip it. 2. The `idle_slot()` method reads the seat via `access()` then immediately does `access_mut()` on the same `LockedBy`, which copies the seat info but in a somewhat roundabout way. 3. The firmware parser's `EntryHeader::size()` field extraction (bits 8-15) only captures an 8-bit size, limiting entries to 255 bytes. This matches the C panthor driver but deserves a comment. 4. The `Firmware::Drop` calling `self.vm.kill()` to break a circular Arc reference is a known pattern but worth flagging for potential future refactoring. --- --- Generated by Claude Code Patch Reviewer