Interrupt handling pattern (OSIntEnter vs. OSIntEnterWAS)

[ritchie]

Hi,

I see that OSIntEnter( ) is still declared in <ucos.h>, but defined as OSIntEnterWAS( ) in ucos.c. Is there a reason for this mismatch? (I'm using NNDK 2.6.8 if that makes any difference.) The matching OSIntExit( ) is still declared and defined correctly.

From the uCOS documentation I understand that I should be wrapping my interrupt handler in an enter/exit pair to prevent a task waiting on a semaphore from inadvertently executing before the ISR returns. Is the following pattern correct to not execute system calls from the interrupt handler? What is the correct way to call OSIntEnter( )?

Also, is there a reason why SetPinIrq( ) doesn't take a context pointer of some sort (similar to OSTaskCreate( ) for example) so a single ISR can trampoline out to a bunch of different processing tasks?

Thanks!

Code: Select all

// need a singleton semaphore instance somewhere because we can't get a ctx pointer into the interrupt handler?
OS_SEM myObj::IRQ1OSSem;

void myObject::HandleInterrupt( /* no ctx ptr possible? */ ) {
	OSIntEnter( ); // undefined, implemented in ucos.c as OSIntEnterWAS( )

	OSSemPost( &IRQ1OSSem );

	OSIntExit( );
}

// Make sure the stack is 4 byte aligned to keep the Coldfire happy
DWORD ProcessTaskStack[ USER_TASK_STK_SIZE ] __attribute__( ( aligned( 4 ) ) );

void myObject::ProcessTask( void *ctx ) {
	myObject *obj = ( myObject * ) ctx;
	while ( 1 ) {
		uint8_t status = OSSemPend( &IRQ1OSSem, 0 /* wait forever */ );
		if ( status == OS_NO_ERR ) {
			obj->doStuff( );  // do some higher latency system calls here outside of the interrupt handler
		} else {
			iprintf("task: ERROR - OSSemPend() returned %d\n", status);
		}
	}
}

myObject::myObject( ) {
	// set up processing task
	OSTaskCreate( ProcessTask,
			this,
			&ProcessTaskStack[ USER_TASK_STK_SIZE ],
			ProcessTaskStack,
			MAIN_PRIO + 1 );
	OSSemInit( &IRQ1OSSem, 0 );

	// init interrupt
	J2[ 45 ].function( PINJ2_45_IRQ1 );		// IRQ1 = pin 45
	SetPinIrq( 45, -1, &HandleInterrupt );	// active low
}

[dciliske]

NOTE: I misread what you were doing and thought you were directly creating an ISR. The solution you seek is in paragraph four, but I'd recommend reading all of this for educational purposes.

I assume that you are coming to this from the world of software and not of hardware. I'm going to split this up into three sections: what happens when an interrupt fires, what the code you are writing does, and what you want to actually do.

So, what happens when an interrupt is triggered by the processor? Let's take a timer interrupt for instance. So, you have a timer that ticks once per second, and that triggers an interrupt. When the timer ticks, it causes an interrupt request to be asserted to the interrupt controller in the processor. The interrupt controller then proceeds to verify that that interrupt is enabled and that it is a higher level than the current interrupt mask. At that point, the cpu is halted, and the controller performs a fetch from the exception Vector Table for the currently triggered interrupt. This is a table containing pointers to subroutines designed to handle the interrupt, the Interrupt Service Routine (ISR). After the fetch, the processor creates an exception frame on the stack and then jumps to the memory location specified from the vector fetch and starts executing. Note that at no time does there exist the capability of passing 'arguments' at the time of the interrupt triggering.

Now that we've established what happens when an interrupt fires, what does your code do? Well, OSIntEnterWAS increments the OSIntNesting count. That's it. OSIntExit will unwind the exception frame and restore to the highest ready task. The problem for an ISR is that an exception frame is not the same as a normal stack frame. That's the reason we have the INTERRUPT macro in 'cfinter.h'. A normal function will not unwind the stack frame correctly and will lead to garbage execution. Second, you are trying to have the interrupt trigger a C++ member function directly; this doesn't make sense. I think you'll understand why this is after reading the afore mentioned explanation of how interrupts fire.

At this point, I realized that you were using SetPinIRQ, not the SetIntC calls, and this explains a lot. I'll leave the first two parts, as they're useful explanations, but let's get down to what you want to do.

First, You don't need your calls OSIntEnterWAS or OSIntExit (the WAS part, by the way, is because the call was removed from usage; I'm not sure why it's still around). Second, given that you mentioned trampoline, you should understand that SetPinIRQ already is trampolining to your function. If you want to change what get's called at a later date, call SetPinIRQ and set the new function. You should also see how to make a function trigger your object's ISR method.

Code: Select all

myObject::myObject *ISRArg = NULL;
void trampoline()
{
    ISRArg->HandleInterrupt();
}

void myObject::HandleInterrupt( /* no ctx ptr possible? */ ) {
   OSSemPost( &IRQ1OSSem );
}

myObject::myObject( /* insert args here */ )
{
....
    ISRArg = this;
    SetPinIRQ( 45, -1, &trampoline );
}

As for calling OSSemPost, just do it like you did (just without the OSInt calls).

-Dan

[ritchie]

Thanks, that clarifies things a bit. If OSIntEnter/OSIntExit don't do what I think they should due to exception vs. normal stack frames, do I need to switch to the INTERRUPT macro?


You're correct that I'm coming from the software world more than the hardware world, and it's been a while since I've messed around with interrupts at this level.

Based on your comment that SetPinIRQ is already trampolining, I assume that my function (member or otherwise) is not the one pointed to by the dispatch table, and the trampoline set up by SetPinIrq( ) also doesn't keep its own table to wrap a context pointer.

My mistake, I didn't document in my code snippet that I'd declared the member function as static and was intending to pass in the object via a context pointer. I've done this before to simulate an actual member function when a singleton or static function is required to be called from a C context.

Can I request that both OSIntEnter( ) and OSIntExit( ) be documented as deprecated in both the header and source files of a future NNDK? It's a bit confusing trying to understand how they should be used from the sources alone based on the fact that one appears removed and the other doesn't. It is also not immediately clear what calls are interrupt safe or not, but maybe I haven't gotten to that part of the documentation yet.

I wasn't necessarily wanting to change what gets called by the SetPinIrq( ), I was after the following pattern:

Code: Select all

OS_SEM IRQ1SEM;
OS_SEM IRQ2SEM;

void init( ) {
	// init interrupt 1
	J2[ 45 ].function( PINJ2_45_IRQ1 );
	SetPinIrq( 45, -1, &shared_trampoline, &IRQ1SEM );

	// init interrupt 2
	J2[ 43 ].function( PINJ2_43_IRQ2 );
	SetPinIrq( 43, -1, &shared_trampoline, &IRQ2SEM );

	// even better: avoid writing my own shared_trampoline wrapper altogether, since it's so common to just post a semaphore from an interrupt and return?
	OSSemSimplePostOnIrq( 43, -1, IRQ2SEM );
}

void shared_trampoline( void *ctx ) {
	OS_SEM *sem = ( OS_SEM * ) ctx;
	OSSemPost( sem );
}

[dciliske]

First, yes, you should use the INTERRUPT macro, not OSIntEnter/OSIntExit; read the following massive explanation to understand why. No, OSIntEnter and OSIntExit should not be marked deprecated. They are still a vital necessity; they simply aren't needed by most users.

Ok, I've been thinking this one over for the last couple days trying to figure out why your conceptions about OSIntEnter and OSIntExit are misguided. It's one of those things where you know something so deeply, that you can intuit a correct or incorrect understanding, but articulating it is hard. Well, here goes the explanation...

Let's start the discussion by taking a large step back... the operating system you are using is largely based on uC/OS, which stretches back over 20 years (In fact the book on it I have in front of me is copyright 1992...). It was originally written for the Intel 80188. At that time, everything was absolutely written in assembly or, at most, C. Keep in mind that these were 8 or 16 bit micros running in the 8-25MHz. When it comes to ISRs, those were pretty much guaranteed to be written in assembly.

With this in mind, OSIntEnter and OSIntExit were never intended to be called from a C function. Furthermore, a normal C function cannot operate as an ISR on most processors. "Wait, what?!" I'm hearing you say. This is true, a normal C function cannot operate as an ISR, as most processors have a separate 'Return from Subroutine' and 'Return from Exception' (RTS and RTE) opcodes. This is certainly the case with the Coldfire. Since there's different return opcodes and they do different things with the stack, clearly the compiler can't know how you intend to use the function, so it will assume that it's just going to be called normally, and return with an 'rts'.

Since we've established that you can't use a C function directly as an ISR, how the hell do any of our interrupt functions actually work? By lying. That's how. Let's take a look at a representative example from 'Pin_Irq.cpp' for the MOD5441X and look at the trampoline ISR for IRQ1.

Code: Select all

// Pin J2.45
INTERRUPT( firq1_pin_isr, 0x2100 )
{
   sim2.eport.epfr = 0x02;   // Clear the interrupt flag
   if ( TheIrqFuncs[1] ) TheIrqFuncs[1]();
}

The important part here is that declaration 'INTERRUPT( firq1_pin_isr, 0x2100 )'. The important part is that this creates a function called 'firq1_pin_isr' that we can link to in the vector table and execute as an ISR. And in that function, it will execute the code we gave here. But what about that 0x2100? What about that all caps INTERRUPT? where's the return type? For this, let's take a look at the header 'cfinter.h'. In it we will find a macro defined called INTERRUPT.

Code: Select all

#define INTERRUPT(x,y)extern "C" { void real_##x();  void x(); } void fake_##x(){\
__asm__  (".global "#x);\
__asm__  (#x":");\
__asm__  ("move.w #0x2700,%sr ");\
__asm__  ("lea      -60(%a7),%a7 ");\
__asm__  ("movem.l  %d0-%d7/%a0-%a6,(%a7) ");\
__asm__  ("move.w (OSISRLevel),%d0 ");\
__asm__  ("move.l %d0,-(%sp) ");\
__asm__  ("move.l (OSIntNesting),%d0");\
__asm__  ("addq.l #1,%d0");\
__asm__  ("move.l %d0,(OSIntNesting)");\
__asm__  ("move.w #"#y",%d0 ");\
__asm__  ("move.w %d0,%sr ");\
__asm__  ("move.w %d0,(OSISRLevel)");\
__asm__  ("jsr real_"#x );\
__asm__  ("move.l (%sp)+,%d0 ");\
__asm__  ("move.w %d0,(OSISRLevel)");\
__asm__  (" jsr      OSIntExit  ");\
__asm__  ("movem.l  (%a7),%d0-%d7/%a0-%a6 ");\
__asm__  ("lea    60(%a7),%a7 ");\
__asm__  ("rte");} void real_##x()

A couple quick things to note real fast: this macro ends up creating three functions. In reality, it should only need to create 2, but to make the compiler happy about inline assembly, that needs to go inside a function. The C/C++ function that becomes the ISR, ends up being named 'real_<insertnamehere>', so in the case of the 'firq1_pin_isr', this creates the function 'real_firq1_pin_isr'. What then are you linking to when telling the vector table to use 'firq1_pin_isr'? That would be a wrapper subroutine, written in assembly, which first masks all interrupts (other than level 7), makes space on the stack, and saves off all the registers. It then saves the OSISRLevel as well, and then increments OSIntNesting directly (instead of calling OSIntEnter). If you look at the lock code around the increment in OSIntNesting inside of OSIntEnter, you'll see why the function call is unnecessary here. Finally, we set the status register to the second argument of the INTERRUPT macro (in our case 0x2100), set OSISRLevel equal to it, and then 'jsr' Jump, Set Return to our 'real' C ISR.

The C ISR then executes, and then returns. We restore the old OSISRLevel from the stack, and then call OSIntExit. Finally, the registers are restored from the stack, the stack pointer is restored, and we return from exception.


-Dan

[ritchie]

Thanks for the in-depth explanation Dan, much appreciated and that makes more sense. I always forget that even though C is 'closer to the metal' than most other languages, sometimes it's still too far away I'll switch to the INTERRUPT macro and give that a shot.

So my understanding is that OSIntExit( ) needs to be maintained to be called from the assembly definition of INTERRUPT, but OSIntEnter( ) is essentially inlined directly into it.

Based on your explanation these functions (at least OSIntExit( )) shouldn't be removed, but a note in ucos.h along the lines of "Recommended not to use these, use the INTERRUPT macro instead and see example xyz or talk to Dan if you don't know why" would be appreciated by those of us trying to learn how to use the system by reading through the library headers. Again it was the mismatch between OSIntEnter( ) (no WAS appended) declaration and OSIntEnterWAS( ) definition that was most unclear.

First, yes, you should use the INTERRUPT macro, not OSIntEnter/OSIntExit; read the following massive explanation to understand why. No, OSIntEnter and OSIntExit should not be marked deprecated. They are still a vital necessity; they simply aren't needed by most users.