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: Add detect_ctx hook and drm_bridge_detect_ctx() helper Date: Tue, 28 Apr 2026 15:25:19 +1000 Message-ID: In-Reply-To: <20260426-dw-hdmi-qp-scramb-v5-2-d778e70c317b@collabora.com> References: <20260426-dw-hdmi-qp-scramb-v5-0-d778e70c317b@collabora.com> <20260426-dw-hdmi-qp-scramb-v5-2-d778e70c317b@collabora.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 This is the core infrastructure patch. Key observations: **Return type change:** `drm_bridge_detect_ctx()` returns `int` rather than= `enum drm_connector_status`, allowing `-EDEADLK` to propagate. This is a r= easonable approach since connector status values are positive. The `WARN_ON= (ret < 0)` fallback at the end catches unexpected negative returns: ```c out: if (WARN_ON(ret < 0)) ret =3D connector_status_unknown; return ret; ``` This is good defensive coding. However, note that when `ctx !=3D NULL` and = `ret =3D=3D -EDEADLK`, the function returns early (correct), but for any *o= ther* negative return from `detect_ctx` with a caller-provided ctx, it fall= s through to the `WARN_ON` and converts to `connector_status_unknown`. That= seems intentional and fine. **Locking in the NULL-ctx path:** When `ctx` is NULL, the function acquires= `connection_mutex` internally. This matches what the DRM core does for the= connector `.detect_ctx` path. The EDEADLK retry loop is correctly structur= ed. **Delegation from drm_bridge_detect():** The existing `drm_bridge_detect()`= now simply delegates to `drm_bridge_detect_ctx(bridge, connector, NULL)`. = This means old callers that only set `.detect` go through an extra function= call but the behavior is preserved since `!bridge->funcs->detect_ctx` retu= rns `bridge->funcs->detect(bridge, connector)` directly. **One subtlety:** When `ctx` is NULL and `detect_ctx` is implemented, the h= elper acquires `connection_mutex` internally. But when going through the ol= d `drm_bridge_detect()` =E2=86=92 `drm_bridge_detect_ctx(NULL)` path for a = bridge that only has `.detect`, no locking is acquired =E2=80=94 which matc= hes the previous behavior. Good. **Documentation** is clear and correctly describes the precedence rules (`d= etect_ctx` over `detect`). **No issues found.** --- --- Generated by Claude Code Patch Reviewer