From 9ffd8aa017262d9762fc4054b901ef6c8beb7715 Mon Sep 17 00:00:00 2001 From: wuzhenghui Date: Tue, 24 Sep 2024 18:05:47 +0800 Subject: [PATCH 1/2] fix(esp_hw_support): fix coverity defects in sleep code --- .../esp_hw_support/port/esp32p4/io_mux.c | 4 +++ components/esp_hw_support/sleep_modes.c | 31 ++++++++++--------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/components/esp_hw_support/port/esp32p4/io_mux.c b/components/esp_hw_support/port/esp32p4/io_mux.c index 64cd2aa200..5f24525a7a 100644 --- a/components/esp_hw_support/port/esp32p4/io_mux.c +++ b/components/esp_hw_support/port/esp32p4/io_mux.c @@ -6,6 +6,7 @@ #include "sdkconfig.h" #include "esp_attr.h" +#include "esp_check.h" #include "freertos/FreeRTOS.h" #include "esp_private/esp_clk_tree_common.h" #include "esp_private/io_mux.h" @@ -14,6 +15,8 @@ #include "hal/rtc_io_ll.h" #include "soc/soc_caps.h" +static const char __attribute__((__unused__)) *IOMUX_TAG = "IO_MUX"; + #define RTCIO_RCC_ATOMIC() PERIPH_RCC_ATOMIC() static portMUX_TYPE s_io_mux_spinlock = portMUX_INITIALIZER_UNLOCKED; @@ -52,6 +55,7 @@ esp_err_t io_mux_set_clock_source(soc_module_clk_t clk_src) void io_mux_enable_lp_io_clock(gpio_num_t gpio_num, bool enable) { + ESP_RETURN_ON_FALSE(rtc_gpio_is_valid_gpio(gpio_num), ESP_ERR_INVALID_ARG, IOMUX_TAG, "RTCIO number error"); portENTER_CRITICAL(&s_io_mux_spinlock); if (enable) { if (s_rtc_io_status.rtc_io_enabled_cnt[gpio_num] == 0) { diff --git a/components/esp_hw_support/sleep_modes.c b/components/esp_hw_support/sleep_modes.c index 44edbe8cfd..e119b46310 100644 --- a/components/esp_hw_support/sleep_modes.c +++ b/components/esp_hw_support/sleep_modes.c @@ -983,6 +983,14 @@ static esp_err_t IRAM_ATTR esp_sleep_start(uint32_t pd_flags, esp_sleep_mode_t m #endif // SOC_PM_SUPPORT_DEEPSLEEP_CHECK_STUB_ONLY #endif +#if SOC_DCDC_SUPPORTED + uint64_t ldo_increased_us = rtc_time_slowclk_to_us(rtc_time_get() - s_config.rtc_ticks_at_ldo_prepare, s_config.rtc_clk_cal_period); + if (ldo_increased_us < LDO_POWER_TAKEOVER_PREPARATION_TIME_US) { + esp_rom_delay_us(LDO_POWER_TAKEOVER_PREPARATION_TIME_US - ldo_increased_us); + } + pmu_sleep_shutdown_dcdc(); +#endif + // Enter Deep Sleep #if!ESP_ROM_SUPPORT_DEEP_SLEEP_WAKEUP_STUB || SOC_PM_SUPPORT_DEEPSLEEP_CHECK_STUB_ONLY || !CONFIG_ESP_SYSTEM_ALLOW_RTC_FAST_MEM_AS_HEAP #if SOC_PMU_SUPPORTED @@ -1016,27 +1024,20 @@ static esp_err_t IRAM_ATTR esp_sleep_start(uint32_t pd_flags, esp_sleep_mode_t m } #endif -#if SOC_DCDC_SUPPORTED -#if CONFIG_ESP_SLEEP_KEEP_DCDC_ALWAYS_ON - if (!deep_sleep) { - // Keep DCDC always on during light sleep, no need to adjust LDO voltage. - } else -#endif - { - uint64_t ldo_increased_us = rtc_time_slowclk_to_us(rtc_time_get() - s_config.rtc_ticks_at_ldo_prepare, s_config.rtc_clk_cal_period); - if (ldo_increased_us < LDO_POWER_TAKEOVER_PREPARATION_TIME_US) { - esp_rom_delay_us(LDO_POWER_TAKEOVER_PREPARATION_TIME_US - ldo_increased_us); - } - pmu_sleep_shutdown_dcdc(); - } -#endif - #if CONFIG_PM_POWER_DOWN_PERIPHERAL_IN_LIGHT_SLEEP && SOC_PM_MMU_TABLE_RETENTION_WHEN_TOP_PD if (pd_flags & PMU_SLEEP_PD_TOP) { esp_sleep_mmu_retention(true); } #endif +#if SOC_DCDC_SUPPORTED && !CONFIG_ESP_SLEEP_KEEP_DCDC_ALWAYS_ON + uint64_t ldo_increased_us = rtc_time_slowclk_to_us(rtc_time_get() - s_config.rtc_ticks_at_ldo_prepare, s_config.rtc_clk_cal_period); + if (ldo_increased_us < LDO_POWER_TAKEOVER_PREPARATION_TIME_US) { + esp_rom_delay_us(LDO_POWER_TAKEOVER_PREPARATION_TIME_US - ldo_increased_us); + } + pmu_sleep_shutdown_dcdc(); +#endif + #if SOC_PMU_SUPPORTED #if SOC_PM_CPU_RETENTION_BY_SW && ESP_SLEEP_POWER_DOWN_CPU esp_sleep_execute_event_callbacks(SLEEP_EVENT_HW_GOTO_SLEEP, (void *)0); From 6520c61cff6d956b4f00cd8baedcde238f285a89 Mon Sep 17 00:00:00 2001 From: wuzhenghui Date: Tue, 24 Sep 2024 18:06:22 +0800 Subject: [PATCH 2/2] change(esp_hw_support): improve gpio deepsleep wakeup configuration code --- .../include/esp_private/io_mux.h | 2 +- .../esp_hw_support/port/esp32p4/io_mux.c | 7 +++--- components/esp_hw_support/sleep_modes.c | 24 ++++++++++++------- .../soc/esp32c5/include/soc/io_mux_reg.h | 2 +- .../soc/esp32c6/include/soc/io_mux_reg.h | 2 +- .../soc/esp32c61/include/soc/io_mux_reg.h | 2 +- .../soc/esp32p4/include/soc/io_mux_reg.h | 2 +- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/components/esp_hw_support/include/esp_private/io_mux.h b/components/esp_hw_support/include/esp_private/io_mux.h index 62d5c4d76d..c2b9d39a77 100644 --- a/components/esp_hw_support/include/esp_private/io_mux.h +++ b/components/esp_hw_support/include/esp_private/io_mux.h @@ -32,7 +32,7 @@ esp_err_t io_mux_set_clock_source(soc_module_clk_t clk_src); #if SOC_LP_IO_CLOCK_IS_INDEPENDENT typedef struct { - uint8_t rtc_io_enabled_cnt[MAX_RTC_GPIO_NUM]; + uint8_t rtc_io_enabled_cnt[MAX_RTC_GPIO_NUM + 1]; uint32_t rtc_io_using_mask; } rtc_io_status_t; diff --git a/components/esp_hw_support/port/esp32p4/io_mux.c b/components/esp_hw_support/port/esp32p4/io_mux.c index 5f24525a7a..a8fe687f44 100644 --- a/components/esp_hw_support/port/esp32p4/io_mux.c +++ b/components/esp_hw_support/port/esp32p4/io_mux.c @@ -15,8 +15,6 @@ #include "hal/rtc_io_ll.h" #include "soc/soc_caps.h" -static const char __attribute__((__unused__)) *IOMUX_TAG = "IO_MUX"; - #define RTCIO_RCC_ATOMIC() PERIPH_RCC_ATOMIC() static portMUX_TYPE s_io_mux_spinlock = portMUX_INITIALIZER_UNLOCKED; @@ -55,7 +53,10 @@ esp_err_t io_mux_set_clock_source(soc_module_clk_t clk_src) void io_mux_enable_lp_io_clock(gpio_num_t gpio_num, bool enable) { - ESP_RETURN_ON_FALSE(rtc_gpio_is_valid_gpio(gpio_num), ESP_ERR_INVALID_ARG, IOMUX_TAG, "RTCIO number error"); + if (gpio_num > MAX_RTC_GPIO_NUM) { + assert(false && "RTCIO number error"); + return; + } portENTER_CRITICAL(&s_io_mux_spinlock); if (enable) { if (s_rtc_io_status.rtc_io_enabled_cnt[gpio_num] == 0) { diff --git a/components/esp_hw_support/sleep_modes.c b/components/esp_hw_support/sleep_modes.c index e119b46310..d6208bdda5 100644 --- a/components/esp_hw_support/sleep_modes.c +++ b/components/esp_hw_support/sleep_modes.c @@ -1967,8 +1967,9 @@ uint64_t esp_sleep_get_gpio_wakeup_status(void) static void gpio_deep_sleep_wakeup_prepare(void) { - for (gpio_num_t gpio_idx = GPIO_NUM_0; gpio_idx < GPIO_NUM_MAX; gpio_idx++) { - if (((1ULL << gpio_idx) & s_config.gpio_wakeup_mask) == 0) { + uint32_t valid_wake_io_mask = SOC_GPIO_DEEP_SLEEP_WAKE_VALID_GPIO_MASK; + for (gpio_num_t gpio_idx = __builtin_ctz(valid_wake_io_mask); valid_wake_io_mask >> gpio_idx; gpio_idx++) { + if ((s_config.gpio_wakeup_mask & BIT64(gpio_idx)) == 0) { continue; } #if CONFIG_ESP_SLEEP_GPIO_ENABLE_INTERNAL_RESISTORS @@ -1994,13 +1995,20 @@ esp_err_t esp_deep_sleep_enable_gpio_wakeup(uint64_t gpio_pin_mask, esp_deepslee } gpio_int_type_t intr_type = ((mode == ESP_GPIO_WAKEUP_GPIO_LOW) ? GPIO_INTR_LOW_LEVEL : GPIO_INTR_HIGH_LEVEL); esp_err_t err = ESP_OK; - for (gpio_num_t gpio_idx = GPIO_NUM_0; gpio_idx < GPIO_NUM_MAX; gpio_idx++, gpio_pin_mask >>= 1) { - if ((gpio_pin_mask & 1) == 0) { - continue; + + uint64_t invalid_io_mask = gpio_pin_mask & ~SOC_GPIO_DEEP_SLEEP_WAKE_VALID_GPIO_MASK; + if (invalid_io_mask != 0) { + for (gpio_num_t gpio_idx = __builtin_ctzll(invalid_io_mask); invalid_io_mask >> gpio_idx; gpio_idx++) { + if (invalid_io_mask & BIT64(gpio_idx)) { + ESP_LOGE(TAG, "gpio %d is an invalid deep sleep wakeup IO", gpio_idx); + return ESP_ERR_INVALID_ARG; + } } - if (!esp_sleep_is_valid_wakeup_gpio(gpio_idx)) { - ESP_LOGE(TAG, "gpio %d is an invalid deep sleep wakeup IO", gpio_idx); - return ESP_ERR_INVALID_ARG; + } + + for (gpio_num_t gpio_idx = __builtin_ctzll(gpio_pin_mask); gpio_pin_mask >> gpio_idx; gpio_idx++) { + if ((gpio_pin_mask & BIT64(gpio_idx)) == 0) { + continue; } err = gpio_deep_sleep_wakeup_enable(gpio_idx, intr_type); diff --git a/components/soc/esp32c5/include/soc/io_mux_reg.h b/components/soc/esp32c5/include/soc/io_mux_reg.h index c5e4f2a87d..53d3758fc1 100644 --- a/components/soc/esp32c5/include/soc/io_mux_reg.h +++ b/components/soc/esp32c5/include/soc/io_mux_reg.h @@ -144,7 +144,7 @@ extern "C" { #define EXT_OSC_SLOW_GPIO_NUM 0 -#define MAX_RTC_GPIO_NUM 8 +#define MAX_RTC_GPIO_NUM 7 #define MAX_PAD_GPIO_NUM 28 #define MAX_GPIO_NUM 32 #define DIG_IO_HOLD_BIT_SHIFT 32 diff --git a/components/soc/esp32c6/include/soc/io_mux_reg.h b/components/soc/esp32c6/include/soc/io_mux_reg.h index aa95643237..698a2b5785 100644 --- a/components/soc/esp32c6/include/soc/io_mux_reg.h +++ b/components/soc/esp32c6/include/soc/io_mux_reg.h @@ -148,7 +148,7 @@ #define EXT_OSC_SLOW_GPIO_NUM 0 -#define MAX_RTC_GPIO_NUM 8 +#define MAX_RTC_GPIO_NUM 7 #define MAX_PAD_GPIO_NUM 30 #define MAX_GPIO_NUM 34 #define DIG_IO_HOLD_BIT_SHIFT 32 diff --git a/components/soc/esp32c61/include/soc/io_mux_reg.h b/components/soc/esp32c61/include/soc/io_mux_reg.h index fff0c3fa6e..1c330bcfdf 100644 --- a/components/soc/esp32c61/include/soc/io_mux_reg.h +++ b/components/soc/esp32c61/include/soc/io_mux_reg.h @@ -129,7 +129,7 @@ extern "C" { #define EXT_OSC_SLOW_GPIO_NUM 0 -#define MAX_RTC_GPIO_NUM 7 +#define MAX_RTC_GPIO_NUM 6 #define MAX_PAD_GPIO_NUM 21 #define MAX_GPIO_NUM 28 #define HIGH_IO_HOLD_BIT_SHIFT 32 diff --git a/components/soc/esp32p4/include/soc/io_mux_reg.h b/components/soc/esp32p4/include/soc/io_mux_reg.h index 85eed69098..02440af903 100644 --- a/components/soc/esp32p4/include/soc/io_mux_reg.h +++ b/components/soc/esp32p4/include/soc/io_mux_reg.h @@ -194,7 +194,7 @@ #define EXT_OSC_SLOW_GPIO_NUM 0 // XTAL_32K_N -#define MAX_RTC_GPIO_NUM 16 +#define MAX_RTC_GPIO_NUM 15 #define MAX_PAD_GPIO_NUM 54 #define MAX_GPIO_NUM 56 #define HIGH_IO_HOLD_BIT_SHIFT 32