Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The HPD IRQ handler unconditionally accesses unclocked DP controller registers during runtime suspend on GPIO HPD and non-Rockchip configurations, causing a synchronous external abort (system crash). --
--- Patch [1]: [PATCH v1 1/7] drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection --- commit 0b74430c48be5d81c8cb19f2cd5f520d6c7a696a Author: Damon Ding <[email protected]> drm/bridge: analogix_dp: Manage pm runtime and IRQ for native HPD pin detection This commit moves enable_irq() and disable_irq() from bridge enable/disable handlers to analogix_dp_bind() and analogix_dp_unbind() to keep HPD detection active. It also acquires a runtime PM reference exclusively for Rockchip native HPD pin configurations to ensure the controller remains powered. > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index 30c0c0b41d5aa..1c98f92e001d8 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c [ ... ] > @@ -1508,6 +1511,14 @@ int analogix_dp_bind(struct analogix_dp_device *dp, > struct drm_device *drm_dev) > goto err_unregister_aux; > } > > + if (analogix_dp_require_pm_for_hpd_irq(dp)) { > + ret = pm_runtime_resume_and_get(dp->dev); > + if (ret) > + goto err_unregister_aux; > + } > + > + enable_irq(dp->irq); > + [Severity: High] Since pm_runtime_resume_and_get() is only called for Rockchip native HPD, other configurations (like GPIO HPD or Exynos) can enter runtime suspend while the IRQ remains enabled here. If the HPD interrupt fires (e.g. via an active GPIO pin) while the device is in runtime suspend, will this cause a crash? The hardirq handler executes and performs unconditional MMIO accesses to the unclocked DP controller: analogix_dp_hardirq() analogix_dp_mute_hpd_interrupt() readl(dp->reg_base + ANALOGIX_DP_COMMON_INT_MASK_4); Accessing memory-mapped registers of a clock-gated peripheral typically causes a synchronous external abort. > return 0; > > err_unregister_aux: -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
