RE: [PATCH] mtd: intel-dg: wake card on operations

Usyskin, Alexander
2025-10-21T12:51:34Z

> Subject: Re: [PATCH] mtd: intel-dg: wake card on operations
>
> On Mon, Oct 20, 2025 at 01:09:10PM +0300, Andy Shevchenko wrote:
> > On Sun, Oct 19, 2025 at 06:01:45PM +0300, Alexander Usyskin wrote:
> > > Enable runtime PM in mtd driver to notify parent graphics driver
> > > that whole card should be kept awake while nvm operations are
> > > performed through this driver.
>
> Ah, and perhaps a bit elaboration why graphics card needs that?
>
The memory power will be cut when whole card is powered down.
Intel DG card does not have separate power control for persistent memory.
Will add this in v2.

> ...
>
> > > + pm_runtime_mark_last_busy(nvm->dev);
> > > + pm_runtime_put_autosuspend(nvm->dev);
> >
> > Please, drop the second (duplicate) call.

Missed the patch where pm_runtime_put_autosuspend includes pm_runtime_mark_last_busy.
Will update in V2, thx

>
> ...
>
> > > + devm_pm_runtime_enable(device);
> >
> > Please, justify why this code is good without error checking. Before doing
> that
> > think for a moment for the cases when devm_*() might be developed in the
> future
> > and return something interesting (if not yet).
>

We should not fail the probe because of runtime pm enablement failure, I suppose.
There are other ways to keep card awake.
The pm_runtime_* functions work without runtime_enable but have no effect.
Thus, we can ignore failure here.

> ...
>
> > > err:
> > > + pm_runtime_put(device);
> > > +err_norpm:
> > > kref_put(&nvm->refcnt, intel_dg_nvm_release);
> > > return ret;
> >
> > Mixing devm with non-devm usually lead to hard to catch bugs in the error
> paths
> > / remove stages with ordering of cleaning resources up.
>

I see that this pattern is reasonably common in drivers.
There can't be devm wrappers for pm_runtime_get/put and these functions works
regardless of enable status.

> --
> With Best Regards,
> Andy Shevchenko
>

- -
Thanks,
Sasha