-----Original Message-----
From: simon.desfarges(a)intel.com [mailto:simon.desfarges(a)intel.com]
Sent: March-17-16 2:42 PM
To: devel(a)lists.zephyrproject.org
Cc: DESFARGES, SIMON
Subject: [devel] [PATCH 1/2] arc_timer: fix tickless idle

From: Simon Desfarges <simon.desfarges(a)intel.com>

When exiting from tickless idle uppon an external IRQ, the TICK timer is
set to fire at next TICK boundary. The current algorithm can lead to a
point that timer0_count register is higher than the timer0_limit register.

In this situation the next TICK will fire after a full wrapp of the
counter
(~133 seconds).

This condition appears when the counter reaches the limit after the
Interrupt Pending flag is checked. At this point the counter is
automatically wrapped to 0, but is set just next the limit to fire at next
TICK boundary by SW.
At exit of the _timer_idle_exit function, the timer handler is called, and
sets the limit to 1 TICK. At this point the situation is:
- limit register == 1 TICK
- count register is just below the old limit register and higher than 1
TICK

To fix this issue, at _timer_idle_exit, the limit register is always set
to 1 TICK and the count register set such as the next TICK fires on time.

Change-Id: Ifa002809d426aa04109592e53d2b02a224f51101
Tracked-On: https://jira.ndg.intel.com/browse/FIRE-4213
Signed-off-by: Simon Desfarges <simon.desfarges(a)intel.com>
---
drivers/timer/arcv2_timer0.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/timer/arcv2_timer0.c b/drivers/timer/arcv2_timer0.c
index 932ceac..5fec988 100644
--- a/drivers/timer/arcv2_timer0.c
+++ b/drivers/timer/arcv2_timer0.c
@@ -220,7 +220,7 @@ void _timer_idle_enter(int32_t ticks)
}

programmed_ticks = ticks;
- programmed_limit = (programmed_ticks * cycles_per_tick);
+ programmed_limit = (programmed_ticks * cycles_per_tick) - 1;

[Peter]: Good catch on this off by one error.

timer0_limit_register_set(programmed_limit);

@@ -276,9 +276,6 @@ void _timer_idle_exit(void)
* A non-timer interrupt occurred. Announce any
* ticks that have elapsed during the tickless idle.
*/
-
- uint32_t remaining_cycles = programmed_limit - current_count;
-
_sys_idle_elapsed_ticks = current_count / cycles_per_tick;
if (_sys_idle_elapsed_ticks > 0) {
update_accumulated_count();
@@ -289,11 +286,8 @@ void _timer_idle_exit(void)
* Ensure the timer will expire at the end of the next tick in case
* the ISR makes any tasks and/or fibers ready to run.
*/
-
- if (remaining_cycles >= cycles_per_tick) {
- timer0_count_register_set(programmed_limit -
- ((remaining_cycles - 1) % cycles_per_tick) - 1);
- }
+ timer0_limit_register_set(cycles_per_tick - 1);
+ timer0_count_register_set(timer0_count_register_get() %
+cycles_per_tick);

[Peter]:

First an historical note. The reason the original code only touched the count register was that it was not known if lowering the limit register to a value below that of the count register would trigger an interrupt. (I did not have access to a board with the ARCv2 timer and so was unable to confirm or deny that behavior.) That being said, if changing the limit value from a value higher than the count to a value lower than the count does not generate an interrupt, then I have no objections on that front.

Second, Using timer0_count_register_get() at this point is a no-no as it introduces a race condition that can cause us to lose track of a whole tick. Remember, the timer is always counting. If the elapsed ticks were announced just prior to a "tick boundary", the call to timer0_count_register_set() could then occur just after the "tick boundary". The result of this scenario is that nearly one whole tick may be lost when the modulus operation is performed. This is why we use the <current_count> retrieved from the start of _timer_idle_exit(). Yes, it does introduce some gradual drift, but this can be ameliorated somewhat by only modifying the count if there is more than one tick left in the tickles idle.

Hope this helps.

}
#else
static void tickless_idle_init(void) {}
--
1.9.1