-
Notifications
You must be signed in to change notification settings - Fork 2k
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: misc cleanup and fixes #6344
Conversation
Needs rebase |
31d5f1a
to
0b7d5b6
Compare
rebased |
0b7d5b6
to
0fc8c66
Compare
reabased again. |
@kaspar030 mind to take a look at this? |
@kaspar030 ping?! Or @PeterKietzmann, @miri64, @kYc0o: would you guys mind to take a look?! |
I'm fine with the code, but I don't have enough insight to test this at the moment. |
* | ||
* pm_reboot() | ||
* pm_off() | ||
* This interface *must* be implemented for every platform in RIOT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this already the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, at least rudimentary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about pm_off()
but before, I haven't seen there is fall-back code.
@@ -1,2 +1,4 @@ | |||
# include module specific includes | |||
export INCLUDES += -I$(RIOTCPU)/cortexm_common/include | |||
|
|||
USEMODULE += cortexm_common_periph |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouln't be USEMODULE += pm_layered
included here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. The decision if the layered module should be used is up to a CPU. Some cortexm CPUs might want to use it (e.g. stms), but others might opt out (e.g. nrf5x)...
It looks OK for me too and Murdock agreed, so I'd give my ACK. |
Why Jenkins shows red? |
So, I'm fine with this PR but I'd prefer @kaspar030 to hit the button. In worst case I will do, this afternoon. |
Just restarted the Jenkins build. Looks fine now. |
|
||
void pm_reboot(void) | ||
{ | ||
/* force an hardware reboot ("Power-Up Clear"), by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok here, but in general, copy&pasting code doesn't remove copyright. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ups, this was not on purpose -> simply forgot to copy the old copyright into my template file... Sorry!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK.
Follow up on #6160
This PR contains some collected cleanup and fixes for the PM, mostly some code re-organization, doxygen fixes and removal of un-used modules/includes.