From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dt-bindings: display: bridge: ti, sn65dsi83: Add dual-link video mode property Date: Fri, 13 Mar 2026 14:32:11 +1000 Message-ID: In-Reply-To: <20260312043743.261475-2-tessolveupstream@gmail.com> References: <20260312043743.261475-1-tessolveupstream@gmail.com> <20260312043743.261475-2-tessolveupstream@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Questionable need for the property.** The driver already determines dual-= link mode from the OF graph port topology (`ctx->lvds_dual_link`). A separa= te boolean `ti,dual-link-video-mode` is redundant =E2=80=94 if dual-link is= already detected, the driver should automatically apply the correct regist= er programming. If the intent is to distinguish between different dual-link= sub-modes, the property name and description don't make that clear. **DT binding description is vague:** ```yaml + ti,dual-link-video-mode: + type: boolean + description: | + Enables configuration settings required for correct dual-link + LVDS operation. Some panels require the horizontal timing + parameters to be adjusted before being programmed into the + device. The horizontal timing values must be divided by + two when operating in dual-link mode. ``` The description says "some panels require" this =E2=80=94 but a boolean pro= perty doesn't encode which panels or under what conditions. If all dual-lin= k panels need this, it should be automatic. If only some do, the binding ne= eds to explain what distinguishes those panels. **No `allOf`/`if-then` constraint.** The property should only be valid for = `ti,sn65dsi84` (and possibly `ti,sn65dsi85`), not `ti,sn65dsi83` which only= supports single-link. The binding should add a conditional to restrict whe= re this property is allowed. --- Generated by Claude Code Patch Reviewer