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/bridge: waveshare-dsi: support DSI LCD kits with LVDS panels Date: Wed, 01 Apr 2026 07:39:19 +1000 Message-ID: In-Reply-To: <20260331-ws-lcd-v2-2-a1add63b6eb6@oss.qualcomm.com> References: <20260331-ws-lcd-v2-0-a1add63b6eb6@oss.qualcomm.com> <20260331-ws-lcd-v2-2-a1add63b6eb6@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review The approach of using `i2c_get_match_data()` with the connector type stored in `.data` is clean and follows established kernel patterns. ```c ws->bridge.type = (uintptr_t)i2c_get_match_data(i2c); ``` This is correct -- `i2c_get_match_data()` returns `const void *`, and casting to `uintptr_t` for the enum value is standard practice. The two OF table entries are well-formed. One question for the author: the commit message says "The only difference with the existing waveshare,dsi2dpi is the bridge's output type (LVDS vs DPI)." -- is that truly the only difference? The ICN6211 (DSI2DPI) and ICN6202 (DSI2LVDS) are different chips. The probe function writes to registers `0xc0`, `0xc2`, `0xac` -- are those register writes valid/harmless for the ICN6202 as well, or is the bridge firmware handling those registers independently? The cover letter says the bridges are "completely handled by the board itself" so this is probably fine, but worth confirming. No code issues. --- Generated by Claude Code Patch Reviewer