Multiple UARTs and multiple interrupts

- R
- Ross Marchant
  
  Contact options for registered users
posted
19 years ago

Thu, Jul 1, 2004 12:32 AM

Hi All

I want some feedback as to whether what I am doing is correct. I am using a DS80C320 with a Phillips SC28L198 8 channel UART. 2 interrupts are triggered by the internal UARTS, 1 by the phillips chip and another by timer 0. I have found that for reliable operation i must turn all interrupts off when servicing the internal uart interrupts, and when putting a character into the ring buffer for transmission. Can anyone see any problem with this? Does anyone with a similar setup do it differently? Do I have to disable all interrupts, or only a few?Below is two snippits of code as an example: ................. // // uputch - // // Puts a character out of a serial port // void uputch(unsigned char chan,unsigned char data) { unsigned char oldie;

oldie = IE; IE = 0; wdog(); switch(chan) { //Internal UARTS case INTUART_1: tx_buff[chan][tx_inptr[chan]] = data; tx_inptr[chan]++; tx_inptr[chan] &= TXBUF_SIZE; if (sending_flg[1] == 0) { sending_flg[1] = 1; TI_1 = 1; } break; case INTUART_0: tx_buff[chan][tx_inptr[chan]] = data; tx_inptr[chan]++; tx_inptr[chan] &= TXBUF_SIZE;

if (sending_flg[0] == 0) { sending_flg[0] = 1; TI_0 = 1; } break;

//Philips UARTS case EXTUART_7: case EXTUART_6: case EXTUART_5: case EXTUART_4: case EXTUART_3: case EXTUART_2: case EXTUART_1: case EXTUART_0: tx_buff[chan][tx_inptr[chan]] = data; tx_inptr[chan]++; tx_inptr[chan] &= TXBUF_SIZE; //Enable TX FIFO empty interrupt *IMR_WR_PTR[chan] = 0x03; break; } IE = oldie; }

...........

// // uart1_isr - // // Interrupt routine for internal UART 1 // interrupt void uart1_isr(void) { unsigned char oldie;

oldie = IE; IE = 0;

if (RI_1) { RI_1 = 0; rx_buff[INTUART_1][rx_inptr[INTUART_1]] = SBUF1; rx_inptr[INTUART_1]++; rx_inptr[INTUART_1] &= RXBUF_SIZE; } if (TI_1) { TI_1 = 0; if (!isempty_tx(INTUART_1)) { SBUF1 = tx_buff[INTUART_1][tx_outptr[INTUART_1]]; tx_outptr[INTUART_1]++; tx_outptr[INTUART_1] &= TXBUF_SIZE; } else { sending_flg[1] = 0; } } IE = oldie; } ...........

tia Ross

- R
- Rick Merrill
  
  Contact options for registered users
Vote on answer
posted
19 years ago

Thu, Jul 1, 2004 2:06 PM

That is required to synchronize the interupts and the buffering. (If you didn't do it, you would get random errors!)

Just make sure (and document) that TXBUF_SIZE is 2^x -1 .

- RM

- T
- Thad Smith
  
  Contact options for registered users
Vote on answer
posted
19 years ago

Thu, Jul 1, 2004 5:05 PM

With the code you presented, the reason you need to disable interrupts is to maintain a consistent state of tx_inptr when the associated interrupt occurs. Only the UART interrupts should need to be disabled. The minimal amount of code that needs to be protected is

As written, if an interrupt occurs between the two statements, tx_inptr[chan] has an incorrect value, which could cause isempty_tx[chan] in the interrupt routine to incorrectly return false.

What happens if you rewrite as tx_inptr[chan] = (tx_inptr[chan]+1) & TXBUF_SIZE;

Assuming that tx_inptr is an unsigned char and that byte accesses are separate and atomic (almost certainly the case for your processor), I think you can leave interrupts enabled. What would happen is that an interrupt would not see the new character until tx_inptr[chan] is updated. Technically, this is not guaranteed by C unless tx_inptr[] is of type volatile sig_atomic_t.

What happens if the interrupt occurs between the pointer update and testing sending_flg? The byte gets written. If the following completion interrupt occurs before uputch resumes (possible with multiple interrupts), sending_flg could be set to 0, then uputch resumes and sets it to 1 and restarts interrupts. The interrupt routine will then see that there are no new characters and again reset sending_flg. This shouldn't be a problem.

You didn't provide any protection against buffer overrun. What that means is that if you buffer more than TXBUF_SIZE-1 characters, you will probably get a false indication of an empty buffer, assuming that is isempty_tx compares the two pointers for equality.

I hesitate to mention this, but the code for the internal UARTs can usually be made more efficient in uputch() by changing the subscripts "[chan]" within the cases to the actual channel used: [INTUART_1], etc. The same would be true of the external channels if you wrote them as separate cases, rather than combined. Also, isempty_tx would be more efficient as a macro than as a non-inline function, since it has a constant argument. This issue would affect interrupt service time. If throughput is not a problem for your code, KISS and don't bother.

Thad

- G
- Gary Kato
  
  Contact options for registered users
Vote on answer
posted
19 years ago

Thu, Jul 1, 2004 10:12 PM

I'm not sure you want to disable interrupts within the UART ISR itself. It has no bearing on the possible interference between the ISR and your regular code. You are not allowing for any higher priority interrupts to happen. Granted, the amount of time that interrupts are off in the ISR is small, but you still might want to enable higher priority interrupts to occur.