From 8df306ab6007941851485bee46fb5c33c1da0d55 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 13 Dec 2021 10:52:22 +0100 Subject: [PATCH] freertos,esp_system: make dependencies on some components optional Dependencies on gdbstub, espcoredump, app_trace will only be added if these components are present in the build. --- components/esp_system/CMakeLists.txt | 21 +++++++++++++++++-- components/esp_system/startup.c | 3 +++ components/freertos/CMakeLists.txt | 18 ++++++++++++---- .../FreeRTOS-Kernel/portable/xtensa/port.c | 2 ++ .../system/g1_components/CMakeLists.txt | 10 --------- tools/test_apps/system/panic/CMakeLists.txt | 2 +- 6 files changed, 39 insertions(+), 17 deletions(-) diff --git a/components/esp_system/CMakeLists.txt b/components/esp_system/CMakeLists.txt index 77b7f567f3..53c5cd7ced 100644 --- a/components/esp_system/CMakeLists.txt +++ b/components/esp_system/CMakeLists.txt @@ -40,8 +40,8 @@ else() # [refactor-todo] requirements due to init code, # should be removable once using component init functions # link-time registration is used. - esp_pm app_update nvs_flash pthread app_trace esp_gdbstub - espcoredump esp_phy efuse + esp_pm app_update nvs_flash pthread + esp_phy efuse LDFRAGMENTS "linker.lf" "app.lf") add_subdirectory(port) @@ -72,3 +72,20 @@ endif() # Force linking UBSAN hooks. If UBSAN is not enabled, the hooks will ultimately be removed # due to -ffunction-sections -Wl,--gc-sections options. target_link_libraries(${COMPONENT_LIB} INTERFACE "-u __ubsan_include") + + +# [refactor-todo] requirements due to init code, should be removable +# once link-time registration of component init functions is used. +if(CONFIG_APPTRACE_ENABLE) + idf_component_optional_requires(PRIVATE app_trace) +endif() + +if(CONFIG_ESP_COREDUMP_ENABLE) + idf_component_optional_requires(PRIVATE espcoredump) +endif() + +# [refactor-todo] requirement from the panic handler, +# need to introduce panic "event" concept to remove this dependency (IDF-2194) +if(CONFIG_ESP_SYSTEM_PANIC_GDBSTUB) + idf_component_optional_requires(PRIVATE esp_gdbstub) +endif() diff --git a/components/esp_system/startup.c b/components/esp_system/startup.c index 8886fda52a..34d53099e2 100644 --- a/components/esp_system/startup.c +++ b/components/esp_system/startup.c @@ -45,7 +45,10 @@ #include "esp_core_dump.h" #endif +#if CONFIG_APPTRACE_ENABLE #include "esp_app_trace.h" +#endif + #include "esp_private/dbg_stubs.h" #include "esp_pm.h" #include "esp_private/pm_impl.h" diff --git a/components/freertos/CMakeLists.txt b/components/freertos/CMakeLists.txt index 69961a95ef..93e3c5aa39 100644 --- a/components/freertos/CMakeLists.txt +++ b/components/freertos/CMakeLists.txt @@ -68,10 +68,11 @@ if(CONFIG_ESP32_IRAM_AS_8BIT_ACCESSIBLE_MEMORY) list(APPEND srcs "FreeRTOS-Kernel/portable/xtensa/xtensa_loadstore_handler.S") endif() -# esp_timer is required by FreeRTOS because we use esp_tiemr_get_time() to do profiling -# app_trace is required by FreeRTOS headers only when CONFIG_APPTRACE_SV_ENABLE=y, -# REQUIRES can't depend on config options, so always require it. -set(required_components app_trace esp_timer) +# esp_timer is required by FreeRTOS when we use esp_timer_get_time() to do profiling +# [refactor-todo]: make this an optional requirement depending on CONFIG_FREERTOS_RUN_TIME_STATS_USING_ESP_TIMER +# Note, many other components implicitly include esp_timer.h via freertos portmacro.h and some +# components implicitly depend on esp_timer via freertos. +set(required_components esp_timer) idf_component_register(SRCS "${srcs}" INCLUDE_DIRS ${include_dirs} @@ -104,3 +105,12 @@ set_source_files_properties( # we can't establish dependency on what we don't yet know, so we force the # linker to not drop this symbol. target_link_libraries(${COMPONENT_LIB} INTERFACE "-u app_main") + +if(CONFIG_APPTRACE_SV_ENABLE) + # FreeRTOS headers have a dependency on app_trace when SystemView tracing is enabled + idf_component_optional_requires(PUBLIC app_trace) +elseif(CONFIG_APPTRACE_ENABLE) + # [refactor-todo]: port.c has a dependency on esp_apptrace_init, for running it on APP CPU. + # this should be resolved when link-time registration of startup functions is added. + idf_component_optional_requires(PRIVATE app_trace) +endif() diff --git a/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c b/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c index 3508653730..70a5b38494 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c +++ b/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c @@ -61,7 +61,9 @@ #include "esp_system.h" #include "esp_log.h" #include "esp_int_wdt.h" +#ifdef CONFIG_APPTRACE_ENABLE #include "esp_app_trace.h" /* Required for esp_apptrace_init. [refactor-todo] */ +#endif #include "FreeRTOS.h" /* This pulls in portmacro.h */ #include "task.h" /* Required for TaskHandle_t, tskNO_AFFINITY, and vTaskStartScheduler */ #include "port_systick.h" diff --git a/tools/test_apps/system/g1_components/CMakeLists.txt b/tools/test_apps/system/g1_components/CMakeLists.txt index 98d99530e6..ad57901d32 100644 --- a/tools/test_apps/system/g1_components/CMakeLists.txt +++ b/tools/test_apps/system/g1_components/CMakeLists.txt @@ -22,9 +22,6 @@ set(extra_allowed_components # These components are currently included into "G1" build, but shouldn't. # After removing the extra dependencies, remove the components from this list as well. set(extra_components_which_shouldnt_be_included - # app_trace is a public dependency of FreeRTOS because of SystemView trace headers. - # To fix, convert this into a weak public dependency conditional on CONFIG_APPTRACE_SV_ENABLE. - app_trace # app_update gets added because of bootloader_support, spi_flash, espcoredump, esp_system. # bootloader_support, spi_flash, espcoredump should be removed from dependencies; # esp_system code that reads app version should be made conditional on app_update being included @@ -51,8 +48,6 @@ set(extra_components_which_shouldnt_be_included esp_eth # esp_event is a dependency of esp_wifi, esp_eth. Both should be removed from G1. esp_event - # esp_gdbstub is a dependency of esp_system, can easily be made conditional on related Kconfig option. - esp_gdbstub # esp_netif is a dependency of lwip and esp_event, should disappear once lwip is removed. esp_netif # esp_phy is a dependency of esp_system and esp_wifi. For the former, it can be made a weak dependency. @@ -69,11 +64,6 @@ set(extra_components_which_shouldnt_be_included # esp_wifi is a dependency of lwip. # [refactor-todo]: investigate making esp_wifi a conditional dependency. esp_wifi - # Similar to esp_gdbstub, this dependency from esp_system can be made weakly dependent - # on CONFIG_ESP_COREDUMP_ENABLE. - # [refactor-todo]: how will the user set CONFIG_ESP_COREDUMP_ENABLE if the component - # is not included into the build, hence Kconfig option is not visible in menuconfig? - espcoredump # esptool_py is a dependency of bootloader, esp_wifi, app_update, partition_table, all of which # should be removed from G1-only build. esptool_py diff --git a/tools/test_apps/system/panic/CMakeLists.txt b/tools/test_apps/system/panic/CMakeLists.txt index a53a793ff7..2132f5b91f 100644 --- a/tools/test_apps/system/panic/CMakeLists.txt +++ b/tools/test_apps/system/panic/CMakeLists.txt @@ -2,7 +2,7 @@ # in this exact order for cmake to work correctly cmake_minimum_required(VERSION 3.5) -set(COMPONENTS esptool_py main) +set(COMPONENTS main espcoredump esp_gdbstub) include($ENV{IDF_PATH}/tools/cmake/project.cmake) project(test_panic)