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: panel: Add LG LP156WF1 Date: Thu, 04 Jun 2026 16:45:20 +1000 Message-ID: In-Reply-To: <20260529110518.624454-5-ivitro@gmail.com> References: <20260529110518.624454-4-ivitro@gmail.com> <20260529110518.624454-5-ivitro@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 **Overall:** Good patch. The binding is a near-exact structural copy of `ad= vantech,idk-2121wr.yaml`, which is the correct template to follow for dual-= link LVDS panels. **Positive observations:** - The `allOf` references to both `lvds-dual-ports.yaml#` and `panel-common.= yaml#` are correct. - The `compatible` uses the `{}` placeholder pattern to avoid false `select= ` matching in `panel-lvds.yaml`, matching the established convention. - Port requirements correctly specify `dual-lvds-odd-pixels` on `port@0` an= d `dual-lvds-even-pixels` on `port@1`. - The exclusion added to `panel-lvds.yaml` is placed in alphabetical order = =E2=80=94 correct. - Panel dimensions (345mm x 194mm) are consistent with a 15.6" 16:9 display. **Minor observations:** 1. **`unevaluatedProperties: false` vs `additionalProperties: false`** =E2= =80=94 The new binding uses `unevaluatedProperties: false` (line 62 of the = new file) while the existing `advantech,idk-2121wr.yaml` uses `additionalPr= operties: false`. This is actually an *improvement*: since the binding uses= `allOf` with `$ref` to compose schemas, `unevaluatedProperties` is the cor= rect JSON Schema keyword =E2=80=94 it accounts for properties defined in re= ferenced schemas, whereas `additionalProperties` does not. No change needed= , just noting the divergence from the template is correct. 2. **Description could mention port-to-pixel mapping** =E2=80=94 The `advan= tech,idk-2121wr.yaml` description explicitly states: ``` The panel expects odd pixels on the first port, and even pixels on the second port, therefore the ports must be marked accordingly ``` The new binding's description omits this detail. It would be a nice-to-h= ave for consistency and clarity, since the port ordering matters for correc= t display output. Not blocking. 3. **Example uses `- |` vs `- |+`** =E2=80=94 The idk-2121wr example uses `= - |+` (keep trailing newlines) while this uses `- |`. Functionally irreleva= nt for dt-schema validation, but noting the difference for consistency. Not= blocking. 4. **Timing values** =E2=80=94 The example specifies `clock-frequency =3D <= 138500000>` (138.5 MHz), which is a reasonable pixel clock for 1920x1080 at= ~60 Hz with the given blanking intervals. Quick sanity check: (1920+40+24+= 16) * (1080+23+3+5) * 60 =3D 2000 * 1111 * 60 =E2=89=88 133.3 MHz =E2=80=94= close enough given rounding and the panel's spec. Fine. --- Generated by Claude Code Patch Reviewer