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

Yao Zi
2025-11-25T03:20:45Z

On Mon, Nov 24, 2025 at 02:08:00PM -0800, Drew Fustini wrote:
> 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?

No, it was picked randomly. A smaller value would reduce latency of
PLL enabling, and I could tune it more carefully by some testing. But
it's hard to predict how much improvement it will bring.

> > + 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?

This aligns with the vendor code, and I haven't yet observed stability
issues without the delay. But I think it's more safe to keep the
behavior similar since it's hard to test all working conditions.

> Thanks,
> Drew

Best regards,
Yao Zi