Delay should sleep the CPU

[jonathanpallant]

Rather than busy-wait the CPU, the delay implementation should set the systick to generate an interrupt on overflow, and sleep with a wfi. This would reduce power consumption.

You might need to put it behind a flag because you'd have to replace DefaultHandler_ for systick with something that didn't loop forever...

[skibon02]

SysTick can not be an option because user can use it, and overriding duration and systick registers may bring unexpected results to user's program
And the most basic usage of SysTick which is executing handler once in N cycles is expected to continue executing while waiting for delay.

[jonathanpallant]

That's not possible because the cortex_m::Delay struct takes ownership of the SYSTICK singleton, and the Delay::delay_us method already changes all the registers in order to perform the sleep. The only thing we'd need to change is setting the NVIC bits to enable interrupt on Systick overflow, and I think that can be done atomically without needing a &mut cortex_m::NVIC (or even &cortex_m::NVIC), so the API to the Delay object doesn't even need to change.

[skibon02]

Oh, i see
I didn't know we have SysTick based delay already
Using sleep mode is a good point

[jannic]

It's not really needed to replace the default handler, because you can avoid actually calling it by disabling interrupts. (WFI still works.)
But that would add quite some code:

        // Disable interrupts globally, so the systick interrupt handler would not
        // be called. (We don't know what it does!)
        let primask = crate::register::primask::read();
        crate::interrupt::disable();
        let full_cycles = ticks >> 24;
        if full_cycles > 0 {
            self.syst.set_reload(0xffffff);
            self.syst.clear_current();
            // Enable systick interrupt generation.
            self.syst.enable_interrupt();
            self.syst.enable_counter();

            for _ in 0..full_cycles {
                let mut has_wrapped = false;
                while !has_wrapped {
                    // Wait for interrupt - SysTick or other
                    crate::asm::wfi();
                    // If interrupts were enabled before, we want them to run during the delay
                    if primask.is_active() {
                        // ...but not the systick interrupt itself, so disable it first
                        // We can't call `self.syst.disable_interrupt();` because that would overwrite the pending flag
                        let csr = self.syst.csr.read();
                        unsafe { self.syst.csr.write( csr & !SYST_CSR_TICKINT) };
                        // Clear an already pending systick interrupt, if any
                        crate::peripheral::SCB::clear_pendst();
                        // Shortly enable interrupts to allow any pending one to run
                        unsafe { crate::interrupt::enable() };
                        crate::interrupt::disable();
                        // Reenable systick interrupt
                        unsafe { self.syst.csr.write( csr | SYST_CSR_TICKINT) };
                        // Recover wrapped flag from previous read of CSR
                        has_wrapped = csr | SYST_CSR_COUNTFLAG != 0;
                    } else {
                        has_wrapped = self.syst.has_wrapped();
                    }
                }
            }
        }

[Cleanup like re-enabling interrupts follows later]

[jonathanpallant]

So this is neat, but I think I'd maybe rather see a more fully-featured SysTick driver in cortex-m - one that handles roll-over using an exception handler. It could maybe also have an optional user-supplied handler which can be plugged in (some static AtomicPtr<fn()> which defaults to null but can be set to point to some function and which is called on systick overflow. I'd probably put it behind a feature so we don't waste the .data space for the atomic overflow value if we don't need to. Perhaps we could think of a way to control the number of overflows per second (maybe 100 Hz as a default) at compile time.

If the user has a custom systick interrupt handler, I trust them to know that the systick delay is a busy-wait and that they should do something better using the interrupt they have already setup.