From 733020d211dba97bb1c8397083a6144038d72527 Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Sun, 16 Jul 2023 11:16:11 +0200 Subject: [PATCH 1/9] See https://github.com/arduino/ArduinoCore-renesas/issues/49. 1. Fix for anomaly in micros() whwn the counter wrapped just before it was read 2. Specify timer configuration as constants to make micros() considerably faster --- .gitignore | 4 +++- cores/arduino/time.cpp | 33 ++++++++++++++++++++------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index f065235b3..216b0ba99 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,6 @@ cores/arduino/mydebug.cpp libraries/Storage/.development cores/arduino/mydebug.cpp.donotuse .DS_Store -.DS_Store? \ No newline at end of file +.DS_Store? +/.vs +/.gitignore diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index 39f56f41b..cde4ba496 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -2,8 +2,10 @@ #include "IRQManager.h" #include "FspTimer.h" +// this file implements the following public funcions: delay, delayMicroseconds, yield, millis, micros +// The millis and micros implementation uses timer AGT0 (24 HMz, 16-bits, count-down mode, 1 ms period) + volatile unsigned long agt_time_ms = 0; -uint32_t _freq_hz = 0; __attribute__((weak)) void delay(uint32_t ms) { R_BSP_SoftwareDelay(ms, BSP_DELAY_UNITS_MILLISECONDS); @@ -17,20 +19,24 @@ __attribute__((weak)) void yield() { } static FspTimer main_timer; - -static uint32_t _top_counter; +// specifying these details as constants makes micros() faster ! +#define _timer_type AGT_TIMER +#define _timer_index 0 +#define _timer_underflow_bit R_AGT0->AGTCR_b.TUNDF +#define _timer_clock_divider TIMER_SOURCE_DIV_8 // dividers 1, 2, 4 and 8 work because _timer_period is < 16-bit. bug in R4 1.0.2: divider 4 acts as 1 !? +#define _timer_clock_freq 24000000UL +#define _timer_ticks_per_us (_timer_clock_freq / ((1 << _timer_clock_divider) * 1000000UL)) +#define _timer_period (_timer_ticks_per_us * 1000UL) static void timer_micros_callback(timer_callback_args_t __attribute((unused)) *p_args) { - agt_time_ms += 1; //1ms + agt_time_ms += 1; } void startAgt() { - main_timer.begin(TIMER_MODE_PERIODIC, AGT_TIMER, 0, 2000.0f, 0.5f, timer_micros_callback); - IRQManager::getInstance().addPeripheral(IRQ_AGT,(void*)main_timer.get_cfg()); + main_timer.begin(TIMER_MODE_PERIODIC, _timer_type, _timer_index, _timer_period, 1, _timer_clock_divider, timer_micros_callback);; + main_timer.setup_overflow_irq(); main_timer.open(); - _top_counter = main_timer.get_counter(); - main_timer.start(); - _freq_hz = main_timer.get_freq_hz(); + main_timer.start(); // bug in R4 1.0.2: calling start() is not necessary: open() starts the counter already !? } unsigned long millis() @@ -43,10 +49,11 @@ unsigned long millis() } unsigned long micros() { - - // Convert time to us + // Return time in us NVIC_DisableIRQ(main_timer.get_cfg()->cycle_end_irq); - uint32_t time_us = ((main_timer.get_period_raw() - main_timer.get_counter()) * 1000 / main_timer.get_period_raw()) + (agt_time_ms * 1000); + uint32_t ms = agt_time_ms; + uint32_t cnt = main_timer.get_counter(); + if (_timer_underflow_bit && (cnt > (_timer_period / 2))) ++ms; // the counter wrapped arround just before it was read NVIC_EnableIRQ(main_timer.get_cfg()->cycle_end_irq); - return time_us; + return ms * 1000 + (((_timer_period - 1) - cnt) / _timer_ticks_per_us); } From c6534866f816ad0a4b68ae33fbbcefa791648b43 Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Sun, 16 Jul 2023 14:10:50 +0200 Subject: [PATCH 2/9] set correct priority for timer --- cores/arduino/time.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index cde4ba496..f46f5274b 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -27,6 +27,7 @@ static FspTimer main_timer; #define _timer_clock_freq 24000000UL #define _timer_ticks_per_us (_timer_clock_freq / ((1 << _timer_clock_divider) * 1000000UL)) #define _timer_period (_timer_ticks_per_us * 1000UL) +#define TIMER_PRIORITY 8 static void timer_micros_callback(timer_callback_args_t __attribute((unused)) *p_args) { agt_time_ms += 1; @@ -34,7 +35,7 @@ static void timer_micros_callback(timer_callback_args_t __attribute((unused)) *p void startAgt() { main_timer.begin(TIMER_MODE_PERIODIC, _timer_type, _timer_index, _timer_period, 1, _timer_clock_divider, timer_micros_callback);; - main_timer.setup_overflow_irq(); + main_timer.setup_overflow_irq(TIMER_PRIORITY); main_timer.open(); main_timer.start(); // bug in R4 1.0.2: calling start() is not necessary: open() starts the counter already !? } From 7bef4fcc241ec5d518ccf3dde49eca813a201c51 Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Mon, 24 Jul 2023 21:05:15 +0200 Subject: [PATCH 3/9] added const qualifiers, gave register access macro function format, broke down long formulas --- cores/arduino/time.cpp | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index f46f5274b..0aa0ec214 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -20,16 +20,16 @@ __attribute__((weak)) void yield() { static FspTimer main_timer; // specifying these details as constants makes micros() faster ! -#define _timer_type AGT_TIMER -#define _timer_index 0 -#define _timer_underflow_bit R_AGT0->AGTCR_b.TUNDF -#define _timer_clock_divider TIMER_SOURCE_DIV_8 // dividers 1, 2, 4 and 8 work because _timer_period is < 16-bit. bug in R4 1.0.2: divider 4 acts as 1 !? -#define _timer_clock_freq 24000000UL -#define _timer_ticks_per_us (_timer_clock_freq / ((1 << _timer_clock_divider) * 1000000UL)) -#define _timer_period (_timer_ticks_per_us * 1000UL) -#define TIMER_PRIORITY 8 - -static void timer_micros_callback(timer_callback_args_t __attribute((unused)) *p_args) { +#define _timer_type AGT_TIMER +#define _timer_index 0 +#define _timer_get_underflow_bit() R_AGT0->AGTCR_b.TUNDF +#define _timer_clock_divider TIMER_SOURCE_DIV_8 // dividers 1, 2 and 8 work because _timer_period is < 16-bit. the divider 4 seems not supported: acts as 1 +#define _timer_clock_freq 24000000UL +#define _timer_counts_per_us (_timer_clock_freq / ((1 << _timer_clock_divider) * 1000000UL)) +#define _timer_period (_timer_counts_per_us * 1000UL) +#define TIMER_PRIORITY 8 + +static void timer_micros_callback(timer_callback_args_t __attribute((unused))* p_args) { agt_time_ms += 1; } @@ -53,8 +53,13 @@ unsigned long micros() { // Return time in us NVIC_DisableIRQ(main_timer.get_cfg()->cycle_end_irq); uint32_t ms = agt_time_ms; - uint32_t cnt = main_timer.get_counter(); - if (_timer_underflow_bit && (cnt > (_timer_period / 2))) ++ms; // the counter wrapped arround just before it was read + uint32_t const down_counts = main_timer.get_counter(); + if (_timer_get_underflow_bit() && (down_counts > (_timer_period / 2))) + { + // the counter wrapped arround just before it was read + ++ms; + } NVIC_EnableIRQ(main_timer.get_cfg()->cycle_end_irq); - return ms * 1000 + (((_timer_period - 1) - cnt) / _timer_ticks_per_us); + uint32_t const up_counts = (_timer_period - 1) - down_counts; + return (ms * 1000) + (up_counts / _timer_counts_per_us); } From 8dcbd0853de112c1216ed3749a89a64ce2dbe7e6 Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Mon, 24 Jul 2023 21:23:27 +0200 Subject: [PATCH 4/9] corrected typo in comments --- cores/arduino/time.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index 0aa0ec214..0db7345d6 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -56,7 +56,7 @@ unsigned long micros() { uint32_t const down_counts = main_timer.get_counter(); if (_timer_get_underflow_bit() && (down_counts > (_timer_period / 2))) { - // the counter wrapped arround just before it was read + // the counter wrapped around just before it was read ++ms; } NVIC_EnableIRQ(main_timer.get_cfg()->cycle_end_irq); From 46d540307c4b767e8ef392439364b9de8501d676 Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Sat, 5 Aug 2023 22:55:06 +0200 Subject: [PATCH 5/9] added support for the Portenta C33 optimized micros() --- cores/arduino/time.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index 0db7345d6..f2903a2df 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -19,21 +19,23 @@ __attribute__((weak)) void yield() { } static FspTimer main_timer; -// specifying these details as constants makes micros() faster ! -#define _timer_type AGT_TIMER -#define _timer_index 0 -#define _timer_get_underflow_bit() R_AGT0->AGTCR_b.TUNDF -#define _timer_clock_divider TIMER_SOURCE_DIV_8 // dividers 1, 2 and 8 work because _timer_period is < 16-bit. the divider 4 seems not supported: acts as 1 -#define _timer_clock_freq 24000000UL -#define _timer_counts_per_us (_timer_clock_freq / ((1 << _timer_clock_divider) * 1000000UL)) -#define _timer_period (_timer_counts_per_us * 1000UL) -#define TIMER_PRIORITY 8 +const uint8_t _timer_type = AGT_TIMER; +const uint8_t _timer_index = 0; +inline uint8_t _timer_get_underflow_bit() { return R_AGT0->AGTCR_b.TUNDF; } +// clock divider 8 works for the Uno R4 and Portenta C33 both because _timer_period is < 16-bit. +// on the Uno R4 the AGT clock is 24 MHz / 8 -> 3000 ticks per ms +// on the Portenta C33 the AGT clock is 50 Mhz / 8 -> 6250 ticks per ms +const timer_source_div_t _timer_clock_divider = TIMER_SOURCE_DIV_8; +uint32_t _timer_period; +const uint8_t TIMER_PRIORITY = 8; static void timer_micros_callback(timer_callback_args_t __attribute((unused))* p_args) { agt_time_ms += 1; } void startAgt() { + const uint32_t _timer_clock_freq = R_FSP_SystemClockHzGet(_timer_type == AGT_TIMER ? FSP_PRIV_CLOCK_PCLKB : FSP_PRIV_CLOCK_PCLKD); + _timer_period = _timer_clock_freq / ((1 << _timer_clock_divider) * 1000UL); main_timer.begin(TIMER_MODE_PERIODIC, _timer_type, _timer_index, _timer_period, 1, _timer_clock_divider, timer_micros_callback);; main_timer.setup_overflow_irq(TIMER_PRIORITY); main_timer.open(); @@ -61,5 +63,5 @@ unsigned long micros() { } NVIC_EnableIRQ(main_timer.get_cfg()->cycle_end_irq); uint32_t const up_counts = (_timer_period - 1) - down_counts; - return (ms * 1000) + (up_counts / _timer_counts_per_us); + return (ms * 1000) + ((up_counts * 1000) / _timer_period); } From 8ad4dd354f0baf17d3632e3e2b5bd4749a80e887 Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Sun, 6 Aug 2023 13:18:44 +0200 Subject: [PATCH 6/9] read counter direct from AGT0 count register: micros() is now 2x faster --- cores/arduino/time.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index f2903a2df..a0f41b9f2 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -22,6 +22,7 @@ static FspTimer main_timer; const uint8_t _timer_type = AGT_TIMER; const uint8_t _timer_index = 0; inline uint8_t _timer_get_underflow_bit() { return R_AGT0->AGTCR_b.TUNDF; } +inline uint16_t _timer_get_counter() { return R_AGT0->AGT; } // clock divider 8 works for the Uno R4 and Portenta C33 both because _timer_period is < 16-bit. // on the Uno R4 the AGT clock is 24 MHz / 8 -> 3000 ticks per ms // on the Portenta C33 the AGT clock is 50 Mhz / 8 -> 6250 ticks per ms @@ -55,7 +56,7 @@ unsigned long micros() { // Return time in us NVIC_DisableIRQ(main_timer.get_cfg()->cycle_end_irq); uint32_t ms = agt_time_ms; - uint32_t const down_counts = main_timer.get_counter(); + uint32_t const down_counts = _timer_get_counter(); if (_timer_get_underflow_bit() && (down_counts > (_timer_period / 2))) { // the counter wrapped around just before it was read From b6a73f86ff3c353a7c34f9b71e21f93c8343cdfa Mon Sep 17 00:00:00 2001 From: Kees-van-der-Oord Date: Mon, 7 Aug 2023 21:26:56 +0200 Subject: [PATCH 7/9] moved all const defintions inline optimized micros() even more ... --- cores/arduino/time.cpp | 51 +++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index a0f41b9f2..11ed45cd7 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -3,9 +3,6 @@ #include "FspTimer.h" // this file implements the following public funcions: delay, delayMicroseconds, yield, millis, micros -// The millis and micros implementation uses timer AGT0 (24 HMz, 16-bits, count-down mode, 1 ms period) - -volatile unsigned long agt_time_ms = 0; __attribute__((weak)) void delay(uint32_t ms) { R_BSP_SoftwareDelay(ms, BSP_DELAY_UNITS_MILLISECONDS); @@ -18,29 +15,25 @@ void delayMicroseconds(unsigned int us) { __attribute__((weak)) void yield() { } -static FspTimer main_timer; -const uint8_t _timer_type = AGT_TIMER; -const uint8_t _timer_index = 0; -inline uint8_t _timer_get_underflow_bit() { return R_AGT0->AGTCR_b.TUNDF; } -inline uint16_t _timer_get_counter() { return R_AGT0->AGT; } -// clock divider 8 works for the Uno R4 and Portenta C33 both because _timer_period is < 16-bit. -// on the Uno R4 the AGT clock is 24 MHz / 8 -> 3000 ticks per ms -// on the Portenta C33 the AGT clock is 50 Mhz / 8 -> 6250 ticks per ms -const timer_source_div_t _timer_clock_divider = TIMER_SOURCE_DIV_8; -uint32_t _timer_period; -const uint8_t TIMER_PRIORITY = 8; +static FspTimer agt_timer; +volatile uint32_t agt_time_ms = 0; static void timer_micros_callback(timer_callback_args_t __attribute((unused))* p_args) { agt_time_ms += 1; } void startAgt() { - const uint32_t _timer_clock_freq = R_FSP_SystemClockHzGet(_timer_type == AGT_TIMER ? FSP_PRIV_CLOCK_PCLKB : FSP_PRIV_CLOCK_PCLKD); - _timer_period = _timer_clock_freq / ((1 << _timer_clock_divider) * 1000UL); - main_timer.begin(TIMER_MODE_PERIODIC, _timer_type, _timer_index, _timer_period, 1, _timer_clock_divider, timer_micros_callback);; - main_timer.setup_overflow_irq(TIMER_PRIORITY); - main_timer.open(); - main_timer.start(); // bug in R4 1.0.2: calling start() is not necessary: open() starts the counter already !? + // configure AGT timer 0 to generate an underflow interrupt every 1 ms + // a clock divider 8 works for both the Uno R4 and Portenta C33 because number of clock ticks + // in 1 ms (period) is an integer number and below the 16-bit counter limit + // on the Uno R4 the AGT clock is 24 MHz / 8 -> 3000 ticks per ms + // on the Portenta C33 the AGT clock is 50 Mhz / 8 -> 6250 ticks per ms + const uint32_t clock_freq = R_FSP_SystemClockHzGet(FSP_PRIV_CLOCK_PCLKB); + const uint32_t period = clock_freq / ((1 << TIMER_SOURCE_DIV_8) * 1000UL); + agt_timer.begin(TIMER_MODE_PERIODIC, AGT_TIMER, 0, period, 1, TIMER_SOURCE_DIV_8, timer_micros_callback);; + agt_timer.setup_overflow_irq(8); + agt_timer.open(); + agt_timer.start(); // bug in R4 1.0.2: calling start() is not necessary: open() starts the counter already !? } unsigned long millis() @@ -54,15 +47,17 @@ unsigned long millis() unsigned long micros() { // Return time in us - NVIC_DisableIRQ(main_timer.get_cfg()->cycle_end_irq); + const timer_cfg_t* cfg = agt_timer.get_cfg(); + NVIC_DisableIRQ(cfg->cycle_end_irq); uint32_t ms = agt_time_ms; - uint32_t const down_counts = _timer_get_counter(); - if (_timer_get_underflow_bit() && (down_counts > (_timer_period / 2))) - { - // the counter wrapped around just before it was read + // read from the R_AGT0 registers directly for performance reasons + uint32_t const down_counts = R_AGT0->AGT; // get the counter value + if (R_AGT0->AGTCR_b.TUNDF && (down_counts > (cfg->period_counts / 2))) { + // if the TUNDF (underflow) bit is set, the counter wrapped around + // just before down_counts was read and agt_time_ms was not yet updated ++ms; } - NVIC_EnableIRQ(main_timer.get_cfg()->cycle_end_irq); - uint32_t const up_counts = (_timer_period - 1) - down_counts; - return (ms * 1000) + ((up_counts * 1000) / _timer_period); + NVIC_EnableIRQ(cfg->cycle_end_irq); + uint32_t const up_counts = (cfg->period_counts - 1) - down_counts; + return (ms * 1000) + ((up_counts * 1000) / cfg->period_counts); } From c47fb074ff79205b208580b0ec4a20429aada6d6 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Tue, 8 Aug 2023 09:49:21 +0200 Subject: [PATCH 8/9] Suffix constant "clock_freq" with "clock_freq_Hz" to make clear the unit of this constant. --- cores/arduino/time.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index 11ed45cd7..aeef8da91 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -28,8 +28,8 @@ void startAgt() { // in 1 ms (period) is an integer number and below the 16-bit counter limit // on the Uno R4 the AGT clock is 24 MHz / 8 -> 3000 ticks per ms // on the Portenta C33 the AGT clock is 50 Mhz / 8 -> 6250 ticks per ms - const uint32_t clock_freq = R_FSP_SystemClockHzGet(FSP_PRIV_CLOCK_PCLKB); - const uint32_t period = clock_freq / ((1 << TIMER_SOURCE_DIV_8) * 1000UL); + const uint32_t clock_freq_Hz = R_FSP_SystemClockHzGet(FSP_PRIV_CLOCK_PCLKB); + const uint32_t period = clock_freq_Hz / ((1 << TIMER_SOURCE_DIV_8) * 1000UL); agt_timer.begin(TIMER_MODE_PERIODIC, AGT_TIMER, 0, period, 1, TIMER_SOURCE_DIV_8, timer_micros_callback);; agt_timer.setup_overflow_irq(8); agt_timer.open(); From 70ef6c76624ca2d522d126e4f0bbb490cee41b08 Mon Sep 17 00:00:00 2001 From: Alexander Entinger Date: Tue, 8 Aug 2023 09:55:06 +0200 Subject: [PATCH 9/9] Split function call across multiple lines and document parameters for better readability. --- cores/arduino/time.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cores/arduino/time.cpp b/cores/arduino/time.cpp index aeef8da91..8fc9a2e08 100644 --- a/cores/arduino/time.cpp +++ b/cores/arduino/time.cpp @@ -30,7 +30,13 @@ void startAgt() { // on the Portenta C33 the AGT clock is 50 Mhz / 8 -> 6250 ticks per ms const uint32_t clock_freq_Hz = R_FSP_SystemClockHzGet(FSP_PRIV_CLOCK_PCLKB); const uint32_t period = clock_freq_Hz / ((1 << TIMER_SOURCE_DIV_8) * 1000UL); - agt_timer.begin(TIMER_MODE_PERIODIC, AGT_TIMER, 0, period, 1, TIMER_SOURCE_DIV_8, timer_micros_callback);; + agt_timer.begin(/* mode */ TIMER_MODE_PERIODIC, + /* type */ AGT_TIMER, + /* channel */ 0, + period, + /* pulse */ 1, + TIMER_SOURCE_DIV_8, + timer_micros_callback);; agt_timer.setup_overflow_irq(8); agt_timer.open(); agt_timer.start(); // bug in R4 1.0.2: calling start() is not necessary: open() starts the counter already !?