[PATCH] clk: zynq: return -ETIMEDOUT if the PLL never locks

[PATCH] clk: zynq: return -ETIMEDOUT if the PLL never locks

[Linkai Gong]

zynq_pll_enable() waits for lock under a spinlock with no
timeout. Poll with a 1ms bound and return the error.

Fixes: 3682af46d55f ("clk: zynq: Factor out PLL driver")
Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
---
 drivers/clk/zynq/pll.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/zynq/pll.c b/drivers/clk/zynq/pll.c
index 44c609378364..96cf45088ba0 100644
--- a/drivers/clk/zynq/pll.c
+++ b/drivers/clk/zynq/pll.c
@@ -10,6 +10,7 @@
 #include <linux/clk-provider.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 
 /**
  * struct zynq_pll - pll clock
@@ -119,6 +120,7 @@ static int zynq_pll_enable(struct clk_hw *hw)
 	unsigned long flags = 0;
 	u32 reg;
 	struct zynq_pll *clk = to_zynq_pll(hw);
+	int ret;
 
 	if (zynq_pll_is_enabled(hw))
 		return 0;
@@ -131,12 +133,12 @@ static int zynq_pll_enable(struct clk_hw *hw)
 	reg = readl(clk->pll_ctrl);
 	reg &= ~(PLLCTRL_RESET_MASK | PLLCTRL_PWRDWN_MASK);
 	writel(reg, clk->pll_ctrl);
-	while (!(readl(clk->pll_status) & (1 << clk->lockbit)))
-		;
+	ret = readl_poll_timeout_atomic(clk->pll_status, reg,
+					reg & (1 << clk->lockbit), 10, 1000);
 
 	spin_unlock_irqrestore(clk->lock, flags);
 
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.25.1

Re: [PATCH] clk: zynq: return -ETIMEDOUT if the PLL never locks

[Michal Simek]

On 9/1/26 14:53, Linkai Gong wrote:
> zynq_pll_enable() waits for lock under a spinlock with no
> timeout. Poll with a 1ms bound and return the error.
> 
> Fixes: 3682af46d55f ("clk: zynq: Factor out PLL driver")
> Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
> ---
>   drivers/clk/zynq/pll.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/zynq/pll.c b/drivers/clk/zynq/pll.c
> index 44c609378364..96cf45088ba0 100644
> --- a/drivers/clk/zynq/pll.c
> +++ b/drivers/clk/zynq/pll.c
> @@ -10,6 +10,7 @@
>   #include <linux/clk-provider.h>
>   #include <linux/slab.h>
>   #include <linux/io.h>
> +#include <linux/iopoll.h>
>   
>   /**
>    * struct zynq_pll - pll clock
> @@ -119,6 +120,7 @@ static int zynq_pll_enable(struct clk_hw *hw)
>   	unsigned long flags = 0;
>   	u32 reg;
>   	struct zynq_pll *clk = to_zynq_pll(hw);
> +	int ret;
>   
>   	if (zynq_pll_is_enabled(hw))
>   		return 0;
> @@ -131,12 +133,12 @@ static int zynq_pll_enable(struct clk_hw *hw)
>   	reg = readl(clk->pll_ctrl);
>   	reg &= ~(PLLCTRL_RESET_MASK | PLLCTRL_PWRDWN_MASK);
>   	writel(reg, clk->pll_ctrl);
> -	while (!(readl(clk->pll_status) & (1 << clk->lockbit)))
> -		;
> +	ret = readl_poll_timeout_atomic(clk->pll_status, reg,
> +					reg & (1 << clk->lockbit), 10, 1000);

BIT(clk->lockbit)

And 10 and 1000 are magic values.

Fix itself is fine but I would prefer to explain more why 1ms upper limit was 
used. I don't think it is going to be a problem and 10-1000us is fine. I just 
want to make sure that it will be clear that this value is not coming from any 
TRM but still at least range is aligned with expectation in HW.

Thanks,
Michal

Re: [PATCH] clk: zynq: return -ETIMEDOUT if the PLL never locks

[Linkai Gong]

On Mon, Sep 07, 2026 at 02:58:18PM +0200, Michal Simek wrote:
> BIT(clk->lockbit)
>
> And 10 and 1000 are magic values.
>
> Fix itself is fine but I would prefer to explain more why 1ms upper limit was
> used. I don't think it is going to be a problem and 10-1000us is fine. I just
> want to make sure that it will be clear that this value is not coming from any
> TRM but still at least range is aligned with expectation in HW.

Thanks for the review.

Agreed on both points. v2 will use BIT(), name the poll delay/timeout,
and clarify in the commit message that 1 ms is a software upper bound
for a stuck PLL under spinlock, not a TRM-derived value; lock is still
expected well within that window on Zynq.

Will send v2 shortly.

Thanks,
Linkai

[PATCH v2] clk: zynq: return -ETIMEDOUT if the PLL never locks

[Linkai Gong]

zynq_pll_enable() waits for lock under a spinlock with no timeout.
A stuck PLL would wedge the enable path with IRQs off.

Poll with readl_poll_timeout_atomic() and return -ETIMEDOUT on
failure. The 1 ms upper bound is a software limit for a stuck PLL,
not a TRM-derived value; Zynq PLL lock is still expected well
within that window.

Changes in v2:
- Use BIT(clk->lockbit)
- Name the poll delay/timeout constants
- Clarify the 1 ms bound in the commit message

Fixes: 3682af46d55f ("clk: zynq: Factor out PLL driver")
Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
---
 drivers/clk/zynq/pll.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/zynq/pll.c b/drivers/clk/zynq/pll.c
index fe90b50e1545..6a98bc60fb91 100644
--- a/drivers/clk/zynq/pll.c
+++ b/drivers/clk/zynq/pll.c
@@ -9,7 +9,9 @@
 #include <linux/clk/zynq.h>
 #include <linux/clk-provider.h>
 #include <linux/slab.h>
+#include <linux/bits.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 
 /**
  * struct zynq_pll - pll clock
@@ -41,6 +43,10 @@ struct zynq_pll {
 #define PLL_FBDIV_MIN	13
 #define PLL_FBDIV_MAX	66
 
+/* Software bound for a stuck PLL under spinlock; not from the TRM. */
+#define PLL_LOCK_POLL_DELAY_US	10
+#define PLL_LOCK_TIMEOUT_US	1000
+
 /**
  * zynq_pll_determine_rate() - Round a clock frequency
  * @hw:		Handle between common and hardware-specific interfaces
@@ -119,6 +125,7 @@ static int zynq_pll_enable(struct clk_hw *hw)
 	unsigned long flags = 0;
 	u32 reg;
 	struct zynq_pll *clk = to_zynq_pll(hw);
+	int ret;
 
 	if (zynq_pll_is_enabled(hw))
 		return 0;
@@ -131,12 +138,14 @@ static int zynq_pll_enable(struct clk_hw *hw)
 	reg = readl(clk->pll_ctrl);
 	reg &= ~(PLLCTRL_RESET_MASK | PLLCTRL_PWRDWN_MASK);
 	writel(reg, clk->pll_ctrl);
-	while (!(readl(clk->pll_status) & (1 << clk->lockbit)))
-		;
+	ret = readl_poll_timeout_atomic(clk->pll_status, reg,
+					reg & BIT(clk->lockbit),
+					PLL_LOCK_POLL_DELAY_US,
+					PLL_LOCK_TIMEOUT_US);
 
 	spin_unlock_irqrestore(clk->lock, flags);
 
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.25.1