This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

C-func call in ISR seems to cause stack corruption

Target: ST uPSD3454 running at 36.8864 Mhz

What seems simple is causing extreme confusion.

Enhanced template code from Keil:

;  Define instructions executed on a hardware timer interrupt.
HW_TIMER_CODE   MACRO
        EXTRN   CODE (TIMER_Update)
        USING   2       ; Registerbank 2 for following code
        LCALL   TIMER_Update ; C-func
        RETI
        ENDM


C-func:

void TIMER_Update( void )
{   unsigned char data i;

    for( i=0; i<NUM_TIMERS; i++ )
    {   timers[i]--;
        if( timers[i] == 0 )
        {   timers[i] = reload[i];
            if( events[i] < 255 )
                events[i]++;
        }
    }
}


HW_TIMER_CODE is ISR and occurring ever 10ms.
TIMER_update is attempt to implement software timers for additional systems functionality.

After about the 3rd HW_TIMER_CODE execution (between 30 and 50ms) system resets.

No stack corruption is detected in trace.

What silly thing am I overlooking?

  • Check to make sure that your TIMER_Update( ) function is operating properly with the USING 2 register bank.

    You might need to also add USING 2 to the TIMER_Update( ) function, or omit the register bank part in the ISR vector routine.

    There might be other problems, but that's the first one I noticed as being a potential problem.

    Oh, the other problem... I hate macros. All of them.

    --Cpt. Vince Foster
    2nd Cannon Place
    Fort Marcy Park, VA

  • Currently functional code:

    New code snippets:

    ;  Define instructions executed on a hardware timer interrupt.
    HW_TIMER_CODE            MACRO
    ;  start of user code to update soft timers via C-func
            EXTRN   CODE (TIMER_Update)
            PUSH    ACC
            LCALL   TIMER_Update
            POP     ACC
            RETI
            ENDM
    


    and the C-func:

    void TIMER_Update( void ) using 2 
    {   unsigned char data i;
    
        for( i=0; i<NUM_TIMERS; i++ )
        {   timers[i]--;
            if( timers[i] == 0 )
            {   timers[i] = reload[i];
                if( events[i] < 255 )
                    events[i]++;
            }
        }
    }
    

    There still seems to be a discrepancy whether or not it would be better (more efficient) to wrap a full context switch in the assembly code (PUSH ACC, PSW, .. POP AR7, AR6, ...) around the call to the C_func rather than using a different register bank.

    This is code that is running under RTS51 Tiny so this exercise will use Reg Bank 0 (in general) Reg Bank 1 (for the timer interrupt) and then extend the timer interrupt to use Reg Bank 2 just for this C-func.

  • You might need to also add USING 2 to the TIMER_Update( ) function, or omit the register bank part in the ISR vector routine.

    Maybe I should have said "exclusive or" (XOR).

    My point was to ensure that both code sections are identical. If you are going to use the "USING" command, do it in both places. Otherwise don't use the "USING" directive in either place (maybe).

    Not that I think that this is your problem, it is just a risk area I saw in your code.

    The Keil compiler does not know if one module has a different 'USING' value than another module. When you have a file "Timer.C" and another file "Interrupts.C" and one of them uses a non-standard "USING" directive, then you could have a problem.

    Case in point:

    /*
    ** Timer.C
    */
    int Get_Timer1( void ) using 3
    {
        return( TimerCounter ); // 'TimerCounter' maintained elsewhere.
    }
    

    And then your interrupt service vector table is in another location

    /*
    ** ISR_Vectors.C
    */
    void Ext1_ISR( void ) interrupt EXTERNAL1_ISR
    {
        ...
        val = Get_Timer1( );
        if( val > LAUNCH_ICBM_DELAY_TIME )
        {
            Launch_Arsenal( ALL );
        }
        ...
    }
    

    When T1_ISR launches Timer1( ), there MAY be a mismatch in the register bank.

    Again, I'm not saying that this is your problem. Just a risk area.

    Are you sure that the uPSD3454 is configured properly? especially with respect to the memory mapping (PSD).

    --Cpt. Vince Foster
    2nd Cannon Place
    Fort Marcy Park, VA

  • There is missing PUSH / POP PSW in your interrupt header, I think. And if your "C_funct" uses DPTR, you must add PUSH / POP DPL,DPH. And if your function uses dual DPTR - you must PUSH both DPL/DPH ... I prefer interrupt handler in "C" style. The compiler can decide what is necessary to PUSH to stack in this moment.

  • Good call Martin Persich, I totally agree. That has got to be it.

    I just saw the who MACRO thing and thought there was some missing context around it. (I should have known. Especially since RETI is right there).

    (I hate MACROs)

    --Cpt. Vince Foster
    2nd Cannon Place
    Fort Marcy Park, VA

  • By using the declaration

    void TIMER_Update( void ) using 2
    { ... }
    

    C51 is intelligent enough to generate

        PUSH    PSW
        MOV     PSW,#010H
        ...
        POP     PSW
        RET
    

    as a wrapper around the C-func.

    RTX51 Tiny is sharing RegBank 0, but the RTOS timer interrupt is using RegBank 1. Therefore, the declaration in the C-func of using 2 was specified to let the compiler keep track of the used / unused registers of RegBank 2.

    All appears to be working correctly now.
    Thanks for the insights.

    (I agree with both "I hate macros. All of them." and "I prefer interrupt handler in "C" style."!)