cpu: PM_NUM_MODES must only count non-IDLE modes

[benpicco]

Contribution description

From the documentation:

• CPUs define up to 4 power modes (from zero, the lowest power mode,
  to PM_NUM_MODES-1, the highest)
• there is an implicit extra idle mode (which has the number PM_NUM_MODES)

I must say that an 'implicit extra mode' is a very surprising concept, but most implementations did get it right that there are in fact PM_NUM_MODES + 1 modes.

This fixes the ones that didn't.

Testing procedure

Run tests/periph_pm and observe the power consumption

I tested the sam0 CPUs and manually checked the code of the other implementations.

Issues/PRs references

I noticed that saml21 would not wake up from STANDBY (mode 1) in tests/periph_pm.
It would briefly wake up, but immediately go back to STANDBY.

It turned out that pm_layered would always call pm_set(3), which is out of range.
As a result, the SLEEPMODE register would never be written and the previous value (STANDBY) would be used.

Another side effect was that the CPU never entered idle mode.
With this patch, IDLE consumption drops from 750µA to 368µA.

[roberthartung]

Thanks for the fix! Will have a look at it, even though I don't have many boards if any available!

[kaspar030]

I must say that an 'implicit extra mode' is a very surprising concept

Yeah, the reasoning was along the lines that a) usually there's no benefit in blocking idle, b) this allows one more blockable mode, c) there's always a mode to go to.

[kaspar030]

It turned out that pm_layered would always call pm_set(3), which is out of range.

Is there a way to prevent this in the future? Drop the extra mode, require implementations to actually count it, assert in pm_set_lowest() that the chosen mode is in range?

[benpicco]

Is there a way to prevent this in the future?

s/PM_NUM_MODES/PM_NUM_SLEEP_MODES/g
(But then again samd2x has 3 IDLE modes that differ in wake-up time)

[benpicco]

Drop the extra mode, require implementations to actually count it

Probably the better solution - In the first version of this PR I did that and added

#ifndef PM_BLOCKER_INITIAL
#if PM_NUM_MODES == 4
#define PM_BLOCKER_INITIAL 0x01010101   /* block all but IDLE by default */
#elif PM_NUM_MODES == 3
#define PM_BLOCKER_INITIAL 0x00010101   /* block all but IDLE by default */
#elif PM_NUM_MODES == 2
#define PM_BLOCKER_INITIAL 0x00000101   /* block all but IDLE by default */
#elif PM_NUM_MODES == 1
#define PM_BLOCKER_INITIAL 0x00000000   /* block all but IDLE by default */
#endif
#endif

because I thought the other implementations got it wrong and forgot to include the IDLE mode in PM_NUM_MODES.

Then I read the documentation again, cursed, and changed it back.

[jue89]

FYI: SAMR30 looks fine with this patch

[benpicco]

Should I split this so we can get at least the changes for sam0 in?

[kaspar030]

(But then again samd2x has 3 IDLE modes that differ in wake-up time)

IMO that should be a compile time choice. Who wants to actually use multiple IDLE modes in a single application?