Re: [PATCH] clocksource/drivers/sun4i: Wait for pending reload before CTRL write
From: Daniel Lezcano
Date: Wed Aug 12 2026 - 07:52:35 EST
On 8/12/26 13:09, Ondřej Jirman wrote:
Hello Daniel,
On Wed, Aug 12, 2026 at 11:56:03AM +0200, Daniel Lezcano wrote:
On 7/25/26 13:33, Ondřej Jirman wrote:
From: Ondrej Jirman <megi@xxxxxx>
The A13 manual says of TMR0_RELOAD: "After the bit is set, it can not be
written again before it's cleared automatically." Both time_stop() and
time_start() read-modify-write the control register, so a reload still in
flight gets rewritten and the write can be dropped, leaving the timer
unarmed. In oneshot mode this happens on every tick, and the tick
eventually stops for good.
Wait for the reload to clear first. All control register updates go
through sun4i_clkevt_time_stop().
Without this my Allwinner A13 based Pocketbook Touch Lux 3 stops
scheduling processes during boot or within a few seconds of executing
userspace. Only sysrq+t over serial port works at this stage, which is
how I discovered the root cause.
Fixes: 7e14183469d8 ("clocksource: sun4i: Fix bug when switching from periodic to oneshot modes")
Signed-off-by: Ondrej Jirman <megi@xxxxxx>
---
BTW, similar issue will likely be also in timer-sun5i.c but I don't have any
device that would exercise it. And it's possible that this triggers more easily
with CONFIG_HZ=1000 which I use.
drivers/clocksource/timer-sun4i.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-sun4i.c b/drivers/clocksource/timer-sun4i.c
index 7bdcc60ad43c..2e7457f671c4 100644
--- a/drivers/clocksource/timer-sun4i.c
+++ b/drivers/clocksource/timer-sun4i.c
@@ -39,6 +39,9 @@
#define TIMER_SYNC_TICKS 3
+/* The reload bit clears a couple of source clock cycles after it is set. */
+#define TIMER_RELOAD_MAX_POLL 100
+
/*
* When we disable a timer, we need to wait at least for 2 cycles of
* the timer source clock. We will use for that the clocksource timer
@@ -53,9 +56,30 @@ static void sun4i_clkevt_sync(void __iomem *base)
cpu_relax();
}
+/*
+ * The control register must not be written while a reload is still in
+ * flight, or the write can be dropped.
+ */
+static int sun4i_clkevt_wait_reload(void __iomem *base, u8 timer)
+{
+ int i;
+
+ for (i = 0; i < TIMER_RELOAD_MAX_POLL; i++) {
+ if (!(readl(base + TIMER_CTL_REG(timer)) & TIMER_CTL_RELOAD))
+ return 0;
+ cpu_relax();
+ }
The loop looks correct but I'm wondering why not use the helper:
ret = readl_poll_timeout_atomic(base + TIMER_CTL_REG(timer), val,
!(val & TIMER_CTL_RELOAD), 0, timeout);
I can. The only difference would be that this would loop up to 1000 * timeout
times, instead of 100 times in case reload bit never returns to 0. But that
never happens anyway.
Sorry I don't get the 1000 * timeout
Does the documentation says anything about the duration before the register is cleared ?
IMO, returning void is fine along with a message, may be a WARN as it is really, AFAIU, not supposed to happen. It will be easy for anyone to find out where the issue is coming from+ return -ETIME;
+}
+
static void sun4i_clkevt_time_stop(void __iomem *base, u8 timer)
{
- u32 val = readl(base + TIMER_CTL_REG(timer));
+ u32 val;
+
+ sun4i_clkevt_wait_reload(base, timer);
If it fails ?
There's not much to do, since sun4i_clkevt_time_stop() does not return anything
anyway and it's used across the file quite extensively. Well, the file is small,
but sun4i_clkevt_time_stop is called in every function exported via timer_of
struct... :)
I can rewrite all these functions to report all errors to timer core, but in
practice reload always resets very quickly, and a condition where the loop even
loops happens once every few seconds or so. I doubt anything but return 0
would ever happen anyway. I'd certainly notice, because it would again cause
instant lockup of my e-reader. Maybe I can just return void from
sun4i_clkevt_wait_reload() and not pretend there are errors to report,
instead?