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

#include ... 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 and :

--- --- ... #define sei() __asm__ __volatile__ ("sei" ::: "memory") ...

--- ---

--- --- ... static __inline__ void __iSeiParam(const uint8_t *__s) { sei(); __asm__ volatile ("" ::: "memory"); (void)__s; } ...

--- ---

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

Anyway, reading 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.

Reply to
Loading thread data ...

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

Reply to

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.