Strange IRQ_CMP0 behaviour of Teensy 3.2, help needed to track down issue

wangnick

Well-known member
Dear all,

I am stumbling into an issue that I can't manage to resolve myself. It involves an ISR procedure that works only under certain circumstances.

Background of the matter is a 64x64 pixel display showing an illuminated street number of the house. This display is measuring the surrounding light to determine its brightness (off during daytime, bright at dawn and dusk, dimmed at night). The measurement is done by charging 47nF across 330R to 3.3V and then letting the cap discharge across an LPT80A and measuring the time until 0.95V are crossed.

The crossing is detected by using the CMP0 analog comparator of the T3.2. An interrupt routine is set up to be called after the crossing occurred. It saves away the bus clock tick that the comparator filled in at the time of crossing. This is the code of the ISR:
Code:
volatile uint8_t cmp_scr;
volatile uint32_t cmp_tick;
void cmp_isr (void) {
  cmp_tick = PIT_CVAL3;
  cmp_scr = CMP0_SCR; // Guaranteed not 0 (either IEF or IER are set, otherwise we would't be called)
  CMP0_SCR = 0b00000110; // rsv, !DMAEN, rsv, !IEF, !IER, CFR cleared, CFF cleared, rsv
}

However, with this code cmp_scr is never set. But if I change the code to the following:
Code:
volatile uint8_t cmp_scr;
volatile uint32_t cmp_tick;
uint8_t avoidbug = 0;
void cmp_isr (void) {
  avoidbug++; // Something needs to happen here, otherwise cmp_scr is never set. Very strange!
  cmp_tick = PIT_CVAL3;
  cmp_scr = CMP0_SCR; // Guaranteed not 0 (either IEF or IER are set, otherwise we would't be called)
  CMP0_SCR = 0b00000110; // rsv, !DMAEN, rsv, !IEF, !IER, CFR cleared, CFF cleared, rsv
}

This code works absolutely fine.

I'm totally puzzled what is going on here. I had a quick glance at the generated assembler, but in both cases the assembler code looks ok. One difference that I spot is that in the 3-line version of the ISR no push and pop of registers is necessary, but that's basically where my capabilities end for now.

I attach an example cmp0issue.ino file that reproduces the issue. I trimmed it down as much as I could. I'm using Teensyduino 1.53 on Arduino 1.8.13, which gives me gcc 5.4.1 20160919 (release) [ARM/embedded-5-branch revision 240496]. To reproduce you need a 330R, a 47nF and a 10k between P12, P11 and GND as indicated in the file header. My rig shows ~28500 busticks when it works, and 48000000 (F_BUS) when it doesn't and cmp_scr is not being set..

Any help appreciated, not only a solution but also how to proceed to home in on the issue.

Kind regards,
Sebastian
 

Attachments

  • cmp0issue.ino
    4.4 KB · Views: 30
Wild guess: Very short ISRs tend to be called twice (or even trice) if the resetting of the ISR flag didn't make it through the bus synchronization fast enough. If the ISR gets called twice, at the second call CMP0_SCR will be zero and it looks like the ISR was never called.

Try to reset the interrupt flags at the beginning of the ISR and place a
Code:
asm volatile("dsb");
at the bottom of the ISR. dsb will wait until the flags are actually reset.
 
...yep..and this is another fine example that shows that volatile not always exactly does what some here think.
 
...yep..and this is another fine example that shows that volatile not always exactly does what some here think.

FYI, I've tried as well with the variables defined without volatile (and then casting them to volatile at the place they are accessed outside the ISR context) but that did not have any effect on the issue.

Kind regards,
Sebastian
 
No that comment was not adressed to you. Your use of volatile is correct.

Have you tried Lunis´ suggestion?
 
Last edited:
Wild guess: Very short ISRs tend to be called twice (or even trice) if the resetting of the ISR flag didn't make it through the bus synchronization fast enough. If the ISR gets called twice, at the second call CMP0_SCR will be zero and it looks like the ISR was never called.

Wow. Now, where is THAT documented, I wonder? In the secret book of the Cortex order?

place a
Code:
asm volatile("dsb");
at the bottom of the ISR. dsb will wait until the flags are actually reset.

Ok, I placed
Code:
asm volatile("dsb");
right after after the
Code:
CMP0_SCR = 0b00000110;
at the end of the ISR and that did the trick! I then changed the dsb to nop and the erroneous behavior returned. Two nops were still not good enough. At three nops the issue went away as well. So indeed the issue seems to have to do with the clearing of the IE flag not yet having arrived in the CMP0_SCR memory location when the bx exception return concludes handler mode.

Thanks a lot for your help!

Kind regards,
Sebastian
 
I saw the suggestion to put:

asm volatile("dsb");

at the bottom of the ISR, is dsb case sensitive?

I've been using the following w/o volatile:

asm("DSB");

Kris
 
I understand that the volatile in asm volatile("dsb"); prevents the compiler from reordering the instruction. Would be useless if it decides to put the dsb barrier in front of the flag resetting. But I might be wrong here.

@Frank (or others): Might be something to put into the user wiki. These memory barrier / volatile issues pop up quite regularly. Any volunteers :) ?
 
the "asm volatile ()" ./ asm () is an interesting topic, too.
Often the "volatile" is just added without thinking... but it prevents optimization which can - in some cases - be useful. of course there are cases where a volatile is needed for asm.. but , not in any case.
And I think that it used "blindly" often in the auido lib, too, where it would work better without...
 
I understand that the volatile in asm volatile("dsb"); prevents the compiler from reordering the instruction. Would be useless if it decides to put the dsb barrier in front of the flag resetting. But I might be wrong here.

Yes. And the "DSB" here makes sure, that the write-pipeline gets written before the ISR is left.
 
Is `__ASM volatile ("dsb 0xF":::"memory")` equivalent to `asm volatile("dsb")`? (`__DSB()` function from teensy4/cmsis_gcc.h) If so, would uses of `__DSB()` be appropriate?
 
Is `__ASM volatile ("dsb 0xF":::"memory")` equivalent to `asm volatile("dsb")`? (`__DSB()` function from teensy4/cmsis_gcc.h) If so, would uses of `__DSB()` be appropriate?

Yes. In this case (end of isr) it's even better. ::memory tells gcc to move outstanding writes before the "dsb"
 
I noticed that the IntervalTimer.cpp implementations (teensy3 and teensy4) don't use a "dsb" instruction at the end of their interrupts after changing status. Should they be there and an issue/PR filed for adding them? Before I do that, I want to make sure it's really required. The more general question is: should ISRs always put a "dsb" at the end if system state is changed (clearing ISR or changing which ISRs are active)?
 
I noticed that the IntervalTimer.cpp implementations (teensy3 and teensy4) don't use a "dsb" instruction at the end of their interrupts after changing status. Should they be there and an issue/PR filed for adding them? Before I do that, I want to make sure it's really required. The more general question is: should ISRs always put a "dsb" at the end if system state is changed (clearing ISR or changing which ISRs are active)?

No they work.
As said, it has to do with the cpu pipelines.
 
No they work.
As said, it has to do with the cpu pipelines.

In that case, when writing an ISR that changes an IRQ config bit or clears the IRQ itself, when is “dsb” needed at the end and when isn’t it needed? I know you’re saying something about “cpu pipelines”, but how do I know whether that needs to be considered or not?

Let me rephrase: What are the necessary conditions for needing to put “dsb” at the end of an ISR? The phrase “cpu pipelines” only makes sense if one knows how that’s related to changing ISR status (clearing the IRQ or changing some IRQ-related config).

Question 2: why does the TimerInterval code work without the “dsb”?
 
Probably better if you read the manuals about the cpu core, its memory interfaces, busses and other docs.
And visit the nxp forum.
Imagine a periphal connected via a slower bus - the cpu continues working, but the write is still pending.

In short: It can happen that the isr ends when the needed write has not happened. interrupt bit is still set, cpu continues with the code.
So, a new interrupt happens...

A long enough delay would do the same as "dsb".

2) because it calls usercode after that. That's long enough.

I think that's enough here (and I don't know the exact details, too) and .. yes, read available docs.
 
I assume it works for the intervaltimer since its ISR needs to walk through all 4 channels and check the status bits. This takes probably long enough to propagate the value to the register. Of course those things also depend on the difference of the peripheral clock frequency to the frequency of the main bus. So, changing one of those might change things. If you want to be sure, check that the memory transactions are done before leaving the ISR. Bad thing is that this can take quite long.

Here an interesting thread from the ARM forum. Joseph Yiu from ARM, who certainly knows what he's talking about, proposes to simply read back the register instead of using a DSB barrier.
https://community.arm.com/support-f...-dsb-isb-on-cortex-m3-m4-m7-single-core-parts

Let me rephrase: What are the necessary conditions for needing to put “dsb” at the end of an ISR?

Here a quote from the linked thread:
>Of course I guess this is highly depended on how bad the vendor did their peripherals ...
:)
 
...and how fast or slow the used bus is :)

I remember accessing the RTC registers is really incredible slow..

Thank you for the link, luni.
 
That link helped, thanks. I think I understand better now why a very large “it depends” is the best answer. :D
 
Back
Top