Introduce CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE #93720
Conversation
bjarki-andreasen force-pushed the nrf-pm-device-runtime-align branch 4 times, most recently from 0352a64 to 2f14d77 Compare bjarki-andreasen changed the title bjarki-andreasen changed the title CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE 
zephyrbot added area: Samples zephyrbot requested review from JordanYates, anangl, ceolin, dcpleung, decsny, jaz1-nordic, kartben, kl-cruz, magp-nordic, masz-nordic, nashif, nika-nordic, tbursztyka, teburd and tmleman 
| I'm not against this, but what is the reasoning behind this
I've never had a situation where only enabling PM on some devices was problematic. |
The reasoning is the 100+ redundant occurrences of |
bjarki-andreasen force-pushed the nrf-pm-device-runtime-align branch from aa44db5 to d79e741 Compare 
| ping @ceolin |
bjarki-andreasen dismissed stale reviews from decsny and nordic-krch via c331965 bjarki-andreasen force-pushed the nrf-pm-device-runtime-align branch from d79e741 to c331965 Compare | I dropped the last commit with the 100+ removals of |
tmleman left a comment
tmleman left a comment There was a problem hiding this comment.
It can always be done later, but it would be worth updating the function documentation in zephyr/pm/device_runtime.h.
Now it won't automatically enable device runtime based solely on what's defined in the devicetree.
| if (IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME) && | ||
| atomic_test_bit(&pm->flags, PM_DEVICE_FLAG_RUNTIME_AUTO)) { | ||
| if (IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE) || | ||
| (IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME) && | ||
| atomic_test_bit(&pm->flags, PM_DEVICE_FLAG_RUNTIME_AUTO))) { |
There was a problem hiding this comment.
Shouldn't it be something more like:
if (IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME) && (IS_ENABLED(CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE) || atomic_test_bit(&pm->flags, PM_DEVICE_FLAG_RUNTIME_AUTO))) { return 0; }This way, it is more logically consistent because in the current version, this if suggests that CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE can be enabled without CONFIG_PM_DEVICE_RUNTIME.
There was a problem hiding this comment.
While both work, I think your approach is a bit clearer, I will refactor it and add documentation regarding the runtime flag
bjarki-andreasen dismissed stale reviews from anangl and decsny via 967abef bjarki-andreasen force-pushed the nrf-pm-device-runtime-align branch from c331965 to 967abef Compare 
Many SoCs which use PM_DEVICE_RUNTIME need every device in the system to have PM_DEVICE_RUNTIME enabled to function. Currently, this is only possible by adding zephyr,pm-device-runtime-auto; to every node in every devicetree which could potentially implement device power management. This is very error prone since its easy to miss a node, especially if users apply overlays, where users need to know and remember to apply or reapply this property. This commit adds a Kconfig, disabled by default, which automatically treats every device as if it had the zephyr,pm-device-runtime-auto property added to every node. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no>
Document the new CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE option. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no>
Mention CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE in 4.3 release notes. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no>
Enable CONFIG_PM_DEVICE_RUNTIME_DEFAULT_ENABLE by default for all nordic SoCs if CONFIG_PM_DEVICE_RUNTIME is used. This will ensure consistent behavior across all nordic SoCs and remove the need for pasting the devicetree propert zephyr,pm-device-runtime-auto everywhere. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no>
bjarki-andreasen force-pushed the nrf-pm-device-runtime-align branch from 967abef to 6552d4e Compare | Added documentation of the new option, and mentioned it in release notes. Additionally refactored an if statement based on input from @tmleman |
|
13 participants
Introduce the option
config PM_DEVICE_RUNTIME_DEFAULT_ENABLEwhich effectively implicitly adds the devicetree flagzephyr,pm-device-runtime-autoto every node in the devicetree. In reality, it ignores the flag and enables PM_DEVICE_RUNTIME unconditionally, but the behavior is identical from the outside.Typically, SoCs which utilize PM_DEVICE_RUNTIME need it enabled for every device. Having to add a flag manually to every node all of the devicetrees for these SoCs is tedious and error prone. It is also a software property being set in the devicetree, so in multiple ways a problematic pattern.
Most of nordics SoCs require all devices to enable
PM_DEVICE_RUNTIMEifPM_DEVICE_RUNTIMEis enabled, and the rest are not harmed by it since they have additional logic in the drivers to deal with runtime not being enabled (which can be optimized away by this change). It looks like this also is the case for Ambiq Apollo (@AlessandroLuo @RichardSWheatley @aaronyegx) and Xtensa Ace (@dcpleung @andyross @nashif), please check if this option would be helpful for your platforms as well :)With the addition in this PR, we will stop running into issues like: