[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAPDyKFqByMzQruYZTBL0T47Ww2KTF1Py91K-wrQ9GuTeW3fx9A@mail.gmail.com>
Date: Tue, 18 Nov 2025 15:44:32 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Rose Wu <ya-jou.wu@...iatek.com>, linux-pm@...r.kernel.org,
rafael.j.wysocki@...el.com, regressions@...ts.linux.dev,
linux-kernel@...r.kernel.org, wsd_upstream <wsd_upstream@...iatek.com>,
linux-mediatek@...ts.infradead.org, "artis. chiu" <artis.chiu@...iatek.com>,
"Johnny-cc. Kao" <Johnny-cc.Kao@...iatek.com>
Subject: Re: [PATCH v4] PM: sleep: core: Fix runtime PM enabling in device_resume_early()
On Tue, 18 Nov 2025 at 15:16, Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Runtime PM should only be enabled in device_resume_early() if it has
> been disabled for the given device by device_suspend_late(). Otherwise,
> it may cause runtime PM callbacks to run prematurely in some cases
> which leads to further functional issues.
>
> Make two changes to address this problem.
>
> First, reorder device_suspend_late() to only disable runtime PM for a
> device when it is going to look for the device's callback or if the
> device is a "syscore" one. In all of the other cases, disabling runtime
> PM for the device is not in fact necessary. However, if the device's
> callback returns an error and the power.is_late_suspended flag is not
> going to be set, enable runtime PM so it only remains disabled when
> power.is_late_suspended is set.
>
> Second, make device_resume_early() only enable runtime PM for the
> devices with the power.is_late_suspended flag set.
>
> Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more asynchronous")
> Reported-by: Rose Wu <ya-jou.wu@...iatek.com>
> Closes: https://lore.kernel.org/linux-pm/70b25dca6f8c2756d78f076f4a7dee7edaaffc33.camel@mediatek.com/
> Cc: 6.16+ <stable@...r.kernel.org> # 6.16+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
Kind regards
Uffe
> ---
>
> v3 -> v4:
> * Reorder the syscore check in device_resume_early() after the
> is_late_suspended check (Ulf)
> * Update the changelog to reflect the above.
>
> v2 -> v3:
> * Also set is_late_suspended for syscore devices to avoid skipping runtime PM
> disabling for them (Ulf).
> * Update the changelog to reflect the above.
>
> v1 -> v2:
> * Add pm_runtime_enable() to device_suspend_late() error path (Rose).
> * Update the changelog to reflect the above.
>
> ---
> drivers/base/power/main.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -903,12 +903,15 @@ static void device_resume_early(struct d
> TRACE_DEVICE(dev);
> TRACE_RESUME(0);
>
> - if (dev->power.syscore || dev->power.direct_complete)
> + if (dev->power.direct_complete)
> goto Out;
>
> if (!dev->power.is_late_suspended)
> goto Out;
>
> + if (dev->power.syscore)
> + goto Skip;
> +
> if (!dpm_wait_for_superior(dev, async))
> goto Out;
>
> @@ -941,11 +944,11 @@ Run:
>
> Skip:
> dev->power.is_late_suspended = false;
> + pm_runtime_enable(dev);
>
> Out:
> TRACE_RESUME(error);
>
> - pm_runtime_enable(dev);
> complete_all(&dev->power.completion);
>
> if (error) {
> @@ -1630,12 +1633,6 @@ static void device_suspend_late(struct d
> TRACE_DEVICE(dev);
> TRACE_SUSPEND(0);
>
> - /*
> - * Disable runtime PM for the device without checking if there is a
> - * pending resume request for it.
> - */
> - __pm_runtime_disable(dev, false);
> -
> dpm_wait_for_subordinate(dev, async);
>
> if (READ_ONCE(async_error))
> @@ -1646,9 +1643,18 @@ static void device_suspend_late(struct d
> goto Complete;
> }
>
> - if (dev->power.syscore || dev->power.direct_complete)
> + if (dev->power.direct_complete)
> goto Complete;
>
> + /*
> + * Disable runtime PM for the device without checking if there is a
> + * pending resume request for it.
> + */
> + __pm_runtime_disable(dev, false);
> +
> + if (dev->power.syscore)
> + goto Skip;
> +
> if (dev->pm_domain) {
> info = "late power domain ";
> callback = pm_late_early_op(&dev->pm_domain->ops, state);
> @@ -1679,6 +1685,7 @@ Run:
> WRITE_ONCE(async_error, error);
> dpm_save_failed_dev(dev_name(dev));
> pm_dev_err(dev, state, async ? " async late" : " late", error);
> + pm_runtime_enable(dev);
> goto Complete;
> }
> dpm_propagate_wakeup_to_parent(dev);
>
>
>
Powered by blists - more mailing lists