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

Multiple ISRs (interrupt service routines)

I'm using the Keil MCB900 eval board with a P89LPC938 at 7.3728MHz and the LPC900 development tools (demo version). I'm using most of Mark Odell's UART and Timer code.

I have setup Timer 1 as the baud rate generator and Timer 0 as a 10ms system timer tick. The timer tick toggles three LEDs at 10ms, 100ms, and 1s. Both the UART and Timer 0 have their own ISR (interrupt service routines) with the appropriate number and separate register banks. Each works fine on its own. But if I initialize both the timer tick AND the UART/Timer 1, it appears the timer tick runs wide open, or much faster than it should, as evidenced by the LEDs.

I have tried setting up different IP (interrupt priority) levels for each interrupt, but that doesn't seem to help. For now, they're both at level 0. Is there anything special (best practices, guidelines) you have to do to make multiple ISRs work?

Here is some of the code. I can post more.

volatile bit data Tick10ms;             // 10ms
volatile bit data Tick100ms;    // 100ms
volatile bit data Tick1s;               // 1s

static void Timer0_ISR(void) interrupt 1 using 2
{
        static U8 num10msTicks;

        TR0 = 0;                // stop timer
        TL0 = TIMER0_10MS_LO;   // reload
        TH0 = TIMER0_10MS_HI;
        TR0 = 1;                        // start timer

        Tick10ms = !Tick10ms;   // 10ms
        ++num10msTicks;

        if (!(num10msTicks % 10)) {     // 100ms
            Tick100ms = !Tick100ms;     // toggle
        }

        if (num10msTicks == 100) {      // 1s time region
            num10msTicks = 0;
            Tick1s = !Tick1s;   // toggle
        }
}

static void uartIsr(void) interrupt 4 using 1
{
    static U8 data inChar;

    if (RI) {
        RI = 0;
        inChar   = SBUF;
        if (s_rxRingEmpty || s_rxWrIdx != s_rxRdIdx) {
            s_rxRing[s_rxWrIdx] = inChar;
            ++s_rxWrIdx;

#if RX_RING_SIZE != 256
            s_rxWrIdx %= RX_RING_SIZE;
#endif
            s_rxRingEmpty = 0;
        }
    }

    if (TI) {
        TI = 0;
        if (s_txRdIdx != s_txWrIdx) {
            SBUF = s_txRing[s_txRdIdx];
            ++s_txRdIdx;

#if TX_RING_SIZE != 256
            s_txRdIdx %= TX_RING_SIZE;
#endif
        }
        else {
            // declare empty after last SCON.TI event
            s_txRingEmpty = 1;
        }
    }
}

void main(void)
{
        P2M1 = 0;               // set output mode
        P1M1 = 0;

        TMR_Init();             // A
        initUart(BAUD_RATE_57600);      // B

        EA = 1;         // enable ints

        printf("Baud rate: 57600\n");  // B

        P2 = 0x00;      // turn off LEDs

        while (1) {             // endless loop
                LED0 = Tick10ms;        // P2^0
                LED1 = Tick100ms;       // P2^1
                LED2 = Tick1s;  // P2^2
        }
}

Parents
  • I was initially looking for any guidelines or rules-of-thumb when running multiple ISRs. Anyway, here's more of it, including the one ISR with more comments:

    
    #define XTAL_7_3728MHZ  7372800 // Common XTAL freqs
    #define XTAL_11_059MHZ  11059200
    
    #define XTAL_FREQ       XTAL_7_3728MHZ  // our frequency
    
    #define TIMER_0_TICKS_PER_SEC   (XTAL_FREQ/2)   // 7372800/2 = 3686400
    #define TIMER_0_TICKS_PER_MS    (XTAL_FREQ/2)/1000      // 3686400/1000 = 3686.4
    #define TIMER0_10MS          (10 * TIMER_0_TICKS_PER_MS)        // 36864 (0x7000)
    #define TIMER0_10MS_HI       65536 - ((U8) (TIMER0_10MS >> 8))    // reload values for 10ms
    #define TIMER0_10MS_LO       65536 - ((U8) TIMER0_10MS)
    
    #define TIMER0_INT_NUM  1       // interrupt
    #define UART_INT_NUM    4       // same as Odell's code
    
    #define SCON_8N1            0x40 // SCON.SM1 = 1, SM0,2 = 0
    #define SCON_REN            0x10
    
    // Timer 0 only
    #define TMOD_GATE_T0         0x08
    #define TMOD_C_T_T0          0x04
    #define TMOD_M1_T0           0x02
    #define TMOD_M0_T0           0x01
    #define T0_MODE_MASK         (TMOD_GATE_T0 | TMOD_C_T_T0 | TMOD_M1_T0 | TMOD_M0_T0)
    #define T0_16BIT_TIMER       (TMOD_M0_T0)
    
    // Timer 1 only
    #define TMOD_GATE_T1        0x80
    #define TMOD_C_T_T1         0x40
    #define TMOD_M1_T1          0x20
    #define TMOD_M0_T1          0x10
    #define T1_MODE_MASK        (TMOD_GATE_T1 | TMOD_C_T_T1 | TMOD_M1_T1 | TMOD_M0_T1)
    #define T1_8BIT_AUTO_RELOAD (TMOD_M1_T1)
    
    #define PCON_SMOD           0x80 // Doubles baud rate timing of Timer 1
    
    #define RX_RING_SIZE    32              // serial receive buffer size
    #define TX_RING_SIZE    32              // serial transmit buffer size
    
    // serial port tx/rx ring variables
    static U8 xdata s_rxRing[RX_RING_SIZE];
    static U8 xdata s_txRing[TX_RING_SIZE];
    static volatile bit data s_rxRingEmpty;
    static volatile bit data s_txRingEmpty;
    static volatile U8 data s_rxWrIdx;
    static volatile U8 data s_rxRdIdx;
    static volatile U8 data s_txWrIdx;
    static volatile U8 data s_txRdIdx;
    static UartBaudRates data s_currentBaudRate;
    static bit data s_translateEol;         // \r into \r\n
    
    // timer tick variables
    volatile bit data Tick10ms;             // 10ms
    volatile bit data Tick100ms;    // 100ms
    volatile bit data Tick1s;               // 1s
    
    //function prototypes
    static bit setUartBaudRate(UartBaudRates baudRate);
    static bit putUartChar(U8 outChar);
    static bit getUartChar(U8 *pInChar);
    
    
    void TMR_Init()
    {
            TR0 = 0;                // stop Timer 0
            ET0 = 0;                // disable Timer 0 int
    
            TMOD &= T0_MODE_MASK;       // configure Timer 0
            TMOD |= T0_16BIT_TIMER; // 16-bit timer mode
            TL0 = TIMER0_10MS_LO;   // set reload values
            TH0 = TIMER0_10MS_HI;
            Tick10ms = 0;   // reset time regions
            Tick100ms = 0;
            Tick1s = 0;
    
            ET0 = 1;                // enable Timer 0 int
            TR0 = 1;                // start Timer 0
    }
    
    bit initUart(UartBaudRates baudRate)
    {
            ES = 0; // stop serial ints
    
            if (setUartBaudRate(baudRate)) return 1;        // Set the baud rate
    
            SCON = SCON_8N1 | SCON_REN;             // Setup the serial port
    
            s_rxRingEmpty = 1;      // flag rings as empty
            s_txRingEmpty = 1;
    
            s_rxWrIdx = 0;  // Setup ring buffer pointers
            s_rxRdIdx = 0;
            s_txWrIdx = 0;
            s_txRdIdx = 0;
    
            s_translateEol = 1;     // EOL translate
    
            ES = 1;         // start serial ints
            return 0;
    }
    
    static bit setUartBaudRate(UartBaudRates baudRate)
    {
        static const S8 code baudRateTable2x[NUM_BAUD_RATES] =
        {
        //  1200  2400  9600  19200  38400  57600  115200
    #if   XTAL_FREQ == XTAL_7_3728MHZ
             -192, -96,   -24,   -12,   -6,    -4,     -2   // timer reload values
    #elif XTAL_FREQ == XTAL_11_059MHZ
             -48,  -24,   -6,    -3,     0,    -1,      0
    #else // Add more if needed.
    #   error Pick a known XTAL frequency from list provided in this file.
    #endif
        };
        static const S8 code baudRateTable1x[NUM_BAUD_RATES] =
        {
        //  1200  2400  9600  19200  38400  57600  115200
    #if   XTAL_FREQ == XTAL_7_3728MHZ
             -384, -192,  -48,  -24,   -12,    -8,     -4
    #elif XTAL_FREQ == XTAL_11_059MHZ
             -24,  -12,   -3,     0,     0,     0,      0
    #else
    #   error Pick a known XTAL frequency from list provided in this file.
    #endif
        };
        S8 data th1Value;
        bit data ieEs;
    
        // Look for cases where we don't want to or can't change the baud rate.
        if (BAUD_RATE_NO_CHANGE == baudRate || s_currentBaudRate == baudRate) {
            return 0;
        }
            // outside of list - error
        if ((unsigned int) baudRate > NUM_BAUD_RATES) {
            return 1;
        }
    
        PCON |= PCON_SMOD;                  // See if we *can* set the desired baud rate
        th1Value = baudRateTable2x[baudRate];       // reload values
        if (!th1Value) {                            // if zero (not available)
            r_pcon  &= ~PCON_SMOD;              // turn off baud rate doubler
            th1Value = baudRateTable1x[baudRate];   // pull values
            if (!th1Value) return 1;        // if zero again, error
        }
    
            ieEs = _testbit_(ES);   // Preserve and clear serial interrupt enable
    
            ET1 = 0;        // no Timer 1 interrupt
    
            s_currentBaudRate = baudRate;   // Save this for getUartOption()
    
            TR1 = 0;                                        // stop timer 1
            TMOD &= T1_MODE_MASK;       // Config Timer 1 for baud rate generation
            TMOD |= T1_8BIT_AUTO_RELOAD;
            TH1 = (U8) th1Value;    // load timer
            TL1 = (U8) th1Value;
            TR1 = 1;                // start timer 1
            ES = ieEs;      // restore previous setting
    
        return 0;
    }
    
    // repost with more comments
    static void uartIsr(void) interrupt 4 using 1
    {
        static U8 data inChar;
    
        if (RI) {           // char waiting
            RI = 0;         // clear interrupt
            inChar = SBUF;  // grab char, but don't put it in buffer yet
            if (s_rxRingEmpty || s_rxWrIdx != s_rxRdIdx) {  // if room in buffer or pointers unequal
                s_rxRing[s_rxWrIdx] = inChar;       // put into ring buffer
                ++s_rxWrIdx;        // update index pointer
    
    #if RX_RING_SIZE != 256                         // ask Mark Odell about this part
                s_rxWrIdx %= RX_RING_SIZE;
    #endif
                s_rxRingEmpty = 0;  // not empty?
            }
        }
    
        if (TI) {   // char to send
            TI = 0; // clear interrupt
            if (s_txRdIdx != s_txWrIdx) {   // if index pointers not equal
                SBUF = s_txRing[s_txRdIdx]; // send char
                ++s_txRdIdx;                                // update index pointer
    
    #if TX_RING_SIZE != 256                         // ask Mark Odell about this part
                s_txRdIdx %= TX_RING_SIZE;
    #endif
            }
            else {
                s_txRingEmpty = 1;  // declare empty after last TI event
            }
        }
    }
    
    

Reply
  • I was initially looking for any guidelines or rules-of-thumb when running multiple ISRs. Anyway, here's more of it, including the one ISR with more comments:

    
    #define XTAL_7_3728MHZ  7372800 // Common XTAL freqs
    #define XTAL_11_059MHZ  11059200
    
    #define XTAL_FREQ       XTAL_7_3728MHZ  // our frequency
    
    #define TIMER_0_TICKS_PER_SEC   (XTAL_FREQ/2)   // 7372800/2 = 3686400
    #define TIMER_0_TICKS_PER_MS    (XTAL_FREQ/2)/1000      // 3686400/1000 = 3686.4
    #define TIMER0_10MS          (10 * TIMER_0_TICKS_PER_MS)        // 36864 (0x7000)
    #define TIMER0_10MS_HI       65536 - ((U8) (TIMER0_10MS >> 8))    // reload values for 10ms
    #define TIMER0_10MS_LO       65536 - ((U8) TIMER0_10MS)
    
    #define TIMER0_INT_NUM  1       // interrupt
    #define UART_INT_NUM    4       // same as Odell's code
    
    #define SCON_8N1            0x40 // SCON.SM1 = 1, SM0,2 = 0
    #define SCON_REN            0x10
    
    // Timer 0 only
    #define TMOD_GATE_T0         0x08
    #define TMOD_C_T_T0          0x04
    #define TMOD_M1_T0           0x02
    #define TMOD_M0_T0           0x01
    #define T0_MODE_MASK         (TMOD_GATE_T0 | TMOD_C_T_T0 | TMOD_M1_T0 | TMOD_M0_T0)
    #define T0_16BIT_TIMER       (TMOD_M0_T0)
    
    // Timer 1 only
    #define TMOD_GATE_T1        0x80
    #define TMOD_C_T_T1         0x40
    #define TMOD_M1_T1          0x20
    #define TMOD_M0_T1          0x10
    #define T1_MODE_MASK        (TMOD_GATE_T1 | TMOD_C_T_T1 | TMOD_M1_T1 | TMOD_M0_T1)
    #define T1_8BIT_AUTO_RELOAD (TMOD_M1_T1)
    
    #define PCON_SMOD           0x80 // Doubles baud rate timing of Timer 1
    
    #define RX_RING_SIZE    32              // serial receive buffer size
    #define TX_RING_SIZE    32              // serial transmit buffer size
    
    // serial port tx/rx ring variables
    static U8 xdata s_rxRing[RX_RING_SIZE];
    static U8 xdata s_txRing[TX_RING_SIZE];
    static volatile bit data s_rxRingEmpty;
    static volatile bit data s_txRingEmpty;
    static volatile U8 data s_rxWrIdx;
    static volatile U8 data s_rxRdIdx;
    static volatile U8 data s_txWrIdx;
    static volatile U8 data s_txRdIdx;
    static UartBaudRates data s_currentBaudRate;
    static bit data s_translateEol;         // \r into \r\n
    
    // timer tick variables
    volatile bit data Tick10ms;             // 10ms
    volatile bit data Tick100ms;    // 100ms
    volatile bit data Tick1s;               // 1s
    
    //function prototypes
    static bit setUartBaudRate(UartBaudRates baudRate);
    static bit putUartChar(U8 outChar);
    static bit getUartChar(U8 *pInChar);
    
    
    void TMR_Init()
    {
            TR0 = 0;                // stop Timer 0
            ET0 = 0;                // disable Timer 0 int
    
            TMOD &= T0_MODE_MASK;       // configure Timer 0
            TMOD |= T0_16BIT_TIMER; // 16-bit timer mode
            TL0 = TIMER0_10MS_LO;   // set reload values
            TH0 = TIMER0_10MS_HI;
            Tick10ms = 0;   // reset time regions
            Tick100ms = 0;
            Tick1s = 0;
    
            ET0 = 1;                // enable Timer 0 int
            TR0 = 1;                // start Timer 0
    }
    
    bit initUart(UartBaudRates baudRate)
    {
            ES = 0; // stop serial ints
    
            if (setUartBaudRate(baudRate)) return 1;        // Set the baud rate
    
            SCON = SCON_8N1 | SCON_REN;             // Setup the serial port
    
            s_rxRingEmpty = 1;      // flag rings as empty
            s_txRingEmpty = 1;
    
            s_rxWrIdx = 0;  // Setup ring buffer pointers
            s_rxRdIdx = 0;
            s_txWrIdx = 0;
            s_txRdIdx = 0;
    
            s_translateEol = 1;     // EOL translate
    
            ES = 1;         // start serial ints
            return 0;
    }
    
    static bit setUartBaudRate(UartBaudRates baudRate)
    {
        static const S8 code baudRateTable2x[NUM_BAUD_RATES] =
        {
        //  1200  2400  9600  19200  38400  57600  115200
    #if   XTAL_FREQ == XTAL_7_3728MHZ
             -192, -96,   -24,   -12,   -6,    -4,     -2   // timer reload values
    #elif XTAL_FREQ == XTAL_11_059MHZ
             -48,  -24,   -6,    -3,     0,    -1,      0
    #else // Add more if needed.
    #   error Pick a known XTAL frequency from list provided in this file.
    #endif
        };
        static const S8 code baudRateTable1x[NUM_BAUD_RATES] =
        {
        //  1200  2400  9600  19200  38400  57600  115200
    #if   XTAL_FREQ == XTAL_7_3728MHZ
             -384, -192,  -48,  -24,   -12,    -8,     -4
    #elif XTAL_FREQ == XTAL_11_059MHZ
             -24,  -12,   -3,     0,     0,     0,      0
    #else
    #   error Pick a known XTAL frequency from list provided in this file.
    #endif
        };
        S8 data th1Value;
        bit data ieEs;
    
        // Look for cases where we don't want to or can't change the baud rate.
        if (BAUD_RATE_NO_CHANGE == baudRate || s_currentBaudRate == baudRate) {
            return 0;
        }
            // outside of list - error
        if ((unsigned int) baudRate > NUM_BAUD_RATES) {
            return 1;
        }
    
        PCON |= PCON_SMOD;                  // See if we *can* set the desired baud rate
        th1Value = baudRateTable2x[baudRate];       // reload values
        if (!th1Value) {                            // if zero (not available)
            r_pcon  &= ~PCON_SMOD;              // turn off baud rate doubler
            th1Value = baudRateTable1x[baudRate];   // pull values
            if (!th1Value) return 1;        // if zero again, error
        }
    
            ieEs = _testbit_(ES);   // Preserve and clear serial interrupt enable
    
            ET1 = 0;        // no Timer 1 interrupt
    
            s_currentBaudRate = baudRate;   // Save this for getUartOption()
    
            TR1 = 0;                                        // stop timer 1
            TMOD &= T1_MODE_MASK;       // Config Timer 1 for baud rate generation
            TMOD |= T1_8BIT_AUTO_RELOAD;
            TH1 = (U8) th1Value;    // load timer
            TL1 = (U8) th1Value;
            TR1 = 1;                // start timer 1
            ES = ieEs;      // restore previous setting
    
        return 0;
    }
    
    // repost with more comments
    static void uartIsr(void) interrupt 4 using 1
    {
        static U8 data inChar;
    
        if (RI) {           // char waiting
            RI = 0;         // clear interrupt
            inChar = SBUF;  // grab char, but don't put it in buffer yet
            if (s_rxRingEmpty || s_rxWrIdx != s_rxRdIdx) {  // if room in buffer or pointers unequal
                s_rxRing[s_rxWrIdx] = inChar;       // put into ring buffer
                ++s_rxWrIdx;        // update index pointer
    
    #if RX_RING_SIZE != 256                         // ask Mark Odell about this part
                s_rxWrIdx %= RX_RING_SIZE;
    #endif
                s_rxRingEmpty = 0;  // not empty?
            }
        }
    
        if (TI) {   // char to send
            TI = 0; // clear interrupt
            if (s_txRdIdx != s_txWrIdx) {   // if index pointers not equal
                SBUF = s_txRing[s_txRdIdx]; // send char
                ++s_txRdIdx;                                // update index pointer
    
    #if TX_RING_SIZE != 256                         // ask Mark Odell about this part
                s_txRdIdx %= TX_RING_SIZE;
    #endif
            }
            else {
                s_txRingEmpty = 1;  // declare empty after last TI event
            }
        }
    }
    
    

Children
  • I was initially looking for any guidelines or rules-of-thumb when running multiple ISRs.

    In that case, you were looking for the completely wrong type of advice. Your problem has nothing to do with interrupts or ISRs, and it is completely invisible in the code you posted first.

    Instead, you should consider if this line is really doing what you want them to do, because it will completely mess up any configuration of Timer 0.

        TMOD &= T1_MODE_MASK;       // Config Timer 1 for baud rate generation
    

  • I was initially looking for any guidelines or rules-of-thumb when running multiple ISRs
    well the big one is
    KISS Keep ISRs Short and Simple.

    The one ISR you show is fairly so (a note: if you keep ring buffers on a power of two size, you do not need the slooow divide, an and will do)

    s_txRdIdx seems to indicate that you are using a short where a char will do, that will increase the ISR processing time about tenfold.

    Is it possible that you have interrupts at differenr priority (as set in the IP SFR) using the same register bank. ?

    Erik

  • Instead, you should consider if this line is really doing what you want them to do, because it will completely mess up any configuration of Timer 0.

    Wow, that was it - the second TMOD config was blowing away the first. You guys are good. :)

    Changes in TMR_Init:

    TMOD &= 0xF0;       // clear Timer 0, mask off Timer 1
    TMOD |= 0x01;   // Timer 0 Mode 1 (16-bit timer )
    

    Changes in setUartBaudRate:

    TMOD &= 0x0F;       // clear Timer 1, mask off Timer 0
    TMOD |= 0x20;   // Timer 1 Mode 2 (8-bit auto-reload)
    

    I will also make the changes to the ISRs. Thanks guys.