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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aSTXQG5yIIGFjflG@x1>
Date: Mon, 24 Nov 2025 14:08:00 -0800
From: Drew Fustini <fustini@...nel.org>
To: Yao Zi <ziyao@...root.org>
Cc: Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Paul Walmsley <pjw@...nel.org>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, Icenowy Zheng <uwu@...nowy.me>,
	linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
	Han Gao <rabenda.cn@...il.com>, Han Gao <gaohan@...as.ac.cn>
Subject: Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait
 for stability

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@...root.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