[TODO] ICU not working in 2.6.3 without period CB

ChibiOS public support forum for topics related to the STMicroelectronics STM32 family of micro-controllers.

Moderator: RoccoMarco

Post Reply
MarkusS
Posts: 14
Joined: Tue Mar 13, 2012 6:52 pm
Has thanked: 1 time

[TODO] ICU not working in 2.6.3 without period CB

Post by MarkusS »

Hi,

I have problems with 2.6.3 using the ICU wihtout a period CB (on a STM32F4Discovery board). If I define a dummy period CB, 2.6.3 works as 2.6.2.Without the periodCB, the widthCB does not get called. Here is a modified PWM_ICU example showing the problem:

Code: Select all

#include "ch.h"
#include "hal.h"

static void pwmpcb(PWMDriver *pwmp)
{
  (void)pwmp;
  palClearPad(GPIOD, GPIOD_LED5);
}

static void pwmc1cb(PWMDriver *pwmp)
{
  (void)pwmp;
  palSetPad(GPIOD, GPIOD_LED5);
}

static PWMConfig pwmcfg = {
  10000,                                    /* 10kHz PWM clock frequency.   */
  10000,                                    /* Initial PWM period 1S.       */
  pwmpcb,
  {
   {PWM_OUTPUT_ACTIVE_HIGH, pwmc1cb},
   {PWM_OUTPUT_DISABLED, NULL},
   {PWM_OUTPUT_DISABLED, NULL},
   {PWM_OUTPUT_DISABLED, NULL}
  },
  0,
  0
};

icucnt_t last_width, last_period;

static void icuwidthcb(ICUDriver *icup)
{
  palTogglePad(GPIOD, GPIOD_LED4);
  last_width = icuGetWidth(icup);
}

static void icuperiodcb(ICUDriver *icup)
{
  last_period = icuGetPeriod(icup);
}

static ICUConfig icucfg = {
  ICU_INPUT_ACTIVE_HIGH,
  10000,                                    /* 10kHz ICU clock frequency.   */
  icuwidthcb,
  NULL, //icuperiodcb, // without the period cb, the widht cb does NOT get called in 2.6.3???
  NULL,
  ICU_CHANNEL_1,
  0
};

/*
 * Application entry point.
 */
int main(void)
{
  /*
   * System initializations.
   * - HAL initialization, this also initializes the configured device drivers
   *   and performs the board-specific initializations.
   * - Kernel initialization, the main() function becomes a thread and the
   *   RTOS is active.
   */
  halInit();
  chSysInit();

  /*
   * Initializes the PWM driver 2 and ICU driver 3.
   * GPIOA15 is the PWM output.
   * GPIOC6 is the ICU input.
   * The two pins have to be externally connected together.
   */
  pwmStart(&PWMD2, &pwmcfg);
  palSetPadMode(GPIOA, 15, PAL_MODE_ALTERNATE(1));
  icuStart(&ICUD3, &icucfg);
  palSetPadMode(GPIOC, 6, PAL_MODE_ALTERNATE(2));
  icuEnable(&ICUD3);
  chThdSleepMilliseconds(2000);

  /*
   * Starts the PWM channel 0 using 75% duty cycle.
   */
  pwmEnableChannel(&PWMD2, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD2, 7500));
  chThdSleepMilliseconds(5000);

  /*
   * Changes the PWM channel 0 to 50% duty cycle.
   */
  pwmEnableChannel(&PWMD2, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD2, 5000));
  chThdSleepMilliseconds(5000);

  /*
   * Changes the PWM channel 0 to 25% duty cycle.
   */
  pwmEnableChannel(&PWMD2, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD2, 2500));
  chThdSleepMilliseconds(5000);

  /*
   * Changes PWM period to half second the duty cycle becomes 50%
   * implicitly.
   */
  pwmChangePeriod(&PWMD2, 5000);
  chThdSleepMilliseconds(5000);

  /*
   * Disables channel 0 and stops the drivers.
   */
  pwmDisableChannel(&PWMD2, 0);
  pwmStop(&PWMD2);
  icuDisable(&ICUD3);
  icuStop(&ICUD3);
  palClearPad(GPIOD, GPIOD_LED4);
  palClearPad(GPIOD, GPIOD_LED5);

  /*
   * Normal main() thread activity, in this demo it does nothing.
   */
  while (TRUE) {
    chThdSleepMilliseconds(500);
  }
  return 0;
}
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: ICU not working in 2.6.3 without period CB

Post by Giovanni »

Hi,

The callbacks should not be NULL in the ICU driver because there is no check on the pointer value before calling them. Are you sure this used to work?

Giovanni
MarkusS
Posts: 14
Joined: Tue Mar 13, 2012 6:52 pm
Has thanked: 1 time

Re: ICU not working in 2.6.3 without period CB

Post by MarkusS »

Yes, it is working with NULL in 2.6.1 and 2.6.2 (at least for me on an STM32F4 and an STM32F1...). And the overflow cb is NULL in the example as well, so how should one know that the other cb's can't be NULL? If it is that way, perhaps we should add an assert there so we can catch it?
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: ICU not working in 2.6.3 without period CB

Post by Giovanni »

Hi,

I found the problem, it is a side effect of the last bug fix performed on that driver (for a valid reason). The driver now checks the state machine before invoking the period callback in order to suppress width callbacks before a period callback happens.

The problem is that if some callback is NULL then the state machine does not "move" between the ICU_ACTIVE and ICU_IDLE states because the related interrupts are disabled, see icu_lld_enable().

Now, is it acceptable to have a driver that does not update its state machine as is supposed to do? not enabling the interrupts does exactly this. On the other hand forcing the interrupts would limit the capture performance at high frequencies.

I have to rethink this, probably the best approach would be to not track ICU_IDLE and ICU_ACTIVE states anymore and detect those conditions using the timer status registers somehow and introducing a function like icuGetCaptureState() or similar.

Giovanni
User avatar
russian
Posts: 368
Joined: Mon Oct 29, 2012 3:17 am
Has thanked: 16 times
Been thanked: 14 times

Re: [TODO] ICU not working in 2.6.3 without period CB

Post by russian »

Looks like I am experiencing the same issue now. I've disabled period callbacks for performance reasons and suddenly I am not getting width callbacks.
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: [TODO] ICU not working in 2.6.3 without period CB

Post by Giovanni »

Hi,

The driver has been reworked in 3.0.0, not it is possible to enable/disable callbacks (and their IRQ) at runtime. Back-porting it to 2.6.x is not planned but you could try that yourself or use 3.0.0.

Giovanni
User avatar
russian
Posts: 368
Joined: Mon Oct 29, 2012 3:17 am
Has thanked: 16 times
Been thanked: 14 times

Re: [TODO] ICU not working in 2.6.3 without period CB

Post by russian »

Giovanni wrote:not it is possible to enable/disable callbacks (and their IRQ) at runtime.

I am not disabling anything at runtime: I've changed the code, re-compiled the code, flashed the new binary without period callback and lost width callback
User avatar
Giovanni
Site Admin
Posts: 14891
Joined: Wed May 27, 2009 8:48 am
Has thanked: 1202 times
Been thanked: 996 times

Re: [TODO] ICU not working in 2.6.3 without period CB

Post by Giovanni »

I wrote "not" instead of "now"...

now it is possible to enable/disable callbacks (and their IRQ) at runtime.


The driver is now able to do that.

Giovanni
Post Reply