Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait for stability

[Drew Fustini <fustini@kernel.org>]

On Thu, Nov 20, 2025 at 01:14:11PM +0000, Yao Zi wrote:
> All PLLs found on TH1520 SoC take 21250ns at maximum to lock, and their
> lock status is indicated by register PLL_STS (offset 0x80 inside AP
> clock controller). We should poll the register to ensure the PLL
> actually locks after enabling it.
> 
> Furthermore, a 30us delay is added after enabling the PLL, after which
> the PLL could be considered stable as stated by vendor clock code.
> 
> Fixes: 56a48c1833aa ("clk: thead: add support for enabling/disabling PLLs")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)

Thanks for working on this patch series.

[...]
> @@ -299,9 +310,21 @@ static void ccu_pll_disable(struct clk_hw *hw)
>  static int ccu_pll_enable(struct clk_hw *hw)
>  {
>  	struct ccu_pll *pll = hw_to_ccu_pll(hw);
> +	u32 reg;
> +	int ret;
>  
> -	return regmap_clear_bits(pll->common.map, pll->common.cfg1,
> -				 TH1520_PLL_VCO_RST);
> +	regmap_clear_bits(pll->common.map, pll->common.cfg1,
> +			  TH1520_PLL_VCO_RST);
> +
> +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> +					      reg, reg & pll->lock_sts_mask,
> +					      5, TH1520_PLL_LOCK_TIMEOUT_US);

Is there a reason for the specific value of 5 uS polling delay?

> +	if (ret)
> +		return ret;
> +
> +	udelay(TH1520_PLL_STABLE_DELAY_US);

Is it the case that the 30 uS delay after the lock bit is set is just so
that it has the same behavior as the vendor's code? Or did you notice
stability problems without this?

Thanks,
Drew