dnadler wrote on Saturday, August 18, 2012:
In the PIC24 port.c is the following:
#if configKERNEL_INTERRUPT_PRIORITY != 1
#error If configKERNEL_INTERRUPT_PRIORITY is not 1 then the #32 in the following macros needs changing to equal the portINTERRUPT_BITS value, which is ( configKERNEL_INTERRUPT_PRIORITY << 5 )
#endif
I don’t see anything like a #32 in the subsequent code ?
Thanks in advance for any help,
Best Regards, Dave
rtel wrote on Sunday, August 19, 2012:
The kernel uses the configKERNEL_INTERRUPT_PRIORITY setting to mask interrupts under a certain priority in critical sections. Where this is done from C code the constant is used directly so nothing needs to change, but where this is done in assembly code the value used is hard coded (although, as this is effectively GCC, it could probably be used in the asm code to). You will find the #32 FreeRTOS/source/portable/MPLAB/portasm_PIC24.S.
I would however not recommend using a value other than 1 for configKERNEL_INTERRUPT_PRIORITY. It is normally best to have the kernel interrupts at the lowest possible interrupt priority as things can get conceptually awkward if interrupts are running below that priority.
Regards.
dnadler wrote on Sunday, August 19, 2012:
I’m stuck here:
- I have an ISR that needs to run at high priority, then give a semaphore.
- can’t give a semaphore from an ISR at higher priority than FreeRTOS, right ?
This port does not implement configMAX_SYSCALL_INTERRUPT_PRIORITY,
so I assume that it is effectively configKERNEL_INTERRUPT_PRIORITY, right ?
Thanks !
Best Regards, Dave
PS: later PIC24 and dsPIC have hardware stack limits;
any interest in implementing this in the PIC24 FreeRTOS port ?
rtel wrote on Sunday, August 19, 2012:
As I recall, on the PIC24, interrupts that use the FreeRTOS safe API have to run at the kernel interrupt priority as the interrupt nesting model is not a sophisticated as that implemented for the larger PIC32. Working around that would be some effort, but not impossible.
Regards.
dnadler wrote on Sunday, August 19, 2012:
Right, so I figured it would be good to push up the FreeRTOS priority so:
- I can give the binary semaphore from the critical ISR,
- I can run some (slow non-critical) ISRs at a lower priority
Is this a bad idea ?
Thanks Richard,
Best Regards, Dave
rtel wrote on Monday, August 20, 2012:
Critical sections in tasks already mask interrupts up to configKERNEL_INTERRUPT_PRIORITY. If you raise configKERNEL_INTERRUPT_PRIORITY then you will need an equivalent critical section implementation that works from interrupts. The core FreeRTOS code includes macros called portSET_INTERRUPT_MASK_FROM_ISR() and portCLEAR_INTERRUPT_MASK_FROM_ISR() for this purpose - but these are *not* defined in the PIC24 port and just get #defined to nothing. If you were to define them, then it might be ok to raise configKERNEL_INTERRUPT_PRIORITY - but I have not tried it.
portSET_INTERRUPT_MASK_FROM_ISR() needs to set the interrupt mask to configKERNEL_INTERRUPT_PRIORITY(), then return the original mask value (before it was raised). portCLEAR_INTERRUPT_MASK_FROM_ISR() takes one parameter, which was the original interrupt mask value so it can restore it. You can see an example implementation in the PIC32 port layer - albeit for a different architecture.
Hope this helps.
Regards.
dnadler wrote on Tuesday, August 21, 2012:
OK, does the following all look correct ?
PIC24F implements priority levels 1-7 (7 is highest). It has unique interrupt vectors for many sources (an interrupt vector is not typically shared; for example a single serial port has many vectors for RX, TX, etc). Each source’s interrupt priority is set individually in “IPC” registers. Setting a source’s IPC to 0 disables the interrupt. At a device Reset, all user interrupt IPC registers are initialized to priority level 4.
The current operating priority level of the CPU is in the “IPL” (interrupt priority level) bits of the “SR” (processor status) register. The IPL is set when an interrupt is serviced, and restored when the SR is restored during the return-from-interrupt instruction. The IPL can be freely set as a means of controlling what interrupts will be serviced (to implement a critical section for example). Setting IPL to 7 disables interrupts.
Hence…
// DRN: "FromISR" critical sections can be called from an ISR at lower
// DRN: priority than kernel (but may never be called from a priority
// DRN: higher than kernel, which could permit reentrancy). To prevent
// DRN: kernel reentrancy, mask: interrupts up through kernel priority.
#if configKERNEL_INTERRUPT_PRIORITY > 1 // DRN: 1 is lowest PIC24F priority (7 is max)
inline unsigned portBASE_TYPE __portSET_INTERRUPT_MASK_FROM_ISR(void) { // DRN
unsigned portBASE_TYPE saved_IPL = _IPL; // DRN
_IPL = configKERNEL_INTERRUPT_PRIORITY; // DRN
return saved_IPL; // DRN
} // DRN
#define portSET_INTERRUPT_MASK_FROM_ISR() __portSET_INTERRUPT_MASK_FROM_ISR() // DRN
#define portCLEAR_INTERRUPT_MASK_FROM_ISR( uxSavedStatusValue ) (_IPL = (uxSavedStatusValue) ) // DRN
#endif // DRNwoops_ wrote on Tuesday, August 21, 2012:
Well, just by reading your code without actually trying it (I’m not in my office for a week), I would say it looks fine. It has the correct intent anyway.
Regards.
woops_ wrote on Tuesday, August 21, 2012:
Just one comment for something you probably have covered anyway. As I recall setting the IPL requires a bit shift to the correct position in the SR. Does writing to _IPL take care of that for you (unless my recollection is incorrect).
Regards.
rtel wrote on Tuesday, August 21, 2012:
Just to avoid confusion - that was me. Woops indeed - hadn’t logged out.
Regards.
dnadler wrote on Tuesday, August 21, 2012:
Thanks Richard. The generated code does the right thing, but its really ugly and the free compiler doesn’t inline (_IPL resolves to a bitfield). Too bad portSET_INTERRUPT_MASK_FROM_ISR() doesn’t take the result as a parameter (would make it much easier to create a simple clean macro). I’ll play around with it and see if I can improve it later in the week (I’m out Wednesday). Thanks as always,
Best Regards, Dave
dnadler wrote on Sunday, September 02, 2012:
Hi Richard - I added the IPL save/restore code and all works OK, but the generated code is copious (especially with the free XC16 compiler which does not actually inline). So I’ve gone back to running the RTOS at priority 1.
I added a “task_reschedule_safe_from_ISR” which provokes a ‘fake’ interrupt at priority 1 -> ISR does taskYIELD (optionally preceded by giving a binary semaphore). This seems to be the fastest and safest way for my high-priority ISR to wake up the waiting task; guarantees completion of any high-priority ISRs or priority 1 critical sections - no risk of re-entrancy.
Thanks as always for the help,
Best Regards, Dave
PS: The Microchip toolchain is truly horrid - debugger barely works, IDE extremely flakey - new MPLAB-X version too buggy to use at all - yikes…