Conversation
Three extern declarations disagreed with their definitions. The g431 one is a real bug: commutation_interval is a volatile uint32_t but the interrupt handler declared it uint16_t, so the early-zero-cross guard read only half the variable. The a153 aTxBuffer extern had the wrong sign and size, harmless today because only the address is taken, and the dshot.c zero_crosses extern was unused with the wrong type. Surfaced by LTO type merging while preparing the LTO change later in this series; they are standing bugs with or without it.
The main loop never returns, so for any global whose only other accessors are interrupt handlers the optimizer may legally cache reads in a register forever or sink stores out of the loop. Today the huge loop body and cross translation unit calls happen to force reloads, but that is luck, not a contract, and the LTO change later in this series removes the cross-TU opacity entirely. The vulnerable set is what the main loop reads that ISRs write (failsafe compares could never fire) and what the main loop writes that ISRs read (updates could never become visible). - signaltimeout: ISR-written, main loop signal-loss failsafe compare - input, running, forward, tenkhzcounter, commutation_intervals, PROCESS_ADC_FLAG: ISR-written, read by main loop logic - e_com_time, tim1_arr, duty_cycle_maximum, zero_input_count, old_routine: main-loop-written, read by ISRs - dma_buffer: written by DMA hardware, there is no code writer at all - a153 externs for inputSet/zero_input_count: missing the volatile qualifier the definitions already have ISR-to-ISR sharing needs nothing: handlers are anchored by the vector table in startup assembly, so every invocation reloads at the function boundary. All accesses are naturally aligned 8/16/32-bit loads and stores, so volatile adds no tearing hazard, only honest memory accesses.
(duty_cycle * tim1_arr) / 2000 and its prop-brake/active-brake variants each ran a soft division in the 20khz interrupt, 50+ cycles apiece on cortex-m0 which has no hardware divider. Replace them with a Q16 reciprocal multiply against pwm_to_arr_scale_q16; the main loop recomputes the scale factor at idle priority only when tim1_arr actually changes (first pass forced via last_tim1_arr = 0). The result is within one timer count of the exact division for every duty value and ARR (floor-of-floor error is bounded by duty/65536 < 1), verified exhaustively host-side up to ARR 6012, the max with the 8khz PWM setting. The scale factor is volatile like the rest of the ISR-shared state; tim1_arr and the scale update non-atomically but the only ISR path reading both is prop brake, and tim1_arr is pinned constant whenever prop brake can be active (motor stopped pins the variable-pwm logic to its 250-interval branch).
The two hot map() calls in setInput run at input frame rate (up to dshot frame rate). map() is recursive binary interpolation, roughly ten nested calls per evaluation, and deviated up to 5/2000 (0.25% throttle) from a true straight line. Replace them with Q16 slopes computed once in main() after minimum_duty_cycle reaches its final value; the replacement is the exact line through the old endpoints, so this is both faster and more linear. Servo input and other low-rate map() callers are unchanged. minimum_duty_cycle never changes after the bake point today (the runtime dshot settings-save path does not reload settings), so the baked slope cannot go stale; if a runtime reload is ever added the bake must move into loadEEpromSettings.
computeDshotDMA unpacked the 16 pulses into a 16-entry int array and then walked it three more times to assemble the CRC nibbles and the 11-bit value. Pack the pulses into one uint16_t as they are measured (bit 15-i is pulse i) and derive everything with shifts and masks: checkCRC = frame & 0xF, calcCRC = ((frame>>4)^(frame>>8)^(frame>>12)) & 0xF, value = frame >> 5, telemetry request = frame & 0x10. The inverted-CRC bidirectional case (~checkCRC) & 0xF equals the old ~checkCRC + 16 for all nibble values. Bit-for-bit equivalent to the old decode and frees the 64 byte dpulse array.
numReadings was a const qualified variable, not a constant expression, so total / numReadings emitted a soft division on every 1khz current sample. As a #define the compiler folds the divide-by-50 into a multiply and shift sequence. Window size and behavior are unchanged.
The dshot telemetry priority swap wrote three or four NVIC registers on every main loop iteration even though the scheme flips rarely. Track the current scheme in input_priority_is_high and skip the writes when nothing changed. The 0xFF boot sentinel forces the first pass to write, preserving the old boot behavior exactly.
Inline hot helpers across translation units (comStep, commutate, the LL wrappers and friends), which matters most on cortex-m0 where call overhead in the commutation interrupts is high. The whole firmware is already compiled and linked in a single gcc invocation, so -flto needs no build restructuring, no archives are involved and no objects persist between builds. An mcu makefile can opt out by clearing LTO_FLAGS_<MCU>. Opt-outs: DroneCAN builds, because gcc 10 LTO trips Werror maybe-uninitialized false positives in the canard/dsdl code and CAN capable targets have ample flash anyway; E230, whose 27k flash cannot fit the -O3 LTO inlining growth. filename[] needs __attribute__((used)) to survive into its .file_name section now that LTO can see nothing references it. On the F051, LTO costs about 2.7KB flash (+12%) in exchange for the inlining; the earlier volatile and extern-mismatch commits in this series are prerequisites, since whole-program visibility removes the cross-TU reload barriers non-volatile shared globals relied on and turns declaration mismatches into wrong-code risks.
The F051 executes flash at one wait state at 48MHz and SRAM at zero. The prefetch buffer hides the wait state on straight line code but every taken branch and interrupt entry pays it, and the commutation interrupts are exactly that kind of branchy code. Tag the hot path RAM_FUNC: the ADC1_COMP/TIM6_DAC/TIM14 handlers, interruptRoutine, PeriodElapsedCallback, commutate, comStep, tenKhzRoutine, getBemfState and the comparator helpers, about 2.9KB of code. For gcc the .ramfunc input section sits inside .data so the existing startup data init copies it, and the linker generates the flash/RAM veneers. For Keil/armclang the scatter file selects .ramfunc into RW_IRAM1 so armlink scatter loading does the same copy before main; toolchains must not differ in hot path timing. The Keil path still needs one build on Windows to confirm. RAM_FUNC is empty on every other MCU. Worst case F051 target keeps 2.1KB of RAM headroom above the 1.5KB heap/stack reserve.
getCompOutputLevel was an out of line cross translation unit call inside the comparator ISR confirmation loop, up to filter_level (12) calls per zero cross at 15-20 cycles each; as a static inline register read each sample costs about 4. LTO would inline it for release gcc builds anyway, but the header inline also covers non-LTO debug builds and Keil, which have no LTO. This shrinks the wall clock sampling window of the confirmation loop from roughly 5us to 1.2us at filter_level 12: same sample count, less immunity to noise bursts longer than the window. That is the main thing high current bench testing needs to watch, and the loop now carries a comment saying so; the filter_level mapping may want retuning.
SysTick_Config enabled the 1khz SysTick interrupt whose handler is empty, paying interrupt entry and exit every millisecond for nothing. Program LOAD/VAL/CTRL directly so the counter keeps running for anything that polls it, without TICKINT. Nothing on the F051 uses the tick: the utility timebase is TIM17 and the empty handler stays in the vector table as a safety net.
This was referenced Jun 15, 2026
The BlueJay custom startup melody (playBlueJayTune and its helpers) only runs when a user has flashed a custom tune blob into the eeprom tune[] area, which ARK boards never do, yet LTO still links the whole engine on every build. Compile it out behind DISABLE_CUSTOM_TUNE, defined for the flash-tight ARK_4IN1_F051 / ARK_4IN1_RAMP_F051 targets; playStartupTune() falls back to the fixed three-beep startup. Other boards are unaffected and keep the custom-tune feature. ARK_4IN1_F051: 25404 -> 24980 B (92.6% -> 91.1% of the 27 KB app region).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Clean reimplementation of AlexKlimaj's am32-firmware#379 as a reviewable series: 11 atomic commits, each building green on its own, with our review fixes folded in where they logically belong (the volatile audit lands before LTO is enabled, the Keil scatter-file placement is part of the RAM-function commit, and the comparator filter-window explanation rides with the inline that shrank it). Intended to be read commit by commit; staged against this fork's main so we can iterate before opening upstream.
Called out explicitly because it is a real trade: enabling LTO costs ~2.7 KB flash (+12%) on the F051 (measured 22.6 K → 25.4 K on SEQURE_4IN1) in exchange for the cross-TU inlining. The worst-case ARK_4IN1_F051 sits at ~92.5% of the 27 K app region; the no-LTO fallback still fits at ~82%, so an escape hatch exists if a future feature needs the space back.
Problem
The F051 (Cortex-M0, 48 MHz, no hardware divider, 1WS flash) runs out of CPU cycles at high eRPM with bidirectional DShot. The big sinks: soft divisions in the 20 kHz interrupt, recursive map() calls at input rate, a triple-pass DShot decode, redundant NVIC priority writes every loop, all ISR code paying the flash wait state on every branch, and cross-TU call overhead in the commutation path. Fixing the last item with LTO is only safe after fixing what LTO exposes (mismatched extern declarations, one a real half-width read on g431) and what it removes (the accidental cross-TU reload barriers that non-volatile ISR-shared globals were relying on — the main loop never returns, so caching its reads or sinking its stores is a legal optimization once the compiler sees the whole program).
Solution
Commit order is dependency order: extern-mismatch fixes and the volatile audit first, then five independent algorithmic changes (Q16 duty scaling, Q16 throttle slopes, single-pass DShot decode, compile-time current-filter window, NVIC dedup), then LTO with CAN/E230 opt-outs, then the F051-specific changes (RAM-resident hot path for gcc and Keil, inlined comparator read, SysTick tick removal).
Verified: one target per ARM family plus the worst-case F051 and a CAN config build warning-free under -Werror at every commit; disassembly shows the comparator ISR fully RAM-resident with zero flash calls and the 20 kHz per-tick path division-free (remaining divisions are the arming one-shot and the 1 kHz PID block); Q16 results are within one timer count of the exact division and the DShot decode is bit-exact including the inverted-CRC case; worst-case RAM keeps 2.1 KB headroom above the heap/stack reserve. Outstanding: one Keil build on Windows to confirm scatter-loading of .ramfunc (flagged in its commit), V203 not built locally (no RISC-V toolchain, only an explicit-cast DMA address is touched), and the bench items from am32-firmware#379 — the comparator filter window change (~5 µs → ~1.2 µs at filter_level 12) is the one to watch at high current.