| View previous topic :: View next topic |
| Author |
Message |
SeeCwriter
Joined: 18 Nov 2013
Posts: 160
|
| Interrupt warning |
 Posted: Thu Dec 10, 2015 12:52 pm |
 |
|
I'm getting a number of warnings "Interrupts disabled during call to
prevent re-entrancy".
I think it has to with some function calls being made in an external isr.
Those functions read and write some data in an external EEPROM via SPI.
The SPI bus doesn't use interrupts, and I don't think "strlen()" uses
interrupts. I'm not using printf()'s or debug statements in those functions.
So, is there something I need to be concerned about?
Using v5.051 IDE. |
|
 |
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Thu Dec 10, 2015 1:11 pm |
|
|
If there is a function(s) used both within the isr and outside the isr, then you'll get this warning.
There are two ways around this issue.
1. Find out which function(s) are being used both within and without the isr. For example, if it is strlen() that is the cause of the issue, define
| Code: | int16 my_strlen(...) {
return (strlen(...));
}
|
and then use my_strlen() inside the isr. The compiler then can isolate the two functions which will eliminate the warning.
2. Refactor your code to NOT perform the EEPROM access inside of an isr. Much better to simply assert a flag which is then tested in your main() loop, and the EEPROM access is performed there. Remember that an isr should be short - get in, get out because time spent in an isr is time the processor may be missing other, more important, events. |
|
|
SeeCwriter
Joined: 18 Nov 2013
Posts: 160
|
|
| Posted: Thu Dec 10, 2015 1:51 pm |
|
|
Suggestion number 1 still generates the warning. The compiler is apparently smart enough to know that the wrapped function is used elsewhere.
Suggestion 2 is problematic. The interrupt is signaling the PIC that power is going down. The PIC has approx. 50ms to save some data to non-volatile memory before power is gone. The EEPROM takes about 5ms per data item to write, and there are 3-pieces of data. So that reduces the time to 35ms. |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Thu Dec 10, 2015 1:57 pm |
|
|
| If 1 didn't work, then there's probably other functions being used within & without. #1 definitely works - you just have to identify the function(s) you have to isolate. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1358
|
|
| Posted: Thu Dec 10, 2015 1:58 pm |
|
|
any function you call in both the ISR and the main code can cause problems...even simple ones. Additionally, a lot of seemingly simple math operations like division or large number multiplication can cause you the same issue, since they use function calls under the hood sometimes.
I can't speak for all chips, but on the PIC24 series for example, CCS allocates almost all variables using a direct memory addressing method. Take the following function for example:
| Code: |
size_t strlen(char *str){
size_t length = 0;
while(str[length] != '\0'){
length++;
}
return length;
}
|
On a PIC24, the compiler will pick a specific memory location for the variable "length", say address 0x800. It doesn't use the stack or registers in this case.
When the main code calls this method, say it goes through a couple of loops so that length is 2. This value is currently stored at address 0x800. Now lets say an interrupt that also calls strlen occurs. the very first line of the function will set length (still at address 0x800) to 0, totally erasing the value of 2 that you had in the main code. The it will continue on until a new length is calculated, saving that value at 0x800 and leave the interrupt. Now you are back in the main code version. When you left you had length=2 and weren't finished with the looping. However, when you came back, you now have another value (whatever the ISR generated)...like 5. Additionally, the pointer to the string you called in the main will probably be replaced with the pointer you used in the ISR.
This can lead to a lot of bugs. Now consider other functions you call that may call strlen() and you don't even realize it. This is why CCS disables interrupts for those calls.
It is very important (as stated by others) to keep interrupts short and simple to avoid issues like this. Even though this is a warning, I would recommend refactoring your code to avoid generating this warning as it can have performance impacts in some cases.
Last edited by jeremiah on Thu Dec 10, 2015 7:40 pm; edited 1 time in total |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1358
|
|
| Posted: Thu Dec 10, 2015 2:01 pm |
|
|
some other options:
1. inline the function. This tells the compiler to use multiple copies with their own memory. EDIT: However, you still need to be careful of any passed in pointer variables that are not atomic! you need to make sure you don't clobber those.
2. make a duplicate function for the ISR and call that instead. |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Thu Dec 10, 2015 3:31 pm |
|
|
| jeremiah wrote: | | ...Additionally, a lot of seemingly simple math operations like division or large number multiplication can cause you the same issue... |
Had that happen with _MUL once. I was accessing a large array in an interrupt and the "under the hood" code was using _MUL to calculate the offset. I ended up refactoring my code to avoid the problem entirely. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19553
|
|
| Posted: Fri Dec 11, 2015 1:54 am |
|
|
Yes, and if the array is large, then it uses the int16 multiplication, even worse....
Generally, though a 'warning', and therefore not 'code threatening', it is telling you that perhaps you are trying to do more in the interrupt than you should. Newguy's answer is 'spot on', in that he 're-thought' and was able to avoid this. So consider 're-thinking' the interrupt code.
Generally keeping the interrupt code quick and short, is better in a whole lot of ways. Avoids the likelyhood of this, but also keeps the worst case latency down (remember that if you are in an ISR, and another interrupt occurs, the latency for the second interrupt, gets extended to include the time taken handling the first.....). Operations performed inside ISR's should be kept small/simple. Specifically things involving hardware (like SPI/I2C), should be very carefully done, with code either fully in the ISR, and ISR driven, or fully outside. Problem here is that though you can avoid warnings by duplicating code, you can't duplicate the hardware itself, and if the hardware is in the process of transferring a byte from a routine in the main code, then this must be finished before any transaction in the ISR.....
Some things are 'tricks'. For instance the classic one is handling the modulus when dealing with buffers. %32 will be performed by the compiler using the and operator, but %30, will have to use division. There really should be a warning in the example code suggesting that you keep buffer sizes to binary multiples (8,16,32 etc.), when using this code, or use alternative ways of handling this (methods posted here many times). |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Fri Dec 11, 2015 7:27 am |
|
|
Since your interrupt is to alert to power down, there is a way you can refactor the code to remove the EEPROM access from inside the ISR but yet retain the urgency of the matter.
Assuming your superloop is laid out to test for various flags, simply alter it thusly:
| Code: | while (TRUE) {
if (flag1 && normal_run_flag) { }
if (flag2 && normal_run_flag) { }
if (!normal_run_flag) {
// power loss imminent - write power down information to external EEPROM
// do your SPI writes here
while (TRUE); // remain here until power down
}
} |
In your imminent power loss ISR, change it to set normal_run_flag = FALSE;
When that interrupt fires, it effectively locks out all normal operations which then means that your critical task (writing to EEPROM) will be handled more-or-less immediately. Same net effect as your original code, but done in a manner that avoids the compiler warning. |
|
|
SeeCwriter
Joined: 18 Nov 2013
Posts: 160
|
|
| Posted: Mon Dec 14, 2015 10:19 am |
|
|
Regarding option 1. The code below still produces the interrupt warning.
| Code: |
#INT_EXT
void EXT_isr(void)
{
isrSetSumFault( TRUE );
}
void SetSumFault( bit sf_on )
{
// intentionally blank for testing.
}
void isrSetSumFault( bit sf_on )
{
SetSumFault( sf_on );
}
|
Edit: But if I remove the function call from isrSetSumFault() and replace it with the same code block that's in SetSumFault() the warning goes away. So it sure appears to me the compiler is smarter than it's being credited.
I'm going to see what I can do with flags. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon Dec 14, 2015 11:09 am |
|
|
I made a test program from your code and it doesn't show any warnings.
I had to add a function prototype to get it to compile. Without that, it gives
an "Undefined identifier -- isrSetSumFault" error. Also, there is no 'bit'
data type in CCS. I changed it to 'int1' so it would compile.
I tested this with compiler vs. 5.051.
| Quote: | Executing: "C:\Program files\Picc\CCSC.exe" +FH "PCH_Test.c" +DF +LY -T -A +M -Z +Y=9 +EA #__18F4620=TRUE
Memory usage: ROM=0% RAM=1% - 1%
0 Errors, 0 Warnings.
Build Successful.
Loaded C:\Program Files\PICC\Projects\PCH_Test\PCH_Test.cof. |
| Code: |
#include <18F4620.h>
#fuses INTRC_IO, NOWDT
#use delay(clock=4M)
void isrSetSumFault( int1 sf_on ); // *** Added function prototype
#INT_EXT
void EXT_isr(void)
{
isrSetSumFault( TRUE );
}
void SetSumFault( int1 sf_on )
{
// intentionally blank for testing.
}
void isrSetSumFault( int1 sf_on )
{
SetSumFault( sf_on );
}
//=====================================
void main()
{
while(TRUE);
} |
|
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1358
|
|
| Posted: Mon Dec 14, 2015 11:43 am |
|
|
I think they are calling SetSumFault( TRUE ) in the main as well. That generates the warning for me.
I would say make a duplicate copy of the function. That way you have two separate copies for each of the contexts (ISR and main).
Optionally, if the function is simple and does only atomic accesses, you can inline the ISR version and call that in the main wrapper version, but you need to be careful because the ISR can clobber your main version this way. |
|
|
SeeCwriter
Joined: 18 Nov 2013
Posts: 160
|
|
| Posted: Mon Dec 14, 2015 11:43 am |
|
|
| PCM Programmer: I think you're version works because you don't call SetSumFault() in main(). |
|
|
SeeCwriter
Joined: 18 Nov 2013
Posts: 160
|
|
| Posted: Mon Dec 14, 2015 11:47 am |
|
|
Making two copies of the function, with one only called from the isr solves the issue.
Edit: But I still need to use a flag because I can't (don't want to) duplicate the EEPROM driver functions. Hopefully I can detect the flag soon enough to be able save data to EEPROM. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19553
|
|
| Posted: Mon Dec 14, 2015 1:53 pm |
|
|
As a comment though, why worry?.
When the EEPROM is being written, that write _must_ complete, before you can start another. As such the best thing to do, is to disable the interrupts around every use of the EEPROM write, which is what the compiler is doing for you....
Whatever method you use, your power system will have to ensure that the supply is maintained for long enough for the writes needed on the power fail, and also if you are in the middle of a write, for this one to complete first. The write is a hardware function, and once triggered, has to complete before you can initiate another write. If you duplicated the functions, you will corrupt the EEPROM.
The writes typically take about 4mSec each (faster on some newer chips, check the data sheet for your chip), so you need to sit down, calculate how many writes are in your ISR, so how long these need, then add the time needed to get into the ISR, and the time needed to complete any write being done when the interrupt occurs (and add any time needed to complete any other ISR that is already executing). This total, is what your supply has to be able maintain, from the moment that the power fail is detected, to the point where the supply drops to the minimum that guarantees EEPROM writes can be done. Generally not hard. If the unit switches off everything drawing unnecessary power, assume capacitor values at perhaps 50% of their 'rating', you can calculate whether your supply can do this.
You won't speed things up by trying to avoid the warning, The hardware write must complete. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
