Zephyr: sys_pm: fragility in residency policy

[pabigot]

The residency policy for system power management captures a list of defined residency durations in the order of SLEEP levels 1, 2, and 3 followed by DEEP_SLEEP levels 1, 2, and 3.

The policy then looks for deepest level for which a requested timeout meets the minimum requirements.

There are a couple problems with the design and implementation:

• The algorithm assumes that the residency durations are monotonically increasing, but nothing verifies it;
• The table includes a complex nesting of preprocessor directives to produce a list with only defined elements, and replicates these directives when defining available sleep modes, creating a potential for inconsistency;
• When a sleep state is enabled that requires an external signal to wake (e.g. deep sleep) the policy may inadvertently result in an unbounded sleep acceptable because enabling CONFIG_SYS_POWER_DEEP_SLEEP_STATES is opt-in, see https://github.com/zephyrproject-rtos/zephyr/pull/22084#issuecomment-578498729

[pabigot]

A potential resolution to part of this would be to remove the deep sleep states (which include power gating) from the residency policy. Note that also there are legitimate uses of K_FOREVER sleeps that are expected to be woken due to external events not monitored by poll, so IMO a "forever timeout" should never power-off the system.

[pabigot]

Tagged as has-pr because if #20950 gets merged this can be reduced to low priority (but shouldn't be closed).

[dleach02]

per discussion in release meeting #20950 has been approved and merged and the priority of this issue has been set to low for now

[wentongwu]

A potential resolution to part of this would be to remove the deep sleep states (which include power gating) from the residency policy. Note that also there are legitimate uses of K_FOREVER sleeps that are expected to be woken due to external events not monitored by poll, so IMO a "forever timeout" should never power-off the system.

deep sleep doesn't power-off whole system, at least some circuit(like PMU in our system) and some always on RAM(which is to save system state, like ARM's registers) is alive help to power on CPU, most of the devices, and power offed RAM again.

And I don't think removing deep sleep from residency policy is good, if your system doesn't support deep sleep, just configure CONFIG_SYS_POWER_DEEP_SLEEP_STATES=n.

[pabigot]

Nordic does support deep sleep, but it can be woken only by external signal from certain triggers: logic level change, analog level change, NFC, USB voltage. It does not support wakeup from a timeout.

Since deep sleep as currently defined for Zephyr does not require that it be possible to wake from a timeout it should be excluded from policies that select sleep state based solely on time until next activity.

[wentongwu]

Nordic does support deep sleep, but it can be woken only by external signal from certain triggers: logic level change, analog level change, NFC, USB voltage. It does not support wakeup from a timeout.

Since deep sleep as currently defined for Zephyr does not require that it be possible to wake from a timeout it should be excluded from policies that select sleep state based solely on time until next activity.

does that mean timer will not work well in deep sleep of that platform?

[pabigot]

The timer does not work at all in deep sleep on that platform. Uptime restarts from zero when the system is woken.

[wentongwu]

I'm not familiar with your platform, but if end users need timer how you handle it?

[mnkp]

@wentongwu it would be good to keep working on your #18007 to clarify and fix all those issues.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[pabigot]

I suspect the residency policy will be updated in the context of new work that eliminates the hard-coded states, which should settle this, but for now I think it should be left open.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.