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


Groups > comp.arch.embedded > #26456

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

From pozz <pozzugno@gmail.com>
Newsgroups comp.arch.embedded
Subject Re: A timer driver for Cortex-M0+... it rarely doesn't work
Date 2017-04-27 11:38 +0200
Organization A noiseless patient Spider
Message-ID <odse1j$kc4$1@dont-email.me> (permalink)
References <odr8bp$op3$1@dont-email.me> <ods8a4$3gr$1@dont-email.me>

Show all headers | View raw


Il 27/04/2017 10:01, David Brown ha scritto:

>> #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.)

Atmel says you *need* to write CTRLB register before reading COUNT 
register from TC0 peripheral.  I don't know why exactly, but if you 
don't, you can't read the correct COUNT value.


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

Yes, you are right and I thought about this possibility... but IMHO it's 
impossibile.

ticks() are called only by TimerSet() and TimerExpired() and those two 
functions are called only in normal background (not interrupt) code.
This means ticks() always runs in a lower priority than TC0 ISR. 
Moreover, I never disable interrupts in any part of the code.

I don't understand what do you mean with "[...] or interrupts take a few 
cycles to work through the system". Is it possible to read a rolled-over 
hw counter value (0x00000003) and a not incremented _ticks_high?

    uint32_t l1 = TC0->COUNT32.COUNT.reg;
    uint32_t h2 = volatileAccess(_ticks_high);

COUNT register is read before _ticks_high second read. If COUNT has 
rolled over and the rolled value is in l1, the interrupt was fired for 
sure... IMHO.  So h2 should contain the incremented value.


> 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;
> }

This doesn't solve the problem you described.

_ticks_high = 1
90    <- h1 = 1
91
...
99
0     <- read l1
1
2     <- h2 = 1 (interrupt has not fired, your assumption)
3

ticks() could be {1,0} that is completely wrong.


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

Yes, I know this method and I used it many times in the past. However it 
has two drawbacks:
- the maximum delay is 2^31 (in my case, around 40 minutes, but I have
   delays of 1h)
- many times you need an additional "timer running/active" flag

The second point is more important. If you want to switch on a LED after 
10 minutes when a button is pressed:

bool tmr_led_active;
Timer32 tmr_led;
void button_pressed_callback(void) {
    TimerSet32(&tmr_led, 10 * 60 * 1000);
    tmr_led_active = true;
}
void main_loop(void) {
   ...
   if (tmr_led_active && TimerExpired32(&tmr_led)) {
     switch_on_led();
     tmr_led_active = false;
   }
}

If you don't check the timer flag, the led will switch on at random times.


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

Yes, I will try your ideas.

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