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: fsl-ldb: support downstream bridges in addition to panels Date: Sun, 12 Apr 2026 12:59:08 +1000 Message-ID: In-Reply-To: <20260408072802.3947600-1-xiaolei.wang@windriver.com> References: <20260408072802.3947600-1-xiaolei.wang@windriver.com> <20260408072802.3947600-1-xiaolei.wang@windriver.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 **Correctness: Good** The core logic is correct. `devm_drm_of_get_bridge()` internally calls `drm= _of_find_panel_or_bridge()`, and if a panel is found, wraps it with `devm_d= rm_panel_bridge_add()` =E2=80=94 so existing panel-only device trees contin= ue to work unchanged. If a bridge is found, it's returned directly. Either = way the result is a `struct drm_bridge *`, which is exactly what `fsl_ldb->= panel_bridge` stores and what `fsl_ldb_attach()` passes to `drm_bridge_atta= ch()`. **Port selection logic: Correct** ```c fsl_ldb->panel_bridge =3D devm_drm_of_get_bridge(dev, dev->of_node, fsl_ldb->ch0_enabled ? 1 : = 2, 0); ``` This correctly mirrors the original logic: ```c panel_node =3D of_node_get(remote1 ? remote1 : remote2); ``` where `remote1` is from port 1 and `remote2` is from port 2. In the dual-li= nk case (both channels enabled), both ports connect to the same downstream = device, so picking port 1 is correct. **Nit 1: Stale error message** ```c if (!fsl_ldb->ch0_enabled && !fsl_ldb->ch1_enabled) return dev_err_probe(dev, -ENXIO, "No panel node found"); ``` Since the driver now supports bridges too, this message should say somethin= g like `"No panel or bridge node found"` or simply `"No remote node found"`. **Nit 2: Unused include** After this patch, nothing in the file uses `struct drm_panel`, `of_drm_find= _panel()`, or any other symbol from ``. The include at the= top of the file could be dropped since `devm_drm_of_get_bridge()` is decla= red in `` (already included). **Nit 3: Needs rebase** The patch is based on tree state `5c3cf37200bc`, but current drm-next has a= dditional code between the removed `of_node_put(panel_node)` and `devm_drm_= panel_bridge_add()` calls =E2=80=94 specifically the `nxp,enable-terminatio= n-resistor` property check at `fsl-ldb.c:347-348`. The patch will need a tr= ivial rebase to resolve this context conflict. The `nxp,enable-termination-= resistor` block should remain in place after the rebase. **Non-issue (for context):** The struct member `panel_bridge` is now slight= ly misnamed since it can hold a non-panel bridge, but renaming it would be = gratuitous churn and is fine to leave. --- Generated by Claude Code Patch Reviewer