Spurious Interrupts when implementing critical region.

[Claude_Sylvain]

Hello,

My question is about a ColdFire (MCF52235 MCU) behavior when implementing critical region to access safely data handled by the PIT1 peripheral interrupt service routine (PIT1 ISR).

Here is the situation:

I setup an interrupt service routine that simply increment a 32-bit counter ( named "ticks" ) every 10mS using the MCF52235 PIT1 peripheral.

I wrote an interface (function) that enable a function that run on main loop (at fast speed) to get the "ticks" value and do some processing.

To make the things safe, I implemented a critical region (disabling/enabling interrupts) inside that function to prevent that function to be interrupted by the PIT1 ISR, thus preventing data corruption.

The problem is that the only mean to create the critical zone is to disable/enable all MCU interrupts.  All other method, like disabling/enabling PIT1 interrupts using the PIE bit in the PIT1 PCSR register (using 'C' instruction or assembly instruction), or disabling/enabling PIT1 interrupts using the associated bit in the IMR register, or disabling/enabling PIT1 interrupts using the ICR56 register, are not working correctly.

When using those methods, there are Spurious Interrupts generated and all interrupt service routines running on the MCU are disrupted a LOT

Am I missing something, or is it possible that creating critical region by disabling/enabling all interrupts are the only mean to create a critical region that work well ???

Below, the significant part of the interface function (using critical region), that show you all the things I tryed to get rid of that problem:

Code:

BOOL ticks_cu_goal(ticks_t *ticks_goal, ticks_t interval){ BOOL result = NO; /* Enter critical region...  * ************************ *//* - This operation is not atomic and produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * -------------------------------------------------------------- */#if FALSE MCF_PIT1_PCSR &= ~MCF_PIT_PCSR_PIE;  /* Disable PIT1 interrupts. */#endif/* - This operation produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * ------------------------------------------------ */#if FALSE /* Disable PIT1 interrupts.  * ------------------------ */ asm {  lea  __IPSBAR[0x00160000+1],a0  bclr.b #3,(a0) };#endif/* - This operation produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * ------------------------------------------------ */#if FALSE /* Mask interrupt source 56 (PIT1). */ MCF_INTC0_IMRH  |= MCF_INTC_IMRH_MASK56;#endif/* - This operation produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * ------------------------------------------------ */#if FALSE /* Disable PIT1 interrupts by programming its Level and Priority. */ MCF_INTC0_ICR56 = 0;#endif/* Not really the better way to go, but it seems to be the only that work!— * ------------------------------------------------------------------------ */#if TRUE /* TODO: Use a bit instruction in assembly to put PIT1 interrupt off. */ mcf5xxx_lo_disable_all_irq();#endif  /* Process safely your data here.  * ****************************** */  /* Exit critical region...  * *********************** *//* - This operation is not atomic and produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * -------------------------------------------------------------- */#if FALSE MCF_PIT1_PCSR |= MCF_PIT_PCSR_PIE;  /* Enable PIT1 interrupts. */#endif/* - This operation produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * ------------------------------------------------ */#if FALSE /* Enable PIT1 interrupts.  * ----------------------- */ asm {  lea  __IPSBAR[0x00160000+1],a0  bset.b #3,(a0) };#endif/* - This operation produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * ------------------------------------------------ */#if FALSE /* Unmask interrupt source 56 (PIT1). */ MCF_INTC0_IMRH  &= ~MCF_INTC_IMRH_MASK56;#endif/* - This operation produce spurious interrupts *   and erronous operation of the PIT1 interrupts. * - DON'T USE. * ------------------------------------------------ */#if FALSE /* Enable PIT1 interrupts by programming its Level and Priority. */ MCF_INTC0_ICR56 = MCF_INTC_ICR_IL(TIMER_NETWORK_LEVEL) |          MCF_INTC_ICR_IP(0);#endif/* Not really the better way to go, but it seems to be the only that work!– * ------------------------------------------------------------------------ */#if TRUE /* TODO: Use a bit instruction in assembly to put PIT1 interrupt on. */ mcf5xxx_lo_enable_all_irq();#endif return (result);}

 

Regards,

Claude Sylvain

Electro-Technica inc.

[mccPaul]

Hi Claude,

It may be that you are trying too hard to protect access to the timer counter. You say that the timer ISR sets a counter and I assume that this is a 32 bit variable.

If so, the compiler should produce code that will atomically write the new value in the ISR. As all of the other uses of this variable should be atomic reads as well, you can assume that no critical section is required.

Spurious interrupts will only occur when the ColdFire's interrupt module cannot determine the source of an interrupt. This happens when the interrupt source is being masked in the IMR or the module's interrupt mask register and the current interrupt mask in the status register is set to a level below the interrupt's level. (This is from a note in the MCF5282 manual chapter 10, section 10.3.2, should be somewhere similar in the 52235 manual).

Basically this means that you shouldn't modify an interrupt mask without masking interrupts of that level and below in the status register if you don't ever want to see a spurious interrupt exception.

The code below is what I use for setting interrupt vectors for the 5282.

Cheers,

Paul.

Code:

void mcf5282_interrupt_init(uint8 source, uint8 ipl, void (*handler)(void)){ // Only for user defined vectors in INTC0 if ((source > 0) && (source < 63))  {  // Interrupts should be disabled to avoid vector problems  // and to ensure that a spurious interrupt exception can't  // be generated.  uint8 sysint = asm_set_ipl(7);  if (handler)  {   // Set interrupt priority level   MCF5282_INTC0_ICR(source) = (ipl & 0x3F);       // Set vector   mcf5xxx_set_handler(source+64, (ADDRESS)handler);    // Clear mask for this handler   if (source < 32)    MCF5282_INTC0_IMRL &= ~(MCF5282_INTC_IMRL_INT(source)          | MCF5282_INTC_IMRL_MASKALL);   else   {    MCF5282_INTC0_IMRL &= ~(MCF5282_INTC_IMRL_MASKALL);      MCF5282_INTC0_IMRH &= ~(MCF5282_INTC_IMRH_INT(source));   }  }  else  {   // Set vector   mcf5xxx_set_handler(source+64, (ADDRESS)handler);    // Set mask for this handler   if (source < 32)   {    MCF5282_INTC0_IMRL |= MCF5282_INTC_IMRL_INT(source);   }   else   {    MCF5282_INTC0_IMRH |= MCF5282_INTC_IMRH_INT(source);   }  }       // As you were...  asm_set_ipl(sysint); }}

 

[Claude_Sylvain]

Hello Paul,

You're right about the atomic nature of 32-bit variables on ColdFire processors, and it is better to me to consider this fact to avoid to protect too hard, as you said.

However, this ColdFire behavior is new too me, and I am really surprised that a programming technic that work perfectly well with the Freescale HC08 MCU's, do not work correctly with the ColdFire processors.

Disabling only the ISR concerned when entering a critical region have the advantage of not disturbing all other system ISR (and associated functionnalities), and I think it is not a thing disregard.

So, disabling all interrupts for implementing critical region is (for me) not the ideal way to go.

Also, the kind processing done on some (ISR) interface function make mandatory the use of critical region, even if the data type used is atomic (and declared as volatile).

Thank you for your code snippet.

Regards,

Claude Sylvain

Electro-Technica inc.