Messages in this thread | | | Date | Tue, 21 Feb 2023 12:44:50 +0900 | Subject | Re: [PATCH] PM: s2idle: Don't allow s2idle when cpuidle isn't supported | From | Hector Martin <> |
| |
On 15/02/2023 06.50, Kazuki Hashimoto wrote: > s2idle isn't supported on platforms that don't support cpuidle as of > 31a3409065d1 ("cpuidle / sleep: Do sanity checks in cpuidle_enter_freeze() > too"), so update the suspend framework to reflect this in order to avoid > breakages. > > Link: https://lore.kernel.org/all/20230204152747.drte4uitljzngdt6@kazuki-mac > Fixes: 31a3409065d1 ("cpuidle / sleep: Do sanity checks in cpuidle_enter_freeze() too") > Signed-off-by: Kazuki Hashimoto <kazukih0205@gmail.com> > --- > kernel/power/suspend.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index 3f436282547c..27507ae7c9c9 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -556,6 +556,12 @@ static int enter_state(suspend_state_t state) > > trace_suspend_resume(TPS("suspend_enter"), state, true); > if (state == PM_SUSPEND_TO_IDLE) { > + struct cpuidle_device *dev = cpuidle_get_device(); > + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); > + > + if (cpuidle_not_available(drv, dev)) > + return -EINVAL; > + > #ifdef CONFIG_PM_DEBUG > if (pm_test_level != TEST_NONE && pm_test_level <= TEST_CPUS) { > pr_warn("Unsupported test mode for suspend to idle, please choose none/freezer/devices/platform.\n");
Well... this turns s2idle from "slighly broken" to "fully broken". I'm not sure that's a good idea.
If this (or something equivalent) goes in, we'll have to carry a revert in the Asahi tree until the PSCI-replacement story goes in, because lots of people are using the current somewhat broken s2idle. It's a lot better than having no sleep modes at all.
Also, if you intend to disable s2idle on these platforms, you have to actually stop advertising it. Advertising it and then refusing to enter s2idle is not acceptable, it means people will get "sleep" buttons and expect sleep to work and then it won't. But then that could also introduce race conditions with userspace checking for sleep support before the cpuidle driver loads. So there is definitely a can of worms here.
- Hector
| |