Re: [PATCH 1/2] PM: sleep: Do not mark devices without PM callbacks active on resume

[Rafael J. Wysocki (Intel)]

On Fri, Nov 6, 2026 at 7:46 PM Rafael J. Wysocki (Intel)
<rafael@xxxxxxxxxx> wrote:
>
> On Mon, Nov 2, 2026 at 3:47 PM Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx> wrote:
> >
> > Devices without system sleep callbacks get power.smart_suspend set in
> > device_prepare_smart_suspend(), unless their parent or a supplier
> > prevents it, so device_resume_noirq() sets their runtime PM status to
> > "active" whenever dev_pm_skip_resume() returns false. That is always
> > the case for PM_EVENT_RESTORE.
> >
> > Nothing resumes such a device, though.
>
> Too bad because it should be resumed in the "restore" phase of
> hibernation.  The concept of keeping devices in runtime suspend across
> hibernation is just fundamentally broken.

But if this happens to work somehow, then oh well.

> > If its bus or driver manages
> > resources from runtime PM callbacks, those resources stay off while the
> > device is now reported as active, and the next runtime suspend releases
> > them a second time.
> >
> > AMBA devices are an example, since the AMBA bus only has runtime PM
> > callbacks, which enable and disable the APB clock. After restoring from
> > hibernation on a Qualcomm Glymur CRD, the runtime-suspended CoreSight
> > devices are marked active, and the following runtime suspend of each of
> > them hits:
> >
> >   qdss already disabled
> >   WARNING: drivers/clk/clk.c:1259 at clk_core_disable+0xa4/0xac
> >   Call trace:
> >    clk_core_disable+0xa4/0xac (P)
> >    clk_disable+0x30/0x4c
> >    amba_pm_runtime_suspend+0x58/0x74
> >    __rpm_callback+0x48/0x1e0
> >    rpm_callback+0x58/0x64
> >    rpm_suspend+0x10c/0x5a4
> >    pm_runtime_work+0xc8/0xd8
> >
> > followed by "qdss already unprepared".
> >
> > On suspend-to-RAM this does not happen, because such devices take the
> > direct_complete path and device_resume_noirq() returns early for them.
> >
> > Leave the runtime PM status of devices without PM callbacks as it is.
>
> If I'm not mistaken, doing so may break the cases in which a device
> without PM callbacks and runtime PM enabled is the parent of a device
> with smart_suspend set because there will be a runtime-active device
> under a runtime-suspended parent.

So yeah, smart_suspend should not be set for devices with existing
runtime PM callbacks even if they have no system-wide PM callbacks.

Please try the attached patch (untested).

> > Fixes: bca84a7b93fd ("PM: sleep: Use DPM_FLAG_SMART_SUSPEND conditionally")
> > Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/base/power/main.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index 88a31e09e4f3..16a1f017051e 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -839,10 +839,15 @@ static void device_resume_noirq(struct device *dev, pm_message_t state, bool asy
> >          * Otherwise, the device is going to be resumed, so set its PM-runtime
> >          * status to "active" unless its power.smart_suspend flag is clear, in
> >          * which case it is not necessary to update its PM-runtime status.
> > +        *
> > +        * A device without system sleep callbacks is not going to be resumed
> > +        * by anyone, though, so leave its PM-runtime status as is. Otherwise,
> > +        * resources managed by its runtime PM callbacks, like the bus clock of
> > +        * an AMBA device, would end up unbalanced on the next runtime suspend.
> >          */
> >         if (skip_resume)
> >                 pm_runtime_set_suspended(dev);
> > -       else if (dev_pm_smart_suspend(dev))
> > +       else if (dev_pm_smart_suspend(dev) && !dev->power.no_pm_callbacks)
> >                 pm_runtime_set_active(dev);
> >
> >         if (dev->pm_domain) {
> >
> > --
> > 2.54.0
> >
---
 drivers/base/power/main.c    |    5 +++--
 drivers/base/power/power.h   |    4 ++++
 drivers/base/power/runtime.c |    6 ++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -2179,13 +2179,14 @@ static bool device_prepare_smart_suspend
 
 	/*
 	 * The "smart suspend" feature is enabled for devices whose drivers ask
-	 * for it and for devices without PM callbacks.
+	 * for it and for devices without any PM callbacks (including runtime PM
+	 * callbacks).
 	 *
 	 * However, if "smart suspend" is not enabled for the device's parent
 	 * or any of its suppliers that take runtime PM into account, it cannot
 	 * be enabled for the device either.
 	 */
-	if (!dev->power.no_pm_callbacks &&
+	if ((!dev->power.no_pm_callbacks || pm_runtime_callbacks_exist(dev)) &&
 	    !dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND))
 		return false;
 
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -131,6 +131,8 @@ extern void wakeup_source_sysfs_remove(s
 
 extern int pm_wakeup_source_sysfs_add(struct device *parent);
 
+extern bool pm_runtime_callbacks_exist(struct device *dev);
+
 #else /* !CONFIG_PM_SLEEP */
 
 static inline void device_pm_sleep_init(struct device *dev) {}
@@ -160,6 +162,8 @@ static inline int pm_wakeup_source_sysfs
 	return 0;
 }
 
+static inline bool pm_runtime_callbacks_exist(struct device *dev) { return false; }
+
 #endif /* !CONFIG_PM_SLEEP */
 
 static inline void device_pm_init(struct device *dev)
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -2136,4 +2136,10 @@ bool pm_runtime_need_not_resume(struct d
 		 dev->power.ignore_children);
 }
 
+bool pm_runtime_callbacks_exist(struct device *dev)
+{
+	return !dev->power.no_callbacks && (GET_CALLBACK(dev, runtime_resume) ||
+		GET_CALLBACK(dev, runtime_suspend));
+}
+
 #endif /* CONFIG_PM_SLEEP */