From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: misc: fastrpc: Add polling mode support Date: Thu, 23 Apr 2026 07:50:44 +1000 Message-ID: In-Reply-To: <20260422092409.4107093-1-ekansh.gupta@oss.qualcomm.com> References: <20260422092409.4107093-1-ekansh.gupta@oss.qualcomm.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: misc: fastrpc: Add polling mode support Author: Ekansh Gupta Patches: 8 Reviewed: 2026-04-23T07:50:44.016413 --- This is a v9 series adding DSP polling mode support to the FastRPC driver. The series is logically structured: patch 1 is a minor refactor, patch 2 converts to GENMASK/FIELD_GET, patch 3 expands the context ID mask, and patch 4 adds the polling mode feature itself. **Key concerns:** 1. **Build-breaking bug in patch 4**: The function `of_machine_get_match()` does not exist in the kernel. The correct API is `of_machine_device_match()` (returns `bool`). This will cause a compilation failure. 2. **`readl_poll_timeout_atomic` on non-MMIO memory**: `readl()` is for memory-mapped I/O registers. The poll address is DMA coherent buffer memory (from `fastrpc_buf`). Using `readl()` on regular RAM is semantically wrong. A simple `READ_ONCE()` loop or `read_poll_timeout` with `READ_ONCE` would be more appropriate. 3. **Atomic busy-wait for 10ms**: `readl_poll_timeout_atomic` uses `udelay()`, holding the CPU in a tight busy loop for up to 10ms. This is a long time to disable preemption. The non-atomic `readl_poll_timeout` (which uses `usleep_range`) would be much better given this isn't an atomic context. 4. **Missing `__maybe_unused` justification**: The `fastrpc_poll_supported_machines` table is marked `__maybe_unused` which suppresses a warning when `CONFIG_OF` is disabled, but the driver already depends on OF via rpmsg/qcom infrastructure. This feels like papering over a warning rather than addressing it properly. Overall the approach is reasonable but needs the compilation fix and the polling mechanism reviewed for correctness. --- --- Generated by Claude Code Patch Reviewer