[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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);
-- 
1.9.1

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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);
-- 
1.9.1

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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);
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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);
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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



--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Jon Masters]

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_Requirements.pdf

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Jon Masters]

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_Requirements.pdf

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Hanjun Guo]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Lorenzo Pieralisi]

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

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Lorenzo Pieralisi]

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

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Lorenzo Pieralisi]

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

[PATCH] ACPI / sleep: Drop acpi_suspend() which is not used

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The acpi_suspend() function has no callers, so drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/sleep.c |   15 ---------------
 drivers/acpi/sleep.h |    2 --
 2 files changed, 17 deletions(-)

Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -806,21 +806,6 @@ static void acpi_sleep_hibernate_setup(v
 static inline void acpi_sleep_hibernate_setup(void) {}
 #endif /* !CONFIG_HIBERNATION */
 
-int acpi_suspend(u32 acpi_state)
-{
-	suspend_state_t states[] = {
-		[1] = PM_SUSPEND_STANDBY,
-		[3] = PM_SUSPEND_MEM,
-		[5] = PM_SUSPEND_MAX
-	};
-
-	if (acpi_state < 6 && states[acpi_state])
-		return pm_suspend(states[acpi_state]);
-	if (acpi_state == 4)
-		return hibernate();
-	return -EINVAL;
-}
-
 static void acpi_power_off_prepare(void)
 {
 	/* Prepare to power off the system */
Index: linux-pm/drivers/acpi/sleep.h
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.h
+++ linux-pm/drivers/acpi/sleep.h
@@ -1,6 +1,4 @@
 
-extern int acpi_suspend(u32 state);
-
 extern void acpi_enable_wakeup_devices(u8 sleep_state);
 extern void acpi_disable_wakeup_devices(u8 sleep_state);
 

Re: [PATCH] ACPI / sleep: Drop acpi_suspend() which is not used

[Lorenzo Pieralisi]

On Tue, Mar 17, 2015 at 02:29:23PM +0000, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The acpi_suspend() function has no callers, so drop it.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> ---
>  drivers/acpi/sleep.c |   15 ---------------
>  drivers/acpi/sleep.h |    2 --
>  2 files changed, 17 deletions(-)
> 
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -806,21 +806,6 @@ static void acpi_sleep_hibernate_setup(v
>  static inline void acpi_sleep_hibernate_setup(void) {}
>  #endif /* !CONFIG_HIBERNATION */
>  
> -int acpi_suspend(u32 acpi_state)
> -{
> -	suspend_state_t states[] = {
> -		[1] = PM_SUSPEND_STANDBY,
> -		[3] = PM_SUSPEND_MEM,
> -		[5] = PM_SUSPEND_MAX
> -	};
> -
> -	if (acpi_state < 6 && states[acpi_state])
> -		return pm_suspend(states[acpi_state]);
> -	if (acpi_state == 4)
> -		return hibernate();
> -	return -EINVAL;
> -}
> -
>  static void acpi_power_off_prepare(void)
>  {
>  	/* Prepare to power off the system */
> Index: linux-pm/drivers/acpi/sleep.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.h
> +++ linux-pm/drivers/acpi/sleep.h
> @@ -1,6 +1,4 @@
>  
> -extern int acpi_suspend(u32 state);
> -
>  extern void acpi_enable_wakeup_devices(u8 sleep_state);
>  extern void acpi_disable_wakeup_devices(u8 sleep_state);
>  
> 
> 
> 

Re: [PATCH] ACPI / sleep: Drop acpi_suspend() which is not used

[Hanjun Guo]

On 2015/3/17 22:29, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The acpi_suspend() function has no callers, so drop it.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

> ---
>  drivers/acpi/sleep.c |   15 ---------------
>  drivers/acpi/sleep.h |    2 --
>  2 files changed, 17 deletions(-)
>
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -806,21 +806,6 @@ static void acpi_sleep_hibernate_setup(v
>  static inline void acpi_sleep_hibernate_setup(void) {}
>  #endif /* !CONFIG_HIBERNATION */
>  
> -int acpi_suspend(u32 acpi_state)
> -{
> -	suspend_state_t states[] = {
> -		[1] = PM_SUSPEND_STANDBY,
> -		[3] = PM_SUSPEND_MEM,
> -		[5] = PM_SUSPEND_MAX
> -	};
> -
> -	if (acpi_state < 6 && states[acpi_state])
> -		return pm_suspend(states[acpi_state]);
> -	if (acpi_state == 4)
> -		return hibernate();
> -	return -EINVAL;
> -}
> -
>  static void acpi_power_off_prepare(void)
>  {
>  	/* Prepare to power off the system */
> Index: linux-pm/drivers/acpi/sleep.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.h
> +++ linux-pm/drivers/acpi/sleep.h
> @@ -1,6 +1,4 @@
>  
> -extern int acpi_suspend(u32 state);
> -
>  extern void acpi_enable_wakeup_devices(u8 sleep_state);
>  extern void acpi_disable_wakeup_devices(u8 sleep_state);
>  
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> .
>

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP

[Rafael J. Wysocki]

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.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.