EmbeddedRelated.com
Forums
Memfault Beyond the Launch

writing ISR for UART

Started by sohagiut February 7, 2008
hy, i am not very expert in C. Can any one give me a sample example on ISR
for UART.i am using MPLAB with c18. how to write IRQ in ISR. also how can
i add "case" in side ISR? can any  one give me any tips or any link.


thanks in advance  
"sohagiut" <sohagiut@yahoo.com> wrote in message 
news:7vednWJSV7sVfjfanZ2dnUVZ_r2nnZ2d@giganews.com...
> hy, i am not very expert in C. Can any one give me a sample example on ISR > for UART.i am using MPLAB with c18. how to write IRQ in ISR. also how can > i add "case" in side ISR? can any one give me any tips or any link.
Generally speaking, most compilers have an _interrupt_ keyword or similar that tell the compiler that it must restore the status register and perhaps other registers when returning from an interrupt. "case" is safe inside an ISR. Most of the classic families of bugs with ISRs have to do with shared RAM or shared memory. This falls under the category of IPC problems in computer science, i.e.: http://en.wikipedia.org/wiki/Inter-process_communication I wish I had the time to enumerate everything you can do wrong with an ISR (regrettably, I have a day job and my fingers are old), but the most common error is to fail to realize that the interrupt may occur at any time. You need to have a critical section protocol to prevent interrupts when modifying data structures or hardware that are shared. The most common technique is to disable interrupts for a short period of time. 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); } Other links that may be helpful. http://en.wikipedia.org/wiki/Critical_section http://www.isd.mel.nist.gov/projects/rtlinux/rtutorial-2.0/doc/ex05_isr.htm http://www.netrino.com/Embedded-Systems/How-To/Interrupt-Service-Routines There are many ways to hang yourself. I'm too lazy to type much more. -- David T. Ashley (dta@e3ft.com) http://www.e3ft.com (Consulting Home Page) http://www.dtashley.com (Personal Home Page) http://gpl.e3ft.com (GPL Publications and Projects)
dear David,
thanks for a nice reply. actually in my case problem is that i am not
expert on C coding. and my idea about my work is segregated. i can't
compile all due to lack of expertise on coding. i become confused how to
complete. Let me tell you about my intension, it's like following:

i want to execute individual primitives of Zigbee protocol. let's take two
primitives like MLME-ASSOCIATE.request and MLME-ASSOCIATE.indication.i'll
use them as two different "case". Actually my main intention is to measure
the power consumption for each primitives of PHY,MAC,NWK and APL. 

I am thinking to do it like this.as i don't find any complete Zigbee
device i choose CC2420 Zigbee node connected with PICDEM Z board
containing a micro-controller PIC18f4620. usually i use the MPLAB IDE to
run the project and MPLAB ICD 2 to debug the program that contains
complete Zigbee stack command. instead of running the complete stack my
intension is to run the individual command and to observe the power change
in the CC2420. to avoide the multiple debugging my plan is like this: i'll
add all the command in the LabVIEW simulator and the each command will
written as a 'case' in C.by using the MPLAB and ICD 2 the program will be
debugged in the PIC18f4620.this program should contain an ISR so that once
i debug the program i just select the command by clicking a button in the
LabVIEW simulator connected to the PICDEM Z by a serial port (UART).
according to the command it will directed to a interrupt table that
trigger an ISR. and accordingly the case will be selected and the register
value of the CC2420 will change. then i'll record the power change.


With a help from a colleague i sketch my program could be like this :


The interrupt service routine for the UART is something like the
following:

at the beginning of main
- open the serial setting the right baud rate 
- enable IRQ

---------------------------

:::few coding line though i am not sure whether it works:::::


#include <p18cxxx.h>
#include <usart.h>

void main (void)
{

// Configure all PORTB pins for output 

TRISB = 0;

/ * 
open the serial setting the right baud rate

Open the USART configured as
 8N1, 2400 baud, in polled mode

*/

	 OpenUSART (USART_TX_INT_OFF &
	 USART_RX_INT_ON &
	 USART_ASYNCH_MODE &
	 USART_EIGHT_BIT &
	 USART_CONT_RX &
	 USART_BRGH_HIGH, 103);


	TXSTA = 0x20;   	// setup USART transmit
	RCSTA = 0x90; 		// setup USART receive
  
 
   // Use high priority interrupt
   IPR1bits.TXIP = 1;

 

""Here i have to  enable IRQ"""'


}


"""now i have to write the ISR. i got an example like following""" but
don't know how to write my own program.::::




/*********************************************************************
* Function:        void UART_ISR(void)
*
* PreCondition:    UART interrupt has occured
*
* Input:           None
*
* Output:          None
*
* Side Effects:    None
*
* Overview:        None
*
* Note:            This function is supposed to be called in the ISR
*                  context.
********************************************************************/

void UART_ISR(void)
{
   // NOTE: All local variables used here should be declared static
   
	static BYTE rxdata;

   // Store a received byte, if pending, if possible
   
	if(PIR1bits.RCIF)
   {
       // Get the byte
       rxdata = RCREG; 

       // Clear the interrupt flag so we don't keep entering this ISR
       PIR1bits.RCIF = 0;


//       YOU CAN USE THE rxdata HERE
//        OR PUT IN A CIRCULAR BUFFER TO BE USED IN THE WHILE(1) IN THE
MAIN
// 	IN THIS EXAMPLE
//      Copy the byte into the local FIFO, if it won't cause an overflow
       

if(RXHeadPtr != RXTailPtr - 1)
       {
           if((RXHeadPtr != vUARTRXFIFO + sizeof(vUARTRXFIFO)) ||
(RXTailPtr != vUARTRXFIFO))
           {
               *RXHeadPtr++ = i;
               if(RXHeadPtr >= vUARTRXFIFO + sizeof(vUARTRXFIFO))
                   RXHeadPtr = vUARTRXFIFO;
           }
       }
   }

#IF 0 		//TX CODE CAN BE SKIPPED

   
		// Transmit a byte, if pending, if possible
   
if(PIR1bits.TXIF)
   {
       if(TXHeadPtr != TXTailPtr)
       {
           TXREG = *TXTailPtr++;
           if(TXTailPtr >= vUARTTXFIFO + sizeof(vUARTTXFIFO))
               TXTailPtr = vUARTTXFIFO;
       }
       else    // Disable the TX interrupt if we are done so that we don't
keep entering this ISR
       {
           PIE1bits.TXIE = 0;
       }
   }

#ENDIF

}




""""Moreover, in the main application, i need to declare the needed
variables:""""""""




#define BAUD_RATE            19200

// Ring buffers for transfering data to and from the UART ISR:
//  - (Head pointer == Tail pointer) is defined as an empty FIFO
//  - (Head pointer == Tail pointer - 1), accounting for wraparound,
//    is defined as a completely full FIFO.  As a result, the max data
//    in a FIFO is the buffer size - 1.
static BYTE vUARTRXFIFO[65];
static BYTE vUARTTXFIFO[17];
static BYTE *RXHeadPtr = vUARTRXFIFO, *RXTailPtr = vUARTRXFIFO;
static BYTE *TXHeadPtr = vUARTTXFIFO, *TXTailPtr = vUARTTXFIFO;

[...]


now the point is how i can make the program complete. could you please
help me out. and another important question how to add my "case" in the
program.


thanks to read the long writing.

thanks in advance and waiting for a kind guidance.
In article <MN2dnVFbIrGdszbanZ2dnUVZ_uidnZ2d@giganews.com>, dta@e3ft.com 
says...
> "sohagiut" <sohagiut@yahoo.com> wrote in message > news:7vednWJSV7sVfjfanZ2dnUVZ_r2nnZ2d@giganews.com... > > hy, i am not very expert in C. Can any one give me a sample example on ISR > > for UART.i am using MPLAB with c18. how to write IRQ in ISR. also how can > > i add "case" in side ISR? can any one give me any tips or any link. > > Generally speaking, most compilers have an _interrupt_ keyword or similar > that tell the compiler that it must restore the status register and perhaps > other registers when returning from an interrupt. > > "case" is safe inside an ISR. > > Most of the classic families of bugs with ISRs have to do with shared RAM or > shared memory. This falls under the category of IPC problems in computer > science, i.e.: > > http://en.wikipedia.org/wiki/Inter-process_communication > > I wish I had the time to enumerate everything you can do wrong with an ISR > (regrettably, I have a day job and my fingers are old), but the most common > error is to fail to realize that the interrupt may occur at any time. You > need to have a critical section protocol to prevent interrupts when > modifying data structures or hardware that are shared. The most common > technique is to disable interrupts for a short period of time. > > 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. 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" <mborgerson@comcast.net> wrote in message 
news:MPG.22154ead1195c0b0989797@newsgroups.comcast.net...
> In article <MN2dnVFbIrGdszbanZ2dnUVZ_uidnZ2d@giganews.com>, dta@e3ft.com > says... >> "sohagiut" <sohagiut@yahoo.com> wrote in message >> news:7vednWJSV7sVfjfanZ2dnUVZ_r2nnZ2d@giganews.com... >> > hy, i am not very expert in C. Can any one give me a sample example on >> > ISR >> > for UART.i am using MPLAB with c18. how to write IRQ in ISR. also how >> > can >> > i add "case" in side ISR? can any one give me any tips or any link. >> >> Generally speaking, most compilers have an _interrupt_ keyword or similar >> that tell the compiler that it must restore the status register and >> perhaps >> other registers when returning from an interrupt. >> >> "case" is safe inside an ISR. >> >> Most of the classic families of bugs with ISRs have to do with shared RAM >> or >> shared memory. This falls under the category of IPC problems in computer >> science, i.e.: >> >> http://en.wikipedia.org/wiki/Inter-process_communication >> >> I wish I had the time to enumerate everything you can do wrong with an >> ISR >> (regrettably, I have a day job and my fingers are old), but the most >> common >> error is to fail to realize that the interrupt may occur at any time. >> You >> need to have a critical section protocol to prevent interrupts when >> modifying data structures or hardware that are shared. The most common >> technique is to disable interrupts for a short period of time. >> >> 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--;
**** INTERRUPT HAPPENS HERE ****
>> 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.
First of all, I just wanted to say _something_ to the OP ... I really can't generate high-quality examples when I don't have a lot of time to throw at it. That being said, are you __SURE__ that is the only problem? How about this scenario: a)The queue is full. b)The "get" code executes to the point marked above as "INTERRUPT HAPPENS HERE". c)The interrupt runs and overwrites the character that was to be retrieved. The issue is that the count has been decremented before the character from the queue body has been copied out.
> 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.
Perhaps. But it also points out that human beings aren't made to produce software and that any of us can screw up a trivial example. It might also point out the fact that I'm never wrong (*). -- David T. Ashley (dta@e3ft.com) http://www.e3ft.com (Consulting Home Page) http://www.dtashley.com (Personal Home Page) http://gpl.e3ft.com (GPL Publications and Projects) (*) In my own mind.
In article <tpudnZrLEfg_VzbanZ2dnUVZ_hSdnZ2d@giganews.com>, dta@e3ft.com 
says...
> "Mark Borgerson" <mborgerson@comcast.net> wrote in message > news:MPG.22154ead1195c0b0989797@newsgroups.comcast.net... > > In article <MN2dnVFbIrGdszbanZ2dnUVZ_uidnZ2d@giganews.com>, dta@e3ft.com > > says... > >> "sohagiut" <sohagiut@yahoo.com> wrote in message > >> news:7vednWJSV7sVfjfanZ2dnUVZ_r2nnZ2d@giganews.com... > >> > hy, i am not very expert in C. Can any one give me a sample example on > >> > ISR > >> > for UART.i am using MPLAB with c18. how to write IRQ in ISR. also how > >> > can > >> > i add "case" in side ISR? can any one give me any tips or any link. > >> > >> Generally speaking, most compilers have an _interrupt_ keyword or similar > >> that tell the compiler that it must restore the status register and > >> perhaps > >> other registers when returning from an interrupt. > >> > >> "case" is safe inside an ISR. > >> > >> Most of the classic families of bugs with ISRs have to do with shared RAM > >> or > >> shared memory. This falls under the category of IPC problems in computer > >> science, i.e.: > >> > >> http://en.wikipedia.org/wiki/Inter-process_communication > >> > >> I wish I had the time to enumerate everything you can do wrong with an > >> ISR > >> (regrettably, I have a day job and my fingers are old), but the most > >> common > >> error is to fail to realize that the interrupt may occur at any time. > >> You > >> need to have a critical section protocol to prevent interrupts when > >> modifying data structures or hardware that are shared. The most common > >> technique is to disable interrupts for a short period of time. > >> > >> 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--; > **** INTERRUPT HAPPENS HERE **** > >> 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. > > First of all, I just wanted to say _something_ to the OP ... I really can't > generate high-quality examples when I don't have a lot of time to throw at > it. > > That being said, are you __SURE__ that is the only problem? > > How about this scenario: > > a)The queue is full. > > b)The "get" code executes to the point marked above as "INTERRUPT HAPPENS > HERE". > > c)The interrupt runs and overwrites the character that was to be retrieved. > The issue is that the count has been decremented before the character from > the queue body has been copied out.
Good point. I looked at my code and I find that I don't decrement the queue length until AFTER I fetch the character. That's why I said your example looked a lot like my code---but I didn't say it was identical. No matter how you write the code, you are going to have problems with data integrity if the queue becomes full. No amount of interrupt masking will save you from that problem!
> > > 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. > > Perhaps. But it also points out that human beings aren't made to produce > software and that any of us can screw up a trivial example.
Which is why it's always good to test your code with extreme cases before you release it to your customers. That won't eliminate all problems, but it can reduce their frequency.
> > It might also point out the fact that I'm never wrong (*). > >
The only programmers I know that have never produced a bug are those that haven't finished their first real application! It's much easier to never be wrong than it is to always be right. You can achieve the former by doing nothing! Thanks for the examples. Just because nobody has complained about my ISRs and queue handlers in the last 10 years doesn't mean the code is perfect. It's always a good idea to reexamine old code before you decide you can simply copy and paste the code into a new compiler with a different processor. Mark Borgerson
Dear Mark Borgerson,
can i have a look your program to test. it might give me some idea for my
code.

thanks

Reza

Mark Borgerson wrote:
> In article <MN2dnVFbIrGdszbanZ2dnUVZ_uidnZ2d@giganews.com>, dta@e3ft.com > says... >> "sohagiut" <sohagiut@yahoo.com> 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 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>>
Dear Mark Borgerson,
can i have a look your program to test. it might give me some idea for my
code.

thanks

Reza

In article <47ac2c51$0$14988$8404b019@news.wineasy.se>, 
david@westcontrol.removethisbit.com says...
> Mark Borgerson wrote: > > In article <MN2dnVFbIrGdszbanZ2dnUVZ_uidnZ2d@giganews.com>, dta@e3ft.com > > says... > >> "sohagiut" <sohagiut@yahoo.com> 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

Memfault Beyond the Launch