FelipeMdeO commented on code in PR #20223:
URL: https://github.com/apache/nuttx/pull/20223#discussion_r4063305603


##########
arch/xtensa/src/esp32s3/esp32s3_idle.c:
##########
@@ -104,7 +104,12 @@ static void up_idlepm(void)
       /* Release the stay above: it only forces this one state change. */
 
       pm_relax(PM_IDLE_DOMAIN, PM_NORMAL);
-      newstate = PM_NORMAL;
+
+      /* Without this, oldstate goes stale after the first wakeup and the
+       * state machine wedges in PM_NORMAL forever -- see commit message.
+       */
+
+      oldstate = PM_NORMAL;

Review Comment:
   Hello, I Agree — and it's not just esp32, the identical pattern shows up in 
all four idle-loop copies: esp32_idle.c, esp32s2_idle.c, and the shared RISC-V 
esp_idle.c (esp32c3/c6) all have the same dead newstate = PM_NORMAL; right 
before pm_checkstate() overwrites it, with oldstate never updated. Same shape, 
same root cause as this PR.
   
   I can't test on esp32/esp32s2/RISC-V hardware right now, only on the 
esp32s3-xiao this PR was built against.
   It would help to get input from the Espressif team here — if they're fine 
with the same one-line fix across all four, I'm happy to expand this PR (or 
split into per-arch commits) to cover them together, since it's the same bug 
either way.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to