From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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@intel.com>
---

v1 -> v2: Corresponds to patch [2/9] (that was posted as [0/9] by mistake) in v1.

---
 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
@@ -1973,7 +1973,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);
@@ -1988,15 +1988,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);

On Thu, Jun 26, 2025 at 8:15 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> 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@intel.com>
> ---
>
> v1 -> v2: Corresponds to patch [2/9] (that was posted as [0/9] by mistake) in v1.
>
> ---
>  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
> @@ -1973,7 +1973,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);
> @@ -1988,15 +1988,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;
> -       }

I kind of see that this change may confuse other things looking at the
PM runtime status to determine whether or not the device needs to be
suspended that possibly run after pm_runtime_force_suspend().

I'm also not quite sure why I thought that this patch would be
necessary in this series because the [5/9] should just work without
it.

Please disregard it unless you see why it is needed here.

>
>         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);
>
>
>
>

On Fri, Jun 27, 2025 at 12:52 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Jun 26, 2025 at 8:15 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > 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@intel.com>
> > ---
> >
> > v1 -> v2: Corresponds to patch [2/9] (that was posted as [0/9] by mistake) in v1.
> >
> > ---
> >  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
> > @@ -1973,7 +1973,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);
> > @@ -1988,15 +1988,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;
> > -       }
>
> I kind of see that this change may confuse other things looking at the
> PM runtime status to determine whether or not the device needs to be
> suspended that possibly run after pm_runtime_force_suspend().
>
> I'm also not quite sure why I thought that this patch would be
> necessary in this series because the [5/9] should just work without
> it.
>
> Please disregard it unless you see why it is needed here.

Well, not quite.

It is needed, but not at this point.  That is,patch [5/9] will work
without it, but then it is needed for the PCI and ACPI PM to work with
pm_runtime_force_suspend().

Namely, say DPM_FLAG_SMART_SUSPEND is set and
pm_runtime_force_suspend() is used as a driver callback in the
"suspend" phase.  The PCI PM sets strict_midlayer, so
pm_runtime_force_suspend() runs the driver runtime PM callback
directly, but power still needs to be removed from the device.
However, pci_pm_suspend_noirq() checks dev_pm_skip_suspend() and it
will bail out if the device is RPM_SUSPENDED.

I guess I should reorder the patches and add the above bit of
explanation to the changelog of the $subject one.

If you can review patches [2-3/9] from this series, I'll be able to go
ahead with the first half of it and the rest can be resent separately.

Sorry for the confusion!

> >
> >         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);
> >
> >
> >
> >