Re: [PATCH v5 1/2] pm: runtime: Add new devm functions
From: Csókás Bence
Date: Thu Mar 27 2025 - 12:11:48 EST
Hi,
On 2025. 03. 27. 15:14, Rafael J. Wysocki wrote:
/-- devm_pm_runtime_get_noresume()
| /-- devm_{pm_runtime_set_active() + pm_runtime_enable() (in this order)}
| | pm_runtime_use_autosuspend()
| |
| | Note that the device cannot be suspended here unless its
runtime PM usage
| | counter is dropped, in which it would need to be bumped up
again later to
| | retain the balance.
| |
| \-> pm_runtime_disable() + pm_runtime_set_suspended() (in this order)
\-> pm_runtime_put_noidle()
Ah, so basically what I've done originally, just calling
`devm_pm_runtime_get_noresume()` _first_ instead of _last_, right?
And pm_runtime_dont_use_autosuspend() is not really necessary after
disabling runtime PM.
It was done this way in devm_pm_runtime_enable() already, see commit
b4060db9251f ("PM: runtime: Have devm_pm_runtime_enable() handle
pm_runtime_dont_use_autosuspend()"). I didn't change anything
behaviourally there.
Also, I think that the driver could be fixed without introducing the
new devm_ stuff which would be way simpler, so why don't you do that
and then think about devm_?
Sure, I could quick-fix this, go through all the possible error paths
and whatnot and ref-count in my head, but it doesn't fix the underlying
problem: in order to properly use PM, you have to do a bunch of calls in
some set order, then undo them in reverse order on error and remove --
exactly the thing devm was designed for, and exactly the thing where
it's easy for a human to forget one case by accident. Thus I prefer to
use the *real* solution, devm.
Bence