* [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en
@ 2009-10-27 2:23 Zhang Rui
2009-10-27 8:21 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Zhang Rui @ 2009-10-27 2:23 UTC (permalink / raw)
To: Brown, Len; +Cc: linux-acpi@vger.kernel.org, Zhang Rui, Rafael J. Wysocki
Introduce kernel parameter acpi_sleep=s3_set_sci_en
some laptop requires SCI_EN being set directly on resume,
or else they hung somewhere in the resume code path.
We already have a blacklist for these lattops but we still need
this option, especially when debugging some suspend/resume problems,
in case there are systems that need this workaround and are not yet
in the blacklist.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
Documentation/kernel-parameters.txt | 5 ++++-
arch/x86/kernel/acpi/sleep.c | 4 ++++
drivers/acpi/sleep.c | 6 ++++++
include/linux/acpi.h | 3 +++
4 files changed, 17 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -182,6 +182,12 @@ static void acpi_pm_end(void)
*/
static bool set_sci_en_on_resume;
+void __init acpi_set_sci_en_on_resume(void)
+{
+ set_sci_en_on_resume = true;
+ return 0;
+}
+
extern void do_suspend_lowlevel(void);
static u32 acpi_suspend_states[] = {
Index: linux-2.6/arch/x86/kernel/acpi/sleep.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c
+++ linux-2.6/arch/x86/kernel/acpi/sleep.c
@@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char
#endif
if (strncmp(str, "old_ordering", 12) == 0)
acpi_old_suspend_ordering();
+#ifdef CONFIG_SUSPEND
+ if (strncmp(str, "s3_set_sci_en", 13) == 0)
+ acpi_set_sci_en_on_resume();
+#endif /* CONFIG_SUSPEND */
str = strchr(str, ',');
if (str != NULL)
str += strspn(str, ", \t");
Index: linux-2.6/include/linux/acpi.h
===================================================================
--- linux-2.6.orig/include/linux/acpi.h
+++ linux-2.6/include/linux/acpi.h
@@ -251,6 +251,9 @@ int acpi_check_mem_region(resource_size_
void __init acpi_no_s4_hw_signature(void);
void __init acpi_old_suspend_ordering(void);
void __init acpi_s4_no_nvs(void);
+#ifdef CONFIG_SUSPEND
+void __init acpi_set_sci_en_on_resume(void);
+#endif /* CONFIG_SUSPEND*/
#endif /* CONFIG_PM_SLEEP */
#define OSC_QUERY_TYPE 0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -241,7 +241,7 @@ and is between 256 and 4096 characters.
acpi_sleep= [HW,ACPI] Sleep options
Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig,
- old_ordering, s4_nonvs }
+ old_ordering, s4_nonvs, s3_set_sci_en }
See Documentation/power/video.txt for information on
s3_bios and s3_mode.
s3_beep is for debugging; it makes the PC's speaker beep
@@ -254,6 +254,9 @@ and is between 256 and 4096 characters.
of _PTS is used by default).
s4_nonvs prevents the kernel from saving/restoring the
ACPI NVS memory during hibernation.
+ s3_set_sci_en causes the kernel to set SCI_EN directly
+ on resume from S3 (which is against the ACPI specification,
+ but some broken systems don't work without it).
acpi_use_timer_override [HW,ACPI]
Use timer override. For some broken Nvidia NF5 boards
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en 2009-10-27 2:23 [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en Zhang Rui @ 2009-10-27 8:21 ` Rafael J. Wysocki 2009-10-27 8:46 ` Zhang Rui 2009-10-28 1:36 ` Zhang Rui 0 siblings, 2 replies; 7+ messages in thread From: Rafael J. Wysocki @ 2009-10-27 8:21 UTC (permalink / raw) To: Zhang Rui; +Cc: Brown, Len, linux-acpi@vger.kernel.org On Tuesday 27 October 2009, Zhang Rui wrote: > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > some laptop requires SCI_EN being set directly on resume, > or else they hung somewhere in the resume code path. > > We already have a blacklist for these lattops but we still need > this option, especially when debugging some suspend/resume problems, > in case there are systems that need this workaround and are not yet > in the blacklist. > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> I'm generally fine with this, but ... > --- > Documentation/kernel-parameters.txt | 5 ++++- > arch/x86/kernel/acpi/sleep.c | 4 ++++ > drivers/acpi/sleep.c | 6 ++++++ > include/linux/acpi.h | 3 +++ > 4 files changed, 17 insertions(+), 1 deletion(-) > > Index: linux-2.6/drivers/acpi/sleep.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/sleep.c > +++ linux-2.6/drivers/acpi/sleep.c > @@ -182,6 +182,12 @@ static void acpi_pm_end(void) > */ > static bool set_sci_en_on_resume; > > +void __init acpi_set_sci_en_on_resume(void) > +{ > + set_sci_en_on_resume = true; > + return 0; > +} > + > extern void do_suspend_lowlevel(void); > > static u32 acpi_suspend_states[] = { > Index: linux-2.6/arch/x86/kernel/acpi/sleep.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c > +++ linux-2.6/arch/x86/kernel/acpi/sleep.c > @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char > #endif > if (strncmp(str, "old_ordering", 12) == 0) > acpi_old_suspend_ordering(); > +#ifdef CONFIG_SUSPEND > + if (strncmp(str, "s3_set_sci_en", 13) == 0) > + acpi_set_sci_en_on_resume(); > +#endif /* CONFIG_SUSPEND */ ... do we really need these #ifdefs? > str = strchr(str, ','); > if (str != NULL) > str += strspn(str, ", \t"); > Index: linux-2.6/include/linux/acpi.h > =================================================================== > --- linux-2.6.orig/include/linux/acpi.h > +++ linux-2.6/include/linux/acpi.h > @@ -251,6 +251,9 @@ int acpi_check_mem_region(resource_size_ > void __init acpi_no_s4_hw_signature(void); > void __init acpi_old_suspend_ordering(void); > void __init acpi_s4_no_nvs(void); > +#ifdef CONFIG_SUSPEND > +void __init acpi_set_sci_en_on_resume(void); > +#endif /* CONFIG_SUSPEND*/ s4_no_nvs is hibernation-specific, but it's not under a #ifdef. > #endif /* CONFIG_PM_SLEEP */ > > #define OSC_QUERY_TYPE 0 > Index: linux-2.6/Documentation/kernel-parameters.txt > =================================================================== > --- linux-2.6.orig/Documentation/kernel-parameters.txt > +++ linux-2.6/Documentation/kernel-parameters.txt > @@ -241,7 +241,7 @@ and is between 256 and 4096 characters. > > acpi_sleep= [HW,ACPI] Sleep options > Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig, > - old_ordering, s4_nonvs } > + old_ordering, s4_nonvs, s3_set_sci_en } > See Documentation/power/video.txt for information on > s3_bios and s3_mode. > s3_beep is for debugging; it makes the PC's speaker beep > @@ -254,6 +254,9 @@ and is between 256 and 4096 characters. > of _PTS is used by default). > s4_nonvs prevents the kernel from saving/restoring the > ACPI NVS memory during hibernation. > + s3_set_sci_en causes the kernel to set SCI_EN directly > + on resume from S3 (which is against the ACPI specification, > + but some broken systems don't work without it). > > acpi_use_timer_override [HW,ACPI] > Use timer override. For some broken Nvidia NF5 boards Thanks, Rafael ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en 2009-10-27 8:21 ` Rafael J. Wysocki @ 2009-10-27 8:46 ` Zhang Rui 2009-10-27 10:15 ` Rafael J. Wysocki 2009-10-28 1:36 ` Zhang Rui 1 sibling, 1 reply; 7+ messages in thread From: Zhang Rui @ 2009-10-27 8:46 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Brown, Len, linux-acpi@vger.kernel.org On Tue, 2009-10-27 at 16:21 +0800, Rafael J. Wysocki wrote: > On Tuesday 27 October 2009, Zhang Rui wrote: > > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > > > some laptop requires SCI_EN being set directly on resume, > > or else they hung somewhere in the resume code path. > > > > We already have a blacklist for these lattops but we still need > > this option, especially when debugging some suspend/resume problems, > > in case there are systems that need this workaround and are not yet > > in the blacklist. > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > I'm generally fine with this, but ... > > > --- > > Documentation/kernel-parameters.txt | 5 ++++- > > arch/x86/kernel/acpi/sleep.c | 4 ++++ > > drivers/acpi/sleep.c | 6 ++++++ > > include/linux/acpi.h | 3 +++ > > 4 files changed, 17 insertions(+), 1 deletion(-) > > > > Index: linux-2.6/drivers/acpi/sleep.c > > =================================================================== > > --- linux-2.6.orig/drivers/acpi/sleep.c > > +++ linux-2.6/drivers/acpi/sleep.c > > @@ -182,6 +182,12 @@ static void acpi_pm_end(void) > > */ > > static bool set_sci_en_on_resume; > > > > +void __init acpi_set_sci_en_on_resume(void) > > +{ > > + set_sci_en_on_resume = true; > > + return 0; > > +} > > + > > extern void do_suspend_lowlevel(void); > > > > static u32 acpi_suspend_states[] = { > > Index: linux-2.6/arch/x86/kernel/acpi/sleep.c > > =================================================================== > > --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c > > +++ linux-2.6/arch/x86/kernel/acpi/sleep.c > > @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char > > #endif > > if (strncmp(str, "old_ordering", 12) == 0) > > acpi_old_suspend_ordering(); > > +#ifdef CONFIG_SUSPEND > > + if (strncmp(str, "s3_set_sci_en", 13) == 0) > > + acpi_set_sci_en_on_resume(); > > +#endif /* CONFIG_SUSPEND */ > > ... do we really need these #ifdefs? > #ifdef CONFIG_SUSPEND ... static bool set_sci_en_on_resume; ... #endif /* CONFIG_SUSPEND */ Or else I have to move set_sci_en_on_resume out of this #ifdef. both solutions are okay to me but I think the current one is cleaner. thanks, rui > > str = strchr(str, ','); > > if (str != NULL) > > str += strspn(str, ", \t"); > > Index: linux-2.6/include/linux/acpi.h > > =================================================================== > > --- linux-2.6.orig/include/linux/acpi.h > > +++ linux-2.6/include/linux/acpi.h > > @@ -251,6 +251,9 @@ int acpi_check_mem_region(resource_size_ > > void __init acpi_no_s4_hw_signature(void); > > void __init acpi_old_suspend_ordering(void); > > void __init acpi_s4_no_nvs(void); > > +#ifdef CONFIG_SUSPEND > > +void __init acpi_set_sci_en_on_resume(void); > > +#endif /* CONFIG_SUSPEND*/ > > s4_no_nvs is hibernation-specific, but it's not under a #ifdef. > > > #endif /* CONFIG_PM_SLEEP */ > > > > #define OSC_QUERY_TYPE 0 > > Index: linux-2.6/Documentation/kernel-parameters.txt > > =================================================================== > > --- linux-2.6.orig/Documentation/kernel-parameters.txt > > +++ linux-2.6/Documentation/kernel-parameters.txt > > @@ -241,7 +241,7 @@ and is between 256 and 4096 characters. > > > > acpi_sleep= [HW,ACPI] Sleep options > > Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig, > > - old_ordering, s4_nonvs } > > + old_ordering, s4_nonvs, s3_set_sci_en } > > See Documentation/power/video.txt for information on > > s3_bios and s3_mode. > > s3_beep is for debugging; it makes the PC's speaker beep > > @@ -254,6 +254,9 @@ and is between 256 and 4096 characters. > > of _PTS is used by default). > > s4_nonvs prevents the kernel from saving/restoring the > > ACPI NVS memory during hibernation. > > + s3_set_sci_en causes the kernel to set SCI_EN directly > > + on resume from S3 (which is against the ACPI specification, > > + but some broken systems don't work without it). > > > > acpi_use_timer_override [HW,ACPI] > > Use timer override. For some broken Nvidia NF5 boards > > Thanks, > Rafael ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en 2009-10-27 8:46 ` Zhang Rui @ 2009-10-27 10:15 ` Rafael J. Wysocki 0 siblings, 0 replies; 7+ messages in thread From: Rafael J. Wysocki @ 2009-10-27 10:15 UTC (permalink / raw) To: Zhang Rui; +Cc: Brown, Len, linux-acpi@vger.kernel.org On Tuesday 27 October 2009, Zhang Rui wrote: > On Tue, 2009-10-27 at 16:21 +0800, Rafael J. Wysocki wrote: > > On Tuesday 27 October 2009, Zhang Rui wrote: > > > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > > > > > some laptop requires SCI_EN being set directly on resume, > > > or else they hung somewhere in the resume code path. > > > > > > We already have a blacklist for these lattops but we still need > > > this option, especially when debugging some suspend/resume problems, > > > in case there are systems that need this workaround and are not yet > > > in the blacklist. > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > I'm generally fine with this, but ... > > > > > --- > > > Documentation/kernel-parameters.txt | 5 ++++- > > > arch/x86/kernel/acpi/sleep.c | 4 ++++ > > > drivers/acpi/sleep.c | 6 ++++++ > > > include/linux/acpi.h | 3 +++ > > > 4 files changed, 17 insertions(+), 1 deletion(-) > > > > > > Index: linux-2.6/drivers/acpi/sleep.c > > > =================================================================== > > > --- linux-2.6.orig/drivers/acpi/sleep.c > > > +++ linux-2.6/drivers/acpi/sleep.c > > > @@ -182,6 +182,12 @@ static void acpi_pm_end(void) > > > */ > > > static bool set_sci_en_on_resume; > > > > > > +void __init acpi_set_sci_en_on_resume(void) > > > +{ > > > + set_sci_en_on_resume = true; > > > + return 0; > > > +} > > > + > > > extern void do_suspend_lowlevel(void); > > > > > > static u32 acpi_suspend_states[] = { > > > Index: linux-2.6/arch/x86/kernel/acpi/sleep.c > > > =================================================================== > > > --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c > > > +++ linux-2.6/arch/x86/kernel/acpi/sleep.c > > > @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char > > > #endif > > > if (strncmp(str, "old_ordering", 12) == 0) > > > acpi_old_suspend_ordering(); > > > +#ifdef CONFIG_SUSPEND > > > + if (strncmp(str, "s3_set_sci_en", 13) == 0) > > > + acpi_set_sci_en_on_resume(); > > > +#endif /* CONFIG_SUSPEND */ > > > > ... do we really need these #ifdefs? > > > > #ifdef CONFIG_SUSPEND > ... > static bool set_sci_en_on_resume; > ... > #endif /* CONFIG_SUSPEND */ > > Or else I have to move set_sci_en_on_resume out of this #ifdef. Please do. Best, Rafael ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en 2009-10-27 8:21 ` Rafael J. Wysocki 2009-10-27 8:46 ` Zhang Rui @ 2009-10-28 1:36 ` Zhang Rui 2009-10-28 18:52 ` Rafael J. Wysocki 1 sibling, 1 reply; 7+ messages in thread From: Zhang Rui @ 2009-10-28 1:36 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Brown, Len, linux-acpi@vger.kernel.org On Tue, 2009-10-27 at 16:21 +0800, Rafael J. Wysocki wrote: > On Tuesday 27 October 2009, Zhang Rui wrote: > > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > > > some laptop requires SCI_EN being set directly on resume, > > or else they hung somewhere in the resume code path. > > > > We already have a blacklist for these lattops but we still need > > this option, especially when debugging some suspend/resume problems, > > in case there are systems that need this workaround and are not yet > > in the blacklist. > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > I'm generally fine with this, but ... > > > --- > > Documentation/kernel-parameters.txt | 5 ++++- > > arch/x86/kernel/acpi/sleep.c | 4 ++++ > > drivers/acpi/sleep.c | 6 ++++++ > > include/linux/acpi.h | 3 +++ > > 4 files changed, 17 insertions(+), 1 deletion(-) > > > > Index: linux-2.6/drivers/acpi/sleep.c > > =================================================================== > > --- linux-2.6.orig/drivers/acpi/sleep.c > > +++ linux-2.6/drivers/acpi/sleep.c > > @@ -182,6 +182,12 @@ static void acpi_pm_end(void) > > */ > > static bool set_sci_en_on_resume; > > > > +void __init acpi_set_sci_en_on_resume(void) > > +{ > > + set_sci_en_on_resume = true; > > + return 0; > > +} > > + > > extern void do_suspend_lowlevel(void); > > > > static u32 acpi_suspend_states[] = { > > Index: linux-2.6/arch/x86/kernel/acpi/sleep.c > > =================================================================== > > --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c > > +++ linux-2.6/arch/x86/kernel/acpi/sleep.c > > @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char > > #endif > > if (strncmp(str, "old_ordering", 12) == 0) > > acpi_old_suspend_ordering(); > > +#ifdef CONFIG_SUSPEND > > + if (strncmp(str, "s3_set_sci_en", 13) == 0) > > + acpi_set_sci_en_on_resume(); > > +#endif /* CONFIG_SUSPEND */ > > ... do we really need these #ifdefs? > yes, we do. acpi_s4_no_nvs also uses #ifdef here. > > str = strchr(str, ','); > > if (str != NULL) > > str += strspn(str, ", \t"); > > Index: linux-2.6/include/linux/acpi.h > > =================================================================== > > --- linux-2.6.orig/include/linux/acpi.h > > +++ linux-2.6/include/linux/acpi.h > > @@ -251,6 +251,9 @@ int acpi_check_mem_region(resource_size_ > > void __init acpi_no_s4_hw_signature(void); > > void __init acpi_old_suspend_ordering(void); > > void __init acpi_s4_no_nvs(void); > > +#ifdef CONFIG_SUSPEND > > +void __init acpi_set_sci_en_on_resume(void); > > +#endif /* CONFIG_SUSPEND*/ > > s4_no_nvs is hibernation-specific, but it's not under a #ifdef. > it's okay to remove this #ifdef. refreshed patch attached. Introduce kernel parameter acpi_sleep=s3_set_sci_en some laptop requires SCI_EN being set directly on resume, or else they hung somewhere in the resume code path. We already have a blacklist for these lattops but we still need this option, especially when debugging some suspend/resume problems, in case there are systems that need this workaround and are not yet in the blacklist. Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- Documentation/kernel-parameters.txt | 5 ++++- arch/x86/kernel/acpi/sleep.c | 4 ++++ drivers/acpi/sleep.c | 6 ++++++ include/linux/acpi.h | 1 + 4 files changed, 15 insertions(+), 1 deletion(-) Index: linux-2.6/drivers/acpi/sleep.c =================================================================== --- linux-2.6.orig/drivers/acpi/sleep.c +++ linux-2.6/drivers/acpi/sleep.c @@ -182,6 +182,12 @@ static void acpi_pm_end(void) */ static bool set_sci_en_on_resume; +void __init acpi_set_sci_en_on_resume(void) +{ + set_sci_en_on_resume = true; + return 0; +} + extern void do_suspend_lowlevel(void); static u32 acpi_suspend_states[] = { Index: linux-2.6/arch/x86/kernel/acpi/sleep.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c +++ linux-2.6/arch/x86/kernel/acpi/sleep.c @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char #endif if (strncmp(str, "old_ordering", 12) == 0) acpi_old_suspend_ordering(); +#ifdef CONFIG_SUSPEND + if (strncmp(str, "s3_set_sci_en", 13) == 0) + acpi_set_sci_en_on_resume(); +#endif /* CONFIG_SUSPEND */ str = strchr(str, ','); if (str != NULL) str += strspn(str, ", \t"); Index: linux-2.6/include/linux/acpi.h =================================================================== --- linux-2.6.orig/include/linux/acpi.h +++ linux-2.6/include/linux/acpi.h @@ -251,6 +251,7 @@ int acpi_check_mem_region(resource_size_ void __init acpi_no_s4_hw_signature(void); void __init acpi_old_suspend_ordering(void); void __init acpi_s4_no_nvs(void); +void __init acpi_set_sci_en_on_resume(void); #endif /* CONFIG_PM_SLEEP */ #define OSC_QUERY_TYPE 0 Index: linux-2.6/Documentation/kernel-parameters.txt =================================================================== --- linux-2.6.orig/Documentation/kernel-parameters.txt +++ linux-2.6/Documentation/kernel-parameters.txt @@ -241,7 +241,7 @@ and is between 256 and 4096 characters. acpi_sleep= [HW,ACPI] Sleep options Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig, - old_ordering, s4_nonvs } + old_ordering, s4_nonvs, s3_set_sci_en } See Documentation/power/video.txt for information on s3_bios and s3_mode. s3_beep is for debugging; it makes the PC's speaker beep @@ -254,6 +254,9 @@ and is between 256 and 4096 characters. of _PTS is used by default). s4_nonvs prevents the kernel from saving/restoring the ACPI NVS memory during hibernation. + s3_set_sci_en causes the kernel to set SCI_EN directly + on resume from S3 (which is against the ACPI specification, + but some broken systems don't work without it). acpi_use_timer_override [HW,ACPI] Use timer override. For some broken Nvidia NF5 boards ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en 2009-10-28 1:36 ` Zhang Rui @ 2009-10-28 18:52 ` Rafael J. Wysocki 2009-12-30 7:04 ` Len Brown 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2009-10-28 18:52 UTC (permalink / raw) To: Zhang Rui, Brown, Len; +Cc: linux-acpi@vger.kernel.org On Wednesday 28 October 2009, Zhang Rui wrote: > On Tue, 2009-10-27 at 16:21 +0800, Rafael J. Wysocki wrote: > > On Tuesday 27 October 2009, Zhang Rui wrote: > > > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > > > > > some laptop requires SCI_EN being set directly on resume, > > > or else they hung somewhere in the resume code path. > > > > > > We already have a blacklist for these lattops but we still need > > > this option, especially when debugging some suspend/resume problems, > > > in case there are systems that need this workaround and are not yet > > > in the blacklist. > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > I'm generally fine with this, but ... > > > > > --- > > > Documentation/kernel-parameters.txt | 5 ++++- > > > arch/x86/kernel/acpi/sleep.c | 4 ++++ > > > drivers/acpi/sleep.c | 6 ++++++ > > > include/linux/acpi.h | 3 +++ > > > 4 files changed, 17 insertions(+), 1 deletion(-) > > > > > > Index: linux-2.6/drivers/acpi/sleep.c > > > =================================================================== > > > --- linux-2.6.orig/drivers/acpi/sleep.c > > > +++ linux-2.6/drivers/acpi/sleep.c > > > @@ -182,6 +182,12 @@ static void acpi_pm_end(void) > > > */ > > > static bool set_sci_en_on_resume; > > > > > > +void __init acpi_set_sci_en_on_resume(void) > > > +{ > > > + set_sci_en_on_resume = true; > > > + return 0; > > > +} > > > + > > > extern void do_suspend_lowlevel(void); > > > > > > static u32 acpi_suspend_states[] = { > > > Index: linux-2.6/arch/x86/kernel/acpi/sleep.c > > > =================================================================== > > > --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c > > > +++ linux-2.6/arch/x86/kernel/acpi/sleep.c > > > @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char > > > #endif > > > if (strncmp(str, "old_ordering", 12) == 0) > > > acpi_old_suspend_ordering(); > > > +#ifdef CONFIG_SUSPEND > > > + if (strncmp(str, "s3_set_sci_en", 13) == 0) > > > + acpi_set_sci_en_on_resume(); > > > +#endif /* CONFIG_SUSPEND */ > > > > ... do we really need these #ifdefs? > > > yes, we do. > acpi_s4_no_nvs also uses #ifdef here. > > > > str = strchr(str, ','); > > > if (str != NULL) > > > str += strspn(str, ", \t"); > > > Index: linux-2.6/include/linux/acpi.h > > > =================================================================== > > > --- linux-2.6.orig/include/linux/acpi.h > > > +++ linux-2.6/include/linux/acpi.h > > > @@ -251,6 +251,9 @@ int acpi_check_mem_region(resource_size_ > > > void __init acpi_no_s4_hw_signature(void); > > > void __init acpi_old_suspend_ordering(void); > > > void __init acpi_s4_no_nvs(void); > > > +#ifdef CONFIG_SUSPEND > > > +void __init acpi_set_sci_en_on_resume(void); > > > +#endif /* CONFIG_SUSPEND*/ > > > > s4_no_nvs is hibernation-specific, but it's not under a #ifdef. > > > it's okay to remove this #ifdef. > > refreshed patch attached. > > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > some laptop requires SCI_EN being set directly on resume, > or else they hung somewhere in the resume code path. > > We already have a blacklist for these lattops but we still need > this option, especially when debugging some suspend/resume problems, > in case there are systems that need this workaround and are not yet > in the blacklist. > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> This one looks better. OK, Len, are you going to apply it or do you want me to handle it? Rafael > --- > Documentation/kernel-parameters.txt | 5 ++++- > arch/x86/kernel/acpi/sleep.c | 4 ++++ > drivers/acpi/sleep.c | 6 ++++++ > include/linux/acpi.h | 1 + > 4 files changed, 15 insertions(+), 1 deletion(-) > > Index: linux-2.6/drivers/acpi/sleep.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/sleep.c > +++ linux-2.6/drivers/acpi/sleep.c > @@ -182,6 +182,12 @@ static void acpi_pm_end(void) > */ > static bool set_sci_en_on_resume; > > +void __init acpi_set_sci_en_on_resume(void) > +{ > + set_sci_en_on_resume = true; > + return 0; > +} > + > extern void do_suspend_lowlevel(void); > > static u32 acpi_suspend_states[] = { > Index: linux-2.6/arch/x86/kernel/acpi/sleep.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c > +++ linux-2.6/arch/x86/kernel/acpi/sleep.c > @@ -162,6 +162,10 @@ static int __init acpi_sleep_setup(char > #endif > if (strncmp(str, "old_ordering", 12) == 0) > acpi_old_suspend_ordering(); > +#ifdef CONFIG_SUSPEND > + if (strncmp(str, "s3_set_sci_en", 13) == 0) > + acpi_set_sci_en_on_resume(); > +#endif /* CONFIG_SUSPEND */ > str = strchr(str, ','); > if (str != NULL) > str += strspn(str, ", \t"); > Index: linux-2.6/include/linux/acpi.h > =================================================================== > --- linux-2.6.orig/include/linux/acpi.h > +++ linux-2.6/include/linux/acpi.h > @@ -251,6 +251,7 @@ int acpi_check_mem_region(resource_size_ > void __init acpi_no_s4_hw_signature(void); > void __init acpi_old_suspend_ordering(void); > void __init acpi_s4_no_nvs(void); > +void __init acpi_set_sci_en_on_resume(void); > #endif /* CONFIG_PM_SLEEP */ > > #define OSC_QUERY_TYPE 0 > Index: linux-2.6/Documentation/kernel-parameters.txt > =================================================================== > --- linux-2.6.orig/Documentation/kernel-parameters.txt > +++ linux-2.6/Documentation/kernel-parameters.txt > @@ -241,7 +241,7 @@ and is between 256 and 4096 characters. > > acpi_sleep= [HW,ACPI] Sleep options > Format: { s3_bios, s3_mode, s3_beep, s4_nohwsig, > - old_ordering, s4_nonvs } > + old_ordering, s4_nonvs, s3_set_sci_en } > See Documentation/power/video.txt for information on > s3_bios and s3_mode. > s3_beep is for debugging; it makes the PC's speaker beep > @@ -254,6 +254,9 @@ and is between 256 and 4096 characters. > of _PTS is used by default). > s4_nonvs prevents the kernel from saving/restoring the > ACPI NVS memory during hibernation. > + s3_set_sci_en causes the kernel to set SCI_EN directly > + on resume from S3 (which is against the ACPI specification, > + but some broken systems don't work without it). > > acpi_use_timer_override [HW,ACPI] > Use timer override. For some broken Nvidia NF5 boards > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en 2009-10-28 18:52 ` Rafael J. Wysocki @ 2009-12-30 7:04 ` Len Brown 0 siblings, 0 replies; 7+ messages in thread From: Len Brown @ 2009-12-30 7:04 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Zhang Rui, linux-acpi@vger.kernel.org > > > > Introduce kernel parameter acpi_sleep=s3_set_sci_en > > > > > > > > some laptop requires SCI_EN being set directly on resume, > > > > or else they hung somewhere in the resume code path. > > > > > > > > We already have a blacklist for these lattops but we still need > > > > this option, especially when debugging some suspend/resume problems, > > > > in case there are systems that need this workaround and are not yet > > > > in the blacklist. > > > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> ... > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > This one looks better. > > OK, Len, are you going to apply it or do you want me to handle it? As this isn't actually S3 specific, lets give this a better name, say acpi_sleep=sci_force_enable what you think? thanks, -Len Brown, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-12-30 7:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-10-27 2:23 [PATCH] ACPI: introduce kernel parameter acpi_sleep=s3_set_sci_en Zhang Rui 2009-10-27 8:21 ` Rafael J. Wysocki 2009-10-27 8:46 ` Zhang Rui 2009-10-27 10:15 ` Rafael J. Wysocki 2009-10-28 1:36 ` Zhang Rui 2009-10-28 18:52 ` Rafael J. Wysocki 2009-12-30 7:04 ` Len Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox