pm: device_runtime: fix unbalanced domain get/put #90941
Conversation
github-actions bot requested review from JordanYates, ceolin, nashif, teburd and tmleman 
JordanYates left a comment
JordanYates left a comment There was a problem hiding this comment.
It would be good to add a test case that fails the previous incorrect behaviour and now passes.
bjarki-andreasen force-pushed the pm-device-runtime-pd-fix-unbalanced branch 2 times, most recently from 9b35619 to e0dd67d Compare 
| pm_device_runtime_get(dev); | ||
| pm_device_runtime_get(dev); | ||
| /* Assert domain was gotten/claimed only once */ | ||
| zassert_equal(pm_device_runtime_usage(dev->pm_base->domain), 1); |
There was a problem hiding this comment.
This is testing your particular implementation of a requirement, not the requirement itself.
It doesn't make any difference whether the domain is claimed once or multiple times, as long as it goes up with the first request and down with the last release.
Presumably the change is being made because there was some situation where the domain wasn't being powered up or down properly? The test should be duplicating that scenario (tested through pm_device_is_powered), not looking at usage counts.
There was a problem hiding this comment.
Agreed, test now tests the same scenario that discovered the bug: every get() increased ref count by 1 immediately, but async_put() only decreased by 1 once the put work was actually invoked, which was an imbalance :)
Assert async put correctly releases device's power domain. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no>
pm_device_runtime has inconsistent behavior regarding getting and putting a device's domain. This commit aligns it to get the domain only once, once device is resumed, and put only once, once device is suspended. Signed-off-by: Bjarki Arge Andreasen <bjarki.andreasen@nordicsemi.no>
bjarki-andreasen force-pushed the pm-device-runtime-pd-fix-unbalanced branch from e0dd67d to a4f53f4 Compare 
|
| ping @ceolin :) |
5 participants
pm_device_runtime has inconsistent behavior regarding getting and putting a device's domain. This commit aligns it to get the domain only once, once device is resumed, and put only once, once device is
suspended.