Is there an expected SoC sequence required by power subsystem to obtain best exit latency?

[albertofloyd]

Currently MCHP SoC supports 2 power states. idle and suspend_to_ram

During suspend_to_ram state entry follwing sequence is performed.

1. Mask all exceptions and interrupts except NMI and HardFault
   __set_PRIMASK(1);

2. Save context for peripheral

3. SoC-specific enable sleep sequence

4. Unmask all interrupts in BASEPRI
   __set_BASEPRI(0);

5. Wait for interrupt

6. SoC-specific disable sleep sequence

7. Restore peripheral context

8. Unmask interrupts

__set_PRIMASK(0);

Now app implement hooks state entry hook is observed that is executed ~100 miliseconds after SoC low power mode exited.
Most of time this time is spent handling _ISRs

Questions:

• Is correct/expected from Zephyr perspective to unmask interrupts at step 8) event though they are eventually re-enabled inside SoC-specific pm_power_state_exit_post_ops ?

• Is there any documentation that indicates how the PM subsystem hooks are to be implemented for SoC-specific sequences?

[albertofloyd]

Adding people to discussion @ceolin @scottwcpg

[ceolin]

Hi @albertofloyd, my understanding is that step 8 should happen in pm_power_state_exit_post_ops. The reason is that the kernel will iterate over devices waking up them only after ``` returns. Though (even if you restore the interruptions in that point), it is guaranteed that the idle thread will not be scheduled out before wake up devices and call pm_power_state_exit_post_ops.

So, in resume I think step 8 should be done only in pm_power_state_exit_post_ops.

One more thing, I saw the following comment in pm_power_state_set on step 8:

/*
 * pm_power_state_exit_post_ops() is not being called
 * after exiting deep sleep, so need to unmask exceptions
 * and interrupts here.
 */

This is much likely wrong (otherwise we have a bug) and I think the reason for this comment is that in the past the scheduler was not locked and the idle thread could be scheduled out before call pm_power_state_exit_post_ops. That is not longer the case.

[albertofloyd]

Thanks @ceolin would the behavior (interrupt unmasking) be altered in any way if DEVICE_POWER_MANAGEMENT is not supported by SoC/board?

Dive into history, seems this code was added due the mentioned schedule problematic sequence.
b935903#diff-b440df135fcada933522797c6b7d94666c062f7cc002d8a2c4f5442c08f4491a

Since this kernel problem is resolved as indicated maybe is time to cleanup @scottwcpg as you express the desire.

[ceolin]

Hi @albertofloyd, device power management should not interfere in the resume steps, it can change the suspend though. As far as I can see, __set_PRIMASK(0); should be called from pm_power_state_exit_post_ops

[albertofloyd]

Thanks @ceolin

[albertofloyd]

@scottwcpg any additional clarifications required in entry/exit suspend-to-ram sleep state (aka deep sleep) ?

[scottwcpg]

Since the SoC code has set BASEPRI=0 when you execute __set_PRIMASK(0) the pended ISR's will run. If you want to prevent ISR's from running until irq_unlock(saved_base_pri) is executed then you need to do a save/restore of PRIMASK in the SoC layer:
primask = __getPRIMASK(); basepri = __get_BASEPRI();
__set_PRIMASK(1);
save and prepare for deep sleep
__set_BASEPRI(0); WFI;
PCR_REGS->SYS_SLP_CTRL = 0; // Set SLEEP_ALL = 0 to clear all internal SLP_EN's
__set_BASEPRI(basepri); // restore base priority to value Zephyr PM code set
// instruction/memory barriers
__set_PRIMASK(primask) // restore primary mask
restore peripherals
return from SoC code
...
Zephyr calls irq_unlock(zephyr_saved_base_pri); // Allow CPU to vector into pended interrupts