Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] soc: starfive: NoC bus clock leaks power during system sleep --
commit f4b88d3831df8dc30fb442edb24499643c924a58 Author: Michal Wilczynski <[email protected]> soc: starfive: Add jh7110-vout-subsystem driver This commit adds a parent driver for the StarFive JH7110 video output subsystem. It claims the NoC display bus clock and reset, and holds a runtime PM reference to keep the PD_VOUT power domain active for as long as any child device exists. > diff --git a/drivers/soc/starfive/jh7110-vout-subsystem.c > b/drivers/soc/starfive/jh7110-vout-subsystem.c > new file mode 100644 > index 0000000000000..af632692e2139 > --- /dev/null > +++ b/drivers/soc/starfive/jh7110-vout-subsystem.c [ ... ] > + bus_clk = devm_clk_get_enabled(dev, NULL); > + if (IS_ERR(bus_clk)) > + return dev_err_probe(dev, PTR_ERR(bus_clk), > + "Failed to enable NoC bus clock\n"); > + > + bus_rst = devm_reset_control_get_exclusive_deasserted(dev, NULL); [Severity: Medium] Does this cause a power leak during system suspend? The NoC bus clock is enabled here, but the pointer to bus_clk is discarded. This means it cannot be disabled later during system sleep transitions. While the genpd framework manages the PD_VOUT domain during system sleep, the clock framework doesn't automatically disable clocks. The software enable count will remain greater than zero, which could keep the NoC bus clock and its parent PLLs active and leak power during suspend (e.g., suspend-to-RAM). [ ... ] > +static struct platform_driver jh7110_vout_subsys_driver = { > + .probe = jh7110_vout_subsys_probe, > + .driver = { > + .name = "jh7110-vout-subsystem", > + .of_match_table = jh7110_vout_subsys_of_match, > + }, > +}; > +module_platform_driver(jh7110_vout_subsys_driver); [Severity: Medium] Because the driver struct lacks a .pm field with system suspend and resume callbacks, it seems there is no mechanism to disable the NoC bus clock when the system goes to sleep. Should the driver store the clock pointer in a private structure and implement dev_pm_ops callbacks to handle disabling and re-enabling the clock? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
