Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Hardware Teardown Order Inversion Risking MMIO Aborts --
commit 3edb8b3d87e91fedec7589ce07bc351965b21765 Author: Michal Wilczynski <[email protected]> drm/bridge: inno-hdmi: Add .disable platform operation This commit adds a new .disable platform operation to the Innosilicon HDMI driver to support platform-specific power down sequences. The new callback is executed during the bridge's atomic disable phase. > diff --git a/drivers/gpu/drm/bridge/inno-hdmi.c > b/drivers/gpu/drm/bridge/inno-hdmi.c > --- a/drivers/gpu/drm/bridge/inno-hdmi.c > +++ b/drivers/gpu/drm/bridge/inno-hdmi.c [ ... ] > @@ -886,6 +886,10 @@ static void inno_hdmi_bridge_atomic_disable(struct > drm_bridge *bridge, > struct drm_atomic_commit *state) > { > struct inno_hdmi *hdmi = bridge_to_inno_hdmi(bridge); > + const struct inno_hdmi_plat_ops *plat_ops = hdmi->plat_data->ops; > + > + if (plat_ops && plat_ops->disable) > + plat_ops->disable(hdmi->dev); > > inno_hdmi_standby(hdmi); [Severity: High] Does calling the platform disable callback before inno_hdmi_standby() risk MMIO aborts? The platform disable hook (such as the JH7110 implementation mentioned in the commit message) typically powers off the PHY and disables clocks. However, inno_hdmi_standby() attempts to perform MMIO writes to PHY registers like HDMI_PHY_DRIVER and HDMI_PHY_PRE_EMPHASIS. Accessing hardware registers after the PHY has been powered off and clocks disabled can cause a synchronous external abort (bus hang) or silent failures on many SoC architectures. Since plat_ops->enable() is called prior to configuring the PHY registers during setup, should the teardown follow a strict LIFO order where inno_hdmi_standby() is called before plat_ops->disable()? > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
