Skip to content

Commit

Permalink
Fix ChibiOS timer overflow for 16-bit SysTick devices. (qmk#8078)
Browse files Browse the repository at this point in the history
  • Loading branch information
tzarc authored and noroadsleft committed Feb 21, 2020
1 parent af87481 commit 5cb8468
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.
5 changes: 5 additions & 0 deletions docs/ChangeLog/20200229/PR8078.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Fix ChibiOS timer overflow for 16-bit SysTick devices

* On 16-bit SysTick devices, the timer subsystem in QMK was incorrectly dealing with overflow.
* When running at a 100000 SysTick frequency (possible on 16-bit devices, but uncommon), this overflow would occur after 0.65 seconds.
* Timers are now correctly handling this overflow case and timing should now be correct on ChibiOS/ARM.
37 changes: 29 additions & 8 deletions tmk_core/common/chibios/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,44 @@

#include "timer.h"

static uint32_t last_systime_ms = 0;
static uint32_t time_ms = 0;
static uint32_t reset_point = 0;
#if CH_CFG_ST_RESOLUTION < 32
static uint32_t last_systime = 0;
static uint32_t overflow = 0;
#endif

void timer_init(void) { timer_clear(); }

void timer_clear(void) {
last_systime_ms = (uint64_t)chVTGetSystemTime();
time_ms = 0;
reset_point = (uint32_t)chVTGetSystemTime();
#if CH_CFG_ST_RESOLUTION < 32
last_systime = reset_point;
overflow = 0;
#endif
}

uint16_t timer_read(void) { return (uint16_t)timer_read32(); }

uint32_t timer_read32(void) {
uint32_t systime_ms = TIME_I2MS((uint32_t)chVTGetSystemTime());
time_ms += systime_ms - last_systime_ms;
last_systime_ms = systime_ms;
return time_ms;
uint32_t systime = (uint32_t)chVTGetSystemTime();

#if CH_CFG_ST_RESOLUTION < 32
// If/when we need to support 64-bit chips, this may need to be modified to match the native bit-ness of the MCU.
// At this point, the only SysTick resolution allowed other than 32 is 16 bit.
// In the 16-bit case, at:
// - CH_CFG_ST_FREQUENCY = 100000, overflow will occur every ~0.65 seconds
// - CH_CFG_ST_FREQUENCY = 10000, overflow will occur every ~6.5 seconds
// - CH_CFG_ST_FREQUENCY = 1000, overflow will occur every ~65 seconds
// With this implementation, as long as we ensure a timer read happens at least once during the overflow period, timing should be accurate.
if (systime < last_systime) {
overflow += ((uint32_t)1) << CH_CFG_ST_RESOLUTION;
}

last_systime = systime;
return (uint32_t)TIME_I2MS(systime - reset_point + overflow);
#else
return (uint32_t)TIME_I2MS(systime - reset_point);
#endif
}

uint16_t timer_elapsed(uint16_t last) { return TIMER_DIFF_16(timer_read(), last); }
Expand Down
5 changes: 2 additions & 3 deletions tmk_core/common/timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ uint16_t timer_elapsed(uint16_t last);
uint32_t timer_elapsed32(uint32_t last);

// Utility functions to check if a future time has expired & autmatically handle time wrapping if checked / reset frequently (half of max value)
inline bool timer_expired(uint16_t current, uint16_t future) { return (uint16_t)(current - future) < 0x8000; }

inline bool timer_expired32(uint32_t current, uint32_t future) { return (uint32_t)(current - future) < 0x80000000; }
#define timer_expired(current, future) (((uint16_t)current - (uint16_t)future) < 0x8000)
#define timer_expired32(current, future) (((uint32_t)current - (uint32_t)future) < 0x80000000)

#ifdef __cplusplus
}
Expand Down

0 comments on commit 5cb8468

Please sign in to comment.