Hi, I just started using FreeRTOS with the AVR128DB32 microcontroller from Microchip and I am having some issues with task synchronization using semaphores.
I am trying to synchronize a blocking function writing to the i2c peripheral with the i2c interrupt using a semaphore. The issue is that if after calling xSemaphoreGiveFromISR I call vPortYieldFromISR() the IDLE stack overflows (no matter how big I make it) and I get a stack overflow error.
However, if do NOT call vPortYieldFromISR() the blocked task gets scheduled at the next context switch (and I can notice some delay in the i2c operations) but no stack overflow occurs. Looking at the implementation of vPortYieldFromISR I found out it is the following:
but I cannot understand how this is supposed to work. In fact, it stores the context of the interrupt and I am not sure if this is correct or not.
the relevant part of the code that gives the issue is the following:
ISR (i2c_VECT) {
// do some checks and send data if needed
if (all data have been sent) {
xSemaphoreGiveFromISR(i2cSemaphore, NULL);
vPortYieldFromISR ();
}
}
I tried solving this issue in the following way and so far it seems to work:
__attribute__((naked)) ISR (i2c_VECT) {
portSAVE_CONTEXT();
// do some checks and send data if needed
if (all data have been sent) {
xSemaphoreGiveFromISR(i2cSemaphore, NULL);
vTaskSwitchContext();
portRESTORE_CONTEXT();
asm volatile ( "reti" );
}
portRESTORE_CONTEXT();
asm volatile ("reti");
}
what I am trying to do there is to manually save / restore the context (by declaring the ISR function naked) and only if all data have been sent I manually call for a context switch
if someone could please tell me if I am missing something I would highly appreciate!
I have also tried enabling only the interrupt of a button and inside of the ISR I called vPortYieldFromISR (). checking at the watermarks levels of the stack of all my task I noticed that every task was getting corrupted, only the IDLE task was more affected since the others were in wait state most of the time. In fact what I noticed is that in this port interrupt are handled with the stack of the task currently in execution when they are fired
I just finished reading this port and I think you are exactly right. vPortYieldFromISR does not seem usable on this port. You can probably remove the duplication of portRESTORE_CONTEXT(); asm volatile ("reti"); sequence:
ISR (i2c_VECT, ISR_NAKED) {
portSAVE_CONTEXT();
/* ISR implementation. */
if (all data have been sent) {
xSemaphoreGiveFromISR(i2cSemaphore, NULL);
vTaskSwitchContext();
}
portRESTORE_CONTEXT();
asm volatile ("reti");
}
However, before of opening the issue I would like to share some ideas so that maybe I can propose a better fix than the one we have discussed previously
I think that it would be helpful to have a stack dedicated to interrupt handlers. In fact my understanding is that right now interrupt handlers use the stack of the task that was interrupted. If inside of an interrupt handler there is significant stack usage (maybe calling nested functions) all of the task must have a stack big enough to handle the interrupt handler (since the interrupt could be fired when each of the task is in execution)
My idea was to have all interrupts declared as naked and than as first thing a macro that saves the context and redirects the stack pointer to a stack dedicated to interrupts.
Then when exiting the interrupt there would be another macro switching back to the previous stack (or a call to vTaskSwitchContext if something like a semaphore was released), restores the context and executes “reti”.
Let me know if this can be a good add or if right now it would be better to open the issue with the fix we have discussed
One danger of that is that you then CAN’T EVER nest interrupts, as the interupt stack would already be in use when you switched to it (unless you checked for that case before loading the new stack pointer).
That port may not currently support nested interrupts, but that just adds another hurdle to trying to implement them.
Note that this a port from one of our partners (hence the Parter-Supported-Ports repo) and the author may provide inputs once you open an issue or a PR.
@aggarg you are definitely right saying that being AVR_AVRDx a port from a partner they should be the one modifying it
also thanks to @richard-damon for pointing out the issue with nested interrupt, maybe I will try to implement the interrupt-dedicated stack by myself and then come back to know what you guys think about it
I opened the issue on the github page, since this was my first time opening an issue if you have any suggestion on how to make it clearer please let me know!