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

[Brian Norris]

Hi Rafael,

[ trimming the rest. I appreciate the replies! Will respond later in
  this thread, but largely -- ack. ]

On Fri, Oct 16, 2026 at 07:42:47PM +0200, Rafael J. Wysocki (Intel) wrote:
> On Thu, Oct 15, 2026 at 10:55 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
> > 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@xxxxxxxxxxxx> wrote:
> > > > 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.
> >
> > 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().
> 
> I'm not quite convinced that runtime PM is suitable for devm_ type of
> management.
> 
> IIRC, devm_pm_runtime_enable() was introduced to avoid some code
> duplication between drivers, but I don't think that it may be expected
> to be universally applicable.

I also don't think it will be univerally applicable, but:

1) it's reasonably widely used. (220 and growing.)
2) it's commonly used for "simple" drivers.
3) those "simple" drivers still often get it wrong.
4) I don't see a major downside in trying to improve it in the ways I'm
   thinking.

For #3: literally the first (alphabetical git grep) use that calls
devm_pm_runtime_enable() without first marking it active is
drivers/char/hw_random/mtk-rng.c, and it has a bug -- it doesn't balance
clk_prepare_enable() and clk_disable_unprepare(). In fact, it performs
no remove() action, because *normally* a simple driver that uses devm_*
for everything is automatically in good shape. Everything will tear down
in reverse order, correctly.

But runtime PM is crazy, so no, that's not true. Runtime PM allows (via
.../power/control -> on) the device to leave in an ACTIVE state, and for
that clock to be left on. I believe if the driver unbinds and binds
again, the clock may be mismanaged forever.

I think a simple driver like that should be simple, and we can make
devm_* + pm_runtime work correctly for at least the most straightforward
of cases. Is that an outlandish goal?

Brian