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/atxmega: Add periph power management #16212

Merged
merged 2 commits into from
Apr 7, 2021
Merged

Conversation

nandojve
Copy link
Contributor

@nandojve nandojve commented Mar 21, 2021

Contribution description

The current xmega don't have a way to disable peripherals that are not in used. Add peripheral management to allow enable only the mcu blocks that will be used by application. This saves power on active and sleep modes. By default, at clock initialization, all peripherals are now disabled and each drive must activate at initialization phase.

The periph_timer and periph_uart were updated with this new feature.

Testing procedure

Tests were conducted with atxmega128-a1u-xpro board in:

  • tests/periph_pm
  • tests/periph_timer_short_relative_set

Issues/PRs references

This is part of #15703.

CC: @maribu @benpicco

@jeandudey jeandudey added Area: cpu Area: CPU/MCU ports Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Mar 22, 2021
@jeandudey jeandudey added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 2, 2021
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

lgtm. I have one suggestion regarding style, but that is opinionated, so feel free to ignore. If you take the suggestion, squash right away.

Let me know when I should start testing. Sorry for the long stall.

Comment on lines 113 to 127
void pm_periph_enable(pwr_reduction_t pwr, bool enable)
{
uint8_t mask = _device_mask(pwr);
uint8_t *reg = _register_addr(pwr);

if (enable) {
*reg &= ~mask; /* Clear to Enable */
}
else {
*reg |= mask;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I have the feeling that the function call overhead here is greater than the implementation. So if this would be provided as static inline instead, I think ROM would be reduced.

If that is the case, would you mind to split this into pm_periph_enable() and pm_periph_disable(), as this would make reading the code much easier.

If not, maybe you could name the C function to e.g. pm_periph_set_enabled() and provide a static inline pm_periph_enable() and static inline pm_periph_disable() as wrapper? The compiler will optimize the wrappers out, so this would come with no cost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I splitted the method as suggested. I built with/without static and noted that flash savings were higher using the method as not static. Because of that I preferred left two non static methods.

nandojve added 2 commits April 2, 2021 14:24
The current xmega don't have a way to disable peripherals that are
not in used.  Add peripheral management to allow enable only the mcu
blocks that will be used by application.  This saves power on active
and sleep modes.  By default, at clock initialization, all peripherals
are now disabled and each drive must activate at initialization phase.
The periph_timer and periph_uart were updated with this new feature.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
The periph_pm requires new field to control the power management
feature.  Add missing config at periph_conf for timers and uart.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@nandojve nandojve requested a review from maribu April 2, 2021 17:32
@maribu maribu added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Apr 3, 2021
@maribu
Copy link
Member

maribu commented Apr 7, 2021

Testing with tests/periph_pm on an atxemga-a1u-xpro:

master:

active: 0.178 A
sleep: 0.156 A

PR:

active: 0.164 A
sleep: 0.156 A

@maribu maribu added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Apr 7, 2021
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. Thanks

@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards 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 labels Apr 7, 2021
@maribu maribu merged commit ac774f3 into RIOT-OS:master Apr 7, 2021
@nandojve nandojve deleted the xmega_pm branch April 7, 2021 23:01
@kaspar030 kaspar030 added this to the Release 2021.04 milestone Apr 23, 2021
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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines 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.

4 participants