I am working on the pm (power management) module of RIOT. Can anyone
tell me why pm_reboot is (often) part of this module?
If no one votes against it, I woud like to move *pm_reboot()* to a
dedicated module, in order to have a reboot_fallback module as we have
planned for pm_fallback and additionally common CPUs can implement
specific reboot functions if required.
Is 'reboot' a good module name? Or should we name it pm_reboot_fallback
(prefix it with pm)?
Background: atmega_common defines a pm_reboot, but no power management
by default. Therefore using pm_fallback is not possible and we would
have to duplicate existing code (bad idea!).
After a short discussion offline we decided to keep it as it is. As the
nucleo board for example, supports saving reigsters across the reset.
Therefor when entering low power modes or making a reboot, it might be
required to perform some more stuff.
The pm_* functions will now all be combined in a single module to make
it easier to develop PM for other cpus.
Looks like it's not that easy. Many platforms define pm_reboot in the
board's file(s).
Additionally pm_layered does not define pm_reboot, the same applies for
pm_off (pm_off can be modeled as pm_set_lowest(); irq_disable();
while(1) in pm_layered I guess ?).
Therefore I will work on removing pm_reboot() from pm_fallback
implementation and create additional modules if needed (at some points
pm_reboot is defined outside of pm anyway).
Looks like it's not that easy. Many platforms define pm_reboot in the
board's file(s).
Only mips-malta has it's own "pm_reboot()" implementation. The other two
define stubs.
Additionally pm_layered does not define pm_reboot, the same applies for
pm_off (pm_off can be modeled as pm_set_lowest(); irq_disable();
while(1) in pm_layered I guess ?).
pm_layered does define pm_set_lowest() as weak exactly like that.
Therefore I will work on removing pm_reboot() from pm_fallback
implementation and create additional modules if needed (at some points
pm_reboot is defined outside of pm anyway).
When designing periph/pm, we intentionally moved reboot from a core
include into periph/pm, as it seemed to fit together with pm_off().
Do you have a WIP branch somewhere? While working on #7241, I had to
implement a lot of what we've discussed, in order to make anything
compile with sumbodulized periph. Maybe you can take a look? the
requirements have changed a little.
Only mips-malta has it's own "pm_reboot()" implementation. The other two
define stubs.
Might be, still, it has to be considered where to place it and how we
define fallbacks cleanly!
Additionally pm_layered does not define pm_reboot, the same applies for
pm_off (pm_off can be modeled as pm_set_lowest(); irq_disable();
while(1) in pm_layered I guess ?).
pm_layered does define pm_set_lowest() as weak exactly like that.
I am not using weak definitions. Because of the PR that claims that
definitions are not chosen correctly in RIOT!
Therefore I will work on removing pm_reboot() from pm_fallback
implementation and create additional modules if needed (at some points
pm_reboot is defined outside of pm anyway).
When designing periph/pm, we intentionally moved reboot from a core
include into periph/pm, as it seemed to fit together with pm_off().
Can't really see a reason why this belongs together IMO, but we can
leave it there for now.
Do you have a WIP branch somewhere? While working on #7241, I had to
implement a lot of what we've discussed, in order to make anything
compile with sumbodulized periph. Maybe you can take a look? the
requirements have changed a little.
I stopped working on this because of the pm_reboot problems. Therefore I
didnt create a WIP branch on my fork yet, as I wasn't able to setup a
working version.
Your PR does not compile for me, almost 50% of the boards in
examples/default/ are not compiling anymore.
At least the first few I checked are missing pm_reboot (undefined
reference).
I just looked in more detail on what you've done.
Using submodules is fine for me, but I would like to change a key aspect
here:
Make all pm_* implementations submodules, so the final CPU *always* has
to select the according pm implementation.
For cortexm_common this would allow us to get rid of all #ifdef stuff
which is really ugly!
Would that be fine? When do you plan to be finished with the submodules?
I would really like to finish the pm architecture and finally use it!
pm.h consists of pm_set_lowest, pm_off and pm_reboot.
pm_fallback therefore should define stubs for all of these [1]
cpu/cortexm_common defines some better implementation and also provides
cortexm_sleep, that's why I created pm_cortexm_common [2].
cpu/stm32_common provides pm_set, but only for some CPUs, but always
uses cortexm_sleep from cortexm_common - However I don't see a
dependency of stm32_common to periph_common? Does this mean, all boards
currenctly always use cortexm_common anyway, if they use stm32_common?
This made me create *pm_stm32_common_layered* [3], as this only provides
pm_set, so others can use pm_layered with this one.
Another cool thing is that cpu/saml21 / cpu/samd21 are just able to use
pm_layered and provide their own pm_set().
Q: Why is the pm_set guarded with ifdefs? Shouldn't all CPUs that use
stm32_common be able to use pm_layered with pm_set? The same applies for
all CPUs that are based on cortexm_common IMO.
I've finished my WIP branch and it can be viewed at [0]