Re: [PATCH] clk: imx: scu: fix autosuspend cleanup on probe failure

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete error path cleanup: usage counter leak and unbalanced runtime PM disable
--

--- Patch [1]: [PATCH] clk: imx: scu: fix autosuspend cleanup on probe failure ---
commit be1da3e608e79b11e6b956430089bc533d7bfdbd
Author: Guangshuo Li <[email protected]>

clk: imx: scu: fix autosuspend cleanup on probe failure

This commit adds missing calls to pm_runtime_dont_use_autosuspend in the
probe failure paths of the imx-scu clock driver. This prevents an unbalanced
usage counter when the autosuspend delay is set to a negative value.

> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index 9b33df9967ece..6d29fcc2306d8 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c

[ ... ]

> @@ -542,6 +542,7 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
>  		ret = pm_runtime_resume_and_get(dev);
>  		if (ret) {
>  			pm_genpd_remove_device(dev);
> +			pm_runtime_dont_use_autosuspend(dev);
>  			pm_runtime_disable(dev);
>  			return ret;
>  		}
> @@ -550,6 +551,9 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
>  	hw = __imx_clk_scu(dev, clk->name, clk->parents, clk->num_parents,
>  			   clk->rsrc, clk->clk_type);
>  	if (IS_ERR(hw)) {
> +		if (!(clk->rsrc == IMX_SC_R_A35 || clk->rsrc == IMX_SC_R_A53 ||
> +		      clk->rsrc == IMX_SC_R_A72))
> +			pm_runtime_dont_use_autosuspend(dev);
>  		pm_runtime_disable(dev);
>  		return PTR_ERR(hw);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but since this modifies the error
path, is there a missing call to pm_runtime_put_noidle here?

Earlier in imx_clk_scu_probe, for non-CPU clocks, pm_runtime_resume_and_get
is called and increments the usage counter on success. If __imx_clk_scu
subsequently fails, it appears the usage count is leaked, which would
prevent the device and its power domain from ever suspending.

Also, pm_runtime_disable is called unconditionally here, but
pm_runtime_enable was only called if the clock was a non-CPU clock. Does
this cause an unbalanced runtime PM disable depth for CPU clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1