[Second solution: disabling interrupts] Re: Help with interrupt software routine and non-atomic operations

I admit that following code instruction by instruction for checking if 
an interrupt trigger could bring to problems is tricky. The use of a 
double buffer (next1_s and next2_s) is obscure at first.

Maybe it could be simpler to disable and enable interrupts, without 
worrying about next pending audio stream.

--- audio.c ---
typedef struct {
   const uint16_t *samples;
   size_t size;
   uint16_t *s;
} AudioStream;

static AudioStream curr_s;      // current stream in playing
static AudioStream *curr;       // used mainly as atomic flags

void Timer_ISR(void) {
   uint16_t sample;
   bool dac_refresh = false;
   if (curr) {        // we are playing a stream
     if (curr->s == NULL) {
       curr->s = curr->samples;
     }
     sample = *(curr->s);
     dac_refresh = true;
     if (++curr->s - curr->samples == curr->size) {
       curr = NULL;   // last sample processed
     }
   }
   if (dac_refresh) DAC_VAL = sample;
}

void play(const uint16_t *samples, size_t size)
{
   disableInterrupts();  // start of critical section
   curr_s.samples = samples;
   curr_s.size = size;
   curr_s.s = NULL;
   curr = &curr_s;
   enableInterrupts();   // end of critical section
}
--- audio.c ---

In this solution, I don't need the init member in AudioStream structure: 
I use s member to understand if it is initialized or not.

Here I don't need volatile for any variables managed in play(): after 
entering critical section, all the variables can't be changed 
asyncronously (interrupts are disabled), so their access can be normally 
optimized.
[It is possible to disable only the Timer interrupt and not all the 
interrupts.]

The only problem is to avoid optimizer to move instructions from inside 
the critical section to outside (before disableInterrupts() or after 
enableInterrupts()).
This is one thing I never taked care until David explanation.

One solution to avoid reordering is to embed a memory barrier in 
disableInterrupts() and enableInterrupts(). The memory barrier 
guarantees that the memory accesses before and after it (inside critical 
section in our case) aren't optimized (reordered), even if curr_s and 
curr aren't volatile (David, is this right?).


If the code is for Atmel AVR architecture, I think it is better to use 
macros from <util/atomic.h>:

#include <util/atomic.h>
...
void play(const uint16_t *samples, size_t size)
{
   ATOMIC_BLOC(ATOMIC_FORCEON) {
     curr_s.samples = samples;
     curr_s.size = size;
     curr_s.s = NULL;
     curr = &curr_s;
   }
}

The macro ATOMIC_BLOCK automatically disable interrupts at the beginning 
(using a memory barrier) and enable interrupt at the end (using again a 
memory barrier).

Incidentally, I noticed a strange thing in <util/atomic.h> and 
<interrupt.h>:

--- <interrupt.h> ---
...
#define sei()  __asm__ __volatile__ ("sei" ::: "memory")
...
--- <interrupt.h> ---

--- <util/atomic.h> ---
...
static __inline__ void __iSeiParam(const uint8_t *__s)
{
   sei();
   __asm__ volatile ("" ::: "memory");
   (void)__s;
}
...
--- <util/atomic.h> ---

Why adding a memory barrier after sei() in __iSeiParam() if sei() macro 
alreay has a memory barrier?

Anyway, reading <util/atomic.h> is very instructive, because many tricks 
of C language and gcc compiler are used there (for example, cleanup 
attribute of a variable).


If the code is for Cortex-Mx architectures, macros proposed by Tim or 
David can be used. I don't know if it is necessary to add DSB and ISB hw 
memory barriers after "cpsid i", to be sure pending interrupts won't 
really trigger after disabling interrupts instruction.

I'm sorry, this post should be as an answer to another thread.