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: display-connector: don't autoenable HPD IRQ Date: Mon, 16 Mar 2026 12:14:39 +1000 Message-ID: In-Reply-To: <20260314-dp-connector-hpd-v1-1-786044cedc17@oss.qualcomm.com> References: <20260314-dp-connector-hpd-v1-0-786044cedc17@oss.qualcomm.com> <20260314-dp-connector-hpd-v1-1-786044cedc17@oss.qualcomm.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 patch adds `IRQF_NO_AUTOEN` to the IRQ request flags and implements `h= pd_enable`/`hpd_disable` callbacks that call `enable_irq()`/`disable_irq()`. **Looks correct.** The `hpd_enable`/`hpd_disable` callbacks are the intende= d mechanism per the `drm_bridge_funcs` documentation: bridges that set `DRM= _BRIDGE_OP_HPD` should implement these. Using `IRQF_NO_AUTOEN` prevents the= IRQ from firing before the DRM connector is fully set up, which is the bug= being fixed. One minor observation: - The `hpd_enable`/`hpd_disable` callbacks are unconditionally wired into `= display_connector_bridge_funcs`, but `conn->hpd_irq` is only valid (>=3D 0)= when an HPD GPIO with IRQ capability was found. If `hpd_irq` is `-EINVAL` = (no HPD GPIO, or IRQ request failed), calling `enable_irq(-EINVAL)` / `disa= ble_irq(-EINVAL)` would be invalid. However, in practice, the `hpd_enable` = callback is only invoked by `drm_bridge_hpd_enable()` when `DRM_BRIDGE_OP_H= PD` is set in `bridge->ops`, and `DRM_BRIDGE_OP_HPD` is only set when `conn= ->hpd_irq >=3D 0`. So this is safe by design, but adding a guard like: ```c if (conn->hpd_irq < 0) return; ``` would be defensive and make the contract clearer. Not strictly necessary th= ough. **Fixes tag looks appropriate** =E2=80=93 the original commit introduced th= e auto-enabled IRQ. --- Generated by Claude Code Patch Reviewer