From 8e6b17df3e9e85f5199ca36f394e2a59a135079e Mon Sep 17 00:00:00 2001
From: Emilio Corigliano <emilio.corigliano@skywarder.eu>
Date: Thu, 8 Dec 2022 14:01:37 +0000
Subject: [PATCH] [ClockUtils] Fixed errors in retrieving clock frequency of
 APB buses

- Created two methods getAPBTimersClock() and getAPBPeripheralsClock()
- Fixed wrong computation of APB2 frequency
- Previous usages of getAPBFrequency() now use getAPBTimersClock() for usage compatibility
---
 .../drivers/canbus/CanDriver/CanDriver.cpp    |  9 ++-
 src/shared/drivers/timer/TimerUtils.h         |  2 +-
 src/shared/drivers/usart/USART.cpp            | 11 ++-
 src/shared/utils/ClockUtils.h                 | 74 ++++++++++++++++---
 4 files changed, 78 insertions(+), 18 deletions(-)

diff --git a/src/shared/drivers/canbus/CanDriver/CanDriver.cpp b/src/shared/drivers/canbus/CanDriver/CanDriver.cpp
index dc9469bdd..38c5da81a 100644
--- a/src/shared/drivers/canbus/CanDriver/CanDriver.cpp
+++ b/src/shared/drivers/canbus/CanDriver/CanDriver.cpp
@@ -117,8 +117,13 @@ CanbusDriver::BitTiming CanbusDriver::calcBitTiming(AutoBitTiming autoBt)
     uint8_t NOpt  = 5;
 
     BitTiming cfgIter;
-    cfgIter.SJW     = 0;
-    uint32_t apbclk = ClockUtils::getAPBFrequency(ClockUtils::APB::APB1);
+    cfgIter.SJW = 0;
+    /*
+     * TODO: This is modified only for compatibility with the past, MUST check
+     * the clock settings in order to use the right method
+     * 'getAPBPeripheralsClock()'
+     */
+    uint32_t apbclk = ClockUtils::getAPBTimersClock(ClockUtils::APB::APB1);
 
     // Iterate over the possible number of quanta in a bit to find the best
     // settings
diff --git a/src/shared/drivers/timer/TimerUtils.h b/src/shared/drivers/timer/TimerUtils.h
index 7d619419f..09ecfdc14 100644
--- a/src/shared/drivers/timer/TimerUtils.h
+++ b/src/shared/drivers/timer/TimerUtils.h
@@ -379,7 +379,7 @@ inline ClockUtils::APB TimerUtils::getTimerInputClock(const TIM_TypeDef *timer)
 
 inline uint32_t TimerUtils::getPrescalerInputFrequency(const TIM_TypeDef *timer)
 {
-    return ClockUtils::getAPBFrequency(getTimerInputClock(timer));
+    return ClockUtils::getAPBTimersClock(getTimerInputClock(timer));
 }
 
 inline uint32_t TimerUtils::getFrequency(TIM_TypeDef *timer)
diff --git a/src/shared/drivers/usart/USART.cpp b/src/shared/drivers/usart/USART.cpp
index 765b4dfcc..05d9e5f24 100644
--- a/src/shared/drivers/usart/USART.cpp
+++ b/src/shared/drivers/usart/USART.cpp
@@ -445,11 +445,16 @@ void USART::setBaudrate(Baudrate baudrate)
      */
     miosix::InterruptDisableLock dLock;
 
-    // USART1 and USART6 is always connected to the APB2, while all the others
+    /*
+     * TODO: This is modified only for compatibility with the past, MUST check
+     * the clock settings in order to use the right method
+     * 'getAPBPeripheralsClock()'
+     */
+    // USART1 and USART6 are always connected to the APB2, while all the others
     // UART/USART peripherals are always connected to APB1
-    uint32_t f = ClockUtils::getAPBFrequency(
+    uint32_t f = ClockUtils::getAPBTimersClock(
         (id == 1 || id == 6 ? ClockUtils::APB::APB2     // High speed APB2
-                            : ClockUtils::APB::APB1));  // Low speed APB1,
+                            : ClockUtils::APB::APB1));  // Low speed APB1
 
     // <<4 in order to shift to left of 4 positions, to create a fixed point
     // number of 4 decimal digits /8 == >>3 in order to divide per 8 (from the
diff --git a/src/shared/utils/ClockUtils.h b/src/shared/utils/ClockUtils.h
index addbdafb8..e7fca74c5 100644
--- a/src/shared/utils/ClockUtils.h
+++ b/src/shared/utils/ClockUtils.h
@@ -41,12 +41,20 @@ enum class APB
 };
 
 /**
- * @brief Computes the output frequency for the given APB bus.
+ * @brief Computes the output clock frequency for peripherals on the given APB.
  *
  * @param bus Advanced Peripheral Bus
- * @return Prescaler input frequency.
+ * @return Clock frequency of peripherals.
  */
-uint32_t getAPBFrequency(APB bus);
+uint32_t getAPBPeripheralsClock(APB bus);
+
+/**
+ * @brief Computes the output clock frequency for timers on the given APB.
+ *
+ * @param bus Advanced Peripheral Bus
+ * @return Clock frequency of timers.
+ */
+uint32_t getAPBTimersClock(APB bus);
 
 /**
  * @brief Enables a peripheral clock source from the APB1 and APB2 peripheral
@@ -62,18 +70,60 @@ bool disablePeripheralClock(void* peripheral);
 
 }  // namespace ClockUtils
 
-inline uint32_t ClockUtils::getAPBFrequency(APB bus)
+inline uint32_t ClockUtils::getAPBPeripheralsClock(APB bus)
+{
+    // The global variable SystemCoreClock from ARM's CMIS allows to know the
+    // CPU frequency.
+    uint32_t inputFrequency = SystemCoreClock;
+
+    // The APB frequency may be a submultiple of the CPU frequency, due to the
+    // bus at which the peripheral is connected being slower.
+    // The RCC->CFGR register tells us how slower the APB bus is running.
+    if (bus == APB::APB1)
+    {
+        // The position of the PPRE1 bit in RCC->CFGR is different in some stm32
+#ifdef _ARCH_CORTEXM3_STM32
+        const uint32_t ppre1 = 8;
+#elif _ARCH_CORTEXM4_STM32F4 | _ARCH_CORTEXM3_STM32F2
+        const uint32_t ppre1 = 10;
+#else
+#error "Architecture not supported by TimerUtils"
+#endif
+
+        if (RCC->CFGR & RCC_CFGR_PPRE1_2)
+        {
+            inputFrequency /= 2 << ((RCC->CFGR >> ppre1) & 0b11);
+        }
+    }
+    else
+    {
+        // The position of the PPRE2 bit in RCC->CFGR is different in some stm32
+#ifdef _ARCH_CORTEXM3_STM32
+        const uint32_t ppre2 = 11;
+#elif _ARCH_CORTEXM4_STM32F4 | _ARCH_CORTEXM3_STM32F2
+        const uint32_t ppre2 = 13;
+#else
+#error "Architecture not supported by TimerUtils"
+#endif
+
+        if (RCC->CFGR & RCC_CFGR_PPRE2_2)
+        {
+            inputFrequency /= 2 << ((RCC->CFGR >> ppre2) & 0b11);
+        }
+    }
+
+    return inputFrequency;
+}
+
+inline uint32_t ClockUtils::getAPBTimersClock(APB bus)
 {
     // The global variable SystemCoreClock from ARM's CMIS allows to know the
     // CPU frequency.
     uint32_t inputFrequency = SystemCoreClock;
 
-    // The timer frequency may be a submultiple of the CPU frequency, due to the
+    // The APB frequency may be a submultiple of the CPU frequency, due to the
     // bus at which the peripheral is connected being slower.
-    // The RCC-ZCFGR register tells us how slower the APB bus is running.
-    // The following formula takes into account that if the APB1 clock is
-    // divided by a factor of two or grater, the timer is clocked at twice the
-    // bus interface.
+    // The RCC->CFGR register tells us how slower the APB bus is running.
     if (bus == APB::APB1)
     {
         // The position of the PPRE1 bit in RCC->CFGR is different in some stm32
@@ -87,7 +137,7 @@ inline uint32_t ClockUtils::getAPBFrequency(APB bus)
 
         if (RCC->CFGR & RCC_CFGR_PPRE1_2)
         {
-            inputFrequency /= 1 << ((RCC->CFGR >> ppre1) & 0x3);
+            inputFrequency /= 1 << ((RCC->CFGR >> ppre1) & 0b11);
         }
     }
     else
@@ -103,7 +153,7 @@ inline uint32_t ClockUtils::getAPBFrequency(APB bus)
 
         if (RCC->CFGR & RCC_CFGR_PPRE2_2)
         {
-            inputFrequency /= 1 << ((RCC->CFGR >> ppre2) >> 0x3);
+            inputFrequency /= 1 << ((RCC->CFGR >> ppre2) & 0b11);
         }
     }
 
@@ -448,7 +498,7 @@ inline bool ClockUtils::disablePeripheralClock(void* peripheral)
                 RCC->APB1ENR &= ~RCC_APB1ENR_TIM2EN;
                 break;
             case TIM3_BASE:
-                RCC->APB1ENR &= ~RCC_APB1ENR_TIM2EN;
+                RCC->APB1ENR &= ~RCC_APB1ENR_TIM3EN;
                 break;
             case TIM4_BASE:
                 RCC->APB1ENR &= ~RCC_APB1ENR_TIM4EN;
-- 
GitLab