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

pm: fix pm configuration #7308

Closed
wants to merge 1 commit into from

Conversation

Hyungsin
Copy link

@Hyungsin Hyungsin commented Jul 4, 2017

This PR fixes PM configuration.

PM functions in periph_common should not be executed if FEATURE_PERIPH_PM is used. PM_LAYERED will be used instead.

@Hyungsin Hyungsin force-pushed the forupstream_pm_blocker branch from b7bb9ae to b6b1aed Compare July 4, 2017 23:14
@Hyungsin Hyungsin changed the title pm: flexible pm_blocker_initial pm: fix pm configuration Jul 4, 2017
@Hyungsin Hyungsin force-pushed the forupstream_pm_blocker branch from b6b1aed to 2f97cf3 Compare July 5, 2017 01:09
@aabadie aabadie added 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 labels Jul 12, 2017
@aabadie aabadie added this to the Release 2017.10 milestone Jul 12, 2017
@haukepetersen
Copy link
Contributor

seems to me that this PR is defeating its purpose, those functions are defined weak as to actually be overridden by other implementation if present... Could you elaborate a little more on you actual problem?

@vincent-d
Copy link
Member

those functions are defined weak as to actually be overridden by other implementation if present

While working on #7351 I saw that it was not working, but I don't know why.

@vincent-d
Copy link
Member

Actually, this is the same as #6709

@Hyungsin
Copy link
Author

Hyungsin commented Jul 13, 2017

@haukepetersen, yeah the problem is that it does not work as expected.
That weak function is actually called when using pm_layer! FYI, my platform is SAMR21.
This problem has been reported multiple times, such as #6709 and #6655, as @vincent-d mentioned.

The fundamental solution would be making this function overridden. But for now, using #ifdef seems to be the simplest way of getting around this problem, at least temporarily.

@jnohlgard
Copy link
Member

While working on Kinetis pm I noticed that adding USEMODULE += pm_layered has no effect unless pm_block/pm_unblock is used at least once in the drivers (periph). The linker doesn't discard the weak pm_set_lowest unless there is something else required from the file containing the strong pm_set_lowest.

@vincent-d
Copy link
Member

The linker doesn't discard the weak pm_set_lowest unless there is something else required from the file containing the strong pm_set_lowest.

The strong symbol is also used is the object file (.a) of pm_layered is linked before periph_common. But our current build system pass them alphabetically, so periph_common is used before pm_layered.

@Hyungsin
Copy link
Author

Hyungsin commented Aug 16, 2017

So, is there any problem if this PR is applied?
I think it is possible to use this PR before having a complete solution, if it has nothing wrong.

@Hyungsin
Copy link
Author

Hyungsin commented Sep 5, 2017

@haukepetersen, @kaspar030, any feedback?

@kaspar030
Copy link
Contributor

This fix is included in #7241.

@Hyungsin
Copy link
Author

@kaspar030, Cool, waiting for your job finished!

@roberthartung
Copy link
Member

This will be superseded by #7597

@kaspar030
Copy link
Contributor

Closed in favor of #7726.

@kaspar030 kaspar030 closed this Oct 28, 2017
@Hyungsin Hyungsin deleted the forupstream_pm_blocker branch April 23, 2018 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

7 participants