[PATCH] clocksource/drivers/sun4i: Wait for pending reload before CTRL write

[Ondřej Jirman]

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 at xff.cz>
> > 
> > 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 at xff.cz>
> > ---
> > 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.

> > +	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?

Best regards,
	o.

> > +
> > +	val = readl(base + TIMER_CTL_REG(timer));
> >   	writel(val & ~TIMER_CTL_ENABLE, base + TIMER_CTL_REG(timer));
> >   	sun4i_clkevt_sync(base);
> >   }
>