Aksal/nuttx
3
1
Fork 1
mirror of https://github.com/apache/nuttx.git synced 2026-10-01 11:26:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

xtensa/espressif+riscv: fix PM_NORMAL stay leak in idle loop

Browse source 
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 <moura.fmo@gmail.com>
Assisted-by: Claude:claude-sonnet-5
This commit is contained in:
Felipe Moura 2026-09-17 15:59:00 -03:00 • committed by Xiang Xiao
parent 35b68203ad
commit 877d1537df
4 changed files with 95 additions and 79 deletions
Download patch file Download diff file Expand all files Collapse all files

44
arch/risc-v/src/common/espressif/esp_idle.c
View file

@ -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

42
arch/xtensa/src/esp32/esp32_idle.c
View file

@ -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

42
arch/xtensa/src/esp32s2/esp32s2_idle.c
View file

@ -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

46
arch/xtensa/src/esp32s3/esp32s3_idle.c
View file

@ -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