From da390618c2ab4868b0dc1ca85f7a25c3799358f1 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 22 Feb 2019 12:20:11 +1100 Subject: [PATCH] cmake: Fix psram workaround compiler flag application Previously, this compiler flag was not being applied regardless of CONFIG_SPIRAM_CACHE_WORKAROUND setting. Explanation: add_compile_options() only applies to source files added after the function is run, or in subdirectories added after the function is run. In this case, no new source files were being added after this function was run. --- components/esp32/CMakeLists.txt | 5 ++++- components/esp32/Kconfig | 3 ++- components/esp32/project_include.cmake | 8 ++++++++ docs/en/api-guides/build-system-cmake.rst | 4 ++++ docs/en/api-guides/build-system.rst | 1 + tools/ci/test_build_system_cmake.sh | 11 +++++++++++ tools/cmake/utilities.cmake | 24 ----------------------- 7 files changed, 30 insertions(+), 26 deletions(-) create mode 100644 components/esp32/project_include.cmake diff --git a/components/esp32/CMakeLists.txt b/components/esp32/CMakeLists.txt index 938f77ef1e..dac411782c 100644 --- a/components/esp32/CMakeLists.txt +++ b/components/esp32/CMakeLists.txt @@ -96,7 +96,10 @@ else() ) if(CONFIG_SPIRAM_CACHE_WORKAROUND) - add_compile_options(-mfix-esp32-psram-cache-issue) + # Note: Adding as a PUBLIC compile option here causes this option to propagate to all components that depend on esp32. + # + # To handle some corner cases, the same flag is set in project_include.cmake + target_compile_options(${COMPONENT_TARGET} PUBLIC -mfix-esp32-psram-cache-issue) else() target_linker_script(${COMPONENT_TARGET} "ld/esp32.rom.spiram_incompatible_fns.ld") endif() diff --git a/components/esp32/Kconfig b/components/esp32/Kconfig index df8b31c73d..ffe2413a80 100644 --- a/components/esp32/Kconfig +++ b/components/esp32/Kconfig @@ -132,7 +132,8 @@ menu "ESP32-specific" help Revision 1 of the ESP32 has a bug that can cause a write to PSRAM not to take place in some situations when the cache line needs to be fetched from external RAM and an interrupt occurs. This enables a - fix in the compiler that makes sure the specific code that is vulnerable to this will not be emitted. + fix in the compiler (-mfix-esp32-psram-cache-issue) that makes sure the specific code that is + vulnerable to this will not be emitted. This will also not use any bits of newlib that are located in ROM, opting for a version that is compiled with the workaround and located in flash instead. diff --git a/components/esp32/project_include.cmake b/components/esp32/project_include.cmake new file mode 100644 index 0000000000..d54d6a9beb --- /dev/null +++ b/components/esp32/project_include.cmake @@ -0,0 +1,8 @@ +if(CONFIG_SPIRAM_CACHE_WORKAROUND) + # We do this here as well as in CMakeLists.txt, because targets that + # are not part of the ESP-IDF build system (for cases where a generic + # non-IDF CMakeLists.txt file is imported into a component) don't depend + # on the esp32 component so don't get the extra flag. This handles that case. + add_compile_options(-mfix-esp32-psram-cache-issue) +endif() + diff --git a/docs/en/api-guides/build-system-cmake.rst b/docs/en/api-guides/build-system-cmake.rst index 31130a860b..14b138ca80 100644 --- a/docs/en/api-guides/build-system-cmake.rst +++ b/docs/en/api-guides/build-system-cmake.rst @@ -775,6 +775,8 @@ the ESP-IDF build system entirely by using a CMake feature called ExternalProjec - The second set of commands adds a library target, which points to the "imported" library file built by the external system. Some properties need to be set in order to add include directories and tell CMake where this file is. - Finally, the generated library is added to `ADDITIONAL_MAKE_CLEAN_FILES`_. This means ``make clean`` will delete this library. (Note that the other object files from the build won't be deleted.) +.. note:: When using an external build process with PSRAM, remember to add ``-mfix-esp32-psram-cache-issue`` to the C compiler arguments. See :ref:`CONFIG_SPIRAM_CACHE_WORKAROUND` for details of this flag. + .. _ADDITIONAL_MAKE_CLEAN_FILES_note: ExternalProject dependencies, clean builds @@ -961,6 +963,8 @@ parameters which can be set, the full list of which is as follows: The example in :example:`build_system/cmake/idf_as_lib` demonstrates the creation of an application equivalent to :example:`hello world application ` using a custom CMake project. +.. note:: The IDF build system can only set compiler flags for source files that it builds. When an external CMakeLists.txt file is used and PSRAM is enabled, remember to add ``-mfix-esp32-psram-cache-issue`` to the C compiler arguments. See :ref:`CONFIG_SPIRAM_CACHE_WORKAROUND` for details of this flag. + .. _cmake-file-globbing: File Globbing & Incremental Builds diff --git a/docs/en/api-guides/build-system.rst b/docs/en/api-guides/build-system.rst index 8f6a98be0e..70eac2755b 100644 --- a/docs/en/api-guides/build-system.rst +++ b/docs/en/api-guides/build-system.rst @@ -614,6 +614,7 @@ $(COMPONENT_LIBRARY) for the project make process to link into the app binary. (Actually, even this is not strictly necessary - if the COMPONENT_ADD_LDFLAGS variable is overridden then the component can instruct the linker to link other binaries instead.) +.. note:: When using an external build process with PSRAM, remember to add ``-mfix-esp32-psram-cache-issue`` to the C compiler arguments. See :ref:`CONFIG_SPIRAM_CACHE_WORKAROUND` for details of this flag. .. _esp-idf-template: https://github.com/espressif/esp-idf-template .. _GNU Make Manual: https://www.gnu.org/software/make/manual/make.html diff --git a/tools/ci/test_build_system_cmake.sh b/tools/ci/test_build_system_cmake.sh index 3fb5cb103d..f63aae7f95 100755 --- a/tools/ci/test_build_system_cmake.sh +++ b/tools/ci/test_build_system_cmake.sh @@ -328,6 +328,17 @@ function run_tests() grep "CONFIG_PARTITION_TABLE_OFFSET=0x10000" sdkconfig || failure "The define from sdkconfig.defaults should be into sdkconfig" grep "CONFIG_PARTITION_TABLE_TWO_OTA=y" sdkconfig || failure "The define from sdkconfig should be into sdkconfig" + print_status "Building a project with CMake library imported and PSRAM workaround, all files compile with workaround" + cp sdkconfig sdkconfig.psram + rm -rf build + echo "CONFIG_SPIRAM_SUPPORT=y" >> sdkconfig.psram + echo "CONFIG_SPIRAM_CACHE_WORKAROUND=y" >> sdkconfig.psram + # note: we do 'reconfigure' here, as we just need to run cmake + idf.py -C $IDF_PATH/examples/build_system/cmake/import_lib -B `pwd`/build reconfigure -D SDKCONFIG="`pwd`/sdkconfig.psram" + grep -q '"command"' build/compile_commands.json || failure "compile_commands.json missing or has no no 'commands' in it" + (grep '"command"' build/compile_commands.json | grep -v mfix-esp32-psram-cache-issue) && failure "All commands in compile_commands.json should use PSRAM cache workaround" + rm sdkconfig.psram + print_status "All tests completed" if [ -n "${FAILURES}" ]; then echo "Some failures were detected:" diff --git a/tools/cmake/utilities.cmake b/tools/cmake/utilities.cmake index 9e7019f0e7..36cfbc7654 100644 --- a/tools/cmake/utilities.cmake +++ b/tools/cmake/utilities.cmake @@ -73,30 +73,6 @@ function(move_if_different source destination) endfunction() - -# add_compile_options variant for C++ code only -# -# This adds global options, set target properties for -# component-specific flags -function(add_cxx_compile_options) - foreach(option ${ARGV}) - # note: the Visual Studio Generator doesn't support this... - add_compile_options($<$:${option}>) - endforeach() -endfunction() - -# add_compile_options variant for C code only -# -# This adds global options, set target properties for -# component-specific flags -function(add_c_compile_options) - foreach(option ${ARGV}) - # note: the Visual Studio Generator doesn't support this... - add_compile_options($<$:${option}>) - endforeach() -endfunction() - - # target_add_binary_data adds binary data into the built target, # by converting it to a generated source file which is then compiled # to a binary object as part of the build -- 2.40.0