[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aScUELnY-TC8kVzV@gen8>
Date: Wed, 26 Nov 2025 08:52:00 -0600
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(-)
>
> 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.
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.
Thanks,
Drew
Powered by blists - more mailing lists