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: optimize stop mode for stm32f* #11758

Merged
merged 2 commits into from
Jul 4, 2019

Conversation

fjmolinas
Copy link
Contributor

Contribution description

This PR is based on previous work in #11359, where STM32L1 has been cut from the PR.

On start-up it configures all GPIO's as AIN. On other boards of the STM32 family (L0 & L4) this is done by default. There for this is only done for STM32F0-7~.

As stated in this AN, doing this saves the consumption of the input Schmitt trigger. The only case where this shouldn't be done is when the pin is connected to an external driver that has a pull-up or pull-down setting, this should be handled by pertinent drivers.

Testing procedure

Supply voltage for all the below measurement was 3.3V. Al pin headers where disconnected where disconnected, supply was measured directly threw the IDD pin either with a multi meter or NUCLEO-LPM01A power measurement extension.

As of know when entering STOP mode consumption on stm32f7 is around 1.24mA, with this PR it drops to 350uA (270uA typ f746zg). To test run:

make BOARD=nucleo-f746zg -C tests/periph_pm/ flash

As of know when entering STOP mode consumption on stm32f4 is around 500uA, with this PR it drops to 120uA (datasheet 120uA typ for f446re). To test run:

make BOARD=nucleo-f446re -C tests/periph_pm/ flash

As of know when entering STOP mode consumption on stm32f3 is around 1mA, with this PR it drops to 15.3uA (datasheet 7.4uA typ for f303re). To test run:

make BOARD=nucleo-f303re -C tests/periph_pm/ flash

As of know when entering STOP mode consumption on stm32f2 is around 1.27mA, with this PR it drops to 200uA (datasheet 200uA typ for f207zg). To test run:

make BOARD=nucleo-f207zg -C tests/periph_pm/ flash

As of know when entering STOP mode consumption on stm32f1 is around 100uA, with this PR it drops to 11uA (13.5uA typ f103rb). Tested on nucleo-f103rb:

make BOARD=nucleo-f103rb -C tests/periph_pm/ flash

As of know when entering STOP mode consumption on stm32f0 is around 507uA, with this PR it drops to 10.7uA (3.5uA typ f072rb). To test run:

make BOARD=nucleo-f072rb -C tests/periph_pm/ flash

NOTE: on nucleo boards if power is measured directly after programming there will be around 200uA of current consumption coming from the stlink. The stlink must start without being connected to the board or power the board externally.

Issues/PRs references

Based on work from #8403 & #10052. Taken from #11359.

@fjmolinas fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: pm Area: (Low) power management labels Jun 28, 2019
@fjmolinas fjmolinas requested a review from aabadie June 28, 2019 08:33
@aabadie
Copy link
Contributor

aabadie commented Jun 28, 2019

I tested this PR on all related platforms: F0, F1, F2, F3, F4 and F7. I confirm the power consumption measures reported.

@aabadie aabadie changed the title pr: optimize stop mode for stm32f* cpu/stm32: optimize stop mode for stm32f* Jun 28, 2019
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Codewise the changes looks good but I'd like to have someone else's opinion on that. Maybe @kaspar030 ?

@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jul 1, 2019
@fjmolinas
Copy link
Contributor Author

@aabadie I wanted to perform a dist/tools/compile_and_test_for_board/compile_and_test_for_board.py for all tests to verify that this would not affect any test. I tested on nucleo-f303ze with no issues, so I don't have any reason for staling anymore.

@aabadie
Copy link
Contributor

aabadie commented Jul 1, 2019

Murdock raised an issue with the current design: adding gpio_pm_init_ain in periph/gpio.c introduces an implicit dependency on periph_gpio feature.

I think this function should be moved to cpu_init and be static inline. Then there's no need to provide a new function in the public API.
Does it make sense to you @fjmolinas ?

@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jul 2, 2019
@fjmolinas
Copy link
Contributor Author

I think this function should be moved to cpu_init and be static inline. Then there's no need to provide a new function in the public API.

Its going to bloat cpu_init code. Anyway I did as you suggested, I'll need to rebase and re-write the commit history and messaged to properly reflect this.

@aabadie
Copy link
Contributor

aabadie commented Jul 2, 2019

Anyway I did as you suggested, I'll need to rebase and re-write the commit history and messaged to properly reflect this.

Thanks, I'll do another round of review ASAP.

*
* @see https://comm.eefocus.com/media/download/index/id-1013834
*/
void _gpio_init_ain(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be made static inline

Copy link
Contributor

Choose a reason for hiding this comment

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

only static should suffice

@kaspar030
Copy link
Contributor

IMO readablility can be improved by a) seperating the two switch cases, and b) turning the "DISABLE_JTAG" test into a C-level if statement:

https://gist.github.com/4fd5be0cfdaeede87c3b87b7d732887f

@fjmolinas
Copy link
Contributor Author

IMO readablility can be improved by a) seperating the two switch cases, and b) turning the "DISABLE_JTAG" test into a C-level if statement:

Thanks for the suggestion, I was thinking the same thing. I applied the diff.

Also I removed the break a because I realized for some cpu's GPIO's are not in a sequential order so I need to check all ports and not break immediately. e.g. "stm32f410"

#define GPIOA_BASE            (AHB1PERIPH_BASE + 0x0000U)
#define GPIOB_BASE            (AHB1PERIPH_BASE + 0x0400U)
#define GPIOC_BASE            (AHB1PERIPH_BASE + 0x0800U)
#define GPIOH_BASE            (AHB1PERIPH_BASE + 0x1C00U)

@fjmolinas
Copy link
Contributor Author

@aabadie are you ok with the changes? Should I squash?

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK, please squash!

fjmolinas added 2 commits July 3, 2019 16:50
- With this PR, On start up all GPIOs are configured as AIN. For stm32l0/4
  this is done by default. Doing this saves the consumption of the input Schmitt
  trigger in STOP mode which can reduce the consumption in at least 70%
  from current master.
- With this PR all GPIOs are set as AIN on start up.
@fjmolinas fjmolinas force-pushed the pr_optimize_pm_stm32f branch from a072670 to 1b7a861 Compare July 3, 2019 14:50
@aabadie
Copy link
Contributor

aabadie commented Jul 4, 2019

Let's go with this one :)

@aabadie aabadie merged commit 59933d2 into RIOT-OS:master Jul 4, 2019
@fjmolinas fjmolinas deleted the pr_optimize_pm_stm32f branch August 7, 2019 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 CI: run tests If set, CI server will run tests on hardware for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants