Bug fix for TIM9 interrupt for PWM Topic is solved

[wild-boar]

I upgraded a project from ChibiOS 18 to 20. I noticed that the interrupt handler for TIM9 under PWM was removed. I put back the changes and it appears to be working. Here are the three code changes. Thanks!

diff --git a/os/hal/ports/STM32/LLD/TIMv1/hal_pwm_lld.c b/os/hal/ports/STM32/LLD/TIMv1/hal_pwm_lld.c
index eb6bebc7e..9b10df847 100644
--- a/os/hal/ports/STM32/LLD/TIMv1/hal_pwm_lld.c
+++ b/os/hal/ports/STM32/LLD/TIMv1/hal_pwm_lld.c
@@ -364,7 +364,22 @@ OSAL_IRQ_HANDLER(STM32_TIM8_CC_HANDLER) {
 
 #if STM32_PWM_USE_TIM9 || defined(__DOXYGEN__)
 #if !defined(STM32_TIM9_SUPPRESS_ISR)
-#error "TIM9 ISR not defined by platform"
+#if !defined(STM32_TIM9_HANDLER)
+#error "STM32_TIM9_HANDLER not defined"
+#endif
+/**
+ *  * @brief   TIM9 interrupt handler.
+ *   *
+ *    * @isr
+ *     */
+OSAL_IRQ_HANDLER(STM32_TIM9_HANDLER) {
+
+  OSAL_IRQ_PROLOGUE();
+
+  pwm_lld_serve_interrupt(&PWMD9);
+
+  OSAL_IRQ_EPILOGUE();
+}
 #endif /* !defined(STM32_TIM9_SUPPRESS_ISR) */
 #endif /* STM32_PWM_USE_TIM9 */
 
@@ -683,6 +698,9 @@ void pwm_lld_start(PWMDriver *pwmp) {
     if (&PWMD9 == pwmp) {
       rccEnableTIM9(true);
       rccResetTIM9();
+#if !defined(STM32_TIM9_SUPPRESS_ISR)
+      nvicEnableVector(STM32_TIM9_NUMBER, STM32_PWM_TIM9_IRQ_PRIORITY);
+#endif
 #if defined(STM32_TIM9CLK)
       pwmp->clock = STM32_TIM9CLK;
 #else
@@ -1059,6 +1077,9 @@ void pwm_lld_stop(PWMDriver *pwmp) {
 
 #if STM32_PWM_USE_TIM9
     if (&PWMD9 == pwmp) {
+#if !defined(STM32_TIM9_SUPPRESS_ISR)
+      nvicDisableVector(STM32_TIM9_NUMBER);
+#endif
       rccDisableTIM9();
     }
 #endif

[Giovanni]

Hi,

The change is intentional, those IRQs sources that share the IRQ handler with other sources are no more defined into the drivers. Recent STM32s define shared handlers in stm32_irq.c so that error is not triggered. Some older STM32s have not yet been updated.

You may define the ISR in your application code making sure to define that macro (perhaps in mcuconf.h) so that the #error is not triggered.

Giovanni

[wild-boar]

Thank you for the response. Looking at all the other ISR handlers, I now understand the problem.

I am using the STM32F4 platform and it is only missing TIM9. I am hoping that you can accept this patch for the STM32F4 platform that adds TIM9 interrupt support back. I have tested it and it works.

I put the patched code here and also attached it. Thanks!

Code: Select all

diff --git a/os/hal/ports/STM32/LLD/TIMv1/stm32_tim9.inc b/os/hal/ports/STM32/LLD/TIMv1/stm32_tim9.inc
new file mode 100644
index 000000000..7aebb4a64
--- /dev/null
+++ b/os/hal/ports/STM32/LLD/TIMv1/stm32_tim9.inc
@@ -0,0 +1,133 @@
+/*
+    ChibiOS - Copyright (C) 2006..2018 Giovanni Di Sirio
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+        http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+/**
+ * @file    TIMv1/stm32_tim9.inc
+ * @brief   Shared TIM9 handler.
+ *
+ * @addtogroup STM32_TIM9_HANDLER
+ * @{
+ */
+
+/*===========================================================================*/
+/* Driver local definitions.                                                 */
+/*===========================================================================*/
+
+/*===========================================================================*/
+/* Derived constants and error checks.                                       */
+/*===========================================================================*/
+
+/* Registry checks for robustness.*/
+#if !defined(STM32_HAS_TIM9)
+#error "STM32_HAS_TIM9 not defined in registry"
+#endif
+
+/* Driver checks for robustness, undefined USE macros are defaulted to
+   FALSE. This makes this module independent from drivers implementation.*/
+#if !defined(STM32_GPT_USE_TIM9)
+#define STM32_GPT_USE_TIM9                  FALSE
+#endif
+#if !defined(STM32_ICU_USE_TIM9)
+#define STM32_ICU_USE_TIM9                  FALSE
+#endif
+#if !defined(STM32_PWM_USE_TIM9)
+#define STM32_PWM_USE_TIM9                  FALSE
+#endif
+#if !defined(STM32_ST_USE_TIM9)
+#define STM32_ST_USE_TIM9                   FALSE
+#endif
+
+#if STM32_HAS_TIM9
+
+/* Priority settings checks.*/
+#if !defined(STM32_IRQ_TIM9_PRIORITY)
+#error "STM32_IRQ_TIM9_PRIORITY not defined in mcuconf.h"
+#endif
+
+#if !OSAL_IRQ_IS_VALID_PRIORITY(STM32_IRQ_TIM9_PRIORITY)
+#error "Invalid IRQ priority assigned to STM32_IRQ_TIM9_PRIORITY"
+#endif
+
+#endif /* STM32_HAS_TIM9 */
+
+/*===========================================================================*/
+/* Driver exported variables.                                                */
+/*===========================================================================*/
+
+/*===========================================================================*/
+/* Driver local variables.                                                   */
+/*===========================================================================*/
+
+/*===========================================================================*/
+/* Driver local functions.                                                   */
+/*===========================================================================*/
+
+static inline void tim9_irq_init(void) {
+#if defined(STM32_TIM9_IS_USED)
+  nvicEnableVector(STM32_TIM9_NUMBER, STM32_IRQ_TIM9_PRIORITY);
+#endif
+}
+
+static inline void tim9_irq_deinit(void) {
+#if defined(STM32_TIM9_IS_USED)
+  nvicDisableVector(STM32_TIM9_NUMBER);
+#endif
+}
+
+/*===========================================================================*/
+/* Driver interrupt handlers.                                                */
+/*===========================================================================*/
+
+#if defined(STM32_TIM9_IS_USED) || defined(__DOXYGEN__)
+/**
+ * @brief   TIM9 interrupt handler.
+ *
+ * @isr
+ */
+OSAL_IRQ_HANDLER(STM32_TIM9_HANDLER) {
+
+  OSAL_IRQ_PROLOGUE();
+
+#if HAL_USE_GPT
+#if STM32_GPT_USE_TIM9
+  gpt_lld_serve_interrupt(&GPTD9);
+#endif
+#endif
+#if HAL_USE_ICU
+#if STM32_ICU_USE_TIM9
+  icu_lld_serve_interrupt(&ICUD9);
+#endif
+#endif
+#if HAL_USE_PWM
+#if STM32_PWM_USE_TIM9
+  pwm_lld_serve_interrupt(&PWMD9);
+#endif
+#endif
+#if 1
+#if STM32_ST_USE_TIM9
+  st_lld_serve_interrupt();
+#endif
+#endif
+
+  OSAL_IRQ_EPILOGUE();
+}
+#endif
+
+/*===========================================================================*/
+/* Driver exported functions.                                                */
+/*===========================================================================*/
+
+/** @} */
diff --git a/os/hal/ports/STM32/STM32F4xx/stm32_isr.c b/os/hal/ports/STM32/STM32F4xx/stm32_isr.c
index 08237e1c3..65767f908 100644
--- a/os/hal/ports/STM32/STM32F4xx/stm32_isr.c
+++ b/os/hal/ports/STM32/STM32F4xx/stm32_isr.c
@@ -51,6 +51,8 @@
 /* Driver interrupt handlers.                                                */
 /*===========================================================================*/
 
+#include "stm32_tim9.inc"
+
 #if (HAL_USE_PAL && (PAL_USE_WAIT || PAL_USE_CALLBACKS)) || defined(__DOXYGEN__)
 #if !defined(STM32_DISABLE_EXTI0_HANDLER)
 /**
@@ -223,6 +225,8 @@ OSAL_IRQ_HANDLER(VectorE0) {
  */
 void irqInit(void) {
 
+  tim9_irq_init();
+
 #if HAL_USE_PAL
   nvicEnableVector(EXTI0_IRQn, STM32_IRQ_EXTI0_PRIORITY);
   nvicEnableVector(EXTI1_IRQn, STM32_IRQ_EXTI1_PRIORITY);
@@ -241,6 +245,8 @@ void irqInit(void) {
  */
 void irqDeinit(void) {
 
+  tim9_irq_deinit();
+
 #if HAL_USE_PAL
   nvicDisableVector(EXTI0_IRQn);
   nvicDisableVector(EXTI1_IRQn);
diff --git a/os/hal/ports/STM32/STM32F4xx/stm32_isr.h b/os/hal/ports/STM32/STM32F4xx/stm32_isr.h
index 6a826f6a3..3499794ec 100644
--- a/os/hal/ports/STM32/STM32F4xx/stm32_isr.h
+++ b/os/hal/ports/STM32/STM32F4xx/stm32_isr.h
@@ -29,6 +29,12 @@
 /* Driver constants.                                                         */
 /*===========================================================================*/
 
+/**
+ * @name    ISRs suppressed in standard drivers
+ * @{
+ */
+#define STM32_TIM9_SUPPRESS_ISR
+
 /**
  * @name    ISR names and numbers remapping
  * @{


[FXCoder]

Hi,
The existing 20.3.x ChibiOS branch is intended to support TIM9 through stm32_tim1_9_10_11.inc

There is a typo in that file at line 191...

Code: Select all

#if defined(STM32_TIM1_IS_USED) || defined(STM32_TIM19_IS_USED)  ||         \


Should be...

Code: Select all

#if defined(STM32_TIM1_IS_USED) || defined(STM32_TIM9_IS_USED)  ||          \


Try removing your patch, edit os/hal/ports/STM32/LLD/TIMv1/stm32_tim1_9_10_11.inc as above and let us know how that goes.
--
Bob

[wild-boar]

It worked! Thank you!

I am hoping that you can accept this patch for the STM32F4 platform that adds TIM9 interrupt support back. I have tested it and it works.

Here are the three files that need to change:

Code: Select all

diff --git a/os/hal/ports/STM32/LLD/TIMv1/stm32_tim1_9_10_11.inc b/os/hal/ports/STM32/LLD/TIMv1/stm32_tim1_9_10_11.inc
index bebcf542e..bffbf3b70 100644
--- a/os/hal/ports/STM32/LLD/TIMv1/stm32_tim1_9_10_11.inc
+++ b/os/hal/ports/STM32/LLD/TIMv1/stm32_tim1_9_10_11.inc
@@ -188,7 +188,7 @@ static inline void tim1_tim9_tim10_tim11_irq_deinit(void) {
 /* Driver interrupt handlers.                                                */
 /*===========================================================================*/
 
-#if defined(STM32_TIM1_IS_USED) || defined(STM32_TIM19_IS_USED)  ||         \
+#if defined(STM32_TIM1_IS_USED) || defined(STM32_TIM9_IS_USED)  ||         \
     defined(__DOXYGEN__)
 /**
  * @brief   TIM1-BRK, TIM9 interrupt handler.


Code: Select all

diff --git a/os/hal/ports/STM32/STM32F4xx/stm32_isr.c b/os/hal/ports/STM32/STM32F4xx/stm32_isr.c
index 08237e1c3..232177aee 100644
--- a/os/hal/ports/STM32/STM32F4xx/stm32_isr.c
+++ b/os/hal/ports/STM32/STM32F4xx/stm32_isr.c
@@ -51,6 +51,8 @@
 /* Driver interrupt handlers.                                                */
 /*===========================================================================*/
 
+#include "stm32_tim1_9_10_11.inc"
+
 #if (HAL_USE_PAL && (PAL_USE_WAIT || PAL_USE_CALLBACKS)) || defined(__DOXYGEN__)
 #if !defined(STM32_DISABLE_EXTI0_HANDLER)
 /**
@@ -223,6 +225,8 @@ OSAL_IRQ_HANDLER(VectorE0) {
  */
 void irqInit(void) {
 
+  tim1_tim9_tim10_tim11_irq_init();
+
 #if HAL_USE_PAL
   nvicEnableVector(EXTI0_IRQn, STM32_IRQ_EXTI0_PRIORITY);
   nvicEnableVector(EXTI1_IRQn, STM32_IRQ_EXTI1_PRIORITY);
@@ -241,6 +245,8 @@ void irqInit(void) {
  */
 void irqDeinit(void) {
 
+  tim1_tim9_tim10_tim11_irq_deinit();
+
 #if HAL_USE_PAL
   nvicDisableVector(EXTI0_IRQn);
   nvicDisableVector(EXTI1_IRQn);

Code: Select all

diff --git a/os/hal/ports/STM32/STM32F4xx/stm32_isr.h b/os/hal/ports/STM32/STM32F4xx/stm32_isr.h
index 6a826f6a3..727a8069a 100644
--- a/os/hal/ports/STM32/STM32F4xx/stm32_isr.h
+++ b/os/hal/ports/STM32/STM32F4xx/stm32_isr.h
@@ -29,6 +29,12 @@
 /* Driver constants.                                                         */
 /*===========================================================================*/
 
+/**
+ * @name    ISRs suppressed in standard drivers
+ * @{
+ */
+#define STM32_TIM9_SUPPRESS_ISR
+
 /**
  * @name    ISR names and numbers remapping
  * @{
@@ -105,7 +111,7 @@
 #define STM32_TIM7_HANDLER          Vector11C
 #define STM32_TIM8_UP_HANDLER       VectorF0
 #define STM32_TIM8_CC_HANDLER       VectorF8
-#define STM32_TIM9_HANDLER          VectorA0
+#define STM32_TIM1_BRK_TIM9_HANDLER VectorA0
 #define STM32_TIM10_HANDLER         VectorA4 /* Note: same as STM32_TIM1_UP */
 #define STM32_TIM11_HANDLER         VectorA8
 #define STM32_TIM12_HANDLER         VectorEC
@@ -122,7 +128,7 @@
 #define STM32_TIM7_NUMBER           55
 #define STM32_TIM8_UP_NUMBER        44
 #define STM32_TIM8_CC_NUMBER        46
-#define STM32_TIM9_NUMBER           24
+#define STM32_TIM1_BRK_TIM9_NUMBER  24
 #define STM32_TIM10_NUMBER          25 /* Note: same as STM32_TIM1_UP */
 #define STM32_TIM11_NUMBER          26
 #define STM32_TIM12_NUMBER          43

[Giovanni]

Moving in bug reports.

Giovanni

[FXCoder]

Same bug also in trunk

[Giovanni]

Hi,

Fixed as bug #1143.

The timers functionality has already been re-introduced in trunk code, I don't want to change the stable 20.3.x except for actual bugs.

Giovanni