Re: [PATCH v2 8/8] PM: runtime: Add Example Driver Patterns section

[Brian Norris]

On Thu, Sep 24, 2026 at 04:13:30PM +0200, Ulf Hansson wrote:
> On Wed, Sep 23, 2026 at 7:48 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:

> > +Basic Probe
> > +~~~~~~~~~~~
> > +
> > +A driver that powers on its hardware during probe and does not use autosuspend
> > +can initialize runtime PM using device-managed 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;
> > +
> > +               /* 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);
> > +
> > +               ret = devm_pm_runtime_enable(dev);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               /*
> > +                * Alternatively, the above two calls can be combined into:
> > +                * ret = devm_pm_runtime_set_active_enabled(dev);
> > +                * if (ret)
> > +                *     return ret;
> > +                */
> > +
> > +               /*
> > +                * 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;
> 
> The above looks nice and simple, but what happens in the error path is
> equally important.
> 
> In principle, nothing prevents the runtime PM callbacks to be invoked
> until pm_runtime_disable() has been called at some point *after* the
> probe callback has returned. This needs to be taken care of correctly.
> 
> Likewise, as it's great with an example for ->probe(), it would be
> nice with a corresponding example for ->remove().

Good points. I've rewritten much of the Examples to now:

(a) largely *not* use devm_*;

(b) include a more interesting set of error handling and remove(); and

(c) only after doing (a) and (b), show an example that uses devm_*() to
    replace the appropriate error handling and remove() teardown.

I hope that will also partially address your other comment about
devm_pm_runtime_enable(), and how it's difficult to use correctly.

> I will have to defer reviewing the remaining parts in the $subject
> patch, a bit limited bandwidth at the moment.

Thanks for the time you spent on it!

I'll wait a bit more in case Rafael had other feedback, before sending a
v3 that (I think) addresses your feedback.

Brian