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

[Theodore A. Roth]

On Mon, 15 Nov 2004, 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.

It's not obvious to me that Geoffrey's method does not preserve the
state of the I flag. In both your code and his the SREG ends up on the
stack during a context switch or interrupt (implicitly with your code
and explicitly with Geoffrey's).

Can someone please explain how the state the I flag is not preserved?

---
Ted Roth
PGP Key ID: 0x18F846E9
Jabber ID: address@hidden