Function call from Interrupt Service Routine

Hey,

The main difference between calling a function from an ISR and from main() is that in the ISR the interrupts are disabled. It is not a problem if the SPI driver uses busy waiting, but if it is waiting for some interrupt... well you can guess.

Can tell you more, if you post some code...

/*
* Spi2UartTestCode.C
*
* Created on: Jan 11, 2016
* Author: Er Kshitij Gupta
Embedded Software Engg.
iMicro System
*/


#include "DSP28x_Project.h" // Device Headerfile and Examples Include File
#include "MAX3109Header.h"
#include "Spi2UartHeader.h"

interrupt void MAX3109_IRQ_ISR(void);
void clear_Max3109_Interrupt(void);


union STSInt_REG tempSTSInt;
union ISR_REG tempISR;
union GlobalIRQ_REG tempGlobalIRQ;
Uint8 c= 0;
Uint8 rx,lvl,flag,k,z,x=0;
Uint8 RcvdChar = 0;
void xmit_aChar(void);
void enable_tx(void);
//void ReceiveEightChars(void);


Uint8 i,j;


void main(void)
{
//Uint8 tempStatus2 =0;
//
tempSTSInt.all = 0;
tempISR.all = 0;
tempGlobalIRQ.all = 0;
flag = 0;
// Step 1. Initialize System Control:
// PLL, WatchDog, enable Peripheral Clocks
// This example function is found in the DSP2833x_SysCtrl.c file.
InitSysCtrl();

// Step 2. Initalize GPIO:
// This example function is found in the DSP2833x_Gpio.c file and
// illustrates how to set the GPIO to it's default state.
// InitGpio(); // Skipped for this example
// For this example, only enable the GPIO for McBSP-A
InitMcbspbGpio();

// Step 3. Clear all interrupts and initialize PIE vector table:
// Disable CPU interrupts
DINT;

// Initialize PIE control registers to their default state.
// The default state is all PIE interrupts disabled and flags
// are cleared.
// This function is found in the DSP2833x_PieCtrl.c file.
InitPieCtrl();

// Disable CPU interrupts and clear all CPU interrupt flags:
IER = 0x0000;
IFR = 0x0000;

// Initialize the PIE vector table with pointers to the shell Interrupt
// Service Routines (ISR).
// This will populate the entire table, even if the interrupt
// is not used in this example. This is useful for debug purposes.
// The shell ISR routines are found in DSP281x_DefaultIsr.c.
// This function is found in DSP2833x_PieVect.c.
InitPieVectTable();

MemCopy(&RamfuncsLoadStart, &RamfuncsLoadEnd, &RamfuncsRunStart);
// Call Flash Initialization to setup flash waitstates (This function must reside in RAM)
InitFlash();

EALLOW; // This is needed to write to EALLOW protected registers
PieVectTable.XINT4 = & MAX3109_IRQ_ISR;
EDIS;


asm(" nop");

init_uart();
Reset_Config_SpiUart();



while (tempSTSInt.bit.ClkReady != 1) // wait for clock ready
{
tempSTSInt.all = Read_STSInt();

}
asm(" nop");
clear_Max3109_Interrupt();


EALLOW;
PieCtrlRegs.PIECTRL.bit.ENPIE = 1; // Enable the PIE block
PieCtrlRegs.PIEIER12.bit.INTx2 = 1; // Enable PIE Gropu 12 INTx2-XINT4

IER |= M_INT12;
EINT; // Enable Global interrupt INTM
ERTM; // Enable Global realtime interrupt DBGM

GpioCtrlRegs.GPBMUX1.bit.GPIO36 = 0;
GpioCtrlRegs.GPBDIR.bit.GPIO36 = 0; // GPIO36 = input
GpioCtrlRegs.GPBQSEL1.bit.GPIO36=0; // Xint4 Synch to SYSCLKOUT only
GpioCtrlRegs.GPBCTRL.bit.QUALPRD1=0xFF;
GpioIntRegs.GPIOXINT4SEL.bit.GPIOSEL = 4;
XIntruptRegs.XINT4CR.bit.POLARITY = 0; // Falling edge interrupt
// Enable XINT4
XIntruptRegs.XINT4CR.bit.ENABLE = 1;

EDIS;
clear_Max3109_Interrupt();


while(1)
{
LedOn(0);

c = xmit_uart('a');
lvl = max3109_port_read(MAX3109R_TXFIFOLVL);
rx = max3109_port_read(MAX3109R_RXFIFOLVL);
}


}

interrupt void MAX3109_IRQ_ISR(void)
{
LedOn(1);
flag = 1;
EINT; // Enable Global interrupt INTM
ERTM;
enable_tx();
clear_Max3109_Interrupt();
asm(" nop");
PieCtrlRegs.PIEACK.all = PIEACK_GROUP12;

}


Uint8 xmit_uart(Uint8 z)
{

if ((tempISR.bit.TxFifoEmpty == 1)|| (tempISR.bit.TxTrigInt !=1))
{
max3109_port_write(MAX3109R_MODE1, 0x02);// disable transmitter
max3109_port_write(MAX3109R_THR, z);
return 1;
}
else
return 0;


}

Uint8 rcv_uart(void)
{
Uint8 rcvd_data= 0;
max3109_port_write(MAX3109R_MODE1, 0x01);
rcvd_data = max3109_port_read(MAX3109R_RHR);
max3109_port_write(MAX3109R_MODE1, 0x00);

return rcvd_data;
}


void clear_Max3109_Interrupt(void)
{
tempISR.all = Read_ISR();
}



void enable_tx(void)
{
max3109_port_write(MAX3109R_MODE1, 0x00);// enable transmitter

}

I programmed MAX3109 for transmitter fifo trigger level as 8 words.
when 8 words are written to TxFIFO MAX3109 will generate an interrupt (XINT4).
This interrupt should clear in ISR.
IRQ can be only be cleared by reading Interrupt Status Register of MAX 3109 which is done by MCBSP(as SPI).

Hi,

You probably should not re-enable interrupts again in the MAX3109_IRQ_ISR function. I am not even sure the execution will reach the clear_Max3109_Interrupt() function.

Could you please place a breakpoint at the clear_Max3109_Interrupt() call in the ISR and check if it is executed at all?

J

I place a break point at ISR on clear_Max3109_Interrupt () call, it goes to this function but read value of Interrupt Status Register of MAX3109 is wrong and interrupt also not cleared.
but in main() it read the correct value and also clears the interrupt.
help me.

Hi,

The definition for function Read_ISR() is missing. Could you plese post is?

J

Hi,

There could be a race condition in your code. As I can tell, nothing prevents the interrupt from firing while you are reading registers in the while loop. This could cause a series of weird behavior.

Could you please try the following modifications:

In the main:
while(1)
{
LedOn(0);

c = xmit_uart('a');
lvl = max3109_port_read(MAX3109R_TXFIFOLVL);
rx = max3109_port_read(MAX3109R_RXFIFOLVL);

if (flag)
{
flag = 0;
enable_tx();
clear_Max3109_Interrupt();
asm(" nop");
}
}

interrupt void MAX3109_IRQ_ISR(void)
{
LedOn(1);
flag = 1;
PieCtrlRegs.PIEACK.all = PIEACK_GROUP12;
}

Hey,

If the problem is really a race condition than you do not have the option to unconditionally call clear_Max3109_Interrupt() from the ISR.

Let's say, that the MCU is executing the max3109_port_read() function that has two mcbsp_xmit() calls. If the interrupt arrives between the two calls than the bytes actually sent through the SPI interface will be wrong. With the following code changes you can check if this is what  really happens. I assume that when you stop the code with debugger, it will be sitting inside the while loop in the ISR. If that is so, we can start working on a solution that fits you.

int spiLock = 0;

Uint8 max3109_port_read(Uint8 regaddr)
{
	Uint8 ret_val,i;

	__inc(&spiLock);

		GpioDataRegs.GPACLEAR.bit. GPIO15 = 1;
		i = mcbsp_xmit(regaddr & 0x7f);
		ret_val = mcbsp_xmit(0x00);
		GpioDataRegs.GPASET.bit. GPIO15 = 1;

	__dec(&spiLock);

	return ret_val;
}

void max3109_port_write(Uint8 regaddr, Uint8 val)
{
	Uint8 y;

	__inc(&spiLock);

		GpioDataRegs.GPACLEAR.bit. GPIO15 = 1;
		y = mcbsp_xmit(0x80 | regaddr);
		y= mcbsp_xmit(val);
		GpioDataRegs.GPASET.bit. GPIO15 = 1;

	__dec(&spiLock);
}

interrupt void MAX3109_IRQ_ISR(void)
{
	if (spiLock)
	{
		while(1);
	}
	__inc(&spiLock);
	LedOn(1);
	flag = 1;
	enable_tx();
	clear_Max3109_Interrupt();
	asm(" nop");
	PieCtrlRegs.PIEACK.all = PIEACK_GROUP12;
	__dec(&spiLock);
}

Hi,

__inc() is a compiler intrinsic (built-in) function that increments the content of the given memory location in an atomic way. See page 155 in  (C28 C/C++ compiler user's guide).

Basically __inc(&i); and i++; would be just the same thing without interrupts involved. But for the i++; version the compiler could generate three instructions:

1. Read the value of i into a register
2. Increment the value of the register
3. Write the register back to i

The problem with this approach is that an interrupt could occur between any two of these instructions and that may lead to a other problems.

For the __inc(&i) version the compiler generates only one instruction that can not be interrupted.

The deal is the same with the __dec(), except it decrements the content of  a memory location.

In your current code base, atomicity is not critical for these instructions, since data transfers in the interrupts are always finished before the ISR returns. But later, when you are building a larger system and will not have the option to wait for IO in the ISR, you will need this.

J

Hi,

I tested this code, you were right it is a race condition. when I stopped the code with debugger, is sitting inside the while loop in the ISR.
Tell me any solution to clear this interrupt as it is coming in between when MCU is executing the max3109_port_write().

Hi,

This is a possible solution. Not optimal, but should work OK with your current code. I only included the changed code.

volatile int spiLock = 0;
volatile int clear_Max3109_pending = 0;

void clear_Max3109_Interrupt(void)
{
	if (spiLock)
	{
		clear_Max3109_pending = 1;
	}
	else
	{
	__inc(&spiLock);
	tempISR.all = Read_ISR();
	clear_Max3109_pending = 0;
	__dec(&spiLock);
	}
}

void lockSPI(void)
{
	__inc(&spiLock);
}

void unlockSPI(void)
{
	__dec(&spiLock);

	if (clear_Max3109_pending)
	{
		clear_Max3109_Interrupt();
	}

	spiLock = 0;
}

Uint8 max3109_port_read(Uint8 regaddr)
{
	Uint8 ret_val,i;

	lockSPI();

		GpioDataRegs.GPACLEAR.bit. GPIO15 = 1;
		i = mcbsp_xmit(regaddr & 0x7f);
		ret_val = mcbsp_xmit(0x00);
		GpioDataRegs.GPASET.bit. GPIO15 = 1;

	unlockSPI();

	return ret_val;
}

void max3109_port_write(Uint8 regaddr, Uint8 val)
{
	Uint8 y;

	lockSPI();

		GpioDataRegs.GPACLEAR.bit. GPIO15 = 1;
		y = mcbsp_xmit(0x80 | regaddr);
		y= mcbsp_xmit(val);
		GpioDataRegs.GPASET.bit. GPIO15 = 1;

	unlockSPI();
}

interrupt void MAX3109_IRQ_ISR(void)
{
	LedOn(1);
	flag = 1;
	enable_tx();
	clear_Max3109_Interrupt();
	asm(" nop");
	PieCtrlRegs.PIEACK.all = PIEACK_GROUP12;
}

The basic idea is that when the SPI is in use when the ISR runs, you have to schedule the problematic function call instead of actually executing it.

For a larger system I would write a transaction based SPI driver. By this I mean that only the driver code would actually transmit anything on the SPI, while the application would send the driver pointers to transaction structures. Transaction structures would hold the bytes to send and pointer to a buffer to read the bytes into. You can chain the transactions like a linked list.

BR,

Janos

Uint8 g =5;
while(1)
{
LedOn(0); // placed a break point here ---1st Break Point

c = xmit_uart(g);

lvl = ReadTxFiFoLevel();
rx = ReadRxFiFoLevel();
asm(" nop");
enable_tx();

g++;
asm(" nop"); // placed a break point here --- 2nd Break Point

}


}

interrupt void MAX3109_IRQ_ISR(void)
{
LedOn(1);
flag = 1;
asm(" nop");
clear_Max3109_Interrupt();
asm(" nop");
RcvdChar = rcv_uart();
asm(" nop");
//asm(" TRAP #21");
PieCtrlRegs.PIEACK.all = PIEACK_GROUP12;
}

It is working but I can't understand its behavior. As in main(), I am transmitting a byte and MAX 3109 generates an interrupt when it receives a byte in Empty Rx Fifo (I configured MAX3109 Interrupt).
When I run the code through debugger, At the second break point, I should get 5 in RcvdChar. But I am getting it in first break point (2nd run).
Can you explain me what is happening ?

Thanks
Kshitij Gupta

Hi,

I am afraid I can not answer this question, since I don't know enough about your circuit and the IC you are using. I guess that would be necessary, but it would take some time to read through the docs...

J

Thank you very much for your support. I think that I am very close to solution.
Only one thing I can't understand that program control is going to ISR after 2nd break point instead of going to ISR before 2nd break point.
Can you tell me about this behavior?
Thanks
Kshitij Gupta

You might try to place a delay loop in front of the second breakpoint. My guess would be some timing issues.

I have added the delay.
delay_loop1(426200);
Now it is going to ISR before 2nd break point. But this overhead is large (98.878400 ms). Is there any option to optimize it.
Thank you very much.

Kshitij Gupta

Well Kshitij, IO takes time. It is completely understandable that you don't want to wait every time this long to receive a character. There are solutions.

But to help you further, it would be nice to know what you are trying to accomplish with all this...

I need more information. By more information I actually mean the complete specification for your task:

• What is the end application?
• Is your driver the only piece of code that will access the MCBSP?
• What is the necessary throughput on the UART?
• What will be the SPI clock frequency?
• What will be the system clock frequency?
• Is it important to write the SPI driver in a generic manner so that it can be used later on parts with multiple MCBSP peripherals?
• Is it important to write the MAX3109 driver in a generic manner so that it later can be used with other serial peripherals, like SPI or I2C?
• Approximately what will be the CPU load in the end application?
• Are you going to use an RTOS?
• How long will be interrupts disabled?
• What is the maximum allowed delay between calling the transmit function and actually sending the data out of the UART?
• The MAX3109 uses 16 bit SPI transfers. Why don't you configure the MCBSP for 16 bit mode?

Let's deal with these questions first, more might come later.

Ok, let me explain the thoughts behind the questions:

• Knowing about the end application may help filling gaps in the requirements and making design decisions that support the overall app structure.
• If your driver is not the only one that access the MCBSP, than you have to write smarter drivers. If you have two external IC-s connected to the same SPI bus, you have to make sure, that while you are talking to one of them, you will not try to send bytes to the other. This requirement results in a different, but much more reusable architecture.
• Knowing the throughput of the UART would help making architecture decisions. If you have to send a lot of data, then you want to eliminate busy waiting in your drivers. If you have to send only a few chars every day, or make a data dump once a month, you may stick with busy waiting.
• Knowing the difference between the SPI and the system clock frequencies would also help assessing if busy waiting is a good idea. If your system runs at 100 Mhz and the SPI runs at 25MHz than busy waiting will not cost too much CPU time. If the system runs at 150MHz and the SPI at 100kHz, than you have to wait at least 12000 clock cycles for every byte. Generally not a good idea.
• If later you have to use the same code on parts with multiple MCBSPs you can not hard code MCBSP register names into your drivers. You must use some kind of redirection like function pointers, pointers to peripheral register structures, custom peripheral interfaces or something along these lines.
• If you later have to use the same driver to access the MAX3109 via other interfaces you have to divide your driver into two parts: one for the MAX3109 and one for the actual MCU peripheral. You have to create a clean interface between them so later you can switch the communication method the MAX3109 driver is using.
• If the CPU load is small in the end application, than busy waiting in the driver will not hurt.
• If you are using RTOS in the application, than your drivers must usually conform to certain rules depending on the OS.
• If other code in your app disables interrupts for extended periods that could create problems in certain cases, especially when you are using an interrupt driven code.
• Max delay introduced by your driver will be affected by the previous point.

Few ideas on how to learn this thing:

• If there are more experienced devs around you, please look at their code.
• Download open source embedded projects and look at the code, check how they solved similar problems.
• Read books. I liked Test-Driven Development for Embedded C from James Grenning and Object Oriented C by Axel-Tobias Schreiner.