FAQs
cancel
Turn on suggestions
Auto-suggest helps you quickly narrow down your search results by suggesting possible matches as you type.
Showing results for 
Search instead for 
Did you mean: 

Why does assigning TIMx->CCRx occasionally generate an interrupt, despite TIMx->CNT is not equal to TIMx->CCRx?

arnold_w
Senior

‎2021-01-23 11:54 AM

Short version: I am working with the STM32F446 family and I use TIM1 as 4 independent timers:

typedef enum {
    TIM1_CHANNEL_1 = 0,
    TIM1_CHANNEL_2 = 1,
    TIM1_CHANNEL_3 = 2,
    TIM1_CHANNEL_4 = 3,
} timerAndChannelSelector_e;
 
void scheduleTimerInts(timerAndChannelSelector_e, uint16_t periodMs, EventCallback_t, uint32_t callbackArg1, uint32_t callbackArg2);
void stopAndResetTimerChannel(timerAndChannelSelector_e);

Occasionally, right after I assign TIMx->CCRx, I get a timer interrupt from that same channel, even though TIMx->CNT is not equal to TIMx->CCRx. What am I doing wrong?

static void determineTimerParam(uint16_t currentTimerCntValue, uint16_t desiredDelayMs,
                                volatile uint32_t* numNeededInts,  volatile uint16_t* timerThresholdLastInt) {
    uint32_t numNeededTicks = ((uint32_t)desiredDelayMs) * ((uint32_t)TIMER_TICK_FREQ_kHz);
 
    if (((uint32_t)currentTimerCntValue + numNeededTicks) <= (uint32_t)TIMER_ARR) {
        *numNeededInts         = 1;
        *timerThresholdLastInt = (uint16_t)(currentTimerCntValue + numNeededTicks);
    } else {
        *numNeededInts         =            ((uint32_t)currentTimerCntValue + numNeededTicks)/((uint32_t)TIMER_ARR + 1);
        *timerThresholdLastInt = (uint16_t)(((uint32_t)currentTimerCntValue + numNeededTicks)%((uint32_t)TIMER_ARR + 1));
        if (currentTimerCntValue != TIMER_ARR) {
            (*numNeededInts)++;
        }
    }
}
 
void scheduleTimerInts(timerAndChannelSelector_e timerAndChannelSelector, uint16_t periodMs, EventCallback_t callback, uint32_t callbackArg1, uint32_t callbackArg2) {
    DISABLE_ALL_INTS_IF_NECESSARY();      // Make sure this entire function is executed uninterrupted
    states_s* s = &allStates[timerAndChannelSelector];
    if (!__HAL_RCC_TIM1_IS_CLK_ENABLED()) {
    	__HAL_RCC_TIM1_CLK_ENABLE();       // Turn on "master clock" to the timer
    }
 
    // If the timer isn't running, calculate pre-scalar and start the actual timer
    if (allChannelStatesAreIdle()) {
    	__TIM1_FORCE_RESET();
    	__TIM1_RELEASE_RESET();
        uint16_t prescalar = (uint16_t)((TIM1_FREQ_Hz / (1000 * TIMER_TICK_FREQ_kHz)) - 1);
        setPreScalarAndPeriod(prescalar, TIMER_ARR);
        NVIC_EnableIRQ(TIM1_CC_IRQn);
        TIM1->CR1 |= 1;                                    // Start timer
    }
    s->periodMs         = periodMs;
    s->timerIntCallback = callback;
    s->callbackArgs1    = callbackArg1;
    s->callbackArgs2    = callbackArg2;
    s->channelState     = CHANNEL_SCHEDULED_FOR_INT_GENERATION;
    determineTimerParam((uint16_t) TIM1->CNT, periodMs, &(s->numTimerIntsLeft), &(s->timerThresholdForLastInt));
    *s->CCRx = (s->numTimerIntsLeft == 1) ? s->timerThresholdForLastInt : TIMER_ARR;  // Generate interrupt when TIMx_CNT reaches this number
// Sometimes the corresponding channel bit (1, 2, 3 or 4) in TIMx->SR get set here, creating an interrupt immediately!!! 
    TIM1->DIER |= s->TIM_DIER_CCxIE;                                  // Enable interrupts for this channel
    if ((s->periodMs == 0) || (*s->CCRx == TIM1->CNT)) {
        TIM1->EGR |= s->TIM_EGR_CCxG;                                 // Generate interrupt immediately
    }
    ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
}

Labels:
0 Kudos
5 REPLIES 5
arnold_w
Senior

‎2021-01-23 12:04 PM

Long version: My full code uses all 14 timers to create 29 timers (TIM1_CHANNEL_1, TIM1_CHANNEL_2, ..., TIM13_CHANNEL_1, TIM14_CHANNEL_1) that can be scheduled independently. In the source code below, I've reduced my original code to only use TIM1, to make the easier to understand, but the problem still occurs even when I only use TIM1.

#define TIMER_TICK_FREQ_kHz  (1)
#define TIM1_FREQ_Hz         (32000000)
#define TIM1_NUM_CHANNELS    (4)
#define TIMER_ARR            (0xFFFF) /* To simplify things, use all timers as 16-bit timers, even though some actually are 32-bit */
 
#define DISABLE_ALL_INTS_IF_NECESSARY()           uint32_t old_primask;          \
                                                  old_primask = __get_PRIMASK(); \
                                                  __disable_irq()
 
#define ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED()    if (!old_primask) {            \
                                                      __enable_irq();            \
                                                  }
 
typedef enum {
    CHANNEL_IDLE                         = 0,
    CHANNEL_SCHEDULED_FOR_INT_GENERATION = 1,
} channelState_e;
 
/// Timer states
typedef struct {
    volatile channelState_e  channelState;             ///< Channel State
             EventCallback_t timerIntCallback;         ///< Timer interrupt callback
             uint16_t        periodMs;                 ///< Period in ms
    volatile uint32_t        numTimerIntsLeft;         ///< Number of timer interrupts left
    volatile uint16_t        timerThresholdForLastInt; ///< Timer threshold for last interrupt
    volatile uint32_t        callbackArgs1;            ///< Callback argument 1
    volatile uint32_t        callbackArgs2;            ///< Callback argument 2
    volatile uint32_t*       CCRx;                     ///< CCRx
    const    uint32_t        TIM_DIER_CCxIE;           ///< TIM_DIER_CCxIE
    const    uint32_t        TIM_EGR_CCxG;             ///< TIM_EGR_CCxG
} states_s;
 
static states_s allStates[] = {
    {CHANNEL_IDLE, NULL, 0, 0, 0, 0, 0, (volatile uint32_t* const)&(TIM1->CCR1), TIM_DIER_CC1IE, TIM_EGR_CC1G},
    {CHANNEL_IDLE, NULL, 0, 0, 0, 0, 0, (volatile uint32_t* const)&(TIM1->CCR2), TIM_DIER_CC2IE, TIM_EGR_CC2G},
    {CHANNEL_IDLE, NULL, 0, 0, 0, 0, 0, (volatile uint32_t* const)&(TIM1->CCR3), TIM_DIER_CC3IE, TIM_EGR_CC3G},
    {CHANNEL_IDLE, NULL, 0, 0, 0, 0, 0, (volatile uint32_t* const)&(TIM1->CCR4), TIM_DIER_CC4IE, TIM_EGR_CC4G}};
 
static Bool_t allChannelStatesAreIdle() {
    for (uint8_t i = 0; i < TIM1_NUM_CHANNELS; i++) {
        if (allStates[i].channelState != CHANNEL_IDLE) {
            return FALSE;
        }
    }
    return TRUE;
}
 
static void setPreScalarAndPeriod(uint16_t preScalar, uint16_t timerPeriod) {
    if (((uint16_t) TIM1->PSC) != preScalar) {
        TIM1->PSC = preScalar;
        TIM1->EGR = TIM_EGR_UG;
        TIM1->SR &= 0xFFFFFFFE;    // Clear update interrupt flag
    }
    TIM1->ARR = timerPeriod;
}
 
static void determineTimerParam(uint16_t currentTimerCntValue, uint16_t desiredDelayMs,
                                volatile uint32_t* numNeededInts,  volatile uint16_t* timerThresholdLastInt) {
    uint32_t numNeededTicks = ((uint32_t)desiredDelayMs) * ((uint32_t)TIMER_TICK_FREQ_kHz);
 
    if (((uint32_t)currentTimerCntValue + numNeededTicks) <= (uint32_t)TIMER_ARR) {
        *numNeededInts         = 1;
        *timerThresholdLastInt = (uint16_t)(currentTimerCntValue + numNeededTicks);
    } else {
        *numNeededInts         =            ((uint32_t)currentTimerCntValue + numNeededTicks)/((uint32_t)TIMER_ARR + 1);
        *timerThresholdLastInt = (uint16_t)(((uint32_t)currentTimerCntValue + numNeededTicks)%((uint32_t)TIMER_ARR + 1));
        if (currentTimerCntValue != TIMER_ARR) {
            (*numNeededInts)++;
        }
    }
}
 
void scheduleTimerInts(timerAndChannelSelector_e timerAndChannelSelector, uint16_t periodMs, EventCallback_t callback, uint32_t callbackArg1, uint32_t callbackArg2) {
    DISABLE_ALL_INTS_IF_NECESSARY();      // Make sure this entire function is executed uninterrupted
    states_s* s = &allStates[timerAndChannelSelector];
    if (!__HAL_RCC_TIM1_IS_CLK_ENABLED()) {
    	__HAL_RCC_TIM1_CLK_ENABLE();       // Turn on "master clock" to the timer
    }
 
    // If the timer isn't running, calculate pre-scalar and start the actual timer
    if (allChannelStatesAreIdle()) {
    	__TIM1_FORCE_RESET();
    	__TIM1_RELEASE_RESET();
        uint16_t prescalar = (uint16_t)((TIM1_FREQ_Hz / (1000 * TIMER_TICK_FREQ_kHz)) - 1);
        setPreScalarAndPeriod(prescalar, TIMER_ARR);
        NVIC_EnableIRQ(TIM1_CC_IRQn);
        TIM1->CR1 |= 1;                                    // Start timer
    }
    s->periodMs         = periodMs;
    s->timerIntCallback = callback;
    s->callbackArgs1    = callbackArg1;
    s->callbackArgs2    = callbackArg2;
    s->channelState     = CHANNEL_SCHEDULED_FOR_INT_GENERATION;
    determineTimerParam((uint16_t) TIM1->CNT, periodMs, &(s->numTimerIntsLeft), &(s->timerThresholdForLastInt));
    *s->CCRx = (s->numTimerIntsLeft == 1) ? s->timerThresholdForLastInt : TIMER_ARR;  // Generate interrupt when TIMx_CNT reaches this number
// Sometimes the corresponding channel bit (1, 2, 3 or 4) in TIMx->SR get set here, creating an interrupt immediately!!!
    TIM1->DIER |= s->TIM_DIER_CCxIE;                                  // Enable interrupts for this channel
    if ((s->periodMs == 0) || (*s->CCRx == TIM1->CNT)) {
        TIM1->EGR |= s->TIM_EGR_CCxG;                                 // Generate interrupt immediately
    }
    ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
}
 
static void shutDownEntireTimer() {
    __TIM1_FORCE_RESET();                // Reset all timer register values
    NVIC_DisableIRQ(TIM1_CC_IRQn);       // Disable timer interrupt in NVIC
    NVIC_ClearPendingIRQ(TIM1_CC_IRQn);  // Clear interrupt flag in NVIC
    for (uint8_t i = 0; i < TIM1_NUM_CHANNELS; i++) {
    	allStates[i].channelState = CHANNEL_IDLE;
    }
    __TIM1_RELEASE_RESET();
}
 
static void stopTimerIntGeneration(states_s* s) {
    TIM1->DIER &= ~s->TIM_DIER_CCxIE;  // Disable interrupt for this channel
    TIM1->SR   &= ~s->TIM_DIER_CCxIE;  // Clear interrupt flag for this channel
    s->timerIntCallback = NULL;
    s->channelState = CHANNEL_IDLE;
    if (allChannelStatesAreIdle()) {
        shutDownEntireTimer();
    }
}
 
void stopAndResetTimerChannel(timerAndChannelSelector_e timerAndChannelSelector) {
    DISABLE_ALL_INTS_IF_NECESSARY();                    // Make sure this entire function is executed uninterrupted
    states_s* s = &allStates[timerAndChannelSelector];
    if (s->channelState == CHANNEL_SCHEDULED_FOR_INT_GENERATION) {
        stopTimerIntGeneration(s);
    }
    ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
}
 
void TIM1_CC_IRQHandler(void) {
    DISABLE_ALL_INTS_IF_NECESSARY();                                // Make sure this entire function is executed uninterrupted
    uint32_t TIM1_SR = TIM1->SR;                                    // Make a copy of the interrupt flags
    TIM1_SR &= (TIM_DIER_CC1IE | TIM_DIER_CC2IE | TIM_DIER_CC3IE | TIM_DIER_CC4IE);  // We will only care about capture/compare interrupts
    TIM1->SR &= ~TIM1_SR;                                           // Clear the interrupt flag we will handle in this interrupt
    EventCallback_t timerIntCallbackCopies[] = {NULL, NULL, NULL, NULL};
    uint32_t callbackArgs1[TIM1_NUM_CHANNELS];
    uint32_t callbackArgs2[TIM1_NUM_CHANNELS];
    states_s* s = &allStates[0];
    for (uint8_t i = 0; i < TIM1_NUM_CHANNELS; i++) {
        if (TIM1_SR & s->TIM_DIER_CCxIE & TIM1->DIER)  {
            s->numTimerIntsLeft--;
            if (s->numTimerIntsLeft == 0) {  // Is it time to call the callback or is this a "hidden interrupt" (=no callback will be called)?
                if (s->timerIntCallback != NULL) {
                    timerIntCallbackCopies[i] = s->timerIntCallback;
                    callbackArgs1[i]          = s->callbackArgs1;
                    callbackArgs2[i]          = s->callbackArgs2;
                }
 
                stopTimerIntGeneration(s);              // This particular channel is now finished
            } else if (s->numTimerIntsLeft == 1) {
                *s->CCRx = s->timerThresholdForLastInt; // Generate interrupt when TIMx_CNT reaches this number
// Sometimes the corresponding channel bit (1, 2, 3 or 4) in TIMx->SR get set here, creating an interrupt immediately!!!
                if ((s->periodMs == 0) || (*s->CCRx == TIM1->CNT)) {
                    TIM1->EGR |= s->TIM_EGR_CCxG;       // Generate interrupt immediately
                }
            }
        }
        s++;
    }
    ENABLE_ALL_INTS_IF_THEY_WERE_ENABLED();
    for (uint8_t i = 0; i < TIM1_NUM_CHANNELS; i++) {
        if (timerIntCallbackCopies[i] != NULL) {
            timerIntCallbackCopies[i](callbackArgs1[i], callbackArgs2[i]);
        }
    }
}

0 Kudos
waclawek.jan
Guru

‎2021-01-23 04:13 PM

The short version is too long too.

> despite TIMx->CNT is not equal to TIMx->CCRx

How do you know? You appear to write to TIMx->CCRx on the fly. Can't CNT run up to the newly set CCRx while still in that functions?

And wasn't the respective TIMx_SR.CCxIF set by matching with the previously set value, just after entering the disables interrupts "zone"?

JW

PS. Not the source of problem here, but don't use RMW to clear TIMx_SR bits.

0 Kudos
arnold_w
Senior
In response to waclawek.jan

‎2021-01-25 11:15 AM

I can't understand why, but the part about RMW seems to make the problem go away. Many thanks!

0 Kudos
waclawek.jan
Guru

‎2021-01-25 12:40 PM

I'd say it's coincidental or indirect (removing RMW makes the code faster).

JW

0 Kudos
arnold_w
Senior
In response to waclawek.jan

‎2021-01-25 01:22 PM

The full story is that I found this problem a few years ago, occasionally the timer interrupts would come way too early. That's when I found that the interrupt flag sometimes got set when TIMx->CCRx was assigned. I solved that by always clearing the interrupt flag right after assigning TIMx->CCRx. That worked fine for a few years, until it was recently found that sometimes the timer interrupts would come 65535 timer ticks too late! Upon investigating it, I found that it seemed to coincide with the interrupt flag getting set when assigning TIMx->CCRx, so basically it seemed the root problem of the newly discovered problem was the problem found a few years ago.

There are too many mysterious things with this problem and, as always, I'm swamped with work to do so I can't investigate this anymore, but I'm no longer able to re-produce the problem.

0 Kudos
Top