Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > comp.arch.embedded > #26450

Re: A timer driver for Cortex-M0+... it rarely doesn't work

From David Brown <david.brown@hesbynett.no>
Newsgroups comp.arch.embedded
Subject Re: A timer driver for Cortex-M0+... it rarely doesn't work
Date 2017-04-27 10:01 +0200
Organization A noiseless patient Spider
Message-ID <ods8a4$3gr$1@dont-email.me> (permalink)
References <odr8bp$op3$1@dont-email.me>

Show all headers | View raw


On 27/04/17 00:55, pozz wrote:
> I don't know if it is an issue specific to Atmel SAMC21 Cortex-M0+
> devices or not.
> 
> I wrote a simple timer driver: a 32-bits hw counter clocked at 875kHz
> (14MHz/16) that triggers an interrupt on overflow (every 1h 21').  In
> the interrupt I increment the 32-bits global variable _ticks_high.
> The 64-bits number composed by _ticks_high (upper 32-bits) and the hw
> counter (lower 32-bits) is my system tick.  This 64-bits software
> counter, incremented at 875kHz, will never overflow during my life, so
> it's good :-)
> 
> In timer.h I have:
> --- Start of timer.h ---
> #include <stdint.h>
> #include <io.h>  // Atmel specific
> 
> #define TIMER_FREQ    875000
> 
> typedef uint64_t Timer;
> extern uint32_t _ticks_high;

Identifiers that begin with an underscore are reserved for file scope -
you are not supposed to use them for "extern" variables.  I would be
extremely surprised to find that a compiler treated them in any special
way, but them's the rules (§7.1.3 in C11 N1570.)

> 
> #define volatileAccess(v) *((volatile typeof((v)) *) &(v))

That looks familiar :-)

> 
> static inline uint64_t ticks(void) {
>   uint32_t h1 = volatileAccess(_ticks_high);
>   TC0->COUNT32.CTRLBSET.reg =
>        TC_CTRLBSET_CMD(TC_CTRLBSET_CMD_READSYNC_Val);
>   uint32_t l1 = TC0->COUNT32.COUNT.reg;
>   uint32_t h2 = volatileAccess(_ticks_high);
>   if (h2 != h1) return ((uint64_t)h2 << 32) + 0;
>   else          return ((uint64_t)h1 << 32) + l1;
> }

(I don't know what the CTRLBSET stuff is for - I am not familiar with
Atmel's chips here.)

If the low part of the counter rolls over and the interrupt has not run
(maybe interrupts are disabled, or you are already in a higher priority
interrupt, or interrupts take a few cycles to work through the system)
then this will be wrong - l1 will have rolled over, but h2 will not show
an updated value yet.

So if the high parts don't match, you need to re-read the low part and
re-check the high parts.  It is perhaps easiest to express in a loop:

static inline uint64_t ticks(void) {
	uint32_t h1 = volatileAccess(_ticks_high);
	while (true) {
		uint32_t l1 = TC0->COUNT32.COUNT.reg;
		uint32_t h2 = volatileAccess(_ticks_high);
		if (h1 == h2) return ((uint64_t) h2 << 32) | l1;
		h1 = h2;
	}
}

You can also reasonably note that this loop is not going to be re-run
more than once, unless you have interrupt functions that last for an
hour and a half, or something similarly bad - and then the failure of
the ticks() function is the least of your problems!  Then you can write:

static inline uint64_t ticks(void) {
	uint32_t h1 = volatileAccess(_ticks_high);
	uint32_t l1 = TC0->COUNT32.COUNT.reg;
	uint32_t h2 = volatileAccess(_ticks_high);
	if (h1 != h2) l1 = TC0->COUNT32.COUNT.reg;
	return ((uint64_t) h2 << 32) | l1;
}


> 
> static inline void TimerSet(Timer *tmr, uint64_t delay) {
>   /* delay is in ms */
>   *tmr = ticks() + delay * TIMER_FREQ / 1000;
> }
> 
> static inline int TimerExpired(Timer *tmr) {
>     return ticks() >= *tmr;
> }

When you want a boolean result, return "bool", not "int".  (This is not
your problem, of course - it's just good habit.)

An alternative method here is to just use the low 32-bit part (with
delays limited to 2^31 ticks), but be sure that you have considered
wraparound.  Keep everything in unsigned at first, to make overflows
work as modulo arithmetic:

static uint32_t ticks32(void) {
	return TC0->COUNT32.COUNT.reg;
}

static inline void TimerSet32(Timer32 *tmr, uint32_t delay) {
	*tmr = ticks32() + delay * TIMER_FREQ / 1000;
}

static inline bool TimerExpired32(Timer32 *tmr) {
	int32_t d = ticks32() - *tmr;
	return (d >= 0);
}

Note that doing the subtraction as unsigned, then converting to signed
int, gives you the wraparound behaviour you need.  Converting an
unsigned int to a signed int when the result is out of range is
implementation dependent (§6.3.1.3), but gcc and many other compilers
define it as modulo arithmetic:
<https://gcc.gnu.org/onlinedocs/gcc/Integers-implementation.html>




> --- End of timer.h ---
> 
> In timer.c I have the ISR:
> --- Start of timer.c ---
> ...
> void TC0_Handler(void) {
>   if (TC0->COUNT32.INTFLAG.reg & TC_INTFLAG_OVF) {
>     ++_ticks_high;
>     TC0->COUNT32.INTFLAG.reg = TC_INTFLAG_OVF;
>   }
> }
> ...
> --- End of timer.c ---
> 
> The idea is simple (and stolen from a post appeared on this newsgroup).

"Good artists copy, great artists steal", as someone famous once said.

> At first, the 64-bits software counter must be calculated disabling
> interrupts, because if a timer interrupt triggers during calculation,
> the overall software counter could be wrong.
> By reading _ticks_high before and after reading hw counter, we can avoid
> to disable the interrupts.
> 
> 
> Now I have a code that uses timers. It's a simple state-machine that
> manages communication over a bus.
> --- Start of bus.c ---
> ...
> int bus_task(void) {
>   switch(bus.state) {
>     case BUS_IDLE:
>       if (TimerExpired(&bus.tmr_answer)) {
>         /* Send new request on the bus */
>         ...
>         TimerSet(&bus.tmr_answer, timeout_answer);
>         bus.state = BUS_WAITING_ANSWER;
>       }
>       break;
> 
>     case BUS_WAITING_ANSWER:
>       if (TimerExpired(&bus.tmr_answer)) {
>         /* No reply */
>         bus.state = BUS_IDLE;
>         TimerSet(&bus.tmr_answer, 0);
>       } else {
>         if (reply_received() == true)    {
>           /* Analyze the reply */
>           bus.state = BUS_IDLE;
>           TimerSet(&bus.tmr_answer, 0);
>         }
>       }
>       break;
>   }
>   return 0;
> }
> ...
> --- End of bus.c ---
> 
> I don't think I need to explain the code in bus.c. The only thing to
> specify is that bus_task() is called continuously in the main loop.
> 
> 99% of the time this code works well. Unfortunately I have seen some
> strange events. Rarely (very rarely, one time in a week) the bus seems
> frozen for a time. After that it restarts the normal activity magically.
> There's a thing that relates those strange events to driver of timers:
> the bus stall time lasts exactly 1h 21', the overflow period of hw counter.

It sounds like you are missing a tick interrupt - maybe you are somehow
blocking the overflow interrupt on occasion.  Maybe the CTRLBSET line is
the problem (since I don't know what it does!)  But it could be the
problem I noted above in ticks().

Diagnosing problems like this are a real pain - and demonstrating that
you have fixed them is even worse.  The key is to find some way to speed
up the hardware timer so that you can provoke the problem regularly -
then you can be sure when you have fixed it.  Are you able to make this
timer overflow at a lower bit count - say, 12 bits rather than 32 bits?
 If not, then try this for your hardware interrupt function:

void TC0_Handler(void) {
	if (TC0->COUNT32.INTFLAG.reg & TC_INTFLAG_OVF) {
		++_ticks_high;
		TC0->COUNT32.INTFLAG.reg = TC_INTFLAG_OVF;
		TC0->COUNT32.COUNT.reg = 0xfffff000;
	}
}

That will force overflows and _ticks_high counts to run much faster.


> 
> I suspect there's a problem in my low-level driver and sometimes, maybe
> near the overflow, the code doesn't work as I expect.
> Maybe the TimerSet() function sometimes sets a wrong value to the
> uint64_t timer, maybe a tick value that will happen only at the next
> overflow of the hw counter.
> 
> Do you see where is the problem?
> 

Back to comp.arch.embedded | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-27 00:55 +0200
  Re: A timer driver for Cortex-M0+... it rarely doesn't work Simon Clubley <clubley@remove_me.eisner.decus.org-Earth.UFP> - 2017-04-26 23:24 +0000
    Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-27 09:32 +0200
      Re: A timer driver for Cortex-M0+... it rarely doesn't work Jack <jack4747@gmail.com> - 2017-04-27 01:15 -0700
  Re: A timer driver for Cortex-M0+... it rarely doesn't work John Speth <johnspeth@yahoo.com> - 2017-04-26 17:17 -0700
    Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-27 09:37 +0200
      Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 10:13 +0200
        Re: A timer driver for Cortex-M0+... it rarely doesn't work John Speth <johnspeth@yahoo.com> - 2017-04-27 10:24 -0700
          Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 22:04 +0200
  Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 10:01 +0200
    Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 10:27 +0200
      Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-27 11:53 +0200
        Re: A timer driver for Cortex-M0+... it rarely doesn't work Stef <stef33d@yahooI-N-V-A-L-I-D.com.invalid> - 2017-05-01 10:09 +0200
          Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-05-02 09:03 +0200
            Re: A timer driver for Cortex-M0+... it rarely doesn't work Stef <stef33d@yahooI-N-V-A-L-I-D.com.invalid> - 2017-05-04 10:06 +0200
    Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-27 11:38 +0200
      Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 12:54 +0200
        Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-27 13:51 +0200
          Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 15:39 +0200
            Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-28 09:24 +0200
              Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-28 11:05 +0200
                Re: A timer driver for Cortex-M0+... it rarely doesn't work pozz <pozzugno@gmail.com> - 2017-04-28 12:18 +0200
                Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-28 13:10 +0200
                ARM developing under Linux [was Re: A timer driver for Cortex-M0+... it rarely doesn't work] pozz <pozzugno@gmail.com> - 2017-05-02 09:14 +0200
                Re: ARM developing under Linux [was Re: A timer driver for Cortex-M0+... it rarely doesn't work] David Brown <david.brown@hesbynett.no> - 2017-05-02 11:34 +0200
                Re: ARM developing under Linux [was Re: A timer driver for Cortex-M0+... it rarely doesn't work] pozz <pozzugno@gmail.com> - 2017-05-03 09:04 +0200
                Re: ARM developing under Linux [was Re: A timer driver for Cortex-M0+... it rarely doesn't work] David Brown <david.brown@hesbynett.no> - 2017-05-03 10:59 +0200
  Re: A timer driver for Cortex-M0+... it rarely doesn't work Dimiter_Popoff <dp@tgi-sci.com> - 2017-04-27 11:02 +0300
    Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 11:06 +0200
      Re: A timer driver for Cortex-M0+... it rarely doesn't work Dimiter_Popoff <dp@tgi-sci.com> - 2017-04-27 14:20 +0300
        Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 16:16 +0200
      Re: A timer driver for Cortex-M0+... it rarely doesn't work Simon Clubley <clubley@remove_me.eisner.decus.org-Earth.UFP> - 2017-04-27 12:03 +0000
        Re: A timer driver for Cortex-M0+... it rarely doesn't work David Brown <david.brown@hesbynett.no> - 2017-04-27 16:18 +0200

csiph-web