Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/stm32: add STOP and STANDBY low-power for stm32f3, unify for all stm32 #11211

Merged
merged 7 commits into from
Mar 23, 2019

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Mar 19, 2019

Contribution description

This PR adds STOP and STANDBY low-power modes to stm32f3 and since it's the last one, it also removes STM32 specific defines.

Testing procedure

Build and flash tests/periph_pm on nucleo-f303re and verify it works.

Issues/PRs references

Built on top of #9521, #11173 and #11178.

@aabadie aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pm Area: (Low) power management Area: cpu Area: CPU/MCU ports labels Mar 19, 2019
@aabadie aabadie requested a review from vincent-d March 19, 2019 16:57
@vincent-d
Copy link
Member

Needs rebase ;)

@aabadie
Copy link
Contributor Author

aabadie commented Mar 20, 2019

rebased (but waiting for #11173 and #stm32f7).

Maybe this one can be merged directly without waiting for the 2 other PRs. What do you think @vincent-d ?

@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 20, 2019
Copy link
Member

@vincent-d vincent-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-wise this is OK, maybe the SRAM2 thing could be made configurable.

I don't have time to test it, sorry.

I'm OK with one PR for all, this is small enough.

cpu/stm32_common/periph/pm.c Show resolved Hide resolved
@aabadie
Copy link
Contributor Author

aabadie commented Mar 21, 2019

maybe the SRAM2 thing could be made configurable.

Done here: 94562f7#diff-44501824d2f98f5e4d5ee081b01fd4f0R77

Hope this is ok for you.

Copy link
Member

@vincent-d vincent-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK for me. Untested ACK.

@vincent-d
Copy link
Member

@aabadie feel free to merge

@aabadie
Copy link
Contributor Author

aabadie commented Mar 21, 2019

@aabadie feel free to merge

Thanks @vincent-d ! I'd like to have a last round of testing. Maybe @fjmolinas can find some time to give this a last try. Otherwise, it won't be before next week.

@fjmolinas
Copy link
Contributor

fjmolinas commented Mar 21, 2019

@aabadie do you want a round of testing on all boards, I only have f4, l0, l1, l4. But not f3 with is the subject of the PR.

PROBLEMS FOUND:

L0:

  • Master: when reseting after STANDBY STOPMODE consumes 3uA
  • This Branch: when reseting after STANDBY STOPMODE consumes 30uA

L1:

  • Master: STANDBY consumes 3uA
  • This Branch consume on STANDBY 0.9 mA

F4 and L4 present no problem

@aabadie
Copy link
Contributor Author

aabadie commented Mar 21, 2019

PROBLEMS FOUND

Thanks for testing @fjmolinas. I see nothing in the code changes of this PR that could explain such a difference, implementations for L0 and L1 are the same. Is your local master up-to-date with upstream ? (I'm wondering if another STM32 pm related PR could have introduced it, like #9521 for example)

@fjmolinas
Copy link
Contributor

fjmolinas commented Mar 21, 2019

@aabadie you are right, I was working with an old master branch. Problems where introduced in #9521. To fix when seting PM_STOP_CONFIG for L0 and L1 do:

#define PM_STOP_CONFIG (PWR_CR_LPSDSR | PWR_CR_ULP | PWR_CR_CWUF)

WUF must be cleared before entering stop mode, and it is cleared by writing PWR_CR_CWUF. It wasn't that explicit in the datasheet since it said that WUF must be 0 but didn't explicitly talk about CWUF.

Maybe add a comment before the configs definition, it should be clear what every flag is doing.

@aabadie
Copy link
Contributor Author

aabadie commented Mar 21, 2019

Thanks for testing and providing the fix @fjmolinas ! I pushed one last commit with it. If the comment is fine for you, I guess we can go with this one.

@fjmolinas
Copy link
Contributor

@aabadie yep! go!

@aabadie aabadie merged commit 10b783d into RIOT-OS:master Mar 23, 2019
@danpetry danpetry added this to the Release 2019.04 milestone Mar 26, 2019
@aabadie aabadie deleted the cpu_stm32f3_cpu branch July 4, 2019 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: pm Area: (Low) power management CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants