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

Well, that's what I stated in the original patch comment. I don't have the HW
to test. I can send a blind fix, or whoever cares and has SoC that uses the
other timer driver that may have the same issue, can do it.

I'll do whatever the real maintainers suggest. ;)

	.o

On Sat, Jul 25, 2026 at 11:43:38AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the same hardware limitation where writing to the control register while the RELOAD bit is set causes the write to be dropped is unaddressed in `timer-sun5i.c`.
> --
> 
> --- Patch [1]: [PATCH] clocksource/drivers/sun4i: Wait for pending reload before CTRL write ---
> commit 936f66d7fd8614d6358ab1a1c7241c4675c9fff4
> Author: Ondrej Jirman <[email protected]>
> 
> clocksource/drivers/sun4i: Wait for pending reload before CTRL write
> 
> This commit fixes a bug in the sun4i timer driver where writes to the
> control register are silently dropped if the RELOAD bit is set. It introduces
> a polling mechanism to wait for the reload bit to clear before modifying
> the register.
> 
> > 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.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but as you suspected, the exact
> same hardware limitation is unaddressed in drivers/clocksource/timer-sun5i.c.
> 
> Looking at sun5i_clkevt_time_stop(), the unprotected read-modify-write of
> the control register occurs here:
> 
> drivers/clocksource/timer-sun5i.c:sun5i_clkevt_time_stop() {
>     ...
>     writel(val & ~TIMER_CTL_ENABLE,
>            ce->base + TIMER_CTL_REG(timer, offset));
>     ...
> }
> 
> If rapid back-to-back scheduling of high-resolution timers causes this to
> be called while the RELOAD bit from a prior time_start() is still set,
> wouldn't the write be dropped, failing to clear the ENABLE bit? 
> 
> Since this can permanently stop clock events and cause the system to freeze
> by halting process scheduling, should a similar wait mechanism be applied
> to the sun5i timer to protect devices that use it as a clockevent device?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1