From ac82038b538b01981abae586a269099db33a7ce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Wed, 4 Jan 2023 11:32:15 +0100 Subject: [PATCH] [zephyr] Use native locking primitives (#24231) * [zephyr] Use native locking primitives Switch from Zephyr POSIX layer to Zephyr native locking primitives to reduce the memory overhead. * Code review --- .../nrfconnect/chip-module/Kconfig.defaults | 4 ++ src/ble/BLEEndPoint.h | 3 +- src/platform/Zephyr/SystemPlatformConfig.h | 4 -- .../nrfconnect/SystemPlatformConfig.h | 4 -- src/platform/telink/SystemPlatformConfig.h | 4 -- src/system/BUILD.gn | 2 + src/system/SystemConfig.h | 58 ++++++++++--------- src/system/SystemMutex.h | 35 ++++++++--- src/system/system.gni | 7 ++- 9 files changed, 73 insertions(+), 48 deletions(-) diff --git a/config/nrfconnect/chip-module/Kconfig.defaults b/config/nrfconnect/chip-module/Kconfig.defaults index 9232a4600d69ef..fedfc8cab28f57 100644 --- a/config/nrfconnect/chip-module/Kconfig.defaults +++ b/config/nrfconnect/chip-module/Kconfig.defaults @@ -64,6 +64,10 @@ config SHELL bool default y +config PTHREAD_IPC + bool + default n + # Generic networking options config NET_SOCKETS_POSIX_NAMES bool diff --git a/src/ble/BLEEndPoint.h b/src/ble/BLEEndPoint.h index 7a50256ca064a4..dba18fc5b43511 100644 --- a/src/ble/BLEEndPoint.h +++ b/src/ble/BLEEndPoint.h @@ -28,12 +28,13 @@ #pragma once #include -#include #include #include + #if CHIP_ENABLE_CHIPOBLE_TEST #include +#include #endif namespace chip { diff --git a/src/platform/Zephyr/SystemPlatformConfig.h b/src/platform/Zephyr/SystemPlatformConfig.h index b8e8872a617d93..0ea9af8c3b05af 100644 --- a/src/platform/Zephyr/SystemPlatformConfig.h +++ b/src/platform/Zephyr/SystemPlatformConfig.h @@ -34,10 +34,6 @@ struct ChipDeviceEvent; // ==================== Platform Adaptations ==================== -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0 - #ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS #define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 0 #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS diff --git a/src/platform/nrfconnect/SystemPlatformConfig.h b/src/platform/nrfconnect/SystemPlatformConfig.h index 803005c4df4531..cd863d5c67dcd4 100644 --- a/src/platform/nrfconnect/SystemPlatformConfig.h +++ b/src/platform/nrfconnect/SystemPlatformConfig.h @@ -34,10 +34,6 @@ struct ChipDeviceEvent; // ==================== Platform Adaptations ==================== -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0 - #ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS #define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 0 #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS diff --git a/src/platform/telink/SystemPlatformConfig.h b/src/platform/telink/SystemPlatformConfig.h index 5e3fce6a5b0023..4e80b6e51ca413 100644 --- a/src/platform/telink/SystemPlatformConfig.h +++ b/src/platform/telink/SystemPlatformConfig.h @@ -34,10 +34,6 @@ struct ChipDeviceEvent; // ==================== Platform Adaptations ==================== -#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1 -#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0 -#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0 - #ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS #define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 0 #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn index 348e3af0ebf5f1..581ed2e23f0bb0 100644 --- a/src/system/BUILD.gn +++ b/src/system/BUILD.gn @@ -67,6 +67,7 @@ buildconfig_header("system_buildconfig") { chip_system_config_mbed_locking = chip_system_config_locking == "mbed" chip_system_config_cmsis_rtos_locking = chip_system_config_locking == "cmsis-rtos" + chip_system_config_zephyr_locking = chip_system_config_locking == "zephyr" chip_system_config_no_locking = chip_system_config_locking == "none" have_clock_gettime = chip_system_config_clock == "clock_gettime" have_clock_settime = have_clock_gettime @@ -85,6 +86,7 @@ buildconfig_header("system_buildconfig") { "CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING=${chip_system_config_freertos_locking}", "CHIP_SYSTEM_CONFIG_MBED_LOCKING=${chip_system_config_mbed_locking}", "CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING=${chip_system_config_cmsis_rtos_locking}", + "CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING=${chip_system_config_zephyr_locking}", "CHIP_SYSTEM_CONFIG_NO_LOCKING=${chip_system_config_no_locking}", "CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS=${chip_system_config_provide_statistics}", "HAVE_CLOCK_GETTIME=${have_clock_gettime}", diff --git a/src/system/SystemConfig.h b/src/system/SystemConfig.h index c214e922ca84f8..4b8680b8f6b782 100644 --- a/src/system/SystemConfig.h +++ b/src/system/SystemConfig.h @@ -232,6 +232,20 @@ #define CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING 0 #endif /* CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING */ +/** + * @def CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING + * + * @brief + * Use Zephyr native locking primitives. + * + * This is recommended and enabled by default for Zephyr RTOS. + * Alternatively, you can use CHIP_SYSTEM_CONFIG_POSIX_LOCKING and Zephyr POSIX compatibility + * layer, but that solution has higher memory overhead. + */ +#ifndef CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING +#define CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING 0 +#endif /* CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING */ + /** * @def CHIP_SYSTEM_CONFIG_POOL_USE_HEAP * @@ -254,37 +268,29 @@ #define CHIP_SYSTEM_CONFIG_NO_LOCKING 0 #endif /* CHIP_SYSTEM_CONFIG_NO_LOCKING */ -#if !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING ||CHIP_SYSTEM_CONFIG_NO_LOCKING) -#error "REQUIRED: CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING" -#endif // !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING) - -#if CHIP_SYSTEM_CONFIG_NO_LOCKING && (CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING) -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_NO_LOCKING && (CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING)" -#endif // CHIP_SYSTEM_CONFIG_NO_LOCKING && (CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING) - -#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING" -#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING +#if !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) +#error "REQUIRED: CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING" +#endif // !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) -#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING" -#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING +#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && (CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) +#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && (CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)" +#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && (CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) -#if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING" -#endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING +#if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && (CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) +#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && (CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)" +#endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && (CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) -#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING" -#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING +#if CHIP_SYSTEM_CONFIG_MBED_LOCKING && (CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) +#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_MBED_LOCKING && (CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)" +#endif // CHIP_SYSTEM_CONFIG_MBED_LOCKING && (CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) -#if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING" -#endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING +#if CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING && (CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) +#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING && (CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)" +#endif // CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING && (CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING) -#if CHIP_SYSTEM_CONFIG_MBED_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING -#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_MBED_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING" -#endif // CHIP_SYSTEM_CONFIG_MBED_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING +#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING +#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING" +#endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING /** * @def CHIP_SYSTEM_HEADER_RESERVE_SIZE diff --git a/src/system/SystemMutex.h b/src/system/SystemMutex.h index 13c8337c6c7ffb..c697ea52b9cc64 100644 --- a/src/system/SystemMutex.h +++ b/src/system/SystemMutex.h @@ -28,9 +28,9 @@ #include // Include dependent headers -#include - +#include #include +#include #if CHIP_SYSTEM_CONFIG_POSIX_LOCKING #include @@ -56,6 +56,10 @@ #include #endif // CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING +#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING +#include +#endif + namespace chip { namespace System { @@ -73,8 +77,7 @@ namespace System { class DLL_EXPORT Mutex { public: - Mutex(); - ~Mutex(); + Mutex() = default; static CHIP_ERROR Init(Mutex & aMutex); #if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING @@ -109,13 +112,14 @@ class DLL_EXPORT Mutex osMutexId_t mCmsisRTOSMutex; #endif // CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING +#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING + k_mutex mZephyrMutex; +#endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING + Mutex(const Mutex &) = delete; Mutex & operator=(const Mutex &) = delete; }; -inline Mutex::Mutex() {} -inline Mutex::~Mutex() {} - #if CHIP_SYSTEM_CONFIG_NO_LOCKING inline CHIP_ERROR Init(Mutex & aMutex) { @@ -163,5 +167,22 @@ inline void Mutex::Unlock(void) } #endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING +#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING +inline CHIP_ERROR Mutex::Init(Mutex & aMutex) +{ + return System::MapErrorZephyr(k_mutex_init(&aMutex.mZephyrMutex)); +} + +inline void Mutex::Lock() +{ + VerifyOrDie(0 == k_mutex_lock(&mZephyrMutex, K_FOREVER)); +} + +inline void Mutex::Unlock(void) +{ + VerifyOrDie(0 == k_mutex_unlock(&mZephyrMutex)); +} +#endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING + } // namespace System } // namespace chip diff --git a/src/system/system.gni b/src/system/system.gni index f65a610bd88161..e91dd0dee460d3 100644 --- a/src/system/system.gni +++ b/src/system/system.gni @@ -59,6 +59,8 @@ if (chip_system_config_locking == "") { chip_system_config_locking = "mbed" } else if (current_os == "cmsis-rtos") { chip_system_config_locking = "cmsis-rtos" + } else if (current_os == "zephyr") { + chip_system_config_locking = "zephyr" } else if (chip_system_config_use_dispatch == false) { chip_system_config_locking = "posix" } else { @@ -71,8 +73,9 @@ assert( chip_system_config_locking == "freertos" || chip_system_config_locking == "none" || chip_system_config_locking == "mbed" || - chip_system_config_locking == "cmsis-rtos", - "Please select a valid mutex implementation: posix, freertos, mbed, cmsis-rtos, none") + chip_system_config_locking == "cmsis-rtos" || + chip_system_config_locking == "zephyr", + "Please select a valid mutex implementation: posix, freertos, mbed, cmsis-rtos, zephyr, none") assert( chip_system_config_clock == "clock_gettime" ||