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.

Sign up for GitHub

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

Jump to bottom

fe310: fix power management configuration #13229

Merged
merged 1 commit into from
Jan 30, 2020
Merged

fe310: fix power management configuration #13229

merged 1 commit into from
Jan 30, 2020

Conversation

nmeum
Copy link
Member

@nmeum nmeum commented Jan 29, 2020

Contribution description

This issue was discovered while implementing link time reordering in #13176. The FE310 provides its own implementation of pm_off and pm_set_lowest. Without this change the common implementation of these functions would be compiled as well and the linker would (in the default order) disable on of the two. This is why it doesn't result on a build failure without #13176, the change proposed here makes sure that the FE310 version of these functions is used.

Testing procedure

Either apply #13176 or just read the code, i.e. verify that a custom implementation of these functions is provided in cpu/fe310/periph/pm.c.

Issues/PRs references

#13176

@nmeum nmeum mentioned this pull request Jan 29, 2020
Merged
aabadie
aabadie reviewed Jan 29, 2020
View reviewed changes
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.

I'm not sure if PROVIDES_PM_OFF should be defined for this CPU since the existing implementation of pm_off is empty.

cpu/fe310/include/periph_cpu.h Outdated Show resolved Hide resolved
@aabadie aabadie added Area: cpu Area: CPU/MCU ports Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Area: pm Area: (Low) power management labels Jan 29, 2020
aabadie
aabadie approved these changes Jan 30, 2020
View reviewed changes
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

@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 Jan 30, 2020
@nmeum nmeum mentioned this pull request Jan 30, 2020
Closed
@aabadie aabadie merged commit 596d023 into RIOT-OS:master Jan 30, 2020
@leandrolanzieri leandrolanzieri added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 20, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabadie aabadie aabadie approved these changes

Assignees
No one assigned
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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2020.04
Development

Successfully merging this pull request may close these issues.
3 participants
@nmeum @aabadie @leandrolanzieri