STM32 PWM driver optimization, changed the behavior of pwmChangePeriod().

git-svn-id: svn://svn.code.sf.net/p/chibios/svn/trunk@2869 35acf78f-673a-0410-8e92-d51de3d6d3f4

docs/Doxyfile_chm

@@ -31,7 +31,7 @@ PROJECT_NAME           = ChibiOS/RT
 # This could be handy for archiving the generated documentation or
 # if some version control system is used.
 
-PROJECT_NUMBER         = 2.3.1
+PROJECT_NUMBER         = 2.3.2
 
 # Using the PROJECT_BRIEF tag one can provide an optional one line description
 # for a project that appears at the top of each page and should give viewer

docs/Doxyfile_html

@@ -31,7 +31,7 @@ PROJECT_NAME           = ChibiOS/RT
 # This could be handy for archiving the generated documentation or
 # if some version control system is used.
 
-PROJECT_NUMBER         = 2.3.1
+PROJECT_NUMBER         = 2.3.2
 
 # Using the PROJECT_BRIEF tag one can provide an optional one line description
 # for a project that appears at the top of each page and should give viewer

os/hal/include/pwm.h

@@ -150,11 +150,9 @@ typedef void (*pwmcallback_t)(PWMDriver *pwmp);
  *          been activated using @p pwmStart().
  * @pre     The PWM unit must have been activated using @p pwmStart().
  * @post    The PWM unit period is changed to the new value.
- * @post    Any active channel is disabled by this function and must be
+ * @note    If a period is specified that is shorter than the pulse width
- *          activated explicitly using @p pwmEnableChannel().
+ *          programmed in one of the channels then the behavior is not
- * @note    Depending on the hardware implementation this function has
+ *          guaranteed.
- *          effect starting on the next cycle (recommended implementation)
- *          or immediately (fallback implementation).
  *
  * @param[in] pwmp      pointer to a @p PWMDriver object
  * @param[in] period    new cycle time in ticks

os/hal/platforms/STM32/pwm_lld.c

@@ -254,35 +254,30 @@ void pwm_lld_init(void) {
 #if STM32_PWM_USE_TIM1
   /* Driver initialization.*/
   pwmObjectInit(&PWMD1);
-  PWMD1.enabled_channels = 0;
   PWMD1.tim = TIM1;
 #endif
 
 #if STM32_PWM_USE_TIM2
   /* Driver initialization.*/
   pwmObjectInit(&PWMD2);
-  PWMD2.enabled_channels = 0;
   PWMD2.tim = TIM2;
 #endif
 
 #if STM32_PWM_USE_TIM3
   /* Driver initialization.*/
   pwmObjectInit(&PWMD3);
-  PWMD3.enabled_channels = 0;
   PWMD3.tim = TIM3;
 #endif
 
 #if STM32_PWM_USE_TIM4
   /* Driver initialization.*/
   pwmObjectInit(&PWMD4);
-  PWMD4.enabled_channels = 0;
   PWMD4.tim = TIM4;
 #endif
 
 #if STM32_PWM_USE_TIM5
   /* Driver initialization.*/
   pwmObjectInit(&PWMD5);
-  PWMD5.enabled_channels = 0;
   PWMD5.tim = TIM5;
 #endif
 }
@@ -300,9 +295,6 @@ void pwm_lld_start(PWMDriver *pwmp) {
   uint32_t clock, psc;
   uint16_t ccer;
 
-  /* Reset channels.*/
-  pwmp->enabled_channels = 0;               /* All channels disabled.       */
-
   if (pwmp->state == PWM_STOP) {
     /* Clock activation and timer reset.*/
 #if STM32_PWM_USE_TIM1
@@ -372,7 +364,6 @@ void pwm_lld_start(PWMDriver *pwmp) {
   }
   else {
     /* Driver re-configuration scenario, it must be stopped first.*/
-    pwmp->enabled_channels = 0;             /* All channels disabled.       */
     pwmp->tim->CR1  = 0;                    /* Timer disabled.              */
     pwmp->tim->DIER = 0;                    /* All IRQs disabled.           */
     pwmp->tim->SR   = 0;                    /* Clear eventual pending IRQs. */
@@ -457,8 +448,8 @@ void pwm_lld_start(PWMDriver *pwmp) {
 
   pwmp->tim->CCER = ccer;
   pwmp->tim->EGR  = TIM_EGR_UG;             /* Update event.                */
-  pwmp->tim->SR   = 0;                      /* Clear pending IRQs.          */
   pwmp->tim->DIER = pwmp->config->callback == NULL ? 0 : TIM_DIER_UIE;
+  pwmp->tim->SR   = 0;                      /* Clear pending IRQs.          */
 #if STM32_PWM_USE_ADVANCED
   pwmp->tim->BDTR = pwmp->config->bdtr | TIM_BDTR_MOE;
 #else
@@ -479,7 +470,6 @@ void pwm_lld_stop(PWMDriver *pwmp) {
 
   /* If in ready state then disables the PWM clock.*/
   if (pwmp->state == PWM_READY) {
-    pwmp->enabled_channels = 0;             /* All channels disabled.       */
     pwmp->tim->CR1  = 0;                    /* Timer disabled.              */
     pwmp->tim->DIER = 0;                    /* All IRQs disabled.           */
     pwmp->tim->SR   = 0;                    /* Clear eventual pending IRQs. */
@@ -519,35 +509,6 @@ void pwm_lld_stop(PWMDriver *pwmp) {
   }
 }
 
-/**
- * @brief   Changes the period the PWM peripheral.
- * @details This function changes the period of a PWM unit that has already
- *          been activated using @p pwmStart().
- * @pre     The PWM unit must have been activated using @p pwmStart().
- * @post    The PWM unit period is changed to the new value.
- * @post    Any active channel is disabled by this function and must be
- *          activated explicitly using @p pwmEnableChannel().
- * @note    The function has effect at the next cycle start.
- *
- * @param[in] pwmp      pointer to a @p PWMDriver object
- * @param[in] period    new cycle time in ticks
- *
- * @api
- */
-void pwm_lld_change_period(PWMDriver *pwmp, pwmcnt_t period) {
-
-  pwmp->enabled_channels = 0;           /* All channels disabled.           */
-  pwmp->tim->DIER &= ~(TIM_DIER_CC1IE |
-                       TIM_DIER_CC2IE |
-                       TIM_DIER_CC3IE |
-                       TIM_DIER_CC4IE); /* Channels sources disabled.       */
-  pwmp->tim->SR    = ~(TIM_SR_CC1IF |
-                       TIM_SR_CC1IF |
-                       TIM_SR_CC1IF |
-                       TIM_SR_CC1IF);   /* Clears eventual pending IRQs.    */
-  pwmp->tim->ARR   = (uint16_t)(period - 1);
-}
-
 /**
  * @brief   Enables a PWM channel.
  * @pre     The PWM unit must have been activated using @p pwmStart().
@@ -565,14 +526,15 @@ void pwm_lld_enable_channel(PWMDriver *pwmp,
                             pwmcnt_t width) {
 
   *(&pwmp->tim->CCR1 + (channel * 2)) = width;      /* New duty cycle.      */
-  if ((pwmp->enabled_channels & (1 << channel)) == 0) {
+  /* If there is a callback defined for the channel then the associated
-    /* The channel is not enabled yet.*/
+     interrupt must be enabled.*/
-    pwmp->enabled_channels |= (1 << channel);
+  if (pwmp->config->channels[channel].callback != NULL) {
-    /* If there is a callback associated to the channel then the proper
+    uint32_t dier = pwmp->tim->DIER;
-       interrupt is cleared and enabled.*/
+    /* If the IRQ is not already enabled care must be taken to clear it,
-    if (pwmp->config->channels[channel].callback) {
+       it is probably already pending because the timer is running.*/
+    if ((dier & (2 << channel)) == 0) {
+      pwmp->tim->DIER = dier | (2 << channel);
       pwmp->tim->SR   = ~(2 << channel);
-      pwmp->tim->DIER |= (2 << channel);
     }
   }
 }
@@ -593,7 +555,6 @@ void pwm_lld_disable_channel(PWMDriver *pwmp, pwmchannel_t channel) {
 
   *(&pwmp->tim->CCR1 + (channel * 2)) = 0;
   pwmp->tim->DIER &= ~(2 << channel);
-  pwmp->enabled_channels &= ~(1 << channel);
 }
 
 #endif /* HAL_USE_PWM */

os/hal/platforms/STM32/pwm_lld.h

@@ -295,10 +295,6 @@ struct PWMDriver {
   PWM_DRIVER_EXT_FIELDS
 #endif
   /* End of the mandatory fields.*/
-  /**
-   * @brief Bit mask of the enabled channels.
-   */
-  uint32_t                  enabled_channels;
   /**
    * @brief Pointer to the TIMx registers block.
    */
@@ -309,6 +305,25 @@ struct PWMDriver {
 /* Driver macros.                                                            */
 /*===========================================================================*/
 
+/**
+ * @brief   Changes the period the PWM peripheral.
+ * @details This function changes the period of a PWM unit that has already
+ *          been activated using @p pwmStart().
+ * @pre     The PWM unit must have been activated using @p pwmStart().
+ * @post    The PWM unit period is changed to the new value.
+ * @note    The function has effect at the next cycle start.
+ * @note    If a period is specified that is shorter than the pulse width
+ *          programmed in one of the channels then the behavior is not
+ *          guaranteed.
+ *
+ * @param[in] pwmp      pointer to a @p PWMDriver object
+ * @param[in] period    new cycle time in ticks
+ *
+ * @notapi
+ */
+#define pwm_lld_change_period(pwmp, period)                                 \
+  ((pwmp)->tim->ARR = (uint16_t)((period) - 1))
+
 /*===========================================================================*/
 /* External declarations.                                                    */
 /*===========================================================================*/
@@ -339,7 +354,6 @@ extern "C" {
   void pwm_lld_init(void);
   void pwm_lld_start(PWMDriver *pwmp);
   void pwm_lld_stop(PWMDriver *pwmp);
-  void pwm_lld_change_period(PWMDriver *pwmp, pwmcnt_t period);
   void pwm_lld_enable_channel(PWMDriver *pwmp,
                               pwmchannel_t channel,
                               pwmcnt_t width);

os/hal/src/pwm.c

@@ -124,11 +124,9 @@ void pwmStop(PWMDriver *pwmp) {
  *          been activated using @p pwmStart().
  * @pre     The PWM unit must have been activated using @p pwmStart().
  * @post    The PWM unit period is changed to the new value.
- * @post    Any active channel is disabled by this function and must be
+ * @note    If a period is specified that is shorter than the pulse width
- *          activated explicitly using @p pwmEnableChannel().
+ *          programmed in one of the channels then the behavior is not
- * @note    Depending on the hardware implementation this function has
+ *          guaranteed.
- *          effect starting on the next cycle (recommended implementation)
- *          or immediately (fallback implementation).
  *
  * @param[in] pwmp      pointer to a @p PWMDriver object
  * @param[in] period    new cycle time in ticks
@@ -142,8 +140,7 @@ void pwmChangePeriod(PWMDriver *pwmp, pwmcnt_t period) {
   chSysLock();
   chDbgAssert(pwmp->state == PWM_READY,
               "pwmChangePeriod(), #1", "invalid state");
-  pwmp->period = period;
+  pwmChangePeriodI(pwmp, period);
-  pwm_lld_change_period(pwmp, period);
   chSysUnlock();
 }
 

os/hal/templates/pwm_lld.c

@@ -92,16 +92,15 @@ void pwm_lld_stop(PWMDriver *pwmp) {
  *          been activated using @p pwmStart().
  * @pre     The PWM unit must have been activated using @p pwmStart().
  * @post    The PWM unit period is changed to the new value.
- * @post    Any active channel is disabled by this function and must be
+ * @note    The function has effect at the next cycle start.
- *          activated explicitly using @p pwmEnableChannel().
+ * @note    If a period is specified that is shorter than the pulse width
- * @note    Depending on the hardware implementation this function has
+ *          programmed in one of the channels then the behavior is not
- *          effect starting on the next cycle (recommended implementation)
+ *          guaranteed.
- *          or immediately (fallback implementation).
  *
  * @param[in] pwmp      pointer to a @p PWMDriver object
  * @param[in] period    new cycle time in ticks
  *
- * @api
+ * @notapi
  */
 void pwm_lld_change_period(PWMDriver *pwmp, pwmcnt_t period) {
 

os/kernel/include/ch.h

@@ -40,7 +40,7 @@
 /**
  * @brief   Kernel version string.
  */
-#define CH_KERNEL_VERSION       "2.3.1unstable"
+#define CH_KERNEL_VERSION       "2.3.2unstable"
 
 /**
  * @brief   Kernel version major number.
@@ -55,7 +55,7 @@
 /**
  * @brief   Kernel version patch number.
  */
-#define CH_KERNEL_PATCH         1
+#define CH_KERNEL_PATCH         2
 
 /*
  * Common values.

readme.txt

@@ -70,6 +70,11 @@
 *** Releases                                                              ***
 *****************************************************************************
 
+*** 2.3.2 ***
+- OPT: STM32 PWM driver implementation simplified.
+- CHANGE: Now pwmChangePeriod() does not implicitly disable the active
+  PWM channels.
+
 *** 2.3.1 ***
 - FIX: Fixed insufficient idle thread stack in Cortex-M0-GCC port (bug 3226671)
   (backported to 2.2.3).

testhal/STM32/PWM-ICU/main.c

@@ -89,7 +89,7 @@ int main(void) {
   palSetPad(IOPORT3, GPIOC_LED);
 
   /*
-   * Initializes the PWM driver 1.
+   * Initializes the PWM driver 1 and ICU driver 4.
    */
   pwmStart(&PWMD1, &pwmcfg);
   palSetPadMode(IOPORT1, 8, PAL_MODE_STM32_ALTERNATE_PUSHPULL);
@@ -98,26 +98,32 @@ int main(void) {
   chThdSleepMilliseconds(2000);
 
   /*
-   * Starts the channel 0 using 25% duty cycle.
+   * Starts the PWM channel 0 using 75% duty cycle.
-   */
-  pwmEnableChannel(&PWMD1, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD1, 2500));
-  chThdSleepMilliseconds(5000);
-
-  /*
-   * Changes the channel 0 to 75% duty cycle.
    */
   pwmEnableChannel(&PWMD1, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD1, 7500));
   chThdSleepMilliseconds(5000);
 
   /*
-   * Changes PWM period to half second and duty cycle to 50%.
+   * Changes the PWM channel 0 to 50% duty cycle.
    */
-  pwmChangePeriod(&PWMD1, 5000);
   pwmEnableChannel(&PWMD1, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD1, 5000));
   chThdSleepMilliseconds(5000);
 
   /*
-   * Disables channel 0.
+   * Changes the PWM channel 0 to 25% duty cycle.
+   */
+  pwmEnableChannel(&PWMD1, 0, PWM_PERCENTAGE_TO_WIDTH(&PWMD1, 2500));
+  chThdSleepMilliseconds(5000);
+
+  /*
+   * Changes PWM period to half second the duty cycle becomes 50%
+   * implicitly.
+   */
+  pwmChangePeriod(&PWMD1, 5000);
+  chThdSleepMilliseconds(5000);
+
+  /*
+   * Disables channel 0 and stops the drivers.
    */
   pwmDisableChannel(&PWMD1, 0);
   pwmStop(&PWMD1);