Re: [avr-gcc-list] generic queue library for AVR GCC?

[E. Weddington]

Bruce D. Lightner wrote:

> Geoffrey Wossum wrote:
>
> > On Thursday 11 November 2004 1:32 pm, gouy yann wrote:
> >
> > > here is my implementation of a fifo.
> > > I hope this will help you.
> >
> >
> > I took a quick look at these routines.  I noticed that they lack
> > any type of "critical section" around where the queue data and
> > pointers are modified.  Calling these routines as-is from an ISR
> > context and from a non-ISR context might result in race conditions
> > and odd, intermittent behavior.
> >
> > Here's my critical section routines.  The enter just pushes the
> > global interrupt state and then disables interrupts, the exit
> > pops the interrupt state.  Observant list readers might note an
> > uncanny resemblance to NutOS's critical section code...
> >
> > extern inline void utos_enter_cs(void)
> > {
> >     asm volatile(
> >         "in  __tmp_reg__, __SREG__" "\n\t"
> >         "push __tmp_reg__"          "\n\t"
> >         "cli"                       "\n\t"
> >     );
> > }
> >
> > extern inline void utos_exit_cs(void)
> > {
> >     asm volatile(
> >         "pop __tmp_reg__"           "\n\t"
> >         "out __SREG__, __tmp_reg__" "\n\t"
> >     );
> > }
> >
> > Since these routines push and pop from the stack, you have to be
> > careful to balance them within a function.  I'll leave where to
> > insert them into the code as an exercise to the reader ;)
>
>
> Below is what I use.  A bit more more efficient in most cases
> and does not require matching push/pop. Your way makes me a bit
> nervous in C, although, given how "avr-gcc" stack frames work
> today, your approach should be OK.
>
> Here are the AVR assembly macros that I use...
>
>     #define begin_critical_section() ({         \
>         uint8_t t;                              \
>         asm volatile (                          \
>                 "in %0, __SREG__" "\n\t"        \
>                 "cli"                           \
>                 : "=r" (t)                      \
>         );                                      \
>         t;                                      \
>      })
>
>     #define end_critical_section(val)           \
>         asm volatile (                          \
>                 "out __SREG__,%0"               \
>                 : /* no outputs */              \
>                 : "r" ((uchar)(val))            \
>         )
>
> Here's example usage...
>
>     {
>         unsigned char sreg = begin_critical_section();
>         sm_qin = sm_qout = 0;   // reset queue pointers
>         end_critical_section(sreg);
>     }
>
> My way also preserves the original state of the interrupt flag,
> which may be essential, depending upon the circumstance.

I agree with Bruce's method; it's better in that it preserves the 
original state of the interrupt flag. The push/pop is not required, and 
probably wasteful.

Though you don't have to do it in inline assembly. It can be done all in C:

#include <avr/io.h>
#include <avr/interrupt.h>
#include <inttypes.h>
....
{
   uint8_t sreg = SREG;
   cli();
   sm_qin = sm_qout = 0;  // reset queue pointers.
   SREG = sreg;  
}