Re: [PATCH v4 13/20] soc: starfive: Add jh7110-vout-subsystem driver

[sashiko-bot@kernel.org]

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 <m.wilczynski@samsung.com>
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/20260915-jh7110-clean-send-v4-0-f0e4fd6f2cc8@samsung.com?part=13