Multiple UARTs and multiple interrupts

Started by Ross Marchant June 30, 2004
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
Ross Marchant wrote:

> 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?
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
Ross Marchant wrote:

> 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?
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
> tx_inptr[chan]++; > tx_inptr[chan] &= TXBUF_SIZE;
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