From cb5a8342d4cca99dedc3d6cded917c7effe58b75 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 8 Mar 2021 23:00:07 +0800 Subject: [PATCH 1/8] esp_system: revert reset of systimer clk at startup --- components/esp_system/port/soc/esp32s2/clk.c | 1 - 1 file changed, 1 deletion(-) diff --git a/components/esp_system/port/soc/esp32s2/clk.c b/components/esp_system/port/soc/esp32s2/clk.c index bf8a12a4cb..2f08e42114 100644 --- a/components/esp_system/port/soc/esp32s2/clk.c +++ b/components/esp_system/port/soc/esp32s2/clk.c @@ -316,7 +316,6 @@ __attribute__((weak)) void esp_perip_clk_init(void) DPORT_CLEAR_PERI_REG_MASK(DPORT_BT_LPCK_DIV_FRAC_REG, DPORT_LPCLK_SEL_8M); DPORT_SET_PERI_REG_MASK(DPORT_BT_LPCK_DIV_FRAC_REG, DPORT_LPCLK_SEL_RTC_SLOW); - periph_ll_reset(PERIPH_SYSTIMER_MODULE); /* Enable RNG clock. */ periph_module_enable(PERIPH_RNG_MODULE); From 5a901131369d3de78badf04e4b70046212193b1e Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 8 Mar 2021 23:00:31 +0800 Subject: [PATCH 2/8] newlib: change microseconds offset type --- components/newlib/port/esp_time_impl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/newlib/port/esp_time_impl.c b/components/newlib/port/esp_time_impl.c index bb3bc77bb3..5317156b63 100644 --- a/components/newlib/port/esp_time_impl.c +++ b/components/newlib/port/esp_time_impl.c @@ -51,7 +51,7 @@ // Offset between FRC timer and the RTC. // Initialized after reset or light sleep. #if defined(CONFIG_ESP_TIME_FUNCS_USE_RTC_TIMER) && defined(CONFIG_ESP_TIME_FUNCS_USE_ESP_TIMER) -uint64_t s_microseconds_offset = 0; +int64_t s_microseconds_offset = 0; #endif #ifndef CONFIG_ESP_TIME_FUNCS_USE_RTC_TIMER From 74de5a7c5890b39ee128b11ffd8c5ece2a8daf81 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 8 Mar 2021 23:01:40 +0800 Subject: [PATCH 3/8] esp_timer: correct startup time --- components/esp_timer/CMakeLists.txt | 3 +- .../private_include/esp_timer_impl.h | 7 +++ components/esp_timer/src/esp_timer.c | 18 +------ components/esp_timer/src/system_time.c | 53 +++++++++++++++++++ 4 files changed, 63 insertions(+), 18 deletions(-) create mode 100644 components/esp_timer/src/system_time.c diff --git a/components/esp_timer/CMakeLists.txt b/components/esp_timer/CMakeLists.txt index 0fbbee530b..d799aa7c23 100644 --- a/components/esp_timer/CMakeLists.txt +++ b/components/esp_timer/CMakeLists.txt @@ -1,7 +1,8 @@ idf_build_get_property(target IDF_TARGET) set(srcs "src/esp_timer.c" - "src/ets_timer_legacy.c") + "src/ets_timer_legacy.c" + "src/system_time.c") if(CONFIG_ESP_TIMER_IMPL_FRC2) list(APPEND srcs "src/esp_timer_impl_frc_legacy.c") diff --git a/components/esp_timer/private_include/esp_timer_impl.h b/components/esp_timer/private_include/esp_timer_impl.h index d572550ad8..63daba0d9e 100644 --- a/components/esp_timer/private_include/esp_timer_impl.h +++ b/components/esp_timer/private_include/esp_timer_impl.h @@ -132,3 +132,10 @@ uint64_t esp_timer_impl_get_counter_reg(void); * @return the value of the alarm register */ uint64_t esp_timer_impl_get_alarm_reg(void); + +#if CONFIG_ESP_TIME_FUNCS_USE_ESP_TIMER +/** + * @brief Initialize esp_timer as system time provider. + */ +void esp_timer_impl_init_system_time(void); +#endif diff --git a/components/esp_timer/src/esp_timer.c b/components/esp_timer/src/esp_timer.c index 81d8c00161..8e370a8604 100644 --- a/components/esp_timer/src/esp_timer.c +++ b/components/esp_timer/src/esp_timer.c @@ -446,9 +446,7 @@ esp_err_t esp_timer_init(void) } #if CONFIG_ESP_TIME_FUNCS_USE_ESP_TIMER - // [refactor-todo] this logic, "esp_rtc_get_time_us() - g_startup_time", is also - // the weak definition of esp_system_get_time; find a way to remove this duplication. - esp_timer_private_advance(esp_rtc_get_time_us() - g_startup_time); + esp_timer_impl_init_system_time(); #endif return ESP_OK; @@ -600,17 +598,3 @@ int64_t IRAM_ATTR esp_timer_get_next_alarm(void) } return next_alarm; } - -// Provides strong definition for system time functions relied upon -// by core components. -#if CONFIG_ESP_TIME_FUNCS_USE_ESP_TIMER -int64_t IRAM_ATTR esp_system_get_time(void) -{ - return esp_timer_get_time(); -} - -uint32_t IRAM_ATTR esp_system_get_time_resolution(void) -{ - return 1000; -} -#endif diff --git a/components/esp_timer/src/system_time.c b/components/esp_timer/src/system_time.c new file mode 100644 index 0000000000..08f0e7d292 --- /dev/null +++ b/components/esp_timer/src/system_time.c @@ -0,0 +1,53 @@ +// Copyright 2017 Espressif Systems (Shanghai) PTE LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Provides strong definition for system time functions relied upon +// by core components. +#include "sdkconfig.h" + +#if CONFIG_ESP_TIME_FUNCS_USE_ESP_TIMER +#include "esp_timer.h" +#include "esp_timer_impl.h" + +#include "esp_private/startup_internal.h" + +#if CONFIG_IDF_TARGET_ESP32 +#include "esp32/rtc.h" +#elif CONFIG_IDF_TARGET_ESP32S2 +#include "esp32s2/rtc.h" +#elif CONFIG_IDF_TARGET_ESP32S3 +#include "esp32s3/rtc.h" +#elif CONFIG_IDF_TARGET_ESP32C3 +#include "esp32c3/rtc.h" +#endif + +// Correction for underlying timer to keep definition +// of system time consistent. +static int64_t s_correction_us = 0; + +void esp_timer_impl_init_system_time(void) +{ + s_correction_us = esp_rtc_get_time_us() - g_startup_time - esp_timer_impl_get_time(); +} + +int64_t IRAM_ATTR esp_system_get_time(void) +{ + return esp_timer_get_time() + s_correction_us; +} + +uint32_t IRAM_ATTR esp_system_get_time_resolution(void) +{ + return 1000; +} +#endif From 131bbbd5c45022dcab41806ea0e951d86d3c799d Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Mon, 8 Mar 2021 23:02:04 +0800 Subject: [PATCH 4/8] esp_system: test system time for rtc compensation --- components/esp_system/test/test_system_time.c | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 components/esp_system/test/test_system_time.c diff --git a/components/esp_system/test/test_system_time.c b/components/esp_system/test/test_system_time.c new file mode 100644 index 0000000000..7cd23cc1e5 --- /dev/null +++ b/components/esp_system/test/test_system_time.c @@ -0,0 +1,36 @@ +#include +#include "unity.h" + +#include "esp_private/system_internal.h" + +#if CONFIG_IDF_TARGET_ESP32 +#include "esp32/clk.h" +#elif CONFIG_IDF_TARGET_ESP32S2 +#include "esp32s2/clk.h" +#elif CONFIG_IDF_TARGET_ESP32S3 +#include "esp32s3/clk.h" +#elif CONFIG_IDF_TARGET_ESP32C3 +#include "esp32c3/clk.h" +#endif + +TEST_CASE("Test effect of rtc clk calibration compensation on system time", "[esp_system]") +{ + uint32_t prev_cal = esp_clk_slowclk_cal_get(); + int64_t t1 = esp_system_get_time(); + + // Modify calibration value + esp_clk_slowclk_cal_set(prev_cal/2); + + // Internally, the origin point of rtc clk has been adjusted + // so that t2 > t1 remains true + int64_t t2 = esp_system_get_time(); + + TEST_ASSERT_GREATER_THAN(t1, t2); + + // Restore calibration value + esp_clk_slowclk_cal_set(prev_cal); + + t2 = esp_system_get_time(); + + TEST_ASSERT_GREATER_THAN(t1, t2); +} From d7e9567c00c300045435f3593a117883642f8b65 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Tue, 9 Mar 2021 09:12:24 +0800 Subject: [PATCH 5/8] esp_system, esp_timer: fix time function description --- components/esp_system/include/esp_private/system_internal.h | 4 ++-- components/esp_timer/include/esp_timer.h | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/components/esp_system/include/esp_private/system_internal.h b/components/esp_system/include/esp_private/system_internal.h index 66912a54b4..2125fa7ff4 100644 --- a/components/esp_system/include/esp_private/system_internal.h +++ b/components/esp_system/include/esp_private/system_internal.h @@ -61,11 +61,11 @@ void esp_reset_reason_set_hint(esp_reset_reason_t hint); */ esp_reset_reason_t esp_reset_reason_get_hint(void); - /** * @brief Get the time in microseconds since startup * - * @returns time since startup in microseconds + * @returns time since g_startup_time; definition should be fixed by system time provider + * no matter the underlying timer used. */ int64_t esp_system_get_time(void); diff --git a/components/esp_timer/include/esp_timer.h b/components/esp_timer/include/esp_timer.h index 97459b45eb..bb8879cd93 100644 --- a/components/esp_timer/include/esp_timer.h +++ b/components/esp_timer/include/esp_timer.h @@ -184,8 +184,7 @@ esp_err_t esp_timer_delete(esp_timer_handle_t timer); /** * @brief Get time in microseconds since boot - * @return number of microseconds since esp_timer_init was called (this normally - * happens early during application startup). + * @return number of microseconds since underlying timer has been started */ int64_t esp_timer_get_time(void); From 50745fa61b4cff1250ff90acfdb474d5e1a1ebc8 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Tue, 9 Mar 2021 11:31:32 +0800 Subject: [PATCH 6/8] newlib: use system time in test --- components/newlib/test/test_time.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/newlib/test/test_time.c b/components/newlib/test/test_time.c index 044c437f5c..64ca582fca 100644 --- a/components/newlib/test/test_time.c +++ b/components/newlib/test/test_time.c @@ -17,6 +17,8 @@ #include "esp_system.h" #include "esp_timer.h" +#include "esp_private/system_internal.h" + #if CONFIG_IDF_TARGET_ESP32 #include "esp32/clk.h" #define TARGET_DEFAULT_CPU_FREQ_MHZ CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ @@ -403,7 +405,7 @@ void test_posix_timers_clock (void) TEST_ASSERT_EQUAL_INT(1000, ts.tv_nsec); TEST_ASSERT(clock_gettime(CLOCK_MONOTONIC, &ts) == 0); - delta_monotonic_us = esp_timer_get_time() - (ts.tv_sec * 1000000L + ts.tv_nsec / 1000L); + delta_monotonic_us = esp_system_get_time() - (ts.tv_sec * 1000000L + ts.tv_nsec / 1000L); TEST_ASSERT(delta_monotonic_us > 0 || delta_monotonic_us == 0); TEST_ASSERT_INT_WITHIN(5000L, 0, delta_monotonic_us); From 44a8dc9342247c9bfb0135f005710dbf8e404cb2 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Thu, 25 Mar 2021 15:00:17 +0800 Subject: [PATCH 7/8] ci: change assertions rtc clk compenstation test --- components/esp_hw_support/test/test_rtc_clk.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/esp_hw_support/test/test_rtc_clk.c b/components/esp_hw_support/test/test_rtc_clk.c index b2032f7156..1a12903905 100644 --- a/components/esp_hw_support/test/test_rtc_clk.c +++ b/components/esp_hw_support/test/test_rtc_clk.c @@ -327,7 +327,7 @@ TEST_CASE("Test rtc clk calibration compensation", "[rtc_clk]") // so that t2 > t1 remains true int64_t t2 = esp_rtc_get_time_us(); - TEST_ASSERT(t2 > t1); + TEST_ASSERT_GREATER_THAN(t1, t2); // Restore calibration value esp_clk_slowclk_cal_set(esp_clk_slowclk_cal_get() * 2); @@ -337,7 +337,7 @@ TEST_CASE("Test rtc clk calibration compensation", "[rtc_clk]") t2 = esp_rtc_get_time_us(); - TEST_ASSERT(t2 > t1); + TEST_ASSERT_GREATER_THAN(t1, t2); } static void trigger_deepsleep(void) @@ -364,7 +364,7 @@ static void check_time_deepsleep_1(void) RESET_REASON reason = rtc_get_reset_reason(0); TEST_ASSERT(reason == DEEPSLEEP_RESET); int64_t end = esp_rtc_get_time_us(); - TEST_ASSERT(end > start); + TEST_ASSERT_GREATER_THAN(start, end); esp_clk_slowclk_cal_set(esp_clk_slowclk_cal_get() * 2); @@ -384,7 +384,7 @@ static void check_time_deepsleep_2(void) RESET_REASON reason = rtc_get_reset_reason(0); TEST_ASSERT(reason == DEEPSLEEP_RESET); int64_t end = esp_rtc_get_time_us(); - TEST_ASSERT(end > start); + TEST_ASSERT_GREATER_THAN(start, end); } TEST_CASE_MULTIPLE_STAGES("Test rtc clk calibration compensation across deep sleep", "[rtc_clk][reset=DEEPSLEEP_RESET, DEEPSLEEP_RESET]", trigger_deepsleep, check_time_deepsleep_1, check_time_deepsleep_2); From 32aa5f7e253ebe6d6ef5c12772fc71435ea89f93 Mon Sep 17 00:00:00 2001 From: Renz Bagaporo Date: Thu, 25 Mar 2021 16:09:33 +0800 Subject: [PATCH 8/8] esp_system: add notes on timekeeping --- components/esp_system/README.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 components/esp_system/README.md diff --git a/components/esp_system/README.md b/components/esp_system/README.md new file mode 100644 index 0000000000..b54343b21b --- /dev/null +++ b/components/esp_system/README.md @@ -0,0 +1,29 @@ +## System Notes + +### Timekeeping + +The following are the timekeeping mechanisms available and their differences: + +1. System time (`esp_system_get_time`) + +Time with the origin at `g_startup_time`. The implementation is not handled by `esp_system`, +but it does provide a default implementation using RTC timer. Currently, `esp_timer` +provides system time, since the hardware timers are under the control of that +component. However, no matter the underlying timer, the system time provider +should maintain the definition of having the origin point at `g_startup_time`. + +2. `esp_timer` time (`esp_timer_get_time`) + +This is the time read from an underlying hardware timer, controlled through config. Origin +is at the point where the underlying timer starts counting. + +3. `newlib` time (`gettimeofday`) + +Timekeeping function in standard library. Can be set (`settimeofday`) or moved forward/backward (`adjtime`); +with the possibility of the changes being made persistent through config. +Currently implemented in terms of system time, as the point of origin is fixed. +If persistence is enabled, RTC time is also used in conjuction with system time. + +4. RTC time (`esp_rtc_get_time_us`) + +Time read from RTC timer. \ No newline at end of file