From: Graeme Gregory graeme.gregory@linaro.org
ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() and acpi_sleep_init() for device power management, so introduce CONFIG_ACPI_GENERIC_SLEEP and select it for x86 and ia64 only to make sleep functions available, and also introduce stub function to allow other drivers to function until S states are defined for ARM64.
It will be no functional change for x86 and IA64.
CC: Rafael J. Wysocki rjw@rjwysocki.net Suggested-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Acked-by: Robert Richter rrichter@cavium.com Acked-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/ia64/Kconfig | 1 + arch/x86/Kconfig | 1 + drivers/acpi/Kconfig | 4 ++++ drivers/acpi/Makefile | 2 +- drivers/acpi/internal.h | 4 ++++ 5 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM) + select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y + select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS select ARCH_HAS_FAST_MULTIPLIER diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index e6c3ddd..a7b9120 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -48,9 +48,13 @@ config ACPI_LEGACY_TABLES_LOOKUP config ARCH_MIGHT_HAVE_ACPI_PDC bool
+config ACPI_GENERIC_SLEEP + bool + config ACPI_SLEEP bool depends on SUSPEND || HIBERNATION + depends on ACPI_GENERIC_SLEEP default y
config ACPI_PROCFS_POWER diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 623b117..2397822 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -23,7 +23,7 @@ acpi-y += nvs.o
# Power management related files acpi-y += wakeup.o -acpi-y += sleep.o +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o acpi-y += device_pm.o acpi-$(CONFIG_ACPI_SLEEP) += proc.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 56b321a..6f08c85 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -161,7 +161,11 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit); /*-------------------------------------------------------------------------- Suspend/Resume -------------------------------------------------------------------------- */ +#ifdef CONFIG_ACPI_GENERIC_SLEEP extern int acpi_sleep_init(void); +#else +static inline int acpi_sleep_init(void) { return -ENXIO; } +#endif
#ifdef CONFIG_ACPI_SLEEP int acpi_sleep_proc_init(void);
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() and acpi_sleep_init() for device power management, so introduce CONFIG_ACPI_GENERIC_SLEEP and select it for x86 and ia64 only to make sleep functions available, and also introduce stub function to allow other drivers to function until S states are defined for ARM64.
It will be no functional change for x86 and IA64.
CC: Rafael J. Wysocki rjw@rjwysocki.net Suggested-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Acked-by: Robert Richter rrichter@cavium.com Acked-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/ia64/Kconfig | 1 + arch/x86/Kconfig | 1 + drivers/acpi/Kconfig | 4 ++++ drivers/acpi/Makefile | 2 +- drivers/acpi/internal.h | 4 ++++ 5 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM)
- select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y
- select ACPI_GENERIC_SLEEP if ACPI
One more nit. If you did
+ select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS select ARCH_HAS_FAST_MULTIPLIER diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index e6c3ddd..a7b9120 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -48,9 +48,13 @@ config ACPI_LEGACY_TABLES_LOOKUP config ARCH_MIGHT_HAVE_ACPI_PDC bool +config ACPI_GENERIC_SLEEP
- bool
config ACPI_SLEEP bool depends on SUSPEND || HIBERNATION
- depends on ACPI_GENERIC_SLEEP default y
config ACPI_PROCFS_POWER diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 623b117..2397822 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -23,7 +23,7 @@ acpi-y += nvs.o # Power management related files acpi-y += wakeup.o -acpi-y += sleep.o +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o acpi-y += device_pm.o acpi-$(CONFIG_ACPI_SLEEP) += proc.o diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 56b321a..6f08c85 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -161,7 +161,11 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit); /*-------------------------------------------------------------------------- Suspend/Resume -------------------------------------------------------------------------- */ +#ifdef CONFIG_ACPI_GENERIC_SLEEP extern int acpi_sleep_init(void); +#else +static inline int acpi_sleep_init(void) { return -ENXIO; } +#endif #ifdef CONFIG_ACPI_SLEEP int acpi_sleep_proc_init(void);
On 2015年03月14日 05:49, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() and acpi_sleep_init() for device power management, so introduce CONFIG_ACPI_GENERIC_SLEEP and select it for x86 and ia64 only to make sleep functions available, and also introduce stub function to allow other drivers to function until S states are defined for ARM64.
It will be no functional change for x86 and IA64.
CC: Rafael J. Wysocki rjw@rjwysocki.net Suggested-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Acked-by: Robert Richter rrichter@cavium.com Acked-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/ia64/Kconfig | 1 + arch/x86/Kconfig | 1 + drivers/acpi/Kconfig | 4 ++++ drivers/acpi/Makefile | 2 +- drivers/acpi/internal.h | 4 ++++ 5 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM)
- select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y
- select ACPI_GENERIC_SLEEP if ACPI
One more nit. If you did
- select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
Thanks Hanjun
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote:
On 2015年03月14日 05:49, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() and acpi_sleep_init() for device power management, so introduce CONFIG_ACPI_GENERIC_SLEEP and select it for x86 and ia64 only to make sleep functions available, and also introduce stub function to allow other drivers to function until S states are defined for ARM64.
It will be no functional change for x86 and IA64.
CC: Rafael J. Wysocki rjw@rjwysocki.net Suggested-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Acked-by: Robert Richter rrichter@cavium.com Acked-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/ia64/Kconfig | 1 + arch/x86/Kconfig | 1 + drivers/acpi/Kconfig | 4 ++++ drivers/acpi/Makefile | 2 +- drivers/acpi/internal.h | 4 ++++ 5 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM)
- select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y
- select ACPI_GENERIC_SLEEP if ACPI
One more nit. If you did
- select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
On 2015/3/17 7:15, Rafael J. Wysocki wrote:
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote:
On 2015年03月14日 05:49, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
[...]
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM)
- select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y
- select ACPI_GENERIC_SLEEP if ACPI
One more nit. If you did
- select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Thanks Hanjun
On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote:
On 2015/3/17 7:15, Rafael J. Wysocki wrote:
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote:
On 2015年03月14日 05:49, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
[...]
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM)
- select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y
- select ACPI_GENERIC_SLEEP if ACPI
One more nit. If you did
- select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Well, so what about selecting ACPI_SLEEP from the architectures that use it?
On 2015/3/17 10:28, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote:
On 2015/3/17 7:15, Rafael J. Wysocki wrote:
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote:
On 2015年03月14日 05:49, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
[...]
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index 074e52b..e8728d7 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -10,6 +10,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select PCI if (!IA64_HP_SIM) select ACPI if (!IA64_HP_SIM)
- select ACPI_GENERIC_SLEEP if ACPI select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_IDE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b7d31ca..9804431 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -22,6 +22,7 @@ config X86_64 ### Arch settings config X86 def_bool y
- select ACPI_GENERIC_SLEEP if ACPI
One more nit. If you did
- select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Well, so what about selecting ACPI_SLEEP from the architectures that use it?
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Thanks Hanjun
On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote:
On 2015/3/17 10:28, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote:
On 2015/3/17 7:15, Rafael J. Wysocki wrote:
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote:
On 2015年03月14日 05:49, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
[...]
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig > index 074e52b..e8728d7 100644 > --- a/arch/ia64/Kconfig > +++ b/arch/ia64/Kconfig > @@ -10,6 +10,7 @@ config IA64 > select ARCH_MIGHT_HAVE_PC_SERIO > select PCI if (!IA64_HP_SIM) > select ACPI if (!IA64_HP_SIM) > + select ACPI_GENERIC_SLEEP if ACPI > select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI > select HAVE_UNSTABLE_SCHED_CLOCK > select HAVE_IDE > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index b7d31ca..9804431 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -22,6 +22,7 @@ config X86_64 > ### Arch settings > config X86 > def_bool y > + select ACPI_GENERIC_SLEEP if ACPI One more nit. If you did
- select ACPI_GENERIC_SLEEP if ACPI_SLEEP
here (and above for ia64), you'd avoid having to make ACPI_SLEEP depend on ACPI_GENERIC_SLEEP which goes somewhat backwards.
In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Well, so what about selecting ACPI_SLEEP from the architectures that use it?
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the only arch setting it at least for the time being, is that correct?
On 2015/3/17 11:23, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote:
On 2015/3/17 10:28, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote:
On 2015/3/17 7:15, Rafael J. Wysocki wrote:
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote:
On 2015年03月14日 05:49, Rafael J. Wysocki wrote: > On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
[...]
>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >> index 074e52b..e8728d7 100644 >> --- a/arch/ia64/Kconfig >> +++ b/arch/ia64/Kconfig >> @@ -10,6 +10,7 @@ config IA64 >> select ARCH_MIGHT_HAVE_PC_SERIO >> select PCI if (!IA64_HP_SIM) >> select ACPI if (!IA64_HP_SIM) >> + select ACPI_GENERIC_SLEEP if ACPI >> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >> select HAVE_UNSTABLE_SCHED_CLOCK >> select HAVE_IDE >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index b7d31ca..9804431 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -22,6 +22,7 @@ config X86_64 >> ### Arch settings >> config X86 >> def_bool y >> + select ACPI_GENERIC_SLEEP if ACPI > One more nit. If you did > > + select ACPI_GENERIC_SLEEP if ACPI_SLEEP > > here (and above for ia64), you'd avoid having to make ACPI_SLEEP > depend on ACPI_GENERIC_SLEEP which goes somewhat backwards. In sleep.c,
#ifdef CONFIG_ACPI_SLEEP acpi_target_system_state() { } #endif
and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, which one of them will be enabled on ARM64 so ACPI_SLEEP will also enabled too.
So if we
+select ACPI_GENERIC_SLEEP if ACPI_SLEEP
and
+acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o
it will lead to errors for acpi_target_system_state() that is declared but not defined, so I will keep the code as it is, what do you think?
No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Well, so what about selecting ACPI_SLEEP from the architectures that use it?
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Yes, you are right.
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the
Sorry, I can't fully understand your intention here, could you please explain it more?
Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know.
only arch setting it at least for the time being, is that correct?
That's pretty sure for now.
Thanks Hanjun
On 03/17/2015 12:10 AM, Hanjun Guo wrote:
On 2015/3/17 11:23, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote:
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Yes, you are right.
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the
Sorry, I can't fully understand your intention here, could you please explain it more?
Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know.
If helpful to the discussion, current SBBR (Server Base Boot Requirements[0]) design guidance is that for power off itself, we will prefer calling an EFI Runtime Service (that will preferentially call an PSCI - ARM Power State Coordination Interface - Secure Monitor Call (SMC - think SMI-like) internally to perform the shutdown/reboot) for the action of powering off or resetting 64-bit ARM SBBR platforms.
Therefore if the alternative of an ACPI-based power off solution were not initially supported, I don't think it would have much practical impact, and it could be addressed after the initial support merged.
That's just my $0.02.
Jon.
[0] The Server Base Boot Requirements are the platform specification we created for 64-bit ARM servers. They mandate the use of EFI and ACPI in compliant platforms: http://infocenter.arm.com/help/topic/com.arm.doc.den0044a/Server_Base_Boot_R...
On 2015/3/17 13:59, Jon Masters wrote:
On 03/17/2015 12:10 AM, Hanjun Guo wrote:
On 2015/3/17 11:23, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote: Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Yes, you are right.
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the
Sorry, I can't fully understand your intention here, could you please explain it more?
Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know.
If helpful to the discussion, current SBBR (Server Base Boot Requirements[0]) design guidance is that for power off itself, we will prefer calling an EFI Runtime Service (that will preferentially call an PSCI - ARM Power State Coordination Interface - Secure Monitor Call (SMC
- think SMI-like) internally to perform the shutdown/reboot) for the
action of powering off or resetting 64-bit ARM SBBR platforms.
Agreed, PSCI is the prefer method for power off on ARM64 I think.
Therefore if the alternative of an ACPI-based power off solution were not initially supported, I don't think it would have much practical impact, and it could be addressed after the initial support merged.
I agree. Actually we already removed ACPI power off code for ARM64 in v9 and v10 regardless the ACPI sepc statement about S5, I just want to confirm with Rafael that how to use ACPI_REDUCED_HARDWARE_ONLY properly to do the same thing as we do in v10 (patch - ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP), or if we have some other way to do that :)
Thanks Hanjun
On Tuesday, March 17, 2015 12:10:02 PM Hanjun Guo wrote:
On 2015/3/17 11:23, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote:
On 2015/3/17 10:28, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote:
On 2015/3/17 7:15, Rafael J. Wysocki wrote:
On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote: > On 2015年03月14日 05:49, Rafael J. Wysocki wrote: >> On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote:
[...]
>>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >>> index 074e52b..e8728d7 100644 >>> --- a/arch/ia64/Kconfig >>> +++ b/arch/ia64/Kconfig >>> @@ -10,6 +10,7 @@ config IA64 >>> select ARCH_MIGHT_HAVE_PC_SERIO >>> select PCI if (!IA64_HP_SIM) >>> select ACPI if (!IA64_HP_SIM) >>> + select ACPI_GENERIC_SLEEP if ACPI >>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >>> select HAVE_UNSTABLE_SCHED_CLOCK >>> select HAVE_IDE >>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>> index b7d31ca..9804431 100644 >>> --- a/arch/x86/Kconfig >>> +++ b/arch/x86/Kconfig >>> @@ -22,6 +22,7 @@ config X86_64 >>> ### Arch settings >>> config X86 >>> def_bool y >>> + select ACPI_GENERIC_SLEEP if ACPI >> One more nit. If you did >> >> + select ACPI_GENERIC_SLEEP if ACPI_SLEEP >> >> here (and above for ia64), you'd avoid having to make ACPI_SLEEP >> depend on ACPI_GENERIC_SLEEP which goes somewhat backwards. > In sleep.c, > > #ifdef CONFIG_ACPI_SLEEP > acpi_target_system_state() > { > } > #endif > > and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, > which one of them will be enabled on ARM64 so ACPI_SLEEP > will also enabled too. > > So if we > > +select ACPI_GENERIC_SLEEP if ACPI_SLEEP > > and > > +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o > > it will lead to errors for acpi_target_system_state() that > is declared but not defined, so I will keep the code as > it is, what do you think? No, we need to hash this out. Having two different Kconfig options meaning almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly.
Do you need ACPI_SLEEP on ARM64 at all?
No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Well, so what about selecting ACPI_SLEEP from the architectures that use it?
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Yes, you are right.
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the
Sorry, I can't fully understand your intention here, could you please explain it more?
Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know.
OK, so in your current patch, please replace ACPI_GENERIC_SLEEP with ACPI_SYSTEM_POWER_STATES_SUPPORT and all should be clear.
On 2015/3/17 22:33, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 12:10:02 PM Hanjun Guo wrote:
On 2015/3/17 11:23, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote:
On 2015/3/17 10:28, Rafael J. Wysocki wrote:
On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote:
On 2015/3/17 7:15, Rafael J. Wysocki wrote: > On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote: >> On 2015年03月14日 05:49, Rafael J. Wysocki wrote: >>> On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote: [...] >>>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >>>> index 074e52b..e8728d7 100644 >>>> --- a/arch/ia64/Kconfig >>>> +++ b/arch/ia64/Kconfig >>>> @@ -10,6 +10,7 @@ config IA64 >>>> select ARCH_MIGHT_HAVE_PC_SERIO >>>> select PCI if (!IA64_HP_SIM) >>>> select ACPI if (!IA64_HP_SIM) >>>> + select ACPI_GENERIC_SLEEP if ACPI >>>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >>>> select HAVE_UNSTABLE_SCHED_CLOCK >>>> select HAVE_IDE >>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>>> index b7d31ca..9804431 100644 >>>> --- a/arch/x86/Kconfig >>>> +++ b/arch/x86/Kconfig >>>> @@ -22,6 +22,7 @@ config X86_64 >>>> ### Arch settings >>>> config X86 >>>> def_bool y >>>> + select ACPI_GENERIC_SLEEP if ACPI >>> One more nit. If you did >>> >>> + select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>> >>> here (and above for ia64), you'd avoid having to make ACPI_SLEEP >>> depend on ACPI_GENERIC_SLEEP which goes somewhat backwards. >> In sleep.c, >> >> #ifdef CONFIG_ACPI_SLEEP >> acpi_target_system_state() >> { >> } >> #endif >> >> and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, >> which one of them will be enabled on ARM64 so ACPI_SLEEP >> will also enabled too. >> >> So if we >> >> +select ACPI_GENERIC_SLEEP if ACPI_SLEEP >> >> and >> >> +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o >> >> it will lead to errors for acpi_target_system_state() that >> is declared but not defined, so I will keep the code as >> it is, what do you think? > No, we need to hash this out. Having two different Kconfig options meaning > almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly. > > Do you need ACPI_SLEEP on ARM64 at all? No, at least for now we don't need it, the spec for sleep is not ready for ARM64 arch, so ACPI_SLEEP will not work at all on ARM64.
Well, so what about selecting ACPI_SLEEP from the architectures that use it?
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Yes, you are right.
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the
Sorry, I can't fully understand your intention here, could you please explain it more?
Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know.
OK, so in your current patch, please replace ACPI_GENERIC_SLEEP with ACPI_SYSTEM_POWER_STATES_SUPPORT and all should be clear.
OK, I will send a updated patch.
Thanks Hanjun
On Tue, Mar 17, 2015 at 03:23:11AM +0000, Rafael J. Wysocki wrote:
[...]
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the only arch setting it at least for the time being, is that correct?
HW reduced only platforms are still required to support sleep states that on arm64 are totally meaningless at present, so I do not think ACPI_REDUCED_HARDWARE_ONLY will cut it.
Factoring out power_off methods from sleep.c ? I know, it is not nicer since you split the S-states management in multiple files.
Side note: is the acpi_suspend() function in sleep.c used in the kernel ?
Lorenzo
On 17 March 2015 at 20:35, Lorenzo Pieralisi lorenzo.pieralisi@arm.com wrote:
On Tue, Mar 17, 2015 at 03:23:11AM +0000, Rafael J. Wysocki wrote:
[...]
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in
sleep.c if
we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains
code
outside of the ACPI_SLEEP-dependent blocks. That code is used for
powering
off ACPI platforms.
I guess you don't want that code on ARM too, right?
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be
the
only arch setting it at least for the time being, is that correct?
HW reduced only platforms are still required to support sleep states that on arm64 are totally meaningless at present, so I do not think ACPI_REDUCED_HARDWARE_ONLY will cut it.
Factoring out power_off methods from sleep.c ? I know, it is not nicer since you split the S-states management in multiple files.
Side note: is the acpi_suspend() function in sleep.c used in the kernel ?
I'm afraid not, I grepped this function in this morning when I was reviewing the code, and only found: ./drivers/acpi/sleep.c: trace_suspend_resume(TPS("acpi_suspend"), acpi_state, true); ./drivers/acpi/sleep.c: trace_suspend_resume(TPS("acpi_suspend"), acpi_state, false); ./drivers/acpi/sleep.c:int acpi_suspend(u32 acpi_state)
./drivers/acpi/sleep.h:extern int acpi_suspend(u32 state);
Thanks Hanjun
Lorenzo
On Tuesday, March 17, 2015 12:35:36 PM Lorenzo Pieralisi wrote:
On Tue, Mar 17, 2015 at 03:23:11AM +0000, Rafael J. Wysocki wrote:
[...]
Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and
+acpi-$(CONFIG_ACPI_SLEEP) += sleep.o
as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if we doing so)?
Well, almost. There is one problem with that, becuase sleep.c contains code outside of the ACPI_SLEEP-dependent blocks. That code is used for powering off ACPI platforms.
I guess you don't want that code on ARM too, right?
Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the only arch setting it at least for the time being, is that correct?
HW reduced only platforms are still required to support sleep states that on arm64 are totally meaningless at present, so I do not think ACPI_REDUCED_HARDWARE_ONLY will cut it.
Factoring out power_off methods from sleep.c ? I know, it is not nicer since you split the S-states management in multiple files.
Side note: is the acpi_suspend() function in sleep.c used in the kernel ?
No, it isn't. I've just sent a patch to drop it.
-
Hanjun Guo -
Hanjun Guo -
Jon Masters -
Lorenzo Pieralisi -
Rafael J. Wysocki