From: Thomas Gleixner <tglx@linutronix.de> To: Stefan Agner <stefan@agner.ch> Cc: Dong Aisheng <dongas86@gmail.com>, Shawn Guo <shawnguo@kernel.org>, Lucas Stach <l.stach@pengutronix.de>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@codeaurora.org>, LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>, kernel@pengutronix.de, linux-clk@vger.kernel.org, LAK <linux-arm-kernel@lists.infradead.org>, "Rafael J. Wysocki" <rjw@sisk.pl> Subject: Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled Date: Fri, 10 Jun 2016 00:55:26 +0200 (CEST) [thread overview] Message-ID: <alpine.DEB.2.11.1606100046150.28031@nanos> (raw) In-Reply-To: <1162370a4389413d2d4ff0eb79ff13ac@agner.ch> On Thu, 9 Jun 2016, Stefan Agner wrote: > On 2016-06-09 13:08, Thomas Gleixner wrote: > > On Tue, 7 Jun 2016, Dong Aisheng wrote: > >> 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());
WARNING: multiple messages have this Message-ID (diff)
From: tglx@linutronix.de (Thomas Gleixner) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled Date: Fri, 10 Jun 2016 00:55:26 +0200 (CEST) [thread overview] Message-ID: <alpine.DEB.2.11.1606100046150.28031@nanos> (raw) In-Reply-To: <1162370a4389413d2d4ff0eb79ff13ac@agner.ch> On Thu, 9 Jun 2016, Stefan Agner wrote: > On 2016-06-09 13:08, Thomas Gleixner wrote: > > On Tue, 7 Jun 2016, Dong Aisheng wrote: > >> 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());
next prev parent reply other threads:[~2016-06-09 22:57 UTC|newest] Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-29 22:49 [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled Stefan Agner 2016-01-29 22:49 ` Stefan Agner 2016-01-29 22:49 ` [PATCH 2/2] clk: imx: return correct frequency for Ethernet PLL Stefan Agner 2016-01-29 22:49 ` Stefan Agner 2016-01-29 23:35 ` [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled Joshua Clayton 2016-01-29 23:35 ` Joshua Clayton 2016-01-30 1:16 ` Stephen Boyd 2016-01-30 1:16 ` Stephen Boyd 2016-04-26 17:04 ` Stefan Agner 2016-04-26 17:04 ` Stefan Agner 2016-04-16 1:00 ` Stephen Boyd 2016-04-16 1:00 ` Stephen Boyd 2016-04-18 1:58 ` Shawn Guo 2016-04-18 1:58 ` Shawn Guo 2016-04-21 3:45 ` Dong Aisheng 2016-04-21 3:45 ` Dong Aisheng 2016-04-26 1:23 ` Shawn Guo 2016-04-26 1:23 ` Shawn Guo 2016-04-26 5:51 ` Dong Aisheng 2016-04-26 5:51 ` Dong Aisheng 2016-04-26 5:51 ` Dong Aisheng 2016-04-26 9:24 ` Shawn Guo 2016-04-26 9:24 ` Shawn Guo 2016-04-26 9:24 ` Shawn Guo 2016-04-26 9:31 ` Lucas Stach 2016-04-26 9:31 ` Lucas Stach 2016-04-26 9:31 ` Lucas Stach 2016-04-26 11:16 ` Dong Aisheng 2016-04-26 11:16 ` Dong Aisheng 2016-04-26 11:16 ` Dong Aisheng 2016-04-26 11:27 ` Dong Aisheng 2016-04-26 11:27 ` Dong Aisheng 2016-04-26 11:27 ` Dong Aisheng 2016-04-27 1:58 ` Shawn Guo 2016-04-27 1:58 ` Shawn Guo 2016-04-27 1:58 ` Shawn Guo 2016-04-27 2:45 ` Dong Aisheng 2016-04-27 2:45 ` Dong Aisheng 2016-04-27 2:45 ` Dong Aisheng 2016-04-27 2:56 ` Fabio Estevam 2016-04-27 2:56 ` Fabio Estevam 2016-04-27 2:56 ` Fabio Estevam 2016-04-27 7:28 ` Stefan Agner 2016-04-27 7:28 ` Stefan Agner 2016-04-27 8:53 ` Dong Aisheng 2016-04-27 8:53 ` Dong Aisheng 2016-04-27 8:53 ` Dong Aisheng 2016-04-27 2:57 ` Dong Aisheng 2016-04-27 2:57 ` Dong Aisheng 2016-04-27 2:57 ` Dong Aisheng 2016-04-27 7:24 ` Shawn Guo 2016-04-27 7:24 ` Shawn Guo 2016-04-27 7:24 ` Shawn Guo 2016-04-27 7:26 ` Stefan Agner 2016-04-27 7:26 ` Stefan Agner 2016-04-27 8:48 ` Dong Aisheng 2016-04-27 8:48 ` Dong Aisheng 2016-04-27 8:48 ` Dong Aisheng 2016-04-27 7:34 ` Stefan Agner 2016-04-27 7:34 ` Stefan Agner 2016-04-27 8:57 ` Dong Aisheng 2016-04-27 8:57 ` Dong Aisheng 2016-04-27 8:57 ` Dong Aisheng 2016-04-27 10:15 ` Thomas Gleixner 2016-04-27 10:15 ` Thomas Gleixner 2016-04-27 10:15 ` Thomas Gleixner 2016-04-29 9:45 ` [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init Dong Aisheng 2016-04-29 9:45 ` Dong Aisheng 2016-04-29 9:55 ` Dong Aisheng 2016-04-29 9:55 ` Dong Aisheng 2016-04-29 12:31 ` Lucas Stach 2016-04-29 12:31 ` Lucas Stach 2016-04-29 12:31 ` Lucas Stach 2016-04-30 2:04 ` Stefan Agner 2016-04-30 2:04 ` Stefan Agner 2016-06-02 15:19 ` Dong Aisheng 2016-06-02 15:19 ` Dong Aisheng 2016-05-25 21:54 ` [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled Stefan Agner 2016-05-25 21:54 ` Stefan Agner 2016-06-02 14:59 ` Dong Aisheng 2016-06-02 14:59 ` Dong Aisheng 2016-06-02 14:59 ` Dong Aisheng 2016-06-06 13:20 ` Thomas Gleixner 2016-06-06 13:20 ` Thomas Gleixner 2016-06-06 13:20 ` Thomas Gleixner 2016-06-07 7:04 ` Dong Aisheng 2016-06-07 7:04 ` Dong Aisheng 2016-06-07 7:04 ` Dong Aisheng 2016-06-09 20:08 ` Thomas Gleixner 2016-06-09 20:08 ` Thomas Gleixner 2016-06-09 20:08 ` Thomas Gleixner 2016-06-09 22:14 ` Stefan Agner 2016-06-09 22:14 ` Stefan Agner 2016-06-09 22:55 ` Thomas Gleixner [this message] 2016-06-09 22:55 ` Thomas Gleixner 2016-06-12 12:24 ` Dong Aisheng 2016-06-12 12:24 ` Dong Aisheng 2016-06-12 12:24 ` Dong Aisheng
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.DEB.2.11.1606100046150.28031@nanos \ --to=tglx@linutronix.de \ --cc=dongas86@gmail.com \ --cc=kernel@pengutronix.de \ --cc=l.stach@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mingo@kernel.org \ --cc=mturquette@baylibre.com \ --cc=rjw@sisk.pl \ --cc=sboyd@codeaurora.org \ --cc=shawnguo@kernel.org \ --cc=stefan@agner.ch \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.