AVR Interrupt Question.

Go To Last Post
18 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi all,

 

I'm working on a project, part of which requires getting the RPM of a PC fan with a tach output. The fan has an open drain output which goes low twice per revolution. And of course the pin has a pullup.

 

What I am doing is using an interrupt input (not a PCINT) detecting a falling edge of the tach. Another interrupt is setup in CTC mode and just increments a volatile uint16_t ("count"). That interrupt runs at 10000 per second (used for other stuff as well). I know that AVR interrupts are stopped while processing an interrupt, but my question is, are ALL interrupts stopped during that interval?

 

I ask because to get fan RPM, I plan to copy the count from the CTC interrupt, then zero it each time I get a fan tach interrupt. My concern is, will copying the count to another variable inside the tach ISR be ATOMIC?

 

I plan to do this:

 

ISR_fan_tach {

    busy = 1; // don't read var while it's being updated

    value = count; // get a copy of count <--IS THIS ATOMIC???

    count = 0; // reset count

    busy = 0; // now "value" can be used

}

 

Note: "count" is 16 bit, hence my concern if the read is atomic.

 

Also note that I know the value gets LARGER as the fan gets slower... I simply left out code not relevant to my question.

 

Thanks!I

 

Gentlemen may prefer Blondes, but Real Men prefer Redheads!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ATOMIC if no other interrupt occurs during the interrupt.
"Whether other interrupts occur" depends on your program and the chip used.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

With the AVR, interrupts are disabled on entry to the ISR and re-enabled on exit. Unless you specifically re-enable the interrupts in your ISR, then there is no nesting possible. Thus the access is atomic.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The read will be atomic within the ISR, but you'll need to disable interrupts in non-ISR code when reading 'count'. Your 'busy' variable will not help, since your ISR could fire just after non-ISR code has checked busy==0.

 

You haven't mentioned which AVR device you're using. Many of them have a timer which can be clocked from an external input. Rather than dealing with thousands of interrupts per second, why not attach the tach signal to the clock input pin and let the timer/counter count the pulses over a fixed period. 

 

Steve

 

Maverick Embedded Technologies Ltd. Home of wAVR and Maven.

wAVR: WiFi AVR ISP/PDI/uPDI Programmer.

Maven: WiFi ARM Cortex-M Debugger/Programmer

https://www.maverick-embedded.co...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First we need to know the AVR, many newer AVR actually have 2 level interrupts but we assume that you don't have that problem.

 

You have no problems in the interrupt then problem is in main() when you use count.

That have to be atomic otherwise you can get one byte from an old count, and the other from a new.

Also if you want a busy flag instead of atomic it should placed in main() and then the interrupt should react upon it. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

why not use the input capture feature of the timer?

this copies the timer count into the capture register each time it is triggered

as long as your tacho signal is not too fast you should have plenty of time to read this register before the next pulse (& 100khz input would give 160 cycles on a 16mhz processor)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why not just let a timer run without any interrupt & save all that interruption??...it will just count & go round & round (this prescaler can determine a good count rate)

 

During your TACH irq , you can grab the timer TCNT. Reading 2 tcnct bytes in proper order grabs TCNT atomically (assuming you using a 16bit timer).

 

Once your TACH IRQ has the current count, the tach irq just sets TCNT=0 & that begins the next round stopwatch.  Knowing how fast the count rate (prescaler) is & the grabbed count value, lets you figure out the time.

For example, if running at 16MHz & prescaler at 1024, you get 15625 counts/sec.   ...a 16 bit value has about 4.2 seconds before rolling over....plenty of time.

You can set the prescaler lower & even get a much tighter timing resolution (but then watch for irq latency jitter becoming more important).

 

This approach assumes the "other stuff" you mention might be done elsewhere, or doesn't depend on knowing a count.

 

Really, as already mentioned,  the timer irq input capture is "made for" this type of capture---why not use that?  

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Sat. Aug 8, 2020 - 04:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:

Why not just let a timer run without any interrupt & save all that interruption??...it will just count & go round & round (this prescaler can determine a good count rate)

 

During your TACH irq , you can grab the timer TCNT. Reading 2 tcnct bytes in proper order grabs TCNT atomically (assuming you using a 16bit timer).

 

Once your TACH IRQ has the current count, the tach irq just sets TCNT=0 & that begins the next round stopwatch.  Knowing how fast the count rate (prescaler) is & the grabbed count value, lets you figure out the time.

For example, if running at 16MHz & prescaler at 1024, you get 15625 counts/sec.   ...a 16 bit value has about 4.2 seconds before rolling over....plenty of time.

You can set the prescaler lower & even get a much tighter timing resolution (but then watch for irq latency jitter becoming more important).

 

This approach assumes the "other stuff" you mention might be done elsewhere, or doesn't depend on knowing a count.

 

Really, as already mentioned,  the timer irq input capture is "made for" this type of capture---why not use that?  

 

Input capture would work (and indeed be preferrable), but I already have a CTC timer running to generate other stuff, so the "count++" part works just like TCNT. May as well use it.

 

Gentlemen may prefer Blondes, but Real Men prefer Redheads!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

scdoubleu wrote:

The read will be atomic within the ISR, but you'll need to disable interrupts in non-ISR code when reading 'count'. Your 'busy' variable will not help, since your ISR could fire just after non-ISR code has checked busy==0.

 

You haven't mentioned which AVR device you're using. Many of them have a timer which can be clocked from an external input. Rather than dealing with thousands of interrupts per second, why not attach the tach signal to the clock input pin and let the timer/counter count the pulses over a fixed period. 

 

Steve

 

 

 Ah... you're right. I planned to have a simple "while (busy);" in the loop, but I see now that busy COULD go high right after it was checked. Gotta do that a different way.

 

Gentlemen may prefer Blondes, but Real Men prefer Redheads!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

With the AVR, interrupts are disabled on entry to the ISR and re-enabled on exit. Unless you specifically re-enable the interrupts in your ISR, then there is no nesting possible. Thus the access is atomic.

 

Thanks.

 

Gentlemen may prefer Blondes, but Real Men prefer Redheads!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In main line:

SREG &= ~_BV(SREG_I);  // not cli()
dst=src;
SREG |=  _BV(SREG_I);  // not sei()

If dst is volatile and in standard RAM, the store is atomic.

If src is volatile and in standard RAM, the fetch is atomic.

cli() and sei() expand to inline assembly that does not explicitly affect any volatile. variables.

There is also an ATOMIC_BLOCK macro about whose correctness and utility I am uncertain.

Iluvatar is the better part of Valar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would not trust that code. There is a good change (risk) that the compiler would change the order of lines (It see no correlation between 2. line and the other 2 lines).

 

For ATOMIC_BLOCK take a look here:

https://www.nongnu.org/avr-libc/...

 

 

Add: 

Also about the code, if interrupt is disabled it will enable it. (atomic will restore to what it was (as default at least see the link) )

 

 

 

Last Edited: Sun. Aug 9, 2020 - 09:35 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sparrow2 wrote:
I would not trust that code. There is a good change (risk) that the compiler would change the order of lines
Why? Volatile memory accesses are supposed to occur in the order given.
Quote:
(It see no correlation between 2. line and the other 2 lines).
Not at all sure what you are getting at.

There should be at least three volatile accesses.  They are required to occur in order.

Under what circumstances would a non-volatile variable require atomic access?

Quote:
For ATOMIC_BLOCK take a look here:

https://www.nongnu.org/avr-libc/...

For most cases, I expect ATOMIC_BLOCK is equivalent to cli(); stuff; sei();

When it works it is because it is not needed or because the

obfuscation made the situation more complicated for the compiler.

Before the memory barrier was added to cli() and sei(),

I expect ATOMIC_BLOCK worked, assuming it did, by confusing the optimizer.

Now it works because of the memory barriers in cli() and sei().

Now ATOMIC_BLOCK might be useful as an orginizational tool when a critical section has multiple exits.

Quote:
Add: 

Also about the code, if interrupt is disabled it will enable it. (atomic will restore to what it was (as default at least see the link) )

Hence the "In main line" qualification.

Usually one will know whether interrupts are disabled in the surrounding code.

If they are, atomic code is unnecessary.

If not, restoring interrupts is the way to go.

ATOMIC_BLOCK's ATOMIC_RESTORESTATE is rarely useful.

 

Note that if src is non-volatile, its fetch could be moved to before the first line.

If dst is non-volatile, its assignment could be moved to after the last line.

Both of these are probably good: they reduce the amount of time spent with interrupts disabled.

If neither is volatile, 'tis unlikely you actually need an atomic access.

Iluvatar is the better part of Valar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

We have been there before and perhaps it was a GCC error!

 

https://www.avrfreaks.net/forum/...

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sparrow2 wrote:
We have been there before and perhaps it was a GCC error!

 

https://www.avrfreaks.net/forum/...

What got neglected in that thread is that the volatiles in "asm volatile" and "volatile int" do rather different things.

Other than the memory barrier, there is not much one can do to prevent the reordering of things,

volatile or otherwise, with basic inline assembly.

ATOMIC_BLOCK includes it from before cli() and sei() did.

 

One potential problem with ATOMIC_RESTORESTATE is that it writes old data to SREG.

My understanding is that the compiler does not know that SREG is the repository of the status flags.

Hence it might rely on the N flag not being changed by "SREG=old_sreg".

So far as I am aware the compiler currently does nothing interesting with comparison flags:

A branch that depends on the N flag would occur immediately after the operation that generated it.

Iluvatar is the better part of Valar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

By using ASM I avoid a problem like this by placing the variable in (low) registers and use the MOVX instruction.(by nature is atomic for 16bit )

And as I remember the CV C compiler make code that also will work for this.  

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

using ASM I avoid a problem like this by placing the variable in (low) registers and use the MOVX instruction.(by nature is atomic for 16bit )

Occasionally asm will be helpful, though there is usually a way to "convince" the compiler to see things your way.  I remember dancing with it to quit placing fixed text strings into ram, once that was proper, I loathed the crum--bum string compare that was generated (I think it eventually got greatly improved)....it would go through a huge look up gyration, then repeat it all a little while later, when it could have made partial use of the first lookup setup--apparently it just didn't "see it".

 

 

 

 

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've never felt the need to reserve registers globally.

Doing it for atomicity, MOVW, presents the same problems as doing ot for speed:

Other code, including library code, needs to be compiled with the same reservations.

Iluvatar is the better part of Valar.