PIC24F Interrupts Issue (Warning:Newbie insid

ruben_saz wrote on Tuesday, March 23, 2010:

Hello there.

Maybe this is a silly question (from an unexperienced user, as you’ll probably notice) but here it goes anyway:

I’m having an issue with the PIC24 port of the freeRtos kernel (Version 5.1.2). My application seems to run fine for a while but after some time it freezes. Every task stops executing and no interrupts are handled, the code just loops in the idle task. I have the ICD 2 debugger connected  to my circuit and i can see that the IPL bits are set, so the CPU level is 7 when it freezes. ** I’ve disabled the nested interrupts by setting the NSTDIS bit, because a while ago i experimented other problem that was  solved by doing this**. Now according to the PIC24F family reference manual:

- All user interrupt sources can be disabled by setting IPL<2:0> = 111.
- The IPL<2:0> status bits are read-only when NSTDIS = 1 (INTCON1<15>).
- When the NSTDIS control bit is set, all interrupts in progress will force the CPU priority to level 7 by setting IPL<2:0> = 111.

So, Is it wrong to disable nested interrupts while using the kernel?, Can someone please point me to some text, paper, reference or anything about precautions on handling nested interrupts? If it is not wrong to disable nested interrupts (I’m guessing it is), what else could be happening, what  else should i check?

NOTE: I assume its wrong to disable nested interrupts since the macro #define portDISABLE_INTERRUPTS() SR |= portINTERRUPT_BITS  writes to this bits. However while i was searching the forum I found this post https://sourceforge.net/projects/freertos/forums/forum/382005/topic/3489944 which suggest to disable nested interrupts “If you don’t need them” to solve their problem.

Any help figuring this out will be appreciated.

Thanks in advance guys :slight_smile:

richard_damon wrote on Tuesday, March 23, 2010:

If you disable all the interrupts this way, then you have stopped the timer tick, so any task that delays on a timeout will get frozen.

ruben_saz wrote on Tuesday, March 23, 2010:

Hi Richard.

I’m not halting the Tick interrupt, at least not deliberately. What I did was disable the nested interrupts, then in some way i don’t know, I’m getting ALL the interrupts disabled randomly and when that happens and I check my SR all the IPL bits are set.
I know that the tick interrupt should be executed in order to maintain the delay timing and the task timeslicing. What I’m asking here is what happens with the kernel if I disable the nested interrupts. Could this lead to problems that are causing my IPL bits being set? I have read that IPL bits are set to 7 (111) while the CPU is servicing an interrupt AND it has nested interrupts disabled (thus preventing other interrupts to run while the first is serviced) . With that in mind I’m thinking my problem is caused because I’m not returning properly from some ISR (or something went wrong at the context switch when I call taskYIELD() from the ISR) or by the fact of disabling nested interrupts.

Please excuse me if I didn’t made my point clear on the first post, maybe it has something to do with the fact that English is not my first language.

Best Regards, and thanks rich ;).

davedoors wrote on Tuesday, March 23, 2010:

Its not clear how you are disabling nested interrupts, but if you don’t want interrupts to nest then just set all the interrupts to use the same priority.

richard_damon wrote on Tuesday, March 23, 2010:

I think the way FreeRTOS does the task switch, it isn’t compatible with NSTDIS on the PIC24/dsPIC series. The function vPortYield explicitly saves and reloads the SR to handle the switching in and out of tasks at either task level or interrupt level. If an interrupt restarts and yields to a task that went to Yield at task level, then the RETFIE at the end of the interrupt will NOT get executed until the task that was interrupted by the interrupt gets run again. This is one reason that there should be nothing after the conditional vPortYield call in the interrupt routine. This works in normal cases, as the Yield function does all of the processing to restore the interrupt level to the right state, *unless* NSTDIS is set, which makes part of the SR read only while interrupt nesting is disabled.

ruben_saz wrote on Tuesday, March 23, 2010:

Thank you guys!

At this point I’ve tested with nested interrupts enabled and it seems to work fine. But first I also corrected some code that seems wrong to me:

void iThermocoupleISR( void )
{
	portBASE_TYPE xTaskWoken;		// indicates if a context switch should occur after this interrupt
	unsigned int iTempBuffer = 0;		
 
	iTempBuffer = SPI1BUF;
	xQueueSendToBackFromISR( xThrmcplQueue, &iTempBuffer, &xTaskWoken );	// Send received word to the queue
	CONF_CS_MAX6675_PORT = 1;		// return chip select to the idle state
	if( xTaskWoken )
		taskYIELD();	// context switch if a higher priority task is woken
	return;
}
 
void __attribute__((interrupt, auto_psv)) _SPI1Interrupt (void)
{
	iThermocoupleISR();
	IFS0bits.SPI1IF = 0; // Clear interrupt flag
}

Running taskYield() inside a function called by the ISR. I don’t for sure if this is wrong but i also changed this in my code.

demanzano wrote on Thursday, March 25, 2010:

Why are you using a separate function ? You should do directly inside _SPI1Interrupt() (the true ISR function).
Also, ack the IRQ as early as you can (ie: put IFSbits.SPI1IF = 0; as soon as inside the function)

just my 0.2 E

richard_damon wrote on Friday, March 26, 2010:

The issue here is that for FreeRTOS, the call to taskYEILD() needs to be the very last thing in the ISR, because what follows will NOT be executed right away (not tile the task that was interrupted runs again). The extra level of functions shouldn’t be an issue (but does increase the stack size needed in every task, as interrupts use the task stack in PIC ports), and I do something similar where I may have multiple serial or I2C channels service by a common function which takes an arrangement to indicate which one is being run.

ruben_saz wrote on Friday, March 26, 2010:

demansano: I guess I did it that way because I’m getting used  to service interrupts from different modules using the same function, for example UART1 and UART2 are serviced by the same code with different parameters passed for each ISR. As richard says, this approach uses more RAM per task, but I like it that way, at least right now, that I’m not running out of RAM :).

I’ll be moving IFS0bits.SPI1IF = 0; to the beginning of the ISR.

richard: I’m not sure if it is correct to call taskYEILD() inside a function that gets called from Inside the ISR, so in the code I’m using  right now, I moved the taskYEILD()  so it’s called Outside any  functions. In the disassembly listing I see that the compiler inserts some stuff and then a return instruction after each function. By calling taskYIELD() from outside the function servicing the ISR I ensure that it will be the very last thing to do in the ISR. Can you tell me how do you handle the conditional context switch when you are using a function to service an interrupt?

Thank you for your comments.

richard_damon wrote on Friday, March 26, 2010:

The key thing is that the last thing that happens before a task is switched out is that the proper stack frame is created so it can be restarted later. vPortYield will create just such a frame, independent of its placement in the interrupt handler. The key thing to notice is that as far as FreeRTOS is concerned, the ISR is really just like any other function, the “funny” redirection which was the interrupt occurs down stack from what FreeRTOS worries about. It needs to be at the end, because what follows may not be executed for a while, so it better not do anything (like clearing the interrupt flag) that needs to get done. When the interrupt task gets resumed, it will pick up from the Yield, and finish the interrupt ISR and return to the task.

What makes this task switch work, is that the state save code saves the interrupt state, and restores the state of the new task. This work for the PIC, since push SR saves enough that the corresponding pop SR can make the CPU enter/leave “interrupt” mode, and the only thing that differs between a RET and a RETFIE is that the latter loads the saved SR in addition to the return address (to match how the routine was called), at least as long as the NSTDIS bit is not set. if NSTDIS is set, then the Yield code does not work, as you then MUST execute the RETFIE to leave the interrupt.

This might not work on other processors, and in that case there is a different way to end the ISR, by using the portEND_SWITCHING_ISR macro which might need more magic to make a yield work from an ISR.

ruben_saz wrote on Friday, March 26, 2010:

Thank you richard, you’ve been really helpfull

anonymous wrote on Monday, March 21, 2011:

There are other issues with FreeRTOS and the PIC24F.  The portMacros as supplied will not support any but interrupt priority 1. (The portCRITICAL_SECTION macro only masks and clears level 1).   An interrupt at any other priority does not get cleared until the original SR is restored.  The interrupt priority will remain at the level of the last interrupt.  In addition, there seems to be something funky going on as I can have an enabled IFSx bit pending (IECx bit is 1) that is not recognized even when the interrupt priority is at zero.  IMHO FreeRTOS is worth almost as much as I paid for it.

edwards3 wrote on Tuesday, March 22, 2011:

The portMacros as supplied will not support any but interrupt priority 1

That is configurable.

An interrupt at any other priority does not get cleared until the original SR is restored.  The interrupt priority will remain at the level of the last interrupt.

I don’t think you have read the documentation on how to use the port.

IMHO FreeRTOS is worth almost as much as I paid for it.

Nothing to do with the user, then?

dkure wrote on Monday, March 28, 2011:

Hi all,

I thought I might weigh in on the discussion. I too am trying to use FreeRTOS with nested interrupts disabled as it will reduce my RAM requirements for each task I am running. (Otherwise i need to allocate space on the stack for 9 interrupts for each of my 10 tasks)

I am keen to try to solve this problem and allow FreeRTOS to run seemisly with either nested OR non-nested interrupts.

For this to work 2 things need to be done
1) Disabling and Enabling of interrupts needs to be done via using the DISI assembler instruction followed by enabling nested interrupts, followed by setting the level to maximum

Something that like this
Disable Interrupts

void DisableInterrupts(void)
{
    asm volatile(
            "PUSH   W0\n"           /* Make some room */
            "DISI   #4\n"           /* Disable interrupts for 4 instructions */
            "BCLR.B INTCON1, #7\n"  /* Allow nested interrupts to allow writing to the SR */
            "MOV    #32, W0\n"      /* Set level to disable interrupts */
            "MOV    W0,  SR\n"
            "POP    W0\n"           /* Restore the stack */
            );
}   
            
void EnableInterrupts(void)
{   
    asm volatile(
            "PUSH   W0\n"           /* Make some room */
            "MOV    #0, W0\n"       /* Set the level to enable interrupts */
            "DISI   #3\n"           /* Disable interrupts for 3 instructions */
            "MOV    W0, SR\n"
            "BSET.B INTCON1, #2\n"  /* Allow nested interrupts to allow writing to the SR */
            "POP    W0\n"           /* Restore the stack */
            );
}           
2) We need to provide vPortYieldFromISR()
Now from what I can see in other ports (using the CortexM3 ports as an example) these seem to raise a software interrupt which does the context switch from assembler.
The benefit of this is that all the other interrupts can be normal C code, and only the software interrupts needs to be written in assembler.
From What I can tell the SW interrupt will look like
Save the Low PC and PC + SRL registers onto the stack 
Save the other registers on the stack, exactly the same way as vPortYield() does
Call ContextSwitch()
Restore the registers from the stack
Make sure the last two bytes on the stack are the Low PC and PC + SRL registers. This may require ORing the SRL and PC
together.
Does anyone have any thoughts of either 1 or 2 above. As I am said i am keen to make a universal solution for this that can be used by others. Any help trying to implement it would be great too.

davedoors wrote on Monday, March 28, 2011:

I have just looked at the serial.c file supplied as part of the PIC24 demo, and as far as I can see all you have to do if you don’t want interrupts to nest is set the priority of all interrupts to configKERNEL_INTERRUPT_PRIORITY. The UART interrupt handler in the same file also shows how to request a context switch inside an interrupt. Presumably the web documentation page for the port will describe that too.

dkure wrote on Monday, March 28, 2011:

Mentioned in this post http://sourceforge.net/projects/freertos/forums/forum/382005/topic/3489944 it’s seems that once a interrupt flag is cleared interrupts can then nest even if they are at the same priority.

From what i can see with my build is that if I disable nesting all my stacks can be reduced (and by large amounts) if I disable nesting.

I have written a test to try to verify the comments in the other thread and from what I can see Interrupt priorities for me seem to be ignored when all levels are the same.

When I replace the portENABLE_INTERUPTS and the disable macro with the code above things start to behave much nicer. (as before enable and disable of interrupts were not working due to read only SR bits with nesting disabled)

Thanks for the response davedoors. That’s what I originally thought would happen too but it’s not what happens in practice for me :frowning:

davedoors wrote on Monday, March 28, 2011:

Mentioned in this post http://sourceforge.net/projects/freertos/forums/forum/382005/topic/3489944

The first post in that thread says

It is important to note that we changed the critical section code to mask ALL interrupts during critical sections (not just the OS tick).  We are using the Pic24 port.  We changed the following lines:

To do that I think you can simply set configKERNEL_INTERRUPT_PRIORITY to the maximum possible interrupt priority, rather than the minimum possible interrupt priority, and then run all interrupts at configKERNEL_INTERRUPT_PRIORITY.

If that does not work, then I think I am missing the point in this thread. Interrupt registers should not get modified manually inside an interrupt routine, for example, it is not valid to enable and disable interrupts inside an ISR in that port (I think). Ports for things like the Cortex have a portSET_INTERRUPT_MASK_FROM_ISR() macro but there is not an equivalent in the PIC24.

richard_damon wrote on Monday, March 28, 2011:

I don’t know if it is documented anywhere except by reading the code, but it you look at how FreeRTOS implements tasks switching on this port, it is clear why the disable nesting bit causes problems. When an interrupt causes a task switch, the new task context is just loaded up, changing the IPL to task level. If NSTDIS = 1, this operation will fail, and your interrupts left disabled. Note that when the task that was stopped gets restated, then the processor will return to ISR, and finish it, which is why there is a admonition that the yield needs to be last, as anything after it will be delayed,

Note that I have extensively used the dsPic port, which is basically identical to the Pic24, except for the need to save some additional registers as port of a context save.

dkure wrote on Tuesday, March 29, 2011:

HI all,

I have made a simple test to try to show what I am seeing with my hardware and why just these interrupt priorities are not working correctly.

I have set all my interrupts to the highest possible level 0x07 and also the timer tick is also at 0x07.
I have nesting enabled.

My serial RX interrupt looks like this

#define INTERRUPT_TIMER 0x00
#define INTERRUPT_U1_TX 0x01
#define INTERRUPT_U1_RX 0x02
#define INTERRUPT_U2_TX 0x03
#define INTERRUPT_U2_RX 0x04
#define INTERRUPT_U3_TX 0x05
#define INTERRUPT_U3_RX 0x06
#define INTERRUPT_U4_TX 0x07
#define INTERRUPT_U4_RX 0x08
#define INTERRUPT_NELMS 9
int interrupt_state;
int interrupts[INTERRUPT_NELMS];
void __attribute__((__interrupt__, auto_psv)) _U1RXInterrupt( void )
{
    interrupt_state |= (1 << INTERRUPT_U1_RX);
char cChar;
portBASE_TYPE xHigherPriorityTaskWoken = pdFALSE;
    /* Check which interrupts we are nested in */
    int i;
    for (i = 0; i < INTERRUPT_NELMS; i++) {
        if (interrupt_state & (1 << i)) {
            interrupts[INTERRUPT_U1_RX] |= (1 << i);
        }
    }
    /* Get the character and post it on the queue of Rxed characters.
    If the post causes a task to wake force a context switch as the woken task
    may have a higher priority than the task we have interrupted. */
    _U1RXIF = serCLEAR_FLAG;
    while( U1STAbits.URXDA )
    {
        cChar = U1RXREG;
        xQueueSendFromISR(Serial_xRxedChars[0], &cChar, &xHigherPriorityTaskWoken);
    }
    /* Clear the overrun bit after we have received the characters
     * The act of clearing the OERR flag resets the RX buffer 
     */
    if(U1STAbits.OERR)
    {
        U1STAbits.OERR = 0;
    }
    interrupt_state &= (~(1 << INTERRUPT_U1_RX));
    portEND_SWITCHING_ISR(xHigherPriorityTaskWoken);

And my timer tick interrupt looks like this

void __attribute__((__interrupt__, auto_psv)) _T1Interrupt( void )
{
    interrupt_state |= (1 << INTERRUPT_TIMER);
    /* Check which interrupts we are nested in */
    int i;
    for (i = 0; i < INTERRUPT_NELMS; i++) {
        if (interrupt_state & (1 << i)) {
            interrupts[INTERRUPT_TIMER] |= (1 << i);
        }
    }
    /* Clear the timer interrupt. */
    IFS0bits.T1IF = 0;
    vTaskIncrementTick();
    interrupt_state &= (~(1 << INTERRUPT_TIMER));
    #if configUSE_PREEMPTION == 1
        portYIELD();
    #endif
}

In my main loop, every 10 seconds I print the following

void MainTask(void *p)
{
    while (1) {
        int i, j;
            syslog(LOG_DEBUG, "=================");
            for (i = 0; i < INTERRUPT_NELMS; i++) {
                char s[10] = "000000000";
                int total = 0;
                for (j = 0; j < INTERRUPT_NELMS; j++) {
                    if (interrupts[i] & (1 << j)) {
                        s[j] = '1';
                        total++;
                    }
                }
                printf("interrupts[%d] = %s %d\r\n", i, s, total);
            }
        sleep(10);
    }
}

The output from the main loop after about 10-20 minutes looks like this

=================
interrupts[0] = 000000000 0
interrupts[1] = 010100000 2
interrupts[2] = 001000000 1
interrupts[3] = 000100000 1
interrupts[4] = 000110000 2
interrupts[5] = 000101000 2
interrupts[6] = 000000100 1
interrupts[7] = 000100010 2
interrupts[8] = 000100001 2

From this we can see that the Uart2 TX is being nested with the other interrupts, even though the priority has been set to the maximum and that the flag has been cleared before we call vPortYield()

This is why I am trying to look for a solution to the problem by instead turning off nested interrupts, as it seems the hardware ignores the priority levels.

dkure wrote on Tuesday, March 29, 2011:

there is a missing array index using i above as it makes the code italic, I’ll use the letter z in place

if (interrupts[z] & (1 << j)) {