Implement deep sleep in embassy-mspm0 by CordlessCoder · Pull Request #6580 · embassy-rs/embassy

CordlessCoder · 2026-07-22T14:27:46Z

I'm generally following the approach embassy-stm32 took where it makes sense - the testing is done on a custom MSPM0L1305 board since that's what I have access to.

I used an LLM to assist me in navigating the TRMs for all the MSPM0 product families, to review the code as I write it and for first drafts of some code and documentation.

CordlessCoder · 2026-07-22T14:30:17Z

I believe I fixed the race condition that caused the time driver to jump backwards when driven by LFCLK.

CordlessCoder · 2026-07-22T14:50:01Z

To reproduce the timer jump bug I used this snippet after adjusting TICK_HZ and switching the clock source from MFCLK to LFCLK.

let mut last = Instant::now();
loop {
    let n = Instant::now();
    defmt::assert!(n >= last, "time went backwards: {}us -> {}us", last.as_micros(), n.as_micros());
    last = n;
}

It reliably panicked after 2 seconds(a single overflow cycle).
After the changes I left it running for 15 minutes without getting a panic.

This was tested on the MSPM0L1305RHB, but I'm quite confident about the fix translating to other MCUs.

CordlessCoder · 2026-07-23T08:08:03Z

I'm measuring current consumption in sleep/standby that matches the datasheet on my MSPM0L1305, so the sleep modes work.
I'll start working on integrating sleep into the executor.

i509VCB · 2026-07-23T22:38:28Z

The assembly output of now() is quite a bit of code, I feel there is some room for optimization:

.section .text._embassy_time_now,"ax",%progbits
        .globl  _embassy_time_now
        .p2align        2
        .type   _embassy_time_now,%function
        .code   16
        .thumb_func
_embassy_time_now:
        .fnstart
        .cfi_startproc
        .save   {r4, r5, r7, lr}
        push {r4, r5, r7, lr}
        .cfi_def_cfa_offset 16
        .cfi_offset lr, -4
        .cfi_offset r7, -8
        .cfi_offset r5, -12
        .cfi_offset r4, -16
        .setfp  r7, sp, #8
        add r7, sp, #8
        .cfi_def_cfa r7, 8
        ldr r1, .LCPI141_0
        ldr r2, .LCPI141_1
        b .LBB141_2
.LBB141_1:
        @MEMBARRIER
        ldr r4, [r2, #16]
        cmp r0, r4
        beq .LBB141_7
.LBB141_2:
        ldr r0, [r2, #16]
        @MEMBARRIER
        ldr r3, [r1]
        uxth r4, r3
.LBB141_3:
        ldr r3, [r1]
        uxth r5, r3
        cmp r4, r5
        mov r3, r4
        beq .LBB141_1
        ldr r3, [r1]
        uxth r4, r3
        cmp r5, r4
        mov r3, r5
        beq .LBB141_1
        ldr r3, [r1]
        uxth r5, r3
        cmp r4, r5
        mov r3, r4
        beq .LBB141_1
        ldr r3, [r1]
        uxth r4, r3
        cmp r5, r4
        mov r3, r5
        bne .LBB141_3
        b .LBB141_1
.LBB141_7:
        movs r2, #1
        ands r2, r0
        lsrs r1, r3, #15
        cmp r2, r1
        bne .LBB141_9
        lsls r2, r1, #15
        eors r2, r3
        lsrs r3, r0, #17
        lsls r0, r0, #15
        movs r1, #0
        b .LBB141_11
.LBB141_9:
        lsrs r1, r0, #17
        lsls r0, r0, #15
        movs r4, #129
        lsls r4, r4, #1
        ldr r5, .LCPI141_2
        ldr r5, [r5]
        tst r5, r4
        beq .LBB141_12
        lsls r2, r2, #15
        eors r2, r3
        movs r3, #0
.LBB141_11:
        adds r0, r0, r2
        adcs r1, r3
.LBB141_12:
        pop {r4, r5, r7, pc}
        .p2align        2
.LCPI141_0:
        .long   1074288640
.LCPI141_1:
        .long   embassy_mspm0::time_driver::driver::DRIVER
.LCPI141_2:
        .long   1074286640

CordlessCoder · 2026-07-23T22:40:43Z

I suspect a lot of the bloat is the "torn read protection" loops - I'll double-check if TI's SDK actually does something similar. If it doesn't - they can be plain reads and that gets rid of the loops.

Edit: Removing them seems to break things :(

i509VCB · 2026-07-26T03:20:16Z

I would like to split the timer changes into a different pull request, along with the rhb packages.

i509VCB · 2026-07-26T03:23:52Z


+// Only TIMG0 and TIMG1 remain clocked in STANDBY, so they are the only timers that can wake the
+// core from deep sleep via the time driver. Reject a `low-power` build on any other timer.
+// TODO: Or maybe allow using them, but disable STANDBY? STOP0 still work.


I do think we should allow timers which cannot run in STANDBY by leaking an appropriate wake guard in time driver init.

Would also be worth documenting that specific timers for time driver use would not allow certain sleep modes.

i509VCB · 2026-07-26T06:15:09Z

Okay the timers were fixed in #6613

i509VCB · 2026-07-27T23:01:35Z

        let r = T::info().regs;
-        let state = T::state();

+        // Busy-poll ENC by self-waking; never let the executor deep-sleep while a software-triggered


This will be better in the future when we have more options for clock source for the ADC (ULPCLK or HFCLK).

What we should do is set up an appropriate wake guard when doing an async read if our clock source happens to be SYSOSC which needs to be kept alive.

Otherwise the system will never truly go to a lower sleep mode if possible because we will effectively be doing a yield_now in a loop.

i509VCB · 2026-07-27T23:41:43Z

-        });
+        let (rise, fall, mask) = want_masks(id);
+
+        // The status bit does not say which edge fired, so the level standing now is the evidence.


This is extra wording for no reason.

Something along the lines of "GPIO_ERR_01 requires that we detect level changes to determine if an edge event happened."

i509VCB · 2026-07-27T23:44:28Z

+        let fired = if level.dio(i as usize) { rise } else { fall };
+
+        if fired.load(Ordering::Relaxed) & mask != 0 {
+            // Withdrawing both requests is what tells the waiting future its edge arrived.


"Clear the mask for this pin since the edge arrived."

i509VCB · 2026-07-27T23:49:41Z

+            // The other edge, or a wait that has since been cancelled. Discard it and stay armed.
+            // Waking at all is what errata GPIO_ERR_01 case 2 needs - the detector has no clock in
+            // STANDBY1, so it only observes the release if the release is itself a wake event - and
+            // leaving the status bit set would re-enter here forever.


"Did not receive the correct event or no event is being waited. Per GPIO_ERR_01 clear this event otherwise entering STANDBY1 will mean the next wake up event is not detected."

i509VCB · 2026-07-27T23:54:02Z

+            // Notify the future that an edge event has occurred by masking the interrupt for this pin.
+            gpio.cpu_int().imask().modify(|w| {
+                w.set_dio(i as usize, false);
+            });


We don't read RIS anymore in the future?

i509VCB · 2026-07-27T23:57:25Z

+        if matches!(polarity, Polarity::RISE | Polarity::RISE_FALL) {
+            rise.fetch_or(mask, Ordering::Relaxed);
+        }
+        if matches!(polarity, Polarity::FALL | Polarity::RISE_FALL) {
+            fall.fetch_or(mask, Ordering::Relaxed);
+        }


I don't think these ors have strong enough ordering.

One thread could be reading and then another thread preempts, reads and writes causing us to lose one bit. I think this needs AcqRel.

I don't believe that's the case, AcqRel would only cause these to form happens-before relationships, which we don't care about because the order that the bits are set in doesn't matter, as long as they're set.

i509VCB · 2026-07-27T23:57:47Z

+        rise.fetch_and(!mask, Ordering::Relaxed);
+        fall.fetch_and(!mask, Ordering::Relaxed);


Same here, I think this needs to be AcqRel.

i509VCB · 2026-07-27T23:59:06Z

+                // the completion signal. Testing the request rather than the status bit also closes
+                // the window before the interrupt is first unmasked: an edge landing there is still
+                // held in the status bit, and unmasking below re-enters the handler to classify it.
+                if (rise.load(Ordering::Relaxed) | fall.load(Ordering::Relaxed)) & mask == 0 {


Note for ordering: Relaxed is fine here since the interrupt handler will preempt if needed (or we will wait).

i509VCB · 2026-07-28T00:00:22Z


+        // Armed before the stale events are cleared, so the write below also discards anything
+        // latched by the arming itself.
+        let _arm = EdgeArm::new(block, key, polarity);


I think we should use OnDrop from embassy-hal-internal here and do the setup and then have the drop closure do the deinit in here.

i509VCB · 2026-07-28T00:04:10Z

+//! Each family implements the SYSCTL power-mode sequence from the device TRM (chapter
+//! "System Control (SYSCTL)" -> "Operating Modes"), cross-checked against TI driverlib's
+//! `DL_SYSCTL_setPowerPolicy*`. The families are split into three behaviors:
+//! - `full.rs` — STOP0/1/2 + STANDBY0/1: all G families and the supported L families.
+//! - `c110x.rs` — STOP0/2 + STANDBY0/1 (no STOP1); STOP0 also clears `USELFCLK`: C-series.
+//! - `h321x.rs` — STOP0/2 + STANDBY0/1 (no STOP1): H321x.
+//!


There is no value here in my opinion top say we copied driverlib. I think this highlighted section is needed in the docs.

i509VCB · 2026-07-28T00:20:55Z

+    /// STOP1; rounded to STOP0 on families without it.
+    Stop1,


Should this even be a variant on chip families that don't support it?

But then each peripheral needs to care about what sleep modes are supported on what family, when all it actually cares about is "don't go into STOP1 or lower".

Retention has turned out to be a lot more complex, it'll be addressed in separate peripheral-specific PRs.

i509VCB · 2026-07-28T00:33:53Z

+
+    /// Use the bus clock (ULPCLK), which runs at the MCLK rate.
+    ///
+    /// Required to receive after a deep sleep. An asynchronous fast clock request forces the
+    /// MCLK/ULPCLK tree back to the SYSOSC base frequency, so a UART on this source recovers a usable
+    /// baud clock; LFCLK is not restored by the request and MFCLK only comes back if it was already
+    /// enabled. Pair with [`Config::low_power_rx_wake`].
+    ///
+    /// The baud divisors are sized for the full ULPCLK rate, so an idle or in-flight transfer needs
+    /// the chip awake — a receiver holds a sleep floor of `Stop0` unless `low_power_rx_wake` is set,
+    /// and a transmitter holds one for the duration of each flush.
+    BusClk,


This is power domain dependent which the HAL does not do yet. I don't see a good reason to add this right now.

This is the only clock source that UART can issue an async fast clock request from, so the only one that will work for a deep sleep wake.
I2C already exposes BusClk as a clock option, even though it also depends on power domains.

embassy/embassy-mspm0/src/i2c.rs

Line 28 in 2d98e96

BusClk,

Power domains have been implemented in mspm0-metapac and are waiting to be merged - power domain support will be implemented in peripheral specific PRs.

CordlessCoder · 2026-07-28T10:48:13Z

The core executor and sleep mode changes have been split off into #6646, this PR will probably be superseded by multiple smaller PRs implementing low-power support for individual peripherals.

chiptool now generates register values in PascalCase rather than SCREAMING_SNAKE, and emits the key/index fieldsets as newtypes carrying associated constants instead of enums. No functional change.

Which power domain a peripheral instance sits in decides whether it can work through deep sleep at all: PD0 is powered in RUN/SLEEP/STOP/STANDBY, PD1 only in RUN and SLEEP, and SYSCTL forces PD1 peripherals to a disabled state on STOP/STANDBY entry. Arming a wake source on a PD1 instance is therefore silently dead. The domain is a per-instance and per-chip property, not a property of the peripheral kind: UART3 is PD1 on G-series but PD0 on L122x/L222x, UART7 is PD0 while UART3-6 are PD1, and TIMA0 differs between families. It has to come from the chip metadata.

CordlessCoder · 2026-07-29T13:53:01Z

It doesn't make much sense to keep this PR around since I've decided to split all of the changes off into smaller PRs.

CordlessCoder changed the title ~~Implement deel sleep in embassy-mspm0~~ Implement deep sleep in embassy-mspm0 Jul 22, 2026

i509VCB reviewed Jul 23, 2026

View reviewed changes

Comment thread embassy-mspm0/src/time_driver/tim.rs Outdated

i509VCB reviewed Jul 26, 2026

View reviewed changes

Comment thread embassy-mspm0/src/time_driver/tim.rs Outdated

CordlessCoder force-pushed the mspm0-low-power branch from 477f25c to 49ef45b Compare July 26, 2026 23:16

bugadani added the e-mspm0 Issues for the MSPM0 family of chips label Jul 27, 2026