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

[Rafael J. Wysocki (Intel)]

Hi Brian,

On Wed, Oct 14, 2026 at 10:15 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > ---
> > >
> > > 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, 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.

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.

> > 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@xxxxxxxxxx/
>
>   (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.

> > 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.

> 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.

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