Merge #562

562: Replace default WFI with simpler NOP r=korken89 a=AfoHT

As noted by #561 there are multiple issues with various hardware implementations and debugging together with sleep modes.

As RTIC strives to be a generic framework (even though this is an implementation targeting cortex-m) the decision having `WFI` as the default `idle` task causes issues in some hardware, raising the barrier to entry.

This changes the default behaviour to do a simple `NOP` instead, adding documentation how to provide a custom `idle` task achieving the old default behaviour. Also removes the automatic setting of SLEEPONEXIT bit when no `idle` was provided, delegating this to the user.

This was discussed on the weekly meeting 2021-12-14 and this was the favoured solution.

Fixes #561 

Co-authored-by: Henrik Tjäder <henrik@grepit.se>

CHANGELOG.md

@@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Changed
+
+- Change default `idle` behaviour to be `NOP` instead of `WFI`
+
 ## [v0.6.0-rc.4] - 2021-11-09
 
 - Updated to use the new generic `Monotonic` trait

book/en/src/by-example/app_idle.md

@@ -8,11 +8,6 @@ When present, the runtime will execute the `idle` task after `init`. Unlike
 `init`, `idle` will run *with interrupts enabled* and it's not allowed to return
 so it must run forever.
 
-When no `idle` function is declared, the runtime sets the [SLEEPONEXIT] bit and
-then sends the microcontroller to sleep after running `init`.
-
-[SLEEPONEXIT]: https://developer.arm.com/docs/100737/0100/power-management/sleep-mode/sleep-on-exit-bit
-
 Like in `init`, locally declared resources will have `'static` lifetimes that are safe to access.
 
 The example below shows that `idle` runs after `init`.
@@ -25,3 +20,29 @@ The example below shows that `idle` runs after `init`.
 $ cargo run --target thumbv7m-none-eabi --example idle
 {{#include ../../../../ci/expected/idle.run}}
 ```
+
+By default the RTIC `idle` task does not try to optimise for any specific targets.
+
+A common useful optimisation is to enable the [SLEEPONEXIT] and allow the MCU
+to enter sleep when reaching `idle`.
+
+>**Caution** some hardware unless configured disables the debug unit during sleep mode.
+>
+>Consult your hardware specific documentation as this is outside the scope of RTIC.
+
+The following example shows how to enable sleep by setting the
+[`SLEEPONEXIT`][SLEEPONEXIT] and providing a custom `idle` task replacing the
+default [`nop()`][NOP] with [`wfi()`][WFI].
+
+[SLEEPONEXIT]: https://developer.arm.com/docs/100737/0100/power-management/sleep-mode/sleep-on-exit-bit
+[WFI]: https://developer.arm.com/documentation/dui0662/b/The-Cortex-M0--Instruction-Set/Miscellaneous-instructions/WFI
+[NOP]: https://developer.arm.com/documentation/dui0662/b/The-Cortex-M0--Instruction-Set/Miscellaneous-instructions/NOP
+
+``` rust
+{{#include ../../../../examples/idle-wfi.rs}}
+```
+
+``` console
+$ cargo run --target thumbv7m-none-eabi --example idle-wfi
+{{#include ../../../../ci/expected/idle-wfi.run}}
+```

book/ru/src/by-example/app.md

@@ -70,11 +70,6 @@ $ cargo run --example init
 `init`, `idle` будет запущена *с включенными прерываниями* и она не может вернуть результат,
 а значит должна работать вечно.
 
-Если функция `idle` не определена, среда вполнения устанавливает бит [SLEEPONEXIT], а затем
-отправляет микроконтроллер в сон после запуска `init`.
-
-[SLEEPONEXIT]: https://developer.arm.com/docs/100737/0100/Power-management/Sleep-mode/Sleep-on-exit-bit
-
 Как и в  `init`, `static mut` переменные будут трансформированы в `&'static mut` ссылки,
 безопасные для доступа. Обратите внимание, данная возможность может
 быть удалена в следующем релизе, см. `task_local` ресурсы.

ci/expected/idle-wfi.run

@@ -0,0 +1,2 @@
+init
+idle

examples/idle-wfi.rs

@@ -0,0 +1,47 @@
+//! examples/idle-wfi.rs
+
+#![deny(unsafe_code)]
+#![deny(warnings)]
+#![no_main]
+#![no_std]
+
+use panic_semihosting as _;
+
+#[rtic::app(device = lm3s6965)]
+mod app {
+    use cortex_m_semihosting::{debug, hprintln};
+
+    #[shared]
+    struct Shared {}
+
+    #[local]
+    struct Local {}
+
+    #[init]
+    fn init(mut cx: init::Context) -> (Shared, Local, init::Monotonics) {
+        hprintln!("init").unwrap();
+
+        // Set the ARM SLEEPONEXIT bit to go to sleep after handling interrupts
+        // See https://developer.arm.com/docs/100737/0100/power-management/sleep-mode/sleep-on-exit-bit
+        cx.core.SCB.set_sleepdeep();
+
+        (Shared {}, Local {}, init::Monotonics())
+    }
+
+    #[idle(local = [x: u32 = 0])]
+    fn idle(cx: idle::Context) -> ! {
+        // Locals in idle have lifetime 'static
+        let _x: &'static mut u32 = cx.local.x;
+
+        hprintln!("idle").unwrap();
+
+        debug::exit(debug::EXIT_SUCCESS); // Exit QEMU simulator
+
+        loop {
+            // Now Wait For Interrupt is used instead of a busy-wait loop
+            // to allow MCU to sleep between interrupts
+            // https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/WFI
+            rtic::export::wfi()
+        }
+    }
+}

macros/src/codegen/idle.rs

@@ -85,7 +85,7 @@ pub fn codegen(
             vec![],
             None,
             quote!(loop {
-                rtic::export::wfi()
+                rtic::export::nop()
             }),
         )
     }

macros/src/codegen/pre_init.rs

@@ -122,12 +122,5 @@ pub fn codegen(app: &App, analysis: &Analysis, extra: &Extra) -> Vec<TokenStream
             ));
         }
     }
-
-    // If there's no user `#[idle]` then optimize returning from interrupt handlers
-    if app.idle.is_none() {
-        // Set SLEEPONEXIT bit to enter sleep mode when returning from ISR
-        stmts.push(quote!(core.SCB.scr.modify(|r| r | 1 << 1);));
-    }
-
     stmts
 }

src/export.rs

@@ -6,6 +6,7 @@ use core::{
 pub use crate::tq::{NotReady, TimerQueue};
 pub use bare_metal::CriticalSection;
 pub use cortex_m::{
+    asm::nop,
     asm::wfi,
     interrupt,
     peripheral::{scb::SystemHandler, DWT, NVIC, SCB, SYST},