From 82a78c441990438c6fd03b94dc7a98fbdb9117fc Mon Sep 17 00:00:00 2001 From: Felipe Moura Date: Thu, 17 Sep 2026 15:59:00 -0300 Subject: [PATCH] xtensa/espressif+riscv: fix PM_NORMAL stay leak in idle loop up_idlepm() (esp32s3_idle.c/esp32_idle.c/esp32s2_idle.c and the shared risc-v esp_idle.c for esp32c3/esp32c6) has a recovery branch that forces the domain back to PM_NORMAL when oldstate is not PM_NORMAL and nothing is currently staying at it: pm_stay(PM_IDLE_DOMAIN, PM_NORMAL); pm_changestate(PM_IDLE_DOMAIN, PM_NORMAL); newstate = PM_NORMAL; pm_stay() here has no matching pm_relax() anywhere in any of the four files. The first time this branch runs, the stay count for PM_NORMAL never returns to 0, and pm_checkstate() (called unconditionally right after this block) can never recommend anything deeper than PM_NORMAL again for the rest of uptime -- the idle loop keeps running, but the governor is permanently pinned at full power, with no further light or deep sleep. Confirmed on real ESP32-S3 hardware (XIAO ESP32-S3, CONFIG_ESPRESSIF_WIFI + CONFIG_PM + CONFIG_SCHED_TICKLESS): reading g_pmdomains[0] live via JTAG/GDB showed a "system" wakelock stuck at state=PM_NORMAL, count=1, acquired a few seconds after boot (right when Wi-Fi coming up briefly moves the domain off PM_NORMAL and this branch then forces it back). Reproduced 4/4 times before this fix (never a single PM_STANDBY transition or light-sleep-return log line across a 40+ minute run), 0/4 after. The trigger is timing-dependent (whether anything else already holds PM_NORMAL at the moment this branch runs), which is likely why it does not reproduce on every single boot. Fix: release the stay right after the one pm_changestate() call it exists to force, matching the comment already there ("Keep working in normal stage") -- a one-shot nudge, not a standing hold. Touching the switch statement right below the fix in all four files exposed a pre-existing nxstyle violation (case labels indented level with the switch's opening brace instead of one level in from it, per NuttX style); reindented alongside since checkpatch lints the whole file. esp32s3_idle.c also had two unrelated stray-indented lines ("Perform IDLE mode power management" / up_idlepm()) in up_idle(); fixed those too, same reason. Signed-off-by: Felipe Moura Assisted-by: Claude:claude-sonnet-5 --- arch/risc-v/src/common/espressif/esp_idle.c | 44 +++++++++++--------- arch/xtensa/src/esp32/esp32_idle.c | 42 ++++++++++--------- arch/xtensa/src/esp32s2/esp32s2_idle.c | 42 ++++++++++--------- arch/xtensa/src/esp32s3/esp32s3_idle.c | 46 +++++++++++---------- 4 files changed, 95 insertions(+), 79 deletions(-) diff --git a/arch/risc-v/src/common/espressif/esp_idle.c b/arch/risc-v/src/common/espressif/esp_idle.c index 1d1384ab7d3ae..2d7d7e262dbb4 100644 --- a/arch/risc-v/src/common/espressif/esp_idle.c +++ b/arch/risc-v/src/common/espressif/esp_idle.c @@ -119,6 +119,10 @@ static void up_idlepm(void) /* Keep working in normal stage */ pm_changestate(PM_IDLE_DOMAIN, PM_NORMAL); + + /* Release the stay above: it only forces this one state change. */ + + pm_relax(PM_IDLE_DOMAIN, PM_NORMAL); newstate = PM_NORMAL; } @@ -158,32 +162,32 @@ static void up_idlepm(void) switch (newstate) { - case PM_NORMAL: - break; + case PM_NORMAL: + break; - case PM_IDLE: - break; + case PM_IDLE: + break; - case PM_STANDBY: - { - /* Enter Force-sleep mode */ + case PM_STANDBY: + { + /* Enter Force-sleep mode */ - esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + - CONFIG_PM_ALARM_NSEC / 1000); - } - break; + esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + + CONFIG_PM_ALARM_NSEC / 1000); + } + break; - case PM_SLEEP: - { - /* Enter Deep-sleep mode */ + case PM_SLEEP: + { + /* Enter Deep-sleep mode */ - esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + - CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); - } - break; + esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + + CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); + } + break; - default: - break; + default: + break; } } else diff --git a/arch/xtensa/src/esp32/esp32_idle.c b/arch/xtensa/src/esp32/esp32_idle.c index 65263a486c3b2..ed366e08cd877 100644 --- a/arch/xtensa/src/esp32/esp32_idle.c +++ b/arch/xtensa/src/esp32/esp32_idle.c @@ -135,6 +135,10 @@ static void esp32_idlepm(void) /* Keep working in normal stage */ pm_changestate(PM_IDLE_DOMAIN, PM_NORMAL); + + /* Release the stay above: it only forces this one state change. */ + + pm_relax(PM_IDLE_DOMAIN, PM_NORMAL); newstate = PM_NORMAL; } @@ -174,31 +178,31 @@ static void esp32_idlepm(void) switch (newstate) { - case PM_NORMAL: - break; + case PM_NORMAL: + break; - case PM_IDLE: - break; + case PM_IDLE: + break; - case PM_STANDBY: - { - /* Enter Force-sleep mode */ + case PM_STANDBY: + { + /* Enter Force-sleep mode */ - esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + - CONFIG_PM_ALARM_NSEC / 1000); - } - break; + esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + + CONFIG_PM_ALARM_NSEC / 1000); + } + break; - case PM_SLEEP: - { - /* Enter Deep-sleep mode */ + case PM_SLEEP: + { + /* Enter Deep-sleep mode */ - esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + - CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); - } + esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + + CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); + } - default: - break; + default: + break; } } else diff --git a/arch/xtensa/src/esp32s2/esp32s2_idle.c b/arch/xtensa/src/esp32s2/esp32s2_idle.c index 351943a84f362..99fd361a302a6 100644 --- a/arch/xtensa/src/esp32s2/esp32s2_idle.c +++ b/arch/xtensa/src/esp32s2/esp32s2_idle.c @@ -100,6 +100,10 @@ static void up_idlepm(void) /* Keep working in normal stage */ pm_changestate(PM_IDLE_DOMAIN, PM_NORMAL); + + /* Release the stay above: it only forces this one state change. */ + + pm_relax(PM_IDLE_DOMAIN, PM_NORMAL); newstate = PM_NORMAL; } @@ -139,31 +143,31 @@ static void up_idlepm(void) switch (newstate) { - case PM_NORMAL: - break; + case PM_NORMAL: + break; - case PM_IDLE: - break; + case PM_IDLE: + break; - case PM_STANDBY: - { - /* Enter Force-sleep mode */ + case PM_STANDBY: + { + /* Enter Force-sleep mode */ - esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + - CONFIG_PM_ALARM_NSEC / 1000); - } - break; + esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + + CONFIG_PM_ALARM_NSEC / 1000); + } + break; - case PM_SLEEP: - { - /* Enter Deep-sleep mode */ + case PM_SLEEP: + { + /* Enter Deep-sleep mode */ - esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + - CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); - } + esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + + CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); + } - default: - break; + default: + break; } } else diff --git a/arch/xtensa/src/esp32s3/esp32s3_idle.c b/arch/xtensa/src/esp32s3/esp32s3_idle.c index 2fd2f8533485b..7b4da12a3e37b 100644 --- a/arch/xtensa/src/esp32s3/esp32s3_idle.c +++ b/arch/xtensa/src/esp32s3/esp32s3_idle.c @@ -100,6 +100,10 @@ static void up_idlepm(void) /* Keep working in normal stage */ pm_changestate(PM_IDLE_DOMAIN, PM_NORMAL); + + /* Release the stay above: it only forces this one state change. */ + + pm_relax(PM_IDLE_DOMAIN, PM_NORMAL); newstate = PM_NORMAL; } @@ -139,31 +143,31 @@ static void up_idlepm(void) switch (newstate) { - case PM_NORMAL: - break; + case PM_NORMAL: + break; - case PM_IDLE: - break; + case PM_IDLE: + break; - case PM_STANDBY: - { - /* Enter Force-sleep mode */ + case PM_STANDBY: + { + /* Enter Force-sleep mode */ - esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + - CONFIG_PM_ALARM_NSEC / 1000); - } - break; + esp_pmstandby(CONFIG_PM_ALARM_SEC * 1000000 + + CONFIG_PM_ALARM_NSEC / 1000); + } + break; - case PM_SLEEP: - { - /* Enter Deep-sleep mode */ + case PM_SLEEP: + { + /* Enter Deep-sleep mode */ - esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + - CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); - } + esp_pmsleep(CONFIG_PM_SLEEP_WAKEUP_SEC * 1000000 + + CONFIG_PM_SLEEP_WAKEUP_NSEC / 1000); + } - default: - break; + default: + break; } } else @@ -228,9 +232,9 @@ void up_idle(void) __asm__ __volatile__ ("waiti 0"); # endif - /* Perform IDLE mode power management */ + /* Perform IDLE mode power management */ - up_idlepm(); + up_idlepm(); #endif /* CONFIG_SUPPRESS_INTERRUPTS || CONFIG_SUPPRESS_TIMER_INTS */ #ifdef CONFIG_ESP32S3_SPEED_UP_ISR