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: lt9211: Require data-lanes on DSI input ports Date: Sun, 12 Apr 2026 13:19:34 +1000 Message-ID: In-Reply-To: <20260407203109.34302-1-marex@nabladev.com> References: <20260407203109.34302-1-marex@nabladev.com> <20260407203109.34302-1-marex@nabladev.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 **Positive aspects:** - The `port-base` + `unevaluatedProperties: false` refactoring is correct a= nd follows the established pattern used by other bridge bindings. - The `data-lanes` property definition with `const: 1..4` items is properly= constraining lanes to sequential 1-based indexes, matching the hardware. - The example update adding `data-lanes =3D <1 2 3 4>` is good and consiste= nt with similar bindings. - The commit message correctly notes the driver dependency at `drivers/gpu/= drm/bridge/lontium-lt9211.c:688`: ```c dsi_lanes =3D drm_of_get_data_lanes_count(endpoint, 1, 4); ``` which returns an error if missing (checked at line 697-698). **Concern =E2=80=94 `required: data-lanes` on multi-protocol ports:** The port descriptions state that these ports serve multiple input protocols: ```yaml port@0: description: Primary MIPI DSI port-1 for MIPI input or LVDS port-1 for LVDS input or DPI input. ``` Yet the patch unconditionally requires `data-lanes`: ```yaml required: - data-lanes ``` `data-lanes` is a DSI-specific concept. If a DT describes the LT9211 with L= VDS or DPI input, the binding would now force inclusion of `data-lanes`, wh= ich is semantically wrong for those protocols. DT bindings should describe = the hardware, not just the current driver implementation. The `ti,sn65dsi83.yaml` binding (lines 44-83) was clearly used as a templat= e for this change, but it notably does **not** include `required: - data-la= nes` on its DSI input ports. The patch is stricter than its reference. **Recommendation:** Drop the two `required: - data-lanes` blocks from both = port@0 and port@1. The property definition alone (without `required`) is su= fficient to document and validate the property when present. If the intent = is truly to make this mandatory for DSI configurations only, a conditional = schema (e.g., `if/then` keyed on the presence of a DSI remote-endpoint) wou= ld be more correct, though likely overkill for this binding. If the argument is "the driver requires it," the correct fix is documenting= the property (which this patch does) and optionally fixing the driver to p= rovide a reasonable default or a better error message, rather than encoding= a driver requirement as a hardware constraint. --- Generated by Claude Code Patch Reviewer