Re: [PATCH v2 3/8] PM: runtime: Misc improvements to runtime_pm.rst

[Brian Norris <briannorris@chromium.org>]

On Thu, Sep 24, 2026 at 04:01:00PM +0200, Ulf Hansson wrote:
> On Wed, Sep 23, 2026 at 7:47 PM Brian Norris <briannorris@chromium.org> wrote:
> > diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
> > index 39fdeeda7a1e..352cdaf0650d 100644
> > --- a/Documentation/power/runtime_pm.rst
> > +++ b/Documentation/power/runtime_pm.rst

> > @@ -315,7 +319,10 @@ removal of their drivers.
> >
> >  Drivers in ->remove() callback should undo the runtime PM changes done
> >  in ->probe(). Usually this means calling pm_runtime_disable(),
> > -pm_runtime_dont_use_autosuspend() etc.
> > +pm_runtime_dont_use_autosuspend() etc. Alternatively, drivers can use
> > +devm_pm_runtime_enable() during probe, which automatically takes care of
> > +calling pm_runtime_disable() and pm_runtime_dont_use_autosuspend() upon driver
> > +detachment.
> 
> As I have stated in earlier discussions at LKML, the
> devm_pm_runtime_enable() API is not entirely easy to use correctly by
> drivers. It means that pm_runtime_disable() gets called at some point
> *after* the ->remove() callback has been invoked, which can cause
> problems, unless the driver's ->remove() callback has managed things
> correctly.

Yeah. And I think it's rare for drivers to have done a thorough job. A
rare exception: I found commit 2d90ecdfa326 ("ASoC: rockchip: i2s: Use
managed hclk and runtime PM cleanup") an interesting outlier -- it adds
an additional devres teardown to power things off afterward.

OTOH, between v1 and v2, I chose to tweak one of the Examples to avoid
devm, precisely because it was committing (or hinting at) these kinds of
mistakes.

> My point is, the above makes it sounds like it's easy to switch to the
> devm managed version, while it certainly isn't that straight forward.

Right, I said as much in the cover letter too:

(possible future work)
<quote>
 * Adjust the way devm_pm_runtime_enable() works, specifically for
   remove()/teardown. Currently, this is very hard to use correctly --
   some common driver patterns may assume that a device will tear down
   while RPM_SUSPENDED; but that's not actually guaranteed. Notably,
   this makes some of the "Examples" section fairly tricky/subtle.
</quote>

Would this be a good moment to pass this possibility by you? What if we
taught the teardown to force a device back to RPM_SUSPENDED? Something
like:

    static void pm_runtime_disable_action(void *data)
    {
            pm_runtime_dont_use_autosuspend(data);
            pm_runtime_disable(data);

            // New code:
            if (pm_runtime_status_suspended(data)) {
                    int (*callback)(struct device *);
                    int ret;

                    callback = GET_CALLBACK(data, runtime_suspend);
                    ret = callback ? callback(data) : 0;
                    if (ret)
                            return;

                    pm_runtime_set_suspended(data);
            }
    }

> Not sure what that means for the documentation though. :-)

Well, I don't feel like the part you quoted is a problem. IMO, it's
totally fair to mention relevant APIs even if they're hard to use --
there is no part of the runtime PM that is easy to use!

But I'm definitely trying to make things easier too. Ideally, we can do
something like the above to make it easier to use. But if we
can't...well, I guess I can try to document pitfalls better -- possibly
in the Examples section, or maybe an extra note in the above quoted
area.

Thanks for looking,
Brian