On Thu, Jun 26, 2025 at 12:10 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
> On Wed, 25 Jun 2025 at 21:25, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > Add a power.needs_force_resume check to pm_runtime_force_suspend() so
> > it need not rely on the runtime PM status of the device when deciding
> > whether or not to return early.
> >
> > With the new check in place, pm_runtime_force_suspend() will also skip
> > devices with the runtime PM status equal to RPM_ACTIVE if they have
> > power.needs_force_resume set, so it won't need to change the RPM
> > status of the device to RPM_SUSPENDED in addition to setting
> > power.needs_force_resume in the case when pm_runtime_need_not_resume()
> > return false.
> >
> > This allows the runtime PM status update to be removed from
> > pm_runtime_force_resume(), so the runtime PM status remains unchanged
> > between the pm_runtime_force_suspend() and pm_runtime_force_resume()
> > calls.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > ---
> > drivers/base/power/runtime.c | 21 ++++++++-------------
> > 1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -1975,7 +1975,7 @@
> > int ret;
> >
> > pm_runtime_disable(dev);
> > - if (pm_runtime_status_suspended(dev))
> > + if (pm_runtime_status_suspended(dev) || dev->power.needs_force_resume)
> > return 0;
> >
> > callback = RPM_GET_CALLBACK(dev, runtime_suspend);
> > @@ -1990,15 +1990,16 @@
> > /*
> > * If the device can stay in suspend after the system-wide transition
> > * to the working state that will follow, drop the children counter of
> > - * its parent, but set its status to RPM_SUSPENDED anyway in case this
> > - * function will be called again for it in the meantime.
> > + * its parent and the usage counters of its suppliers. Otherwise, set
> > + * power.needs_force_resume to let pm_runtime_force_resume() know that
> > + * the device needs to be taken care of and to prevent this function
> > + * from handling the device again in case the device is passed to it
> > + * once more subsequently.
> > */
> > - if (pm_runtime_need_not_resume(dev)) {
> > + if (pm_runtime_need_not_resume(dev))
> > pm_runtime_set_suspended(dev);
> > - } else {
> > - __update_runtime_status(dev, RPM_SUSPENDED);
> > + else
> > dev->power.needs_force_resume = true;
> > - }
> >
> > return 0;
> >
> > @@ -2029,12 +2030,6 @@
> > if (!dev->power.needs_force_resume)
> > goto out;
> >
> > - /*
> > - * The value of the parent's children counter is correct already, so
> > - * just update the status of the device.
> > - */
> > - __update_runtime_status(dev, RPM_ACTIVE);
> > -
> > callback = RPM_GET_CALLBACK(dev, runtime_resume);
> >
> > dev_pm_disable_wake_irq_check(dev, false);
> >
>
> As I mentioned for patch4, pm_runtime_force_suspend() is being used
> from driver's ->remove() callback too.
>
> If such a driver/device gets probed again, we need a fresh start. It
> seems like we need to clear the needs_force_resume flag in
> pm_runtime_reinit(). In fact, that looks like an existing bug, even
> before $subject patch, right?
If it is used in ->remove(), then yes, it needs to be cleared in
_reinit(), at least in principle.
