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

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

[Ondřej Jirman]

From: Ondrej Jirman <megi@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@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();
+	}
+
+	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);
+
+	val = readl(base + TIMER_CTL_REG(timer));
 	writel(val & ~TIMER_CTL_ENABLE, base + TIMER_CTL_REG(timer));
 	sun4i_clkevt_sync(base);
 }
-- 
2.54.0

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

[Daniel Lezcano]

On 7/25/26 13:33, Ondřej Jirman wrote:
> From: Ondrej Jirman <megi@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@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);


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

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

Re: [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@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@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);
> >   }
> 

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

[Daniel Lezcano]

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@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@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.

Sorry I don't get the 1000 * timeout

Does the documentation says anything about the duration before the 
register is cleared ?

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

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

[Ondřej Jirman]

On Wed, Aug 12, 2026 at 01:49:18PM +0200, Daniel Lezcano wrote:
> 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@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@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.
> 
> Sorry I don't get the 1000 * timeout

If you look at implementation of readl_poll_timeout_atomic() the timeout
parameter means that readl_poll_timeout_atomic() will do at most timeout * 1000
thight loops, so minimum timeout is 1000 loops.

> Does the documentation says anything about the duration before the register
> is cleared ?

Not directly, but for start after stop, manual says to wait at least 2 Tcycles
(input clock cycles to the timer).

Regards,
	o.

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