Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/bridge: inno-hdmi: sleep in atomic context via .disable hook - [Medium] drm/bridge: inno-hdmi: hardware teardown LIFO ordering violation --
commit f7201b9edd5364bfbb0fe7f6dfc6706a0f43adf3 Author: Michal Wilczynski <[email protected]> drm/bridge: inno-hdmi: Add .disable platform operation This commit introduces a new .disable platform operation to the Innosilicon HDMI driver and invokes it in inno_hdmi_bridge_atomic_disable(). It allows platform-specific drivers to implement power down sequences for PHYs and clocks. > diff --git a/drivers/gpu/drm/bridge/inno-hdmi.c > b/drivers/gpu/drm/bridge/inno-hdmi.c > index 1264152fb1434..06917e83c5e53 100644 > --- 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); [Severity: High] Does this code introduce a sleep in atomic context? The inno_hdmi_bridge_atomic_disable() function runs from drm_atomic_helper_commit_tail(), which is an atomic context where sleeping is strictly forbidden. However, the commit message mentions this hook is meant for calling phy_power_off() and clk_disable_unprepare(), which can sleep. [Severity: Medium] Will this break LIFO teardown ordering? Disabling the platform PHY and clocks via plat_ops->disable() before calling inno_hdmi_standby() leaves the internal HDMI controller configured while its underlying hardware is powered down. To maintain proper LIFO ordering, shouldn't inno_hdmi_standby() be called first to put the internal PHY registers in standby before disabling the platform clocks and PHY? > > inno_hdmi_standby(hdmi); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
