[PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled
From: Thomas Gleixner <hidden>
Date: 2016-06-09 22:57:29
Also in:
linux-clk, lkml
Subsystem:
hibernation (aka software suspend, aka swsusp), suspend to ram, the rest · Maintainers:
"Rafael J. Wysocki", Linus Torvalds
On Thu, 9 Jun 2016, Stefan Agner wrote:
On 2016-06-09 13:08, Thomas Gleixner wrote:quoted
On Tue, 7 Jun 2016, Dong Aisheng wrote:quoted
Then it may need introduce a lot changes and increase many new core APIs. Is that a problem?No. That's all better than each driver having broken workarounds. It's a common problem so it wants to be addressed at the core level. There you have a central point to do this and you can still catch abusers which call stuff from the wrong context. The hacks in the drivers don't allow that because they look at the context, i.e. irq disabled, instead of checking the system state.IMHO, the hacky part of my patch was how I detected whether to use sleep or delay. That said I am ok with API extension too, I guess it is fairly common use case... I found at least 6 clock prepare functions with sleep in it (and some udelays, all between 1-100). Your proposed solution uses "early_boot_or_suspend_resume" which I did not found as a convenient function in the wild :-) How would you implement that?
Early boot is simple. Supsend/resume is not that hard either. We have a patch
in RT which does exactly what you need. See below.
Then the state check simply becomes:
system_state != SYSTEM_RUNNING
Thanks,
tglx
8<---------------------------
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 350dfb08aee3..5e63b681f58e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h@@ -473,6 +473,7 @@ extern enum system_states { SYSTEM_HALT, SYSTEM_POWER_OFF, SYSTEM_RESTART, + SYSTEM_SUSPEND, } system_state; #define TAINT_PROPRIETARY_MODULE 0
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index b7342a24f559..bfd9e0982f15 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c@@ -285,6 +285,8 @@ static int create_image(int platform_mode) local_irq_disable(); + system_state = SYSTEM_SUSPEND; + error = syscore_suspend(); if (error) { printk(KERN_ERR "PM: Some system devices failed to power down, "
@@ -314,6 +316,7 @@ static int create_image(int platform_mode) syscore_resume(); Enable_irqs: + system_state = SYSTEM_RUNNING; local_irq_enable(); Enable_cpus:
@@ -437,6 +440,7 @@ static int resume_target_kernel(bool platform_mode) goto Enable_cpus; local_irq_disable(); + system_state = SYSTEM_SUSPEND; error = syscore_suspend(); if (error)
@@ -470,6 +474,7 @@ static int resume_target_kernel(bool platform_mode) syscore_resume(); Enable_irqs: + system_state = SYSTEM_RUNNING; local_irq_enable(); Enable_cpus:
@@ -555,6 +560,7 @@ int hibernation_platform_enter(void) goto Enable_cpus; local_irq_disable(); + system_state = SYSTEM_SUSPEND; syscore_suspend(); if (pm_wakeup_pending()) { error = -EAGAIN;
@@ -567,6 +573,7 @@ int hibernation_platform_enter(void) Power_up: syscore_resume(); + system_state = SYSTEM_RUNNING; local_irq_enable(); Enable_cpus:
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index f9fe133c13e2..80ebc0726290 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c@@ -359,6 +359,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup) arch_suspend_disable_irqs(); BUG_ON(!irqs_disabled()); + system_state = SYSTEM_SUSPEND; + error = syscore_suspend(); if (!error) { *wakeup = pm_wakeup_pending();
@@ -375,6 +377,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup) syscore_resume(); } + system_state = SYSTEM_RUNNING; + arch_suspend_enable_irqs(); BUG_ON(irqs_disabled());