Re: [PATCH v4 2/2] PM: runtime: Add Example Driver Patterns section

[Brian Norris <briannorris@chromium.org>]

Hi Rafael,

On Thu, Oct 15, 2026 at 10:11:04PM +0200, Rafael J. Wysocki (Intel) wrote:
> On Wed, Oct 14, 2026 at 10:15 PM Brian Norris <briannorris@chromium.org> wrote:
> > On Wed, Oct 14, 2026 at 09:07:54PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > On Thu, Oct 1, 2026 at 5:51 PM Brian Norris <briannorris@chromium.org> wrote:
> > > >
> > > > The runtime PM API surface is pretty large, but there are a few common
> > > > patterns that many drivers should follow. Add some illustrative
> > > > examples, to help guide the most common audience for runtime PM docs --
> > > > driver writers.
> > > >
> > > > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > > > ---
> > > >
> > > > Changes in v4:
> > > >  * "state" to "status", to be consistent with Introduction terminology
> > > >
> > > > Changes in v3:
> > > >  * Rewrite Examples to avoid devm_* at first, to highlight correct
> > > >    approaches
> > > >  * Include error-handling and remove() in any probe() Examples
> > > >  * Advise more caution when using devm_*, as ordering issues are subtle
> > > >
> > > > Changes in v2:
> > > >  * Add appropriate teardown to "Probe with Hardware Powered Off"
> > > >    Example, as the remove() + power-off behavior is subtle here, and
> > > >    easy to get wrong
> > > >
> > > >  Documentation/power/runtime_pm.rst | 475 ++++++++++++++++++++++++++++-
> > > >  1 file changed, 473 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
> > > > index e4407b687f8b..cb25e0bb4ca7 100644
> > > > --- a/Documentation/power/runtime_pm.rst
> > > > +++ b/Documentation/power/runtime_pm.rst
> > > > @@ -429,7 +429,8 @@ in ->probe(). Usually this means calling pm_runtime_disable(),
> > > >  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.
> > > > +detachment. See also the Examples in `Section 10`_ for some effective
> > > > +strategies for driver teardown.
> > > >
> > > >  The user space can effectively disallow the driver of the device to power manage
> > > >  it at run time by changing the value of its /sys/devices/.../power/control
> > > > @@ -654,7 +655,9 @@ The implementation is well suited for asynchronous use in interrupt contexts.
> > > >  However such use inevitably involves races, because the PM core can't
> > > >  synchronize ->runtime_suspend() callbacks with the arrival of I/O requests.
> > > >  This synchronization must be handled by the driver, using its private lock.
> > > > -Here is a schematic pseudo-code example::
> > > > +Here is a schematic pseudo-code example:
> > > > +
> > > > +.. code-block:: c
> > > >
> > > >         foo_read_or_write(struct foo_priv *foo, void *data)
> > > >         {
> > > > @@ -720,3 +723,471 @@ pm_runtime_autosuspend_expiration() from within the ->runtime_suspend()
> > > >  callback while holding its private lock.  If the function returns a nonzero
> > > >  value then the delay has not yet expired and the callback should return
> > > >  -EAGAIN.
> > > > +
> > > > +.. _Section 10:
> > > > +
> > > > +10. Example Driver Patterns
> > > > +===========================
> > > > +
> > > > +The runtime PM API is large and complex, but most device drivers follow a small
> > > > +set of canonical patterns when interacting with runtime PM. This section
> > > > +illustrates standard patterns for device probing, performing I/O, and handling
> > > > +interrupts.
> > > > +
> > > > +Probe and Initialization
> > > > +------------------------
> > > > +
> > > > +Basic Probe
> > > > +~~~~~~~~~~~
> > > > +
> > > > +A driver that powers on its hardware during probe and does not use autosuspend
> > > > +can initialize runtime PM using standard helpers:
> > > > +
> > > > +.. code-block:: c
> > > > +
> > > > +       static int foo_probe(struct platform_device *pdev)
> > > > +       {
> > > > +               struct device *dev = &pdev->dev;
> > > > +               struct foo_priv *priv;
> > > > +               int ret;
> > > > +
> > > > +               priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > > +               if (!priv)
> > > > +                       return -ENOMEM;
> > > > +
> > > > +               platform_set_drvdata(pdev, priv);
> > > > +
> > > > +               /* Power on and initialize hardware registers... */
> > > > +
> > > > +               /*
> > > > +                * The code above left hardware powered on and operational, so
> > > > +                * tell the PM core that the device is active before enabling
> > > > +                * runtime PM.
> > > > +                */
> > > > +               pm_runtime_set_active(dev);
> > > > +               pm_runtime_enable(dev);
> > > > +
> > > > +               /*
> > > > +                * Perform remaining driver setup (e.g. register interrupts, device nodes).
> > > > +                * If any step fails after enabling runtime PM:
> > > > +                */
> > > > +               ret = foo_register_device(priv);
> > > > +               if (ret)
> > > > +                       goto err_pm_disable;
> > > > +
> > > > +               /*
> > > > +                * Upon successful return from ->probe(), the driver core
> > > > +                * automatically executes pm_request_idle(dev), allowing the
> > > > +                * device to suspend asynchronously if its usage counter is zero.
> > > > +                */
> > > > +               return 0;
> > > > +
> > > > +       err_pm_disable:
> > > > +               pm_runtime_disable(dev);
> > > > +               if (!pm_runtime_status_suspended(dev)) {
> > > > +                       /*
> > > > +                        * Power back off, in case device did not suspend yet.
> > > > +                        * For example, leverage your .runtime_suspend()
> > > > +                        * implementation:
> > > > +                        */
> > > > +                       foo_runtime_suspend(dev);
> > >
> > > There are two reasons why this may not work, here and below.
> > >
> > > First off, if a non-trivial PM domain or bus type etc is involved, its
> > > .runtime_suspend() callback may need to be invoked in addition to
> > > foo_runtime_suspend() to actually power off the device.
> >
> > Good point. While it's a platform_device (not a complex bus), I suppose
> > the PM domain may be relevant, at least for remove(). (probe() is
> > already handled automatically by the device core.)
> 
> If this is a platform device, the PM domain detach will power it off,
> which I forgot about,

I kinda get lost in that too sometimes... so even remove() is OK
actually. Good.

> so the driver only needs to prepare the device
> for that.  If there is no PM domain, the driver runtime suspend
> callback function can be used for the power off in principle.

What's the distinction? To me, power domains are not the only thing that
constitute "powering on the device" -- drivers may be handling resets,
clocks, regulators, etc., too. In either case, those are things that the
driver is powering on in probe(), and should be torn down in error-exit
and in remove().

But in either case, those are probably things the driver is doing in its
runtime_suspend callback.

Maybe I'm clouding things by mentioning "runtime_suspend" here; the
concept here is "power off anything you turned on in probe". So if you'd
like another spin here, I may just call it foo_initialize() and
foo_uninitialize(), with some comments about "power on any necessary
resources: clocks, regulators, resets, ...".

> So I think that the original code flow here is fine with the caveat
> that if a PM domain is there, the driver may not actually need to
> carry out the power off by itself.

I kinda think the point of most of this power domain architecture is
that a typical driver just doesn't have to worry about it at all (in
probe(), and if you use runtime PM properly), so I'd prefer not
mentioning it in the Basic example. But sure.

> > > That's why
> > > pm_runtime_force_suspend() looks for the callback to invoke.
> >
> > Would you be OK with pm_runtime_force_suspend() as an alternative here?
> > It seems like some drivers have started using that in remove(), which
> > doesn't quite match its docs, but otherwise seems OK to me. I was
> > considering that already here, and I even think it might be a good idea
> > to do automatically in devm_*, as I noted here:
> >
> >   https://lore.kernel.org/all/arVWVA3lks7SopQH@google.com/
> >
> >   (note: there's definitely at least one typo in there; should be
> >   !pm_runtime_status_suspended(); and it needs some other adjustments to
> >   actually verify the device was suspended at time of enable(); ... so
> >   consider it a concept, not a patch.)
> >
> > But I kinda wanted a maintainer opinion before I went too far on that.
> 
> That shouldn't be necessary as per the above.

OK, yes, not necessary in this particular example.

But my hope is that devm_pm_runtime_enable() can be made easier to use.
It would be nice if we had the property:

  If your device was suspended (most resources powered off) when
  devm_pm_runtime_enable() is called, the PM helpers will suspend it
  (power it back off) on remove().

To me, PM runtime is pretty unique relative to other devm-supported
subsystems: even if you carefully undo everything you did in probe in
exactly the right inverse order, you still get things wrong -- because
PM runtime is so stateful (especially forbid()), and so you have to
account for an extra unwind step (call runtime_suspend-like functions in
remove()).

And to be clear, that proposal would only be useful for the "Probe with
Hardware Powered Off" case (which I think is actually quite common, and
quite common to do incorrectly), and not this "Basic Probe" (that powers
on during probe).

> > > Second, if the driver expects the device to be powered up and
> > > operational at the beginning of probe, which this one does AFAICS,
> >
> > No, that's not the intention. See:
> >
> >   "A driver that powers on its hardware during probe"
> 
> So this effectively assumes no PM domain because platform_probe()
> calls dev_pm_domain_attach() with PD_FLAG_ATTACH_POWER_ON set.

Yes or no, depending on your understanding of what "powers on its
hardware" means. Holistically, I don't usually consider something "on"
if its power domain is on but it's still in reset, or its clocks are
off, or its regulators are off, or ...

Is there some better terminology you'd prefer here?

> > And the pseudocode/comment:
> >
> >                /* Power on and initialize hardware registers... */
> >
> > i.e., probe() is doing the power-on work, but it's elided.
> >
> > Perhaps that wasn't obvious enough, so I can replace that with
> 
> No, it's fine with me except that there is a difference between the
> "PM domain present" and "no PM domain" cases and at least some drivers
> may need to take it into account.

I'm not really getting what the distinction is, from a driver writer
point of view. With or without a PM domain, the driver most likely
doesn't need to worry about the distinction. (Caveat: if you have 2 or
more domains, you have to manage that yourself as the driver core
doesn't handle it. But that becomes obvious rather quickly.)

Brian

> > more-convincing code-like structures
> > (regulator_get()/regulator_enable(), etc., for example) if you think
> > that would help.