Re: writing ISR for UART

In article <47ac2c51$0$14988$8404b019@xxxxxxxxxxxxxxx>,
david@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx says...
Mark Borgerson wrote:
In article <MN2dnVFbIrGdszbanZ2dnUVZ_uidnZ2d@xxxxxxxxxxxx>, dta@xxxxxxxx
says...
"sohagiut" <sohagiut@xxxxxxxxx> wrote in message
<snip>
For example, in the code below, if the interrupt occurs between
approximately points /* 1 */ and /* 2 */ there may be serious logical
trouble. I'll leave it to you to figure out why.

struct
{
int putpoint;
int getpoint;
int nchars;
unsigned char body[QSIZE];
} queue;

_interrupt_ void rx_isr(void)
{
unsigned char c;

/* Get c from hardware */

if (queue.nchars < QSIZE)
{
queue.body[queue.putpoint] = c;
queue.nchars++;
queue.putpoint++;
if (queue.putpoint >= QSIZE)
queue.putpoint = 0;
}
}


unsigned char get_a_char_from_queue(void)
{
unsigned char return_value = 0;

/* 1 */
if (queue.nchars)
{
queue.nchars--;
return_value = queue.body[queue.getpoint];
queue.getpoint++;
if (queue.getpoint >= QSIZE)
queue.getpoint = 0;
}
/* 2 */

return(return_value);
}

This looks a lot like code I've used successfully for more than
a decade. Is there really a problem if q.nchars is accessed
atomically and the q.nchars-- operation occurs in a single
uninterruptible instruction? On the 68K machine where i
run the code,
q.nchars--; becomes subq.w #1, 4(A1)


so if the interrupt handler adds a character to the queue while
the get_a_char_from_queue() function is executing there
should be no problems. q.nchars is the only variable that
is modified by both the interrupt handler and by the
getchar routine.

I forsee a problem if queue.nchars was a 16-bit or
32-bit integer and the code was running on an 8-bit
processor. nchars could get messed up if an interrupt
occured between the mulitple instructions needed to
decrement the value.


The reason that your code (with the modifications you mentioned in
another post, and a little care to avoid buffer overflows) is safe on
the m68k is that the m68k has an atomic decrement instruction. For many
embedded processors, that is not the case. First, it could be that the
nchars variable is wider than the processor (16-bit int on an 8-bit
processor, for example). This is generally due to the programmer not
understanding their target - it's rare that you would need more than 256
bytes of buffer on an 8-bit micro, so the programmer has picked the
wrong types when they used "int". Secondly, many modern processors are
RISC load-store architectures, so that the decrement is done in three
operations (load the old value, decrement it, store it again) and the
interrupt can break into any of these.

If you *don't* have access to such an atomic decrement, you have two
options - disable interrupts around the critical code to make the
operation atomic, or use a better buffer structure! (Hint - do you
really need nchars?)


I've seen and used queue code that compares the get and put pointers to
detect whether a character is available. I avoided it for a couple of
reasons:

* the pointer comparisons are not atomic on the 68K although that may
not matter when checking for getptr != putptr
* there are times when it is handy to have a count of the characters
in the buffer. With that count you can implement a faster loop to
pull characters from the queue. It's easier to increment and
decrement a counter at one instruction each, than it is to get
the queue length by pointer arithmetic that accounts for queue
wraparound.

I do use that technique in my serial handler for the AT91SAM7
processor. There, I use the DMA support in the Atmel chips,
so there is not an interrupt for each character or FIFO.
That leaves me with comparing the getptr with the DMA
receive pointer to detect incoming characters.

One of the first uses for my serial I/O code on a new system
is always as part of the monitor used to upload new code. If
you start getting a lot of checksum errors when uploading
new code, it's a good indication that you've missed something
in the serial I/O driver.


The simple example code here also lacks a few elements:
1: there is no error reporting for queue full errors
or queue empty errors. Without those error reports,
the program can't distinguish between a valid 0x00
character and the result of a get operation on an
empty queue.
2. There should be a ChAvailable() function that returns
the number of characters available. This prevents either
blocking on an empty queue or the queue empty 0x00
character problem.
3. Many UARTS have just one interrupt, which has to handle
receive, transmit and error interrupts, so the
ISR will be more complex than that shown. If the UART
has a FIFO, it gets to be even more fun!






I think this discussion points out one of the reasons that
it is a really good idea to understand the processor and
the assembly language if you are going to write interrupt
handlers.


<<SNIP>>



Mark Borgerson

.

Relevant Pages

  • Re: writing ISR for UART
    ... so if the interrupt handler adds a character to the queue while ... and a little care to avoid buffer overflows) is safe on ... detect whether a character is available. ...
    (comp.arch.embedded)
  • Re: writing ISR for UART
    ... so if the interrupt handler adds a character to the queue while ... nchars could get messed up if an interrupt ... I've seen and used queue code that compares the get and put pointers to detect whether a character is available. ...
    (comp.arch.embedded)
  • Re: writing ISR for UART
    ... i add "case" in side ISR? ... most compilers have an _interrupt_ keyword or similar ... so if the interrupt handler adds a character to the queue while ... c)The interrupt runs and overwrites the character that was to be retrieved. ...
    (comp.arch.embedded)
  • Re: writing ISR for UART
    ... i add "case" in side ISR? ... most compilers have an _interrupt_ keyword or similar ... so if the interrupt handler adds a character to the queue while ... c)The interrupt runs and overwrites the character that was to be retrieved. ...
    (comp.arch.embedded)
  • Re: Lahman, how ya doing?
    ... Object* recipient; int event_id; ... Timer ... Not relevant to my simulation, but generalizing to a case where any amount of time can pass between when an event is pushed on to the queue and when it is popped off and processed, it seems you can have two identical events going to the same object. ... given tick. ...
    (comp.object)