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/tidss: Add external data and sync signal edge configuration Date: Sat, 16 May 2026 12:00:17 +1000 Message-ID: In-Reply-To: <20260513-beagley-ai-display-v2-9-9e9bcefde6bc@ideasonboard.com> References: <20260513-beagley-ai-display-v2-0-9e9bcefde6bc@ideasonboard.com> <20260513-beagley-ai-display-v2-9-9e9bcefde6bc@ideasonboard.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Implements the DPI0_CLK_CTRL register write via syscon. The logic looks correct but: **Nit**: The `+ 0x0` offset addition is superfluous: ```c + regmap_write(dispc->syscon_dpi_io_ctrl, + dispc->syscon_dpi_io_ctrl_offset + 0x0, ``` The `+ 0x0` should be removed. It appears to be a leftover from a pattern where multiple registers at different offsets would be written. If only one register is accessed, just use `dispc->syscon_dpi_io_ctrl_offset` directly. **Logic check**: The polarity logic looks correct: - `!ipc` means data is NOT driven on negedge (i.e., driven on posedge), set `DATA_CLK_INVDIS` (disable inversion = rising edge) - `rf` means sync is driven on posedge, set `SYNC_CLK_INVDIS` (disable inversion = rising edge) The `INVDIS` naming (inversion-disable) is a bit confusing but the logic matches the intent. --- Generated by Claude Code Patch Reviewer