esp_system: link time, not runtime, dependency on esp_timer

components/esp_common/include/esp_private/system_internal.h

@@ -62,13 +62,8 @@ void esp_reset_reason_set_hint(esp_reset_reason_t hint);
 esp_reset_reason_t esp_reset_reason_get_hint(void);
 
 /** 
- * @brief Set function which provides `esp_system_get_time`.
- *
- * @param time_fn function which provides system time
- * @param resolution resolution in microseconds of the time provider function specified
+ * @brief Get the time in microseconds since startup
  */
-void esp_system_set_time_provider(esp_system_time_fn_t time_fn, uint32_t resolution);
-
 
 
 #ifdef __cplusplus

components/esp_system/include/esp_system.h

@@ -28,8 +28,6 @@
 extern "C" {
 #endif
 
-typedef int64_t (*esp_system_time_fn_t)(void);
-
 typedef enum {
     ESP_MAC_WIFI_STA,
     ESP_MAC_WIFI_SOFTAP,

components/esp_system/port/cpu_start.c

@@ -98,21 +98,9 @@ static volatile bool s_cpu_inited[SOC_CPU_CORES_NUM] = { false };
 static volatile bool s_resume_cores;
 #endif
 
-uint64_t g_startup_time = 0;
-
 // If CONFIG_SPIRAM_IGNORE_NOTFOUND is set and external RAM is not found or errors out on testing, this is set to false.
 bool g_spiram_ok = true;
 
-static int64_t default_system_time_fn(void)
-{
-    int64_t t = 0;
-    static spinlock_t s_time_lock = SPINLOCK_INITIALIZER;
-    spinlock_acquire(&s_time_lock, SPINLOCK_WAIT_FOREVER);
-    t = (esp_rtc_get_time_us() - g_startup_time); 
-    spinlock_release(&s_time_lock);
-    return t;
-}
-
 #if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
 void startup_resume_other_cores(void)
 {
@@ -370,7 +358,6 @@ void IRAM_ATTR call_start_cpu0(void)
     // Now that the clocks have been set-up, set the startup time from RTC
     // and default RTC-backed system time provider.
     g_startup_time = esp_rtc_get_time_us();
-    esp_system_set_time_provider(default_system_time_fn, 1000000L / rtc_clk_slow_freq_get_hz());
 
     intr_matrix_clear();
 

components/esp_system/startup.c

@@ -78,6 +78,8 @@
 #define STRINGIFY(s) STRINGIFY2(s)
 #define STRINGIFY2(s) #s
 
+uint64_t g_startup_time = 0;
+
 // App entry point for core 0
 extern void start_app(void);
 

components/esp_system/system_time.c

@@ -15,27 +15,32 @@
 #include "esp_system.h"
 #include "esp_attr.h"
 
-typedef struct {
-    esp_system_time_fn_t fn;        // time provider function
-    uint32_t resolution;            // resolution in microseconds of the time provider
-} system_time_provider_t;
+#include "soc/spinlock.h"
+#include "soc/rtc.h"
 
-// This is expected to be modified only on startup, so
-// it should be safe to not put locks on it.
-static system_time_provider_t s_system_time_provider;
+#include "sdkconfig.h"
 
-int64_t IRAM_ATTR esp_system_get_time(void)
+#if CONFIG_IDF_TARGET_ESP32
+#include "esp32/rtc.h"
+#elif CONFIG_IDF_TARGET_ESP32S2
+#include "esp32s2/rtc.h"
+#endif
+
+#include "esp_private/startup_internal.h"
+
+// A component in the build should provide strong implementations that make use of
+// and actual hardware timer to provide timekeeping functions.
+int64_t IRAM_ATTR __attribute__((weak)) esp_system_get_time(void)
 {
-    return (*s_system_time_provider.fn)();
+    int64_t t = 0;
+    static spinlock_t s_time_lock = SPINLOCK_INITIALIZER;
+    spinlock_acquire(&s_time_lock, SPINLOCK_WAIT_FOREVER);
+    t = (esp_rtc_get_time_us() - g_startup_time); 
+    spinlock_release(&s_time_lock);
+    return t;
 }
 
-uint32_t IRAM_ATTR esp_system_get_time_resolution(void)
+uint32_t IRAM_ATTR __attribute__((weak)) esp_system_get_time_resolution(void)
 {
-    return s_system_time_provider.resolution;
-}
-
-void esp_system_set_time_provider(esp_system_time_fn_t time_fn, uint32_t resolution)
-{
-    s_system_time_provider.fn = time_fn;
-    s_system_time_provider.resolution = resolution;
+    return 1000000L / rtc_clk_slow_freq_get_hz();
 }

components/esp_timer/CMakeLists.txt

@@ -1,8 +1,7 @@
 idf_build_get_property(target IDF_TARGET)
 
-set(srcs    "src/esp_timer.c"
-            "src/ets_timer_legacy.c"
-            "src/timekeeping.c")
+set(srcs "src/esp_timer.c"
+         "src/ets_timer_legacy.c")
 
 if(CONFIG_ESP_TIMER_IMPL_FRC2)
     list(APPEND srcs "src/esp_timer_impl_frc_legacy.c")

components/esp_timer/private_include/esp_timer_impl.h

@@ -117,8 +117,3 @@ 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);
-
-/**
- * @brief Initialize and provide system timekeeping functions.
- */
-esp_err_t esp_timer_timekeeping_impl_init(void);

components/esp_timer/src/esp_timer.c

@@ -24,10 +24,27 @@
 #include "freertos/task.h"
 #include "freertos/semphr.h"
 #include "freertos/xtensa_api.h"
+#include "soc/spinlock.h"
 #include "esp_timer.h"
 #include "esp_timer_impl.h"
+
+#include "esp_private/startup_internal.h"
+#include "esp_private/esp_timer_private.h"
+
+#if CONFIG_IDF_TARGET_ESP32
+#include "esp32/rtc.h"
+#elif CONFIG_IDF_TARGET_ESP32S2
+#include "esp32s2/rtc.h"
+#endif
+
 #include "sdkconfig.h"
 
+#if defined( CONFIG_ESP32_TIME_SYSCALL_USE_FRC1 ) || \
+    defined( CONFIG_ESP32_TIME_SYSCALL_USE_RTC_FRC1 ) || \
+    defined( CONFIG_ESP32S2_TIME_SYSCALL_USE_FRC1 ) || \
+    defined( CONFIG_ESP32S2_TIME_SYSCALL_USE_RTC_FRC1 )
+#define WITH_FRC 1
+#endif
 
 #ifdef CONFIG_ESP_TIMER_PROFILING
 #define WITH_PROFILING 1
@@ -378,10 +395,11 @@ esp_err_t esp_timer_init(void)
         goto out;
     }
 
-    err = esp_timer_timekeeping_impl_init();
-    if (err != ESP_OK) {
-        goto out;
-    }
+#if WITH_FRC
+    // [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);
+#endif
 
     return ESP_OK;
 
@@ -508,4 +526,27 @@ int64_t IRAM_ATTR esp_timer_get_next_alarm(void)
     }
     timer_list_unlock();
     return next_alarm;
-}
+}
+
+int64_t IRAM_ATTR esp_timer_get_time(void)
+{
+    if(is_initialized()) {
+        return esp_timer_impl_get_time();
+    } else {
+        return 0;
+    }
+}
+
+// Provides strong definition for system time functions relied upon
+// by core components.
+#if WITH_FRC
+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 1;
+}
+#endif

components/esp_timer/src/esp_timer_impl_frc_legacy.c

@@ -209,8 +209,6 @@ int64_t IRAM_ATTR esp_timer_impl_get_time(void)
     return result;
 }
 
-int64_t esp_timer_get_time(void) __attribute__((alias("esp_timer_impl_get_time")));
-
 void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp)
 {
     portENTER_CRITICAL_SAFE(&s_time_update_lock);

components/esp_timer/src/esp_timer_impl_lac.c

@@ -149,8 +149,6 @@ int64_t IRAM_ATTR esp_timer_impl_get_time(void)
     return esp_timer_impl_get_counter_reg() / TICKS_PER_US;
 }
 
-int64_t esp_timer_get_time(void) __attribute__((alias("esp_timer_impl_get_time")));
-
 void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp)
 {
     portENTER_CRITICAL_SAFE(&s_time_update_lock);

components/esp_timer/src/esp_timer_impl_systimer.c

@@ -68,9 +68,6 @@ int64_t IRAM_ATTR esp_timer_impl_get_time(void)
     return systimer_hal_get_time(SYSTIMER_COUNTER_0);
 }
 
-// Xtensa architecture doesn't have tail call optimization, using alias here can improve performance somehow
-int64_t esp_timer_get_time(void) __attribute__((alias("esp_timer_impl_get_time")));
-
 void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp)
 {
     portENTER_CRITICAL_SAFE(&s_time_update_lock);

components/esp_timer/src/timekeeping.c

@@ -1,45 +0,0 @@
-// Copyright 2020 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.
-
-#include "esp_timer.h"
-
-#include "esp_private/esp_timer_private.h"
-#include "esp_private/system_internal.h"
-
-#include "sdkconfig.h"
-
-#if defined( CONFIG_ESP32_TIME_SYSCALL_USE_FRC1 ) || \
-    defined( CONFIG_ESP32_TIME_SYSCALL_USE_RTC_FRC1 ) || \
-    defined( CONFIG_ESP32S2_TIME_SYSCALL_USE_FRC1 ) || \
-    defined( CONFIG_ESP32S2_TIME_SYSCALL_USE_RTC_FRC1 )
-#define WITH_FRC 1
-#endif
-
-#if WITH_FRC
-void esp_timer_timekeeping_impl_init(void)
-{
-    // esp_system_get_time here calls the previous system time provider.
-    // This should add the time elapsed from g_startup_time up to esp_timer_init,
-    // therefore keeping it as the point of reference (g_startup_time, that is).
-    esp_timer_private_advance(esp_system_get_time());
-
-    // esp_timer provides microsecond-resolution timers to the system
-    esp_system_set_time_provider(esp_timer_get_time, 1);
-}
-#else
-void esp_timer_timekeeping_impl_init(void)
-{
-    // Do not override default system time provider
-}
-#endif