PIC24 nested interrupts and nested FreeRTOS funtion calls

dragonflight1 wrote on Thursday, November 07, 2013:

This is a followup on the post https://sourceforge.net/p/freertos/discussion/382005/thread/f2c275d1/
in the middle of which Richard Damon posted

There are two separate issues with using multiple interrupt levels and allowing interrupt that access FreeRTOS to nest. First is the need to be able to have a critical section inside the interrupts. The PIC24 port could define this operation, but doesn’t due to the second issue.

The second issue is task switching forced by an ISR. The method that the PIC24/dsPIC ports use, is not compatible with nesting interrupts.

I would like to fix this for the PIC24’s. I had assumed that it worked because the port instructions included instructions to change configKERNEL_INTERRUPT_PRIORITY.

I hope to suggest some solutions and hope that others will comment and/or suggest others.

dragonflight1 wrote on Thursday, November 07, 2013:

First Problem:
I had done a quick sanity check on xQueueGenericSendFromISR() and saw that it bracketed its code by

uxSavedInterruptStatus = portSET_INTERRUPT_MASK_FROM_ISR();
portCLEAR_INTERRUPT_MASK_FROM_ISR( uxSavedInterruptStatus );

and that it did not seem to call portENTER_CRITICAL() or portEXIT_CRITICAL().
Of course I didn’t check on the definitions (they are null) so there is no critical section mgmt.

So question 1 is - are all of the “from_ISR” functions equally bracketed and do they not call portENTER_CRITICAL() or portEXIT_CRITICAL().

Assuming the afferative to this, defining portSET_INTERRUPT_MASK_FROM_ISR() and portCLEAR_INTERRUPT_MASK_FROM_ISR(x) appropriately should fix Richard’s first point.

rtel wrote on Thursday, November 07, 2013:

Yes, all “FromISR” functions use interrupt safe critical nesting, none use portENTER_CRITICAL()/portEXIT_CRITICAL(), but the interrupt safe versions are only defined for ports that support a full interrupt nesting model (like the PIC32).


dragonflight1 wrote on Thursday, November 07, 2013:

His second is somewhat more difficult.

The issue is with calling portYield() from a nested interrupt. The new task will run instead of the interrupt that was interrupted.

I believe that as a workaround if the timer is defined to be priority 1 and portYield() is only called from other priority 1 interrupt routines all will be fine (assuming always his first point is fixed).
Of course this defeats the advantage of quick preemption this feature added.

dragonflight1 wrote on Thursday, November 07, 2013:

Richard you are too quick!
Correct me if I am wrong, I only need to define the two macros to make the calls interrupt safe - ignoring for the moment the issue of portYield()?

dragonflight1 wrote on Friday, November 08, 2013:

I have three possible solutions to the second problem (nesting interrupts)

  1. as per my comment above, if portYield() is only called from interrupt routines with priority 1 all will be ok so

    • use a “spare” interrupt - by default perhaps the timer 5, with priority 1.
    • #define portYieldFromISR(x) if( x ) _T5IF = 1
    • and in the interrupt routine call portYield()
  2. use the compiler option to insert code before the prologue of an interrupt routine.

    • insert asm( “mov w15,stacksave_T1” ) - need unique identifier for each interrupt
    • in portYieldFromISR check statsave[-1]&0xE00 to decide if it is in a nested interrupt
    • as long as all interrupts that call portYield add the asm code the system will operate correctly
    • to work properly (optimized)
      • all interrupt routines must call portYieldFromISR
      • a single external should be used for all interrupt calls such as xQueueReceiveFromISR(x,y,CommonYIELD)
      • calling sequence would be
          if( CommonYIELD ) {
                CommonYIELD = 0;
    + things could be made "nicer" with macros
  1. Create a wrapper (similar to PIC32) to use a single common stack for interrupts. I was looking into this regardless - pretty obvious when you start having nested interrupts
    * in this case only interrupts that needed to call portYieldFromISR would need the wrapper
    * using the preprologue option makes the overhead quite reasonable

I would greatly appreciate comments, corrections, suggestions, further options.
I will try to implement options 1 and 3. 1 is practically free, and I was interested in 3 anyway. My biggest problem will be how to test it as my current stuff “works” though definitely wrong!

dragonflight1 wrote on Monday, November 11, 2013:

I have “successfully” implemented solution 1. I was able to avoid using an additional interrupt and just used the timer1 interrupt. It took me 10 times as long to figure out to test it than do it!!
I have 6 levels of nested interrupts with random interrupt times of 0-2msecs.
It definitely failed without my changes, so I feel reasonably confidently about it. If anyone is interested in playing/testing it, I would happily supply the changes.


richard_damon wrote on Monday, November 11, 2013:

Do you do something to keep the timekeeping of Timer 1 still intact? It could be tricky to tell in the timer 1 interrupt if the tick counter needs to be bumped or not.

dragonflight1 wrote on Monday, November 11, 2013:

It is tricky and I think I can do better, but here is what I did

#define portYIELDfromISR() {
extern portBASE_TYPE tmrSave, yieldPending;
if( !yieldPending ) {
yieldPending = 1;
tmrSave = TMR1;
if( _T1IF )
tmrSave = 0xFFFF;
_T1IF = 1; /* trigger the timer interrupt */

(sorry I couldn’t figure out how to get the text right!)
then check if the TMR1 < tmrSave to see if it is a real tick.


EDIT: I tried to improve it on the fly and screwed up!
EDIT2: fixed tmrSave = 0xFFFF;

richard_damon wrote on Monday, November 11, 2013:

I think you need to put that inside an interrupt critical section, as if you get an higher priority interrupt between testing yieldPending and setting it, you may mis-detect the presence of a counter rollover, as it will set _T1IF, and you will then think that that meant counter rollover.

EDIT: your disi call should fix that.

Also, in the case of _T1IF being set, don’t you want to set tmrSave to -1 or 0xFFFF so that TMR1 < tmrSave is true?

This does presume that the counter can’t roll around enough to case the compare to fail, but if you are stuck in interrupts that long, you are sort of bound to miss some.

dragonflight1 wrote on Monday, November 11, 2013:

You are right about setting it to 0xFFFF.

I spent a while thinking about the “rollover problem” and decided
a) if _T1IF is not set you have 1ms to get to the interrupt routine before the comparison is wrong
b) if _T1IF is set you have 1ms - TMR1 counts to get to the interrupt routine (which is 1ms since the last interrupt)

So in either case you have the same time (1ms) to service the interrupt before you lose one (in case a) you actually have more). I was sure it couldn’t be so, but I’ve convinced myself!

Anyway, I’m just setting up a test with a second timer counting ticks to see if there is any discrepancy, but I’m struggling (some times I can’t program my way out of a wet paper bag!)


richard_damon wrote on Tuesday, November 12, 2013:

One comment, your “1 ms” assumes a 1 KHz tick rate, which in my opinion is generally fast for your system tick. I generally find that I can live with a 10-100ms tick period just fine.

As far as roll-over, as long as the maximum period it takes to service all nesting interrupts + the maximum Critical Section is less than your tick period you should be ok, Remember that in your case b) implies that you have been within the ISRs or Critical Section since the timer last rolled over,

One more case that I would need to think about is if the timer interrupt is servicing a tick interrupt, and gets interrupted. You probably need some sort of disi period to clear the _T1IF flag and decide if you need to increment the system tick where you can’t get interrupted.

For using disi(), one trick is that you need to know how many cycles to disable for, which is often hard to know in C, but you can do a disi() with a large count, and then a disi(0) when you are done.

dragonflight1 wrote on Tuesday, November 12, 2013:

Yes, I have been playing with the interrupt. You need 1 critical section to set yieldPending, after that you just have to be careful. I struggled for quite some time before I realized that I was losing interrupts because my interrupts were taking too long, so I may have to reconsider my previous reasoning.
When I shortened it a little I had 3 1ms timer interrupts (system and 2 checking), 3 interrupts randomly approx 1/ms each sending to a queue and a 50us delay loop and calling portYIELDFromISR while running at 16 mips, I ran for 1/2 hour with no dropped/added ticks. When I made the delay loop 100us I lost about 1 tick/20 secs.
I need to figure a way of measuring what is going on, I’m suspicious
I think I’m going to replace the disi’s with setting the SR so that I don’t disable all the interrupts.

It was a lot easier with a separate interrupt - but not as much fun!

As for the tick rate, 1ms is what the demo had and if it works for 1ms, it will certainly work at 10. With the yield from ISR strategy, it is less useful to have a fast tick timer


dragonflight1 wrote on Tuesday, November 12, 2013:

Well I found a bug - a missing else - after I reorganized the code to make it more “readable”!! oh well. In any case each interrupt is now delay 300us and after 15 min no dropped ticks. I will let it run overnight and see.


It ran all night and the results are perfect. I have three counters, one in the system timer, one in an additional timer running at priority 1 (same as the system timer) and one in a timer running at priority 6.
After 9 hours, the priority 6 interrupt and the system timer agree, whereas the second timer running at priority 1 has lost 1 tick