SAMD10 Xplained RTC Problem...

[westfw]

Anyone have experience with the Atmel "RTC" modules that they're putting on their small ARMs?   I'm having mysterious problems!  [copied from AVRFreaks...)
=====

I haven't quite solved the debug strangeness, but the code overall is working better, except for a little problem that seems like it's probably a generic timer issue rather than something SAM specific.

In order to get microsecond and millisecond counters, I've configured the RTC to have a 1MHz clock (us) and COMP[0] = 999 to cause an interrupt every 1000 ticks (ms)  The ISR increments the ms counter, and to get elapsed microseconds I add the current RTC counter value to 1000*mscount from the ISR.  It seems straightforward, barring minor complications like making the math interrupt safe and accounting for counter resets that haven't had the ISR serviced yet.   I *thought* I had those taken care of.


But sometimes my microsecond count moves backwards, and I can't figure our why. :-(   Am I missing some well-known technique for dividing up a timer between HW and ISR?


Here is the meat of the code:

Code: [Select]

void RTC_init()
{
    //Configure GCLK Generator 2 to use a divided XOSC as input, and feed this clock to RTC
    // This gives us a 1MHz RTC clock.
    GCLK->GENDIV.reg = ((8 << GCLK_GENDIV_DIV_Pos) | (2 << GCLK_GENDIV_ID_Pos));
    GCLK->GENCTRL.reg = ((2 << GCLK_GENCTRL_ID_Pos) | (GCLK_GENCTRL_SRC_XOSC) | (GCLK_GENCTRL_GENEN));
    GCLK->CLKCTRL.reg = ((GCLK_CLKCTRL_GEN_GCLK2) | (GCLK_CLKCTRL_CLKEN) | (GCLK_CLKCTRL_ID(RTC_GCLK_ID)));
   
    RTC->MODE0.CTRL.reg = 0;  // disable, so that we can reset
    RTC->MODE0.CTRL.reg = RTC_MODE0_CTRL_SWRST; //reset
    RTC->MODE0.CTRL.reg = RTC_MODE0_CTRL_MODE_COUNT32 | RTC_MODE0_CTRL_MATCHCLR;
    RTC->MODE0.COMP[0].reg = 1000 - 1;
    RTC->MODE0.INTENSET.reg = RTC_MODE0_INTENSET_CMP0;
    RTC->MODE0.CTRL.reg |= RTC_MODE0_CTRL_ENABLE;
    RTC->MODE0.READREQ.reg |= RTC_READREQ_RCONT|RTC_READREQ_RREQ;  // continuous read sync
    RTC->MODE0.INTFLAG.reg = 0xFF;
   
    NVIC_SetPriority (RTC_IRQn, (1<<__NVIC_PRIO_BITS) - 1);
    NVIC_EnableIRQ(RTC_IRQn);
}

void RTC_Handler(void)
{
    millisecondcount++;
    RTC->MODE0.INTFLAG.reg = 0xFF;  // Clear all possible interrupts
}

volatile uint32_t lastmicros;
volatile uint32_t question = 0;
volatile uint32_t asked = 0;

uint32_t micros()
{
    uint32_t thismicros;
   
    cpu_irq_enter_critical();
    asked++;  // how many calls to us?
    thismicros = RTC->MODE0.COUNT.reg;
    if (RTC->MODE0.INTFLAG.bit.CMP0) {
        /*
         * Check for a ms interrupt pending but not yet
         * serviced, and correct for it.
         */
        thismicros += 1000;
    }
    thismicros += 1000*millisecondcount;

    if (thismicros < lastmicros) {
        // We shouldn't get here.  But allow it to BKPT
        question++;  // how many "questionable" results
    }
    lastmicros = thismicros;
    cpu_irq_leave_critical();
    return thismicros;
}

"question", which ought to stay zero, runs to about 7000 in the first 10 seconds of execution (so, not an overflow problem, either.)

The main loop is calling micros() pretty much continuously.  "asked" is about 3 million.

[ralphd]

Perhaps RTC->MODE0.COUNT.reg is not reset to zero until the ISR runs?
Try removing:

Code: [Select]

    if (RTC->MODE0.INTFLAG.bit.CMP0) {
...

and see if it makes a difference.

[westfw]

Perhaps RTC->MODE0.COUNT.reg is not reset to zero until the ISR runs?

Hmm.   Interesting idea.   I can check for count being greater than the supposed max...

Nope.  I never see COUNT exceed the compare value (999 in my case.)

If I want an interrupt every 1000 clock ticks, I wonder if I want 998, 999, or 1000.  (0..999 is 1000 ticks, but the reset to zero is supposed to happen 1 tick after the clock value...)

[Chris C]

I'm not an ARM guy, so I can't check this code fully.  But I recognize a familiar issue here:

Code: [Select]

thismicros = RTC->MODE0.COUNT.reg;
// What if the interrupt occurs RIGHT HERE, after reading 'thismicros', but before checking the interrupt flag?
if (RTC->MODE0.INTFLAG.bit.CMP0) {
Then you get both a large thismicros (probably >990), and an additional 1,000 erroneously added to it.  An error which will not recur next time, so the microsecond counter will appear to move backwards.  Doesn't matter that you're in a critical section, the interrupt flag still gets raised.

It's an easy fix:

Code: [Select]

thismicros = RTC->MODE0.COUNT.reg;
if (RTC->MODE0.INTFLAG.bit.CMP0) {
        if (thismicros > RTC->MODE0.COUNT.reg) thismicros = RTC->MODE0.COUNT.reg; // there was a rollover, and in a potentially bad spot - so get thismicros again, post-rollover
        thismicros += 1000;
}


[Brutte]

Code: [Select]

void RTC_Handler(void)
{
    millisecondcount++;
    RTC->MODE0.INTFLAG.reg = 0xFF;  // Clear all possible interrupts
}Not sure if that could be a problem but if INTFLAG holds RTC IRQ then you can clear a pending one this way. Either clear all but RTC or better add an assert at return:

Code: [Select]

assert(no pending RTC IRQs till here);

[westfw]

// What if the interrupt occurs RIGHT HERE, after reading 'thismicros', but before checking the interrupt flag?

Good thought!  But I made it like the following, and it didn't change anything :-(

Code: [Select]

thismicros = RTC->MODE0.COUNT.reg;

if (RTC->MODE0.INTFLAG.bit.CMP0) {
/*
* Check for a ms interrupt pending but not yet
* serviced, and correct for it.
*/
thismicros = RTC->MODE0.COUNT.reg;  //re-read
thismicros += 1000;
}
thismicros += 1000*millisecondcount;

if INTFLAG holds RTC IRQ then you can clear a pending one this way. Either clear all but RTC

]
This INTFLAG is RTC specific, and has several bits related to RTC.  I'm clearing ALL of  them, even though only the COMP0 interrupt is enabled.
The INT flags do not clear automatically, as far as I can tell, so you must clear them explicitly.

[Chris C]

Hmm...  Try moving the declaration for 'thismicros' out of the 'micros()' function, and into global space.  (It's a quick test for a possible overzealous compiler optimization, that would defeat your recent change.)

[c4757p]

No need to move it into global scope, that's what volatile is for - and moving it into global scope hardly guarantees that anyway...

But as long as the register itself is declared volatile, which is almost certainly is, it won't make a difference.

Also, the correct way to 'test' if a read/write is being optimized out is to read the assembly output yourself. With a GCC ARM toolchain you can view the assembly interleaved with source like this:

Code: [Select]

arm-none-eabi-objdump -S firmware.elf

Example output from main.c at a random line in a random bit of STM32 code:

Code: [Select]

  RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_HSI|RCC_OSCILLATORTYPE_HSI14;
 80001f6:       2324            movs    r3, #36 ; 0x24
 80001f8:       18fb            adds    r3, r7, r3
 80001fa:       2212            movs    r2, #18
 80001fc:       601a            str     r2, [r3, #0]

You can see the store (str) for yourself.

[Chris C]

In the case that the interrupt flag is set, the code assigns 'thismicros' a second time from the same register, without the first assigned value of 'thismicros' ever being used at all.  So as far as the compiler knows, the first assignment is completely useless.  In the interest of shortening this code path, the compiler may therefore reorganize the code as:

Code: [Select]

if (RTC->MODE0.INTFLAG.bit.CMP0) {
/*
* Check for a ms interrupt pending but not yet
* serviced, and correct for it.
*/
thismicros = RTC->MODE0.COUNT.reg;  //re-read
thismicros += 1000;
} else {
thismicros = RTC->MODE0.COUNT.reg;
}
thismicros += 1000*millisecondcount;
This ordering would reintroduce the issue.  Now that you see what I'm thinking, I think you'll agree that the register being volatile wouldn't prevent this.

But making the variable global means that it can be read at any time by an interrupt, so the compiler can no longer optimize out any assignment to it.  (Of course this is impossible since interrupts are disabled, but the compiler has no knowledge of that.)

I agree it's better to examine the assembly output to detect optimization issues, but I'm making no assumptions as to whether [westfw] is able to read that.  And since I'm not sure if this is really what's happening, I suggested the simplest and most expedient test.

[c4757p]

Simplest is to set the local variable volatile, which directly accomplishes what you want. Setting it global does not. You'd think it should - I think it should - but it doesn't.

(And if he can't look for a simple store in an assembler listing it's high time he learned how. It's particularly easy when interleaved with the C source.)

[westfw]

the code assigns 'thismicros' a second time from the same register, without the first assigned value of 'thismicros' ever being used at all.

The register itself is volatile, so it should work fine, and the assembler looks fine.

Code: [Select]

        thismicros = myRTC->MODE0.COUNT.reg;
 2cc:   6910            ldr     r0, [r2, #16]    // first load of register
        if (RTC->MODE0.INTFLAG.bit.CMP0) {
 2ce:   7a13            ldrb    r3, [r2, #8]
 2d0:   07dc            lsls    r4, r3, #31
 2d2:   d503            bpl.n   2dc <micros+0x28>
                /*
                 * Check for a ms interrupt pending but not yet
                 * serviced, and correct for it.
                 */
                thismicros = RTC->MODE0.COUNT.reg;  //re-read
 2d4:   6910            ldr     r0, [r2, #16]    // second load
                thismicros += 1000;
 2d6:   23fa            movs    r3, #250        ; 0xfa
 2d8:   009b            lsls    r3, r3, #2
 2da:   18c0            adds    r0, r0, r3
        }

[westfw]

Looks like there are some messy "clock synchronization" issues coming into play.  the way I read the docs, it can take up to 6 RTC prescaler-input clocks for synchronization to occur.  There's supposed to be "continuous sync", but I don't think I understand how that behaves :-(

I got the problem to almost "go away" by increasing the RTC input clock from 1MHz to 16MHz, and setting the RTC prescaler to 4 (divide by 16) instead of 1.
So it definitely looks related to synchronization (making it really hard to look at with a debugger, as well.  Grr.)  Any additional ideas? This isn't really a very satisfying solution; I still don't understand exactly how it's failing.  It certainly seems like the same "interrupt pending" logic should correct for synchronization delay (but - reading the count experiences the sync delay, but not reading the interrupt status?)  :-(
 
This does explain an anomaly I was seeing where the count register was "jumping" by about 5ms within the space of a couple instructions. Maybe.

Code: [Select]

thismicros = myRTC->MODE0.COUNT.reg;     // **** read approx 999
     //**** Maybe overflow happens here?
if (RTC->MODE0.INTFLAG.bit.CMP0) {
/*
  * Check for a ms interrupt pending but not yet
  * serviced, and correct for it.
  */
thismicros = RTC->MODE0.COUNT.reg;  //**** re-read approx 4
thismicros += 1000;
}(Down to about 22000 reversals in about 1e9 calls over 1800s.)

[Chris C]

It certainly seems like the same "interrupt pending" logic should correct for synchronization delay (but - reading the count experiences the sync delay, but not reading the interrupt status?)  :-(

If interrupt status is not delayed, but counter reading is, I'd try this:

Code: [Select]

thismicros = myRTC->MODE0.COUNT.reg;     // read approx 999
//**** Maybe overflow happens here?
if (RTC->MODE0.INTFLAG.bit.CMP0) {
    thismicros = myRTC->MODE0.COUNT.reg;     // still might read approx 999 due to sync?
    if (thismicros < 900) thismicros += 1000; // correct only if rollover not delayed
}

If that actually works for the reason hypothesized, be warned that if you turn the RTC input clock back down 1Mhz, an additional issue may pop up.  'RTC_Handler' might be able to enter, increment 'millisecondcount', clear the flag, and return - all before the counter appears to roll over!  That would take additional logic to fix.

Simplest is to set the local variable volatile, which directly accomplishes what you want. Setting it global does not. You'd think it should - I think it should - but it doesn't.

Ok, that's piqued my curiosity.  Setting a local variable volatile clearly prevents optimization of reads - the value is always re-read each time the variable is referenced.  But I never saw anything that suggests it would prevent optimization of writes, especially when there is no chance the written value will ever be read.  I've successfully used global variables to do this without fail.  Most frequently I use it to prevent the optimizer from removing "useless" delay loops, code meant to serve only as a breakpoint location, etc.  Next time I have need of such a thing I'll try volatile and see if it works.