Skip to content

Feat/clock domain#667

Draft
FoniksFox wants to merge 19 commits into
developmentfrom
feat/ClockDomain
Draft

Feat/clock domain#667
FoniksFox wants to merge 19 commits into
developmentfrom
feat/ClockDomain

Conversation

@FoniksFox

@FoniksFox FoniksFox commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Closes #665

Add ClockDomain, a centralised clock validator and applicator

Clock configuration is a precomputed ClockTree (from a host tool or hand-written). The firmware validates it at compile time and applies it at runtime. No solver runs in the firmware.

  • ClockTree stores only decisions (M/N/P/Q/R, prescalers, source enums); all frequencies are derived via accessors (sysclk(t), source_frequency(t, src), etc.)
  • Board<> takes a mandatory ClockTree as the second template parameter
  • Peripherals register clock requirements via ClockDomain::Device with typed models: SPIClockModel<Group, MaxBaud, MinBaud>, ADCClockModel<MaxADCCLK>, SDClockModel<MaxFreq, MinFreq>
  • ClockDomain::build() validates PLL ranges, bus consistency, and peripheral requirements at compile time - refuses to compile on failure
  • ClockDomain::Init::init(tree) applies the validated tree to HAL registers
  • ADC prescaler computed from kernel clock at init (no longer user-specified)
  • SPI/SD use mandatory frequency ranges
  • Removed HALconfig

Clock-tree solver was not implement in compile time because of the added complexity (worst cases were not possible to compile unless willing to let the compiler do billions of operations). The solver may be implement at a further date via a brute force function in the domain that only exists on simulator builds, such that you can build your project with the simulation preset, solve the clock-tree and print it to a header file (it could be added as a build step or as a standalone script).

FoniksFox added 11 commits June 21, 2026 03:42
diff --git c/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp i/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp
index 680bf82..4c09a80 100644
--- c/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp
+++ i/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp
@@ -18,6 +18,7 @@
 #include <array>

 #include "ErrorHandler/ErrorHandler.hpp"
+#include "HALAL/Models/Clocks/ClockDomain.hpp"

 #ifndef SCHEDULER_TIMER_DOMAIN
 /* default is tim2 */
@@ -293,6 +294,7 @@ struct TimerDomain {

     struct Config {
         uint8_t timer_idx;
+        TimerRequest request;
         SelectionTrigger1 trgo1;
         SelectionTrigger2 trgo2;
     };
@@ -639,6 +641,7 @@ struct TimerDomain {

                 Config cfg = {
                     .timer_idx = timer_idxmap[reqint],
+                    .request = requests[i].request,
                     .trgo1 = requests[i].trgo1,
                     .trgo2 = requests[i].trgo2
                 };
@@ -672,7 +675,7 @@ struct TimerDomain {

                 uint8_t reqint = remaining_32bit_timers[count_32bit_requests];
                 Config cfg =
-                    {.timer_idx = timer_idxmap[reqint], .trgo1 = e.trgo1, .trgo2 = e.trgo2};
+                    {.timer_idx = timer_idxmap[reqint], .request = e.request, .trgo1 = e.trgo1, .trgo2 = e.trgo2};
                 cfgs[cfg_idx++] = cfg;

                 // unordered remove
@@ -719,7 +722,7 @@ struct TimerDomain {
                 ST_LIB::compile_error("This only processes TimerRequest::AnyGeneralPurpose");
             }
             uint8_t reqint = remaining_timers[i];
-            Config cfg = {.timer_idx = timer_idxmap[reqint], .trgo1 = e.trgo1, .trgo2 = e.trgo2};
+            Config cfg = {.timer_idx = timer_idxmap[reqint], .request = e.request, .trgo1 = e.trgo1, .trgo2 = e.trgo2};
             cfgs[cfg_idx++] = cfg;
         }

@@ -732,8 +735,27 @@ struct TimerDomain {
         TIM_HandleTypeDef* hal_tim;
         TIM_MasterConfigTypeDef master{};
         uint8_t timer_idx;
+        uint32_t clock_frequency = 0;  // from ClockTree
     };

+    static constexpr bool is_timer_on_apb1(TimerRequest r) {
+        switch (r) {
+            case TimerRequest::GeneralPurpose32bit_2:
+            case TimerRequest::GeneralPurpose_3:
+            case TimerRequest::GeneralPurpose_4:
+            case TimerRequest::GeneralPurpose32bit_5:
+            case TimerRequest::Basic_6:
+            case TimerRequest::Basic_7:
+            case TimerRequest::SlaveTimer_12:
+            case TimerRequest::SlaveTimer_13:
+            case TimerRequest::SlaveTimer_14:
+            case TimerRequest::GeneralPurpose32bit_23:
+            case TimerRequest::GeneralPurpose32bit_24:
+                return true;
+            default: return false;
+        }
+    }
+
     static void (*callbacks[TimerDomain::max_instances])(void*);
     static void* callback_data[TimerDomain::max_instances];

@@ -742,7 +764,7 @@ struct TimerDomain {

         static void TIM_Default_Callback(void* raw) { (void)raw; }

-        static void init(std::span<const Config, N> cfgs) {
+        static void init(std::span<const Config, N> cfgs, const ClockDomain::ClockTree& tree) {
             Scheduler_global_timer = cmsis_timers[timer_idxmap[SCHEDULER_TIMER_DOMAIN]];
             rcc_enable_timer(Scheduler_global_timer);

@@ -802,6 +824,8 @@ struct TimerDomain {
                 inst->tim = tim;
                 inst->hal_tim = handle;
                 inst->timer_idx = e.timer_idx;
+                inst->clock_frequency = is_timer_on_apb1(e.request)
+                    ? ClockDomain::timer_apb1(tree) : ClockDomain::timer_apb2(tree);
                 TIM_MasterConfigTypeDef sMasterConfig = {};
                 sMasterConfig.MasterOutputTrigger = static_cast<uint32_t>(e.trgo1);
                 sMasterConfig.MasterOutputTrigger2 = static_cast<uint32_t>(e.trgo2);
diff --git c/Inc/HALAL/Services/Time/TimerWrapper.hpp i/Inc/HALAL/Services/Time/TimerWrapper.hpp
index 051849b..d41878c 100644
--- c/Inc/HALAL/Services/Time/TimerWrapper.hpp
+++ i/Inc/HALAL/Services/Time/TimerWrapper.hpp
@@ -135,19 +135,10 @@ template <const TimerDomain::Timer& dev> struct TimerWrapper {
     }

     inline uint32_t get_clock_frequency() {
-        uint32_t result;
-        if constexpr (this->is_on_APB1) {
-            result = HAL_RCC_GetPCLK1Freq();
-            if ((RCC->D2CFGR & RCC_D2CFGR_D2PPRE1) != RCC_HCLK_DIV1) {
-                result *= 2;
-            }
-        } else {
-            result = HAL_RCC_GetPCLK2Freq();
-            if ((RCC->D2CFGR & RCC_D2CFGR_D2PPRE2) != RCC_HCLK_DIV1) {
-                result *= 2;
-            }
+        if (instance->clock_frequency == 0) {
+            PANIC("Timer clock not configured by ClockDomain");
         }
-        return result;
+        return instance->clock_frequency;
     }

     template <TimerPin pin>
diff --git c/Inc/ST-LIB.hpp i/Inc/ST-LIB.hpp
index 168dacd..e65ec55 100644
--- c/Inc/ST-LIB.hpp
+++ i/Inc/ST-LIB.hpp
@@ -316,7 +316,7 @@ public:

         MPUDomain::Init<mpuN, cfg.mpu_cfgs>::init();
         GPIODomain::Init<gpioN>::init(cfg.gpio_cfgs);
-        TimerDomain::Init<timN>::init(cfg.tim_cfgs);
+        TimerDomain::Init<timN>::init(cfg.tim_cfgs, cfg.clock_tree);
         DMADomain::Init<dmaN>::init(cfg.dma_cfgs);
         SPIDomain::Init<spiN>::init(
             cfg.spi_cfgs,
@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

ST-LIB Release Plan

  • Current version: 6.1.3
  • Pending changesets: 2
  • Highest requested bump: major
  • Next version if merged now: 7.0.0

Pending changes

  • major Add ClockDomain, a centralised clock validator and applicator (.changesets/clock-redesign.md)
  • patch Fix a warning about symbols in HardfaultTrace (doesn't change any behaviour) (.changesets/fix-hwarfault-warning.md)

@FoniksFox FoniksFox requested a review from Copilot June 21, 2026 11:08

This comment was marked as off-topic.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of code duplicated here which shouldn't be, TimerDomain is not to be used like TimerWrapper. TimerWrapper already handles clocks, so remove TimerRequest from TimerDomain::Config and clock_frequency from the timer instance, as you can already get that from the timeridx or the TIM_TypeDef, it's fine to have a runtime check for if a timer is in apb1 or apb2 if you only have the TIM_TypeDef and that's exactly the reason why I have that function in TimerDomain.

@victor-Lopez25 victor-Lopez25 Jun 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need the ClockDomain from the TimerDomain to know what apb1 and apb2 frequencies are, you should probably use a global ClockDomain, that would keep it simpler and you could even get the function which uses HAL code to check a TIM_TypeDef's frequency to work. The only place where it is used is in InputCapture, which is pretty important...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just use a global:

if constexpr (this->is_on_APB1) {
  return ClockDomain::timer_apb1(global_clock_tree);
} else {
  return ClockDomain::timer_apb2(global_clock_tree);
}

You're also adding a PANIC where there's no reason to have one, a timer is always in apb1 or apb2 so it doesn't make much sense to add a possibility for failiure.

Comment thread Inc/ST-LIB.hpp
Comment on lines +247 to 252
// Build: validate the tree against peripheral requirements
ClockDomain::build(ctx.template span<ClockDomain>(), ClockTreeDef);

return ConfigBundle{
.clock_tree = ClockTreeDef,
.mpu_cfgs = MPUDomain::template build<mpuN>(ctx.template span<MPUDomain>()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no way to have it work like the others, so we can simplify creating Board?

Comment thread Inc/ST-LIB.hpp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we already have a global ClockDomain instance, Board::clock_tree(), why are we passing the clock tree to every domain? I get that you can't access Board from a domain but you could just make it global from ClockDomain since you already know there will only ever be one.

FoniksFox and others added 6 commits June 21, 2026 15:28
* fix: Fix a warning about symbols missmatch

* chore: Add changeset
diff --git c/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp i/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp
index db1a014..8219d4f 100644
--- c/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp
+++ i/Inc/HALAL/Models/TimerDomain/TimerDomain.hpp
@@ -672,11 +672,8 @@ struct TimerDomain {
                 }

                 uint8_t reqint = remaining_32bit_timers[count_32bit_requests];
-                Config cfg = {
-                    .timer_idx = timer_idxmap[reqint],
-                    .trgo1 = e.trgo1,
-                    .trgo2 = e.trgo2
-                };
+                Config cfg =
+                    {.timer_idx = timer_idxmap[reqint], .trgo1 = e.trgo1, .trgo2 = e.trgo2};
                 cfgs[cfg_idx++] = cfg;

                 // unordered remove
@@ -723,11 +720,7 @@ struct TimerDomain {
                 ST_LIB::compile_error("This only processes TimerRequest::AnyGeneralPurpose");
             }
             uint8_t reqint = remaining_timers[i];
-            Config cfg = {
-                .timer_idx = timer_idxmap[reqint],
-                .trgo1 = e.trgo1,
-                .trgo2 = e.trgo2
-            };
+            Config cfg = {.timer_idx = timer_idxmap[reqint], .trgo1 = e.trgo1, .trgo2 = e.trgo2};
             cfgs[cfg_idx++] = cfg;
         }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flash latency set to 3 instead of 4

3 participants