Re: [PATCH v4 14/24] clk: mediatek: pllfh: Add ops for PLLs using set/clr regs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing rollback on error path in `mtk_pll_prepare_setclr` leads to permanent PLL power leak.
--

--- Patch [14]: [PATCH v4 14/24] clk: mediatek: pllfh: Add ops for PLLs using set/clr regs ---
commit 6499dae9fef2cef096ef23a01c06293418528fef
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

clk: mediatek: pllfh: Add ops for PLLs using set/clr regs
    
Add new set of PLL FHCTL clock operations, that uses the new prepare
and unprepare methods, added in PLL for MT8189 SoC to support this
logic.

> diff --git a/drivers/clk/mediatek/clk-pllfh.c b/drivers/clk/mediatek/clk-pllfh.c
> index 4878ac6da7da0..8dc50461c485b 100644
> --- a/drivers/clk/mediatek/clk-pllfh.c
> +++ b/drivers/clk/mediatek/clk-pllfh.c
[ ... ]
> @@ -47,6 +47,16 @@ static const struct clk_ops mtk_pllfh_ops = {
>  	.set_rate	= mtk_fhctl_set_rate,
>  };
>  
> +const struct clk_ops mtk_pllfh_setclr_ops = {
> +	.is_prepared	= mtk_pll_is_prepared,
> +	.prepare	= mtk_pll_prepare_setclr,
[Severity: Medium]
Could mtk_pll_prepare_setclr() leak power on the error path?

Looking at drivers/clk/mediatek/clk-pll.c:mtk_pll_prepare_setclr() which was
introduced earlier in this series:

	mtk_pll_prepare_common(pll, pll->en_set_addr);

	if (pll->data->flags & HAVE_RST_BAR) {
		writel(pll->data->rst_bar_mask, pll->rst_bar_set_addr);

		ret = readl_poll_timeout(pll->rst_bar_addr, val,
					(val & pll->data->rst_bar_mask), 1,
					RST_BAR_TIMEOUT);
		if (ret)
			return ret;
	}

If readl_poll_timeout() fails, it returns the error directly without calling
mtk_pll_unprepare_common() to undo the state change.

Since the Common Clock Framework won't call .unprepare() on a clock that
failed to prepare, wouldn't the PLL be left permanently powered on?

> +	.unprepare	= mtk_pll_unprepare_setclr,
> +	.recalc_rate	= mtk_pll_recalc_rate,
> +	.determine_rate = mtk_pll_determine_rate,
> +	.set_rate	= mtk_fhctl_set_rate,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-mt8189-clocks-system-base-v4-0-e356b813a64c@collabora.com?part=14