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: <aScZ5mwesY3wQTQl@pie>
Date: Wed, 26 Nov 2025 15:16:54 +0000
From: Yao Zi <ziyao@...root.org>
To: Drew Fustini <fustini@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>,
	devicetree@...r.kernel.org, Stephen Boyd <sboyd@...nel.org>,
	Michael Turquette <mturquette@...libre.com>,
	linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
	linux-clk@...r.kernel.org, Guo Ren <guoren@...nel.org>,
	Han Gao <rabenda.cn@...il.com>, Han Gao <gaohan@...as.ac.cn>,
	Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <pjw@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Fu Wei <wefu@...hat.com>
Subject: Re: [PATCH 2/7] clk: thead: th1520-ap: Poll for PLL lock and wait
 for stability

On Wed, Nov 26, 2025 at 08:52:00AM -0600, 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@...root.org>
> > ---
> >  drivers/clk/thead/clk-th1520-ap.c | 34 +++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> [...]
> > +/*
> > + * All PLLs in TH1520 take 21250ns at maximum to lock, let's take its double
> > + * for safety.
> > + */
> > +#define TH1520_PLL_LOCK_TIMEOUT_US	44
> > +#define TH1520_PLL_STABLE_DELAY_US	30
> 
> I'm taking a second look at this and I think it might be best to add a
> define for the polling loop delay of 5. It could be helpful when other
> people read the code later.
> 
> [...]
> > +	ret = regmap_read_poll_timeout_atomic(pll->common.map, TH1520_PLL_STS,
> > +					      reg, reg & pll->lock_sts_mask,
> > +					      5, TH1520_PLL_LOCK_TIMEOUT_US);
> 
> The loop delay is only used here but I think using a #define would make
> it more readable.

There are TH1520_PLL_LOCK_TIMEOUT_US and TH1520_PLL_STABLE_DELAY_US
defined because they're meaningful constants, either specified by TRM or
implied by vendor code, however the 5us delay is only a randomly-picked
value, as what I've mentioned before.

Anyway, I'm fine with a separate definition. So please go ahead if it
looks better to you.

> Other than that:
> Reviewed-by: Drew Fustini <fustini@...nel.org>
> 
> If no other changes are needed I could fix this up on apply. Let's see
> what other comments there may be. It's too late for me to send a 6.19
> clk pull request so this will have to target the next merge window. I
> can put it into linux-next once 6.19-rc1 is released.

Many thanks for it.

> Thanks,
> Drew

Best regards,
Yao Zi