clk: sunxi-ng: ccu_common: Replace ktime-dependent lock wait loop with udelay
Commit Message
The Allwinner sunxi-ng CCU driver uses readl_relaxed_poll_timeout() in
ccu_helper_wait_for_lock() to wait for PLLs to lock.
This poll macro relies on ktime_get() and the kernel timekeeping
infrastructure. During early boot or sensitive clock transitions
(such as CPUfreq DVFS scaling) where timer interrupts or timekeeping may
be suspended or unstable, ktime_get() fails to increment. This turns the
timeout calculation into an infinite busy-wait loop, causing RCU stalls
and system freezes if the PLL lock bit does not assert immediately.
Decouple the PLL lock wait loop from the timekeeping framework by replacing
readl_relaxed_poll_timeout() with an iteration loop using udelay(1) up to
100ms.
Assisted-by: LLM
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Notes:
Tested on Allwinner H618 (Mellow Fly-C5) and H616 boards in Armbian, resolving intermittent boot freezes and CPUfreq DVFS scaling lockups when timekeeping interrupts were suspended.
drivers/clk/sunxi-ng/ccu_common.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
Comments
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] clk: sunxi-ng: custom udelay loop instead of readl_relaxed_poll_timeout_atomic()
--
commit 303433ce9fcf08821cef4553ec1030effcdd3fb9
Author: Alastair D'Silva <alastair@d-silva.org>
clk: sunxi-ng: ccu_common: Replace ktime-dependent lock wait loop with udelay
This commit replaces the use of readl_relaxed_poll_timeout() in the sunxi-ng
CCU driver with a custom udelay() loop. The goal is to avoid system freezes
caused by timekeeping suspensions during early boot or CPUfreq DVFS scaling
when waiting for PLLs to lock.
> diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
> index 43d8eca6abeed..00daedadb5c83 100644
> --- a/drivers/clk/sunxi-ng/ccu_common.c
> +++ b/drivers/clk/sunxi-ng/ccu_common.c
[ ... ]
> @@ -35,7 +36,14 @@ void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock)
> else
> addr = common->base + common->reg;
>
> - WARN_ON(readl_relaxed_poll_timeout(addr, reg, reg & lock, 100, 70000));
> + for (i = 0; i < 100000; i++) {
> + if (readl_relaxed(addr) & lock)
> + return;
> + udelay(1);
> + }
[Severity: Low]
Does this custom busy-wait loop unnecessarily reinvent the kernel's standard
polling abstractions?
Since ccu_helper_wait_for_lock() executes whenever a PLL lock is waited upon
during clock configuration, such as early boot or CPUfreq transitions, avoiding
ktime dependencies is definitely necessary. However, the kernel already natively
provides readl_relaxed_poll_timeout_atomic() in <linux/iopoll.h> which is
designed specifically to perform timekeeping-independent hardware polling.
Would it be better to use readl_relaxed_poll_timeout_atomic() here instead of
circumventing the standard I/O polling macros with a hand-rolled loop?
> +
> + if (!(readl_relaxed(addr) & lock))
> + pr_warn("%s: clock lock timeout\n", clk_hw_get_name(&common->hw));
> }
> EXPORT_SYMBOL_NS_GPL(ccu_helper_wait_for_lock, "SUNXI_CCU");
@@ -7,6 +7,7 @@
#include <linux/clk.h>
#include <linux/clk-provider.h>
+#include <linux/delay.h>
#include <linux/device.h>
#include <linux/iopoll.h>
#include <linux/module.h>
@@ -25,7 +26,7 @@ struct sunxi_ccu {
void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock)
{
void __iomem *addr;
- u32 reg;
+ int i;
if (!lock)
return;
@@ -35,7 +36,14 @@ void ccu_helper_wait_for_lock(struct ccu_common *common, u32 lock)
else
addr = common->base + common->reg;
- WARN_ON(readl_relaxed_poll_timeout(addr, reg, reg & lock, 100, 70000));
+ for (i = 0; i < 100000; i++) {
+ if (readl_relaxed(addr) & lock)
+ return;
+ udelay(1);
+ }
+
+ if (!(readl_relaxed(addr) & lock))
+ pr_warn("%s: clock lock timeout\n", clk_hw_get_name(&common->hw));
}
EXPORT_SYMBOL_NS_GPL(ccu_helper_wait_for_lock, "SUNXI_CCU");