Interrupts issue MCF52259

[adelvallem]

Hi everyone.

I'm developing and application running with Coldfirev2 MCF52259 and Codewarrior 11.1

I want to use a timer and decided to use the DMTIM0 and UART1

configured as follows:

void DTIM0_Init(void)
{
  /* DTMR0: PS=0,CE=0,OM=0,ORRI=1,FRR=1,CLK=2,RST=0 */
  MCF_DTIM0_DTMR = 0x1CU;             
  MCF_DTIM0_DTRR = 0xC350UL;     //10mS
  /* DTXMR0: DMAEN=0,HALTED=0,??=0,??=0,??=0,??=0,??=0,MODE16=0 */
  MCF_DTIM0_DTXMR = 0x00U;   
  /* DTER0: ??=0,??=0,??=0,??=0,??=0,??=0,REF=1,CAP=1 */
  MCF_DTIM0_DTER = 0x03U; 
  /* DTMR0: RST=1 */
  MCF_DTIM0_DTMR |= 0x0001U;  
  MCF_INTC0_ICR19 = 0x3CU;
}

void UART1_Init(void)
{
  MCF_GPIO_PUBPAR = 0x05;                         // PUBPAR: PUBPAR1=1,PUBPAR0=1
  setReg8(MCF_UART1_UCR, 0x30U);                  // reset transmiter
  setReg8(MCF_UART1_UCR, 0x20U);                  // reset receiver
  setReg8(MCF_UART1_UCR, 0x10U);                  // reset the mode pointer
  setReg8(MCF_UART1_UIMR, 0x02U);                 // enable rx to generate interrupts also Tx(?)
  setReg8(MCF_UART1_UACR, 0x00U);                 // 
  setReg8(MCF_UART1_UCSR, 0xDDU);                 // internal clock for tx & rx
  setReg8(MCF_UART1_UMR1, 0x13U);                 // no parity 8 bits
  //setReg8(MCF_UART1_UMR1, 0x53U);                 // FFULL interrupts, no parity 8 bits
  setReg8(MCF_UART1_UMR2, 0x07U);                 // 1 bit stop
  setReg8(MCF_UART1_UBG1, 0x00U);                 // set to 38400
  setReg8(MCF_UART1_UBG2, 0x41U);
  setReg8(MCF_UART1_UCR, 0x05U);                  // enable transmiter and receiver
  MCF_INTC0_ICR14 = 0x3F;                         // ICR014: IL=7,IP=7
}

both work almost as expected.

My issue is that from time to time the uart doesn't get serviced on time or I don't know but I get  an overflow error in the ISR which was defined as follows:

__declspec (interrupt) void UART1_isr()
{
  if((MCF_UART1_USR & MCF_UART_USR_OE) == MCF_UART_USR_OE) /
  {
    UART0_Send_Char('\x21');     // if error send '!'
    MCF_UART1_UCR |= MCF_UART_UCR_RESET_ERROR;
  }
  if((MCF_UART1_UISR & MCF_UART_UISR_FFULL_RXRDY) == MCF_UART_UISR_FFULL_RXRDY)
  { 
    rxbuff[rxbuffpos] = MCF_UART1_URB; 
    rxbytes++;
  }
}

I know I have an error because I get the '!' in my terminal.

At certain times I disable the interrupts to get a value without any risk of a sudden change.

DISABLE_ALL_INTERRUPTS;
bytes = rxbytes;
ENABLE_ALL_INTERRUPTS;

Now I noticed I have an spurious  interrupt, and created a simple ISR in order to know what is going on and placed a breakpoint to read the interrupt registers at that moment.

I noticed the IPRL0 bits are not being cleared when the interrupt is serviced

Is this the normal behaviour? the manual says:

0 The corresponding interrupt source does not have an interrupt pending
1 The corresponding interrupt source has an interrupt pending

So shouldn't they be cleared?

Every time my code disables the interrutps I get this spurious int.

my timer routine is

__declspec(interrupt) void DTIM0(void) //1ms
{
  //MCF_DTIM0_DTER = 0x01;
  MCF_DTIM0_DTER = 0x03;
  MCF_DTIM0_DTMR &= ~MCF_DTIM_DTMR_ORRI;
  ++tick10ms;
  if (tick10ms >= 251) tick10ms = 1;

  if ((tick10ms % 5) == 0)
  {
    ++tick50ms;
    if (tick50ms >= 251) tick50ms = 1;
  }
  MCF_DTIM0_DTCN = 0;
  MCF_DTIM0_DTMR |= MCF_DTIM_DTMR_ORRI;
  MCF_DTIM0_DTMR |= 0x0001U;
}

Any help is welcome, I need to find out why my uart is getting overflowed, and I thought this might be the reason, but I'm kinda lost.

Thank you

[adelvallem · Accepted solution]

Hi TomE.

I found in the uC manual in section 28.5.1.1 this

Unmask appropriate bits in the core’s status register (SR) to enable interrupts.

I ignored this instruction because my interrupt had already been defined in the highest level and the SR section says about the bits 10-8

Interrupt level mask. Defines current interrupt level. Interrupt requests are inhibited for all priority levels less than or equal to current level, except edge-sensitive level 7 requests, which cannot be masked.

Anyway I changed this bits, at boot up they are configured to 7 level, and I set them to level 6, as follow

This is the very first thing my code does, to avoid any sudden interruption

  asm
  {
    move.w  d7,temp_reg
    move.w  sr,d7
    and.l   #0xf6ff,d7
    move.w  d7,sr
    move.w  temp_reg,d7
  }

What I guess is that my uart interrupt isn't an edge sensitive request.

adelvalle

[TomE]

Your "solution" is wrong on so many (interrupt) levels.

First, Level 7 interrupts cannot be masked. You can't disable them. They're the same as the NMI (Non Maskable Interrupt) on other primitive CPUs that only have one normal and one NMI interrupt. That means that you can't disable interrupts in critical regions. It also means that when the code is in the middle of an interrupt service routine it can (and will) get interrupted by the SAME interrupt.

So on this architecture (M68K and Coldfire) you should never use this NMI except for "unrecoverable and serious errors".

It isn't an "edge triggered interrupt". Not like you'd expect from other architectures.

Next you have to obey the note:

16.3.6 Interrupt Control Registers (ICRnx)

It is the responsibility of the software to program the ICRnx registers with unique and non-overlapping level and priority definitions. Failure to program the ICRnx registers in this manner can result in undefined behavior. If a specific interrupt request is completely unused, the ICRnx value can remain in its reset (and disabled) state.

So your "MCF_INTC0_ICR19" and "MCF_INTC0_ICR14" have to be set to different values. I would recommend 0x80 and 0x81.

The normal way to enable interrupts is to set the status register bits to zero, not 6. Unless you're using multiple levels (and allowing interrupts at one level to interrupt the execution of other levels) the only sensible values for the IPL bits is all zeros or all ones.

What does the "ENABLE_ALL_INTERRUPTS" macro you're using do? Why aren't you using that to enable interrupts after startup if it is enabling interrupts the right way?

Then you have to watch out for this:

16.3.2 Interrupt Mask Registers (IMRHn, IMRLn)

A spurious interrupt may occur if an interrupt source is being masked in the  interrupt controller mask register (IMR) or a module’s interrupt mask register while the interrupt mask in the status register (SR[I]) is set to a value lower than the interrupt’s level. This is because by the time the status register acknowledges this interrupt, the interrupt has been masked. A spurious interrupt is generated because the CPU cannot determine the interrupt source.
To avoid this situation for interrupts sources with levels 1–6, first write a higher level interrupt mask to the status register, before setting the mask in the IMR or the module’s interrupt mask register. After the mask is set, return the interrupt mask in the status register to its previous value. Because level 7 interrupts cannot be disabled in the status register prior to masking, use of the IMR or module interrupt

So why is it so complicated? Because it is a "real computer" and not a 1970's 8-bitter with bits bolted on.

The recommended way to handle the interrupt acknowledge cycle is not to write the ACK bit with an assign, but to read the register and then write that same value back. That automatically handles the case where there are multiple interrupts indicated in the one register and you want to handle all that are present. If you do "reg |= bit" you risk clearing ones you didn't intend to and losing interrupts.

Tom

[adelvallem]

Hi TomE.

Wow I was really failing to understand the interrupts process.

I followed your advices, my interrupts are no longer level 7,  their respective ICR value were never the same value. Also initialized the SR  to 0, I haven't yet implemented the interrupt acknowledgment changes but so far the application is working correctly.

I still have one doubt, you mentioned referring to spurious interrupts in the RM:

 This is because by the time the status register acknowledges this interrupt, the interrupt has been masked.

Then my macro ENABLE_INTERRUPTS which does the next thing would be incomplete

#define INT14_EN  0x4000                // UART1
#define INT19_EN  0x80000               // DTIM0

#define ENABLE_BITS    (unsigned long)(MCF_INTC_IMRL_MASKALL| INT19_EN | INT14_EN)

#define ENABLE_ALL_INTERRUPTS   MCF_INTC0_IMRL = ~ENABLE_BITS
#define DISABLE_ALL_INTERRUPTS  MCF_INTC0_IMRL = MCF_INTC_IMRL_MASKALL

Then, what would be the best approach to disable the interrupts? 

disable_interrupts()
{
- set SR to a high level
- set IMRH & IMRL
- set SR back to its original level
}

?

Thank you.

adelvalle

[TomE]

The best way to disable the interrupts is the IPL bits in the status/control register. Write a small assembly function that reads the 16 bit CCR, or's in "7" and writes it back. Enable by anding to zero those bits.

That is the "master CPU interrupt enable" bit of the system.

That's equivalent to the usually single "interrupt enable bit" in the control/status register of simpler CPUs.

Normally you shouldn't have to touch the registers in the interrupt controller after it has been set up.

There's a lot of registers there, but a lot of them are for debugging. To "add visibility" to what the interrupt controller is doing (like all the interrupts currently pending and what vector it is going to generate when allowed to). You don't need this unless something's not working like you'd expect. Then having all the visible is useful.

"Not touching the interrupt controller" applies with sensible peripherals. Something like a UART. You use the individual interrupt enables, status and ack bits in the peripheral registers and ignore the controller completely.

Where you might have to mess with the enable bits in the interrupt controller (is when you have something like a GPIO bit causing an interrupt when it is at a certain level, and you can't force it back off again in the interrupt handler. Then you may need to disable the interrupt in the controller and then enable it when you want it to trigger again. There are also "broken" peripherals in some chips that don't play nicely (they weren't designed for this system, but came from somewhere else), and with them you might have to disable them in the IMR register.

If you're using the "LEVEL" interrupts or the Software ones, then the "ack bits" are in the interrupt controller.

In summary:

1. With the CPU IPL at "7"
2. Setup all the peripherals (or do one at a time, whatever)
3. Enable interrupts in a peripheral and enable in the IMR registers
4. When ready to run... Set the CPU IPL to "0".

When an interrupt happens:

1. In the ISR, read the peripheral status to see which interrupts are pending,
2. ACK all pending by writing that exact work back to the register, OR
3. Mask out (zero) all bits in the read copy you're not handling, set the one you are and write that back
4. Handle the interrupt, like reading a byte from the UART. That removes the interrupt request
5. Do something with that byte (store or whatever)
6. Return. That's it. You don't have to do any more.

When you're in the mainline and want to access some variables shared with an interrupt routine, in order to access without the interrupt happening - this INCLUDES disabling interrupts in peripherals or in the IMR bits:

1. Disable interrupts in the CCR
2. Do whatever you need to do
3. Enable interrupts in the CCR

If you find you want to use multiple levels - different levels for different interrupts, you can't justdisable/enable. You should read and save the current CCR IPL values, then disable, then access, and then RESTORE the original ones. That's because interrupts AREN'T DISABLED in an interrupt service routine. Only ones up to the level of that interrupt are disabled. Higher ones can interrupt the interrupt service routine.

So disabling interrupts looks like:

int old_ipl = asm_set_ipl(7);
... mess with whatever...
asm_set_ipl(old_ipl)

Where that function is in the "mcf5xxx.S" file in our system:

asm_set_ipl:
_asm_set_ipl:
  link  a6,#-8
  movem.l d6-d7,(sp)

  move.w  sr,d7   /* current sr  */

  move.l  d7,d0   /* prepare return value  */
  andi.l  #0x0700,d0  /* mask out IPL  */
  lsr.l #8,d0   /* IPL   */

  move.l  8(a6),d6  /* get argument  */
  andi.l  #0x07,d6    /* least significant three bits  */
  lsl.l #8,d6   /* move over to make mask  */

  andi.l  #0x0000F8FF,d7  /* zero out current IPL  */
  or.l  d6,d7     /* place new IPL in sr   */
  move.w  d7,sr

  movem.l (sp),d6-d7
  lea   8(sp),sp
  unlk  a6
  rts

You should have been provided with a function like the above somewhere in all of the source files and libraries in your system.

We're using 125 interrupts in our system. So you want to be careful how you assign these. best to have them all in the one table in the one file so you can make sure you don't have them assigned twice or something. You can also see which ones are higher than which other ones.

BTW, the multiple levels allow interrupts to interrupt lower ones while they're running. It allows the CPU to set itself to a middle level to block all the lower ones while allowing the higher ones. The multiple (8) different priorities within one level only controls which one gets to run first when multiple ones at the same level are pending. Good luck...

Tom

[TomE]

The good news is that these are really old chips, so the chance that you're the first person to get something wrong is pretty close to zero.

I can see about half of the necessary interrupt setup code, but can't see the rest.

If you haven't programmed all of the OTHER registers, then that's probably part of your problem.

Type "spurious" into the search bar on these pages and read through the results.

https://community.nxp.com/t5/ColdFire-68K-Microcontrollers/Coldfire-V2-Kernel-ISR-invoked-even-while...

But read through the "04-25-2017 01:29 AM" reply on this one:

https://community.nxp.com/t5/ColdFire-68K-Microcontrollers/Spurious-interrupt-with-declspec-interrup...

Tom