ICU driver's "overflow" handler seems poorly defined

[genosensor]

Hi,

[Note that I'm working with 2.6.10, but I have verified that all this valid for 17.x as well]

The ICU driver's interrupt handler ends with the following statement:

Code: Select all

  if ((sr & STM32_TIM_SR_UIF) != 0)
    _icu_isr_invoke_overflow_cb(icup);


This causes the overflow handler, if registered, to be called every time the counter rolls over from 0xffff to 0,
generating a periodic callback at the configured frequency/2**16
This agrees with the documentation, so it is not a bug. But, how is it useful?
How does the API allow the application to detect capture overflow?!
Wouldn't it be more useful to arrange for the overflow handler to be called on capture rather than counter overflow?

Here is a revised ISR that invokes the overflow callback only when there is a capture overflow:

Code: Select all

static void icu_lld_serve_interrupt(ICUDriver *icup) {
  uint16_t sr = icup->tim->SR;
  uint16_t mask = icup->tim->DIER & STM32_TIM_DIER_IRQ_MASK;
  if (icup->config->overflow_cb && (sr & overcap))
    mask |= overcap;
  sr &= mask;
  icup->tim->SR = ~sr;
  if (sr & overcap)
    _icu_isr_invoke_overflow_cb(icup);
  if (icup->config->channel == ICU_CHANNEL_1) {
    if (sr & STM32_TIM_SR_CC1IF)
      _icu_isr_invoke_period_cb(icup);
    if (sr & STM32_TIM_SR_CC2IF)
      _icu_isr_invoke_width_cb(icup);
  } else {
    if (sr & STM32_TIM_SR_CC2IF)
      _icu_isr_invoke_period_cb(icup);
    if (sr & STM32_TIM_SR_CC1IF)
      _icu_isr_invoke_width_cb(icup);
  }
}


Note that, in the icu_lld_enable(), the statement enabling the UIE interrupt if the overflow_cb != NULL should be removed.

Thoughts?

[genosensor]

In my previous post, I forgot to include the definition of overcap referenced in the revised interrupt service routine:

Code: Select all

#define overcap (STM32_TIM_SR_CC2OF | STM32_TIM_SR_CC1OF)