* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
@ 2016-04-19 12:49 ` kbuild test robot
2016-04-19 13:00 ` Sudeep Holla
2016-04-20 9:57 ` Vikas Sajjan
2016-05-09 23:59 ` Rafael J. Wysocki
2 siblings, 1 reply; 7+ messages in thread
From: kbuild test robot @ 2016-04-19 12:49 UTC (permalink / raw)
To: linux-ia64
[-- Attachment #1: Type: text/plain, Size: 5448 bytes --]
Hi,
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.6-rc4 next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-x000-201616 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the linux-review/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500 HEAD c51fc2a756d7b0dce908a4ca043d1d458c400af5 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
drivers/acpi/processor_idle.c: In function 'acpi_processor_cstate_first_run_checks':
>> drivers/acpi/processor_idle.c:943:3: error: 'status' undeclared (first use in this function)
status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
^
drivers/acpi/processor_idle.c:943:3: note: each undeclared identifier is reported only once for each function it appears in
drivers/acpi/processor_idle.c: In function 'acpi_processor_power_init':
>> drivers/acpi/processor_idle.c:1062:14: warning: unused variable 'status' [-Wunused-variable]
acpi_status status;
^
vim +/status +943 drivers/acpi/processor_idle.c
937 if (max_cstate < ACPI_C_STATES_MAX)
938 pr_notice("ACPI: processor limited to max C-state %d\n",
939 max_cstate);
940 first_run++;
941
942 if (acpi_gbl_FADT.cst_control && !nocst) {
> 943 status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
944 acpi_gbl_FADT.cst_control, 8);
945 if (ACPI_FAILURE(status))
946 ACPI_EXCEPTION((AE_INFO, status,
947 "Notifying BIOS of _CST ability failed"));
948 }
949 }
950 #else
951
952 static inline int disabled_by_idle_boot_param(void) { return 0; }
953 static inline void acpi_processor_cstate_first_run_checks(void) { }
954 static int acpi_processor_get_power_info(struct acpi_processor *pr)
955 {
956 return -ENODEV;
957 }
958
959 static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
960 struct cpuidle_device *dev)
961 {
962 return -EINVAL;
963 }
964
965 static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
966 {
967 return -EINVAL;
968 }
969
970 #endif
971
972 int acpi_processor_hotplug(struct acpi_processor *pr)
973 {
974 int ret = 0;
975 struct cpuidle_device *dev;
976
977 if (disabled_by_idle_boot_param())
978 return 0;
979
980 if (nocst)
981 return -ENODEV;
982
983 if (!pr->flags.power_setup_done)
984 return -ENODEV;
985
986 dev = per_cpu(acpi_cpuidle_device, pr->id);
987 cpuidle_pause_and_lock();
988 cpuidle_disable_device(dev);
989 acpi_processor_get_power_info(pr);
990 if (pr->flags.power) {
991 acpi_processor_setup_cpuidle_cx(pr, dev);
992 ret = cpuidle_enable_device(dev);
993 }
994 cpuidle_resume_and_unlock();
995
996 return ret;
997 }
998
999 int acpi_processor_cst_has_changed(struct acpi_processor *pr)
1000 {
1001 int cpu;
1002 struct acpi_processor *_pr;
1003 struct cpuidle_device *dev;
1004
1005 if (disabled_by_idle_boot_param())
1006 return 0;
1007
1008 if (nocst)
1009 return -ENODEV;
1010
1011 if (!pr->flags.power_setup_done)
1012 return -ENODEV;
1013
1014 /*
1015 * FIXME: Design the ACPI notification to make it once per
1016 * system instead of once per-cpu. This condition is a hack
1017 * to make the code that updates C-States be called once.
1018 */
1019
1020 if (pr->id == 0 && cpuidle_get_driver() == &acpi_idle_driver) {
1021
1022 /* Protect against cpu-hotplug */
1023 get_online_cpus();
1024 cpuidle_pause_and_lock();
1025
1026 /* Disable all cpuidle devices */
1027 for_each_online_cpu(cpu) {
1028 _pr = per_cpu(processors, cpu);
1029 if (!_pr || !_pr->flags.power_setup_done)
1030 continue;
1031 dev = per_cpu(acpi_cpuidle_device, cpu);
1032 cpuidle_disable_device(dev);
1033 }
1034
1035 /* Populate Updated C-state information */
1036 acpi_processor_get_power_info(pr);
1037 acpi_processor_setup_cpuidle_states(pr);
1038
1039 /* Enable all cpuidle devices */
1040 for_each_online_cpu(cpu) {
1041 _pr = per_cpu(processors, cpu);
1042 if (!_pr || !_pr->flags.power_setup_done)
1043 continue;
1044 acpi_processor_get_power_info(_pr);
1045 if (_pr->flags.power) {
1046 dev = per_cpu(acpi_cpuidle_device, cpu);
1047 acpi_processor_setup_cpuidle_cx(_pr, dev);
1048 cpuidle_enable_device(dev);
1049 }
1050 }
1051 cpuidle_resume_and_unlock();
1052 put_online_cpus();
1053 }
1054
1055 return 0;
1056 }
1057
1058 static int acpi_processor_registered;
1059
1060 int acpi_processor_power_init(struct acpi_processor *pr)
1061 {
> 1062 acpi_status status;
1063 int retval;
1064 struct cpuidle_device *dev;
1065
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24581 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:49 ` kbuild test robot
@ 2016-04-19 13:00 ` Sudeep Holla
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2016-04-19 13:00 UTC (permalink / raw)
To: kbuild test robot
Cc: Sudeep Holla, kbuild-all, Rafael J. Wysocki, linux-acpi,
linux-kernel, linux-arm-kernel, Lorenzo Pieralisi, Al Stone,
Prashanth Prakash, Ashwin Chaugule, x86, linux-ia64
Hi,
On 19/04/16 13:49, kbuild test robot wrote:
> Hi,
>
> [auto build test ERROR on pm/linux-next]
> [also build test ERROR on v4.6-rc4 next-20160419]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url: https://github.com/0day-ci/linux/commits/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: i386-randconfig-x000-201616 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: the linux-review/Sudeep-Holla/ACPI-processor_idle-Add-ACPI-v6-0-LPI-support/20160419-203500 HEAD c51fc2a756d7b0dce908a4ca043d1d458c400af5 builds fine.
> It only hurts bisectibility.
>
Thanks for the report, it's now fixed locally.
It will part of the next version.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-04-19 12:49 ` kbuild test robot
@ 2016-04-20 9:57 ` Vikas Sajjan
2016-04-20 10:09 ` Sudeep Holla
2016-05-09 23:59 ` Rafael J. Wysocki
2 siblings, 1 reply; 7+ messages in thread
From: Vikas Sajjan @ 2016-04-20 9:57 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel@lists.infradead.org, Lorenzo Pieralisi,
linux-ia64, Al Stone, Prashanth Prakash, x86, Ashwin Chaugule
Hi Sudeep,
On Tue, Apr 19, 2016 at 6:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
> called Low Power Idle(LPI) states. Since new architectures like ARM64
> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
> encapsulate all the code supporting the old style C-states(_CST)
>
> This patch will help to extend the processor_idle module to support
> LPI.
>
> Cc: x86@kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/ia64/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> drivers/acpi/Kconfig | 3 ++
> drivers/acpi/processor_idle.c | 74 +++++++++++++++++++++++++++++--------------
> include/acpi/processor.h | 3 +-
> 5 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index b534ebab36ea..717de2a146e2 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -16,6 +16,7 @@ config IA64
> select PCI if (!IA64_HP_SIM)
> select ACPI if (!IA64_HP_SIM)
> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE 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 2dc18605831f..eb03fd0d63b9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -37,6 +37,7 @@ config X86
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
> select ARCH_SUPPORTS_INT128 if X86_64
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 82b96ee8624c..ec289078667c 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
> config ARCH_MIGHT_HAVE_ACPI_PDC
> bool
>
> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> + bool
> +
> config ACPI_GENERIC_GSI
> bool
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 444e3745c8b3..1f3fe54194b5 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -59,6 +59,12 @@ module_param(latency_factor, uint, 0644);
>
> static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
>
> +struct cpuidle_driver acpi_idle_driver = {
> + .name = "acpi_idle",
> + .owner = THIS_MODULE,
> +};
> +
> +#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> static
> DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX], acpi_cstate);
>
> @@ -804,11 +810,6 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
> acpi_idle_do_entry(cx);
> }
>
> -struct cpuidle_driver acpi_idle_driver = {
> - .name = "acpi_idle",
> - .owner = THIS_MODULE,
> -};
> -
> /**
> * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
> * device i.e. per-cpu data
> @@ -925,6 +926,49 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> return 0;
> }
>
> +static inline void acpi_processor_cstate_first_run_checks(void)
> +{
> + static int first_run;
> +
> + if (first_run)
> + return;
> + dmi_check_system(processor_power_dmi_table);
> + max_cstate = acpi_processor_cstate_check(max_cstate);
> + if (max_cstate < ACPI_C_STATES_MAX)
> + pr_notice("ACPI: processor limited to max C-state %d\n",
> + max_cstate);
> + first_run++;
> +
> + if (acpi_gbl_FADT.cst_control && !nocst) {
> + status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
> + acpi_gbl_FADT.cst_control, 8);
> + if (ACPI_FAILURE(status))
> + ACPI_EXCEPTION((AE_INFO, status,
> + "Notifying BIOS of _CST ability failed"));
> + }
> +}
> +#else
> +
> +static inline int disabled_by_idle_boot_param(void) { return 0; }
> +static inline void acpi_processor_cstate_first_run_checks(void) { }
> +static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +{
> + return -ENODEV;
> +}
> +
> +static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> + struct cpuidle_device *dev)
> +{
> + return -EINVAL;
> +}
> +
> +static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +{
> + return -EINVAL;
> +}
> +
> +#endif
> +
> int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> int ret = 0;
> @@ -1018,29 +1062,11 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> acpi_status status;
> int retval;
> struct cpuidle_device *dev;
> - static int first_run;
>
> if (disabled_by_idle_boot_param())
> return 0;
>
> - if (!first_run) {
> - dmi_check_system(processor_power_dmi_table);
> - max_cstate = acpi_processor_cstate_check(max_cstate);
> - if (max_cstate < ACPI_C_STATES_MAX)
> - printk(KERN_NOTICE
> - "ACPI: processor limited to max C-state %d\n",
> - max_cstate);
> - first_run++;
> - }
> -
> - if (acpi_gbl_FADT.cst_control && !nocst) {
> - status > - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Notifying BIOS of _CST ability failed"));
> - }
> - }
> + acpi_processor_cstate_first_run_checks();
>
> acpi_processor_get_power_info(pr);
> pr->flags.power_setup_done = 1;
Not related to your change,
The acpi_processor_get_power_info() function can return failure, so i
thought it makes sense to check for the return value
and then set the flag pr->flags.power_setup_done appropriately.
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 6f1805dd5d3c..50f2423d31fa 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -242,7 +242,8 @@ extern int acpi_processor_get_performance_info(struct acpi_processor *pr);
> DECLARE_PER_CPU(struct acpi_processor *, processors);
> extern struct acpi_processor_errata errata;
>
> -#ifdef ARCH_HAS_POWER_INIT
> +#if defined(ARCH_HAS_POWER_INIT) && \
> + defined(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE)
> void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
> unsigned int cpu);
> int acpi_processor_ffh_cstate_probe(unsigned int cpu,
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-20 9:57 ` Vikas Sajjan
@ 2016-04-20 10:09 ` Sudeep Holla
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2016-04-20 10:09 UTC (permalink / raw)
To: Vikas Sajjan
Cc: Sudeep Holla, Rafael J. Wysocki, linux-acpi, linux-kernel,
linux-arm-kernel@lists.infradead.org, Lorenzo Pieralisi,
linux-ia64, Al Stone, Prashanth Prakash, x86, Ashwin Chaugule
On 20/04/16 10:56, Vikas Sajjan wrote:
> Hi Sudeep,
>
> On Tue, Apr 19, 2016 at 6:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
>> called Low Power Idle(LPI) states. Since new architectures like ARM64
>> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
>> encapsulate all the code supporting the old style C-states(_CST)
>>
>> This patch will help to extend the processor_idle module to support
>> LPI.
>>
[...]
>> @@ -1018,29 +1062,11 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>> acpi_status status;
>> int retval;
>> struct cpuidle_device *dev;
>> - static int first_run;
>>
>> if (disabled_by_idle_boot_param())
>> return 0;
>>
>> - if (!first_run) {
>> - dmi_check_system(processor_power_dmi_table);
>> - max_cstate = acpi_processor_cstate_check(max_cstate);
>> - if (max_cstate < ACPI_C_STATES_MAX)
>> - printk(KERN_NOTICE
>> - "ACPI: processor limited to max C-state %d\n",
>> - max_cstate);
>> - first_run++;
>> - }
>> -
>> - if (acpi_gbl_FADT.cst_control && !nocst) {
>> - status >> - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
>> - if (ACPI_FAILURE(status)) {
>> - ACPI_EXCEPTION((AE_INFO, status,
>> - "Notifying BIOS of _CST ability failed"));
>> - }
>> - }
>> + acpi_processor_cstate_first_run_checks();
>>
>> acpi_processor_get_power_info(pr);
>> pr->flags.power_setup_done = 1;
>
> Not related to your change,
> The acpi_processor_get_power_info() function can return failure, so i
> thought it makes sense to check for the return value
> and then set the flag pr->flags.power_setup_done appropriately.
>
Makes sense, will do that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-04-19 12:30 ` [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE Sudeep Holla
2016-04-19 12:49 ` kbuild test robot
2016-04-20 9:57 ` Vikas Sajjan
@ 2016-05-09 23:59 ` Rafael J. Wysocki
2016-05-11 15:07 ` Sudeep Holla
2 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2016-05-09 23:59 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-acpi, linux-kernel, linux-arm-kernel, Lorenzo Pieralisi,
Al Stone, Prashanth Prakash, Ashwin Chaugule, x86, linux-ia64
On Tuesday, April 19, 2016 01:30:09 PM Sudeep Holla wrote:
> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
> called Low Power Idle(LPI) states. Since new architectures like ARM64
> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
> encapsulate all the code supporting the old style C-states(_CST)
>
> This patch will help to extend the processor_idle module to support
> LPI.
>
> Cc: x86@kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> arch/ia64/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> drivers/acpi/Kconfig | 3 ++
> drivers/acpi/processor_idle.c | 74 +++++++++++++++++++++++++++++--------------
> include/acpi/processor.h | 3 +-
> 5 files changed, 57 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index b534ebab36ea..717de2a146e2 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -16,6 +16,7 @@ config IA64
> select PCI if (!IA64_HP_SIM)
> select ACPI if (!IA64_HP_SIM)
> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE 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 2dc18605831f..eb03fd0d63b9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -37,6 +37,7 @@ config X86
> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
> select ARCH_SUPPORTS_ATOMIC_RMW
> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
> select ARCH_SUPPORTS_INT128 if X86_64
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 82b96ee8624c..ec289078667c 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
> config ARCH_MIGHT_HAVE_ACPI_PDC
> bool
>
> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> + bool
It might be better to do
config ACPI_PROCESSOR_CST
bool
depends on ia64 || x86
Should it depend on ARCH_HAS_POWER_INIT too?
> +
> config ACPI_GENERIC_GSI
> bool
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 444e3745c8b3..1f3fe54194b5 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -59,6 +59,12 @@ module_param(latency_factor, uint, 0644);
>
> static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
>
> +struct cpuidle_driver acpi_idle_driver = {
> + .name = "acpi_idle",
> + .owner = THIS_MODULE,
> +};
> +
> +#ifdef CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
> static
> DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX], acpi_cstate);
>
> @@ -804,11 +810,6 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev,
> acpi_idle_do_entry(cx);
> }
>
> -struct cpuidle_driver acpi_idle_driver = {
> - .name = "acpi_idle",
> - .owner = THIS_MODULE,
> -};
> -
> /**
> * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE
> * device i.e. per-cpu data
> @@ -925,6 +926,49 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> return 0;
> }
>
> +static inline void acpi_processor_cstate_first_run_checks(void)
> +{
> + static int first_run;
> +
> + if (first_run)
> + return;
> + dmi_check_system(processor_power_dmi_table);
> + max_cstate = acpi_processor_cstate_check(max_cstate);
> + if (max_cstate < ACPI_C_STATES_MAX)
> + pr_notice("ACPI: processor limited to max C-state %d\n",
> + max_cstate);
> + first_run++;
> +
> + if (acpi_gbl_FADT.cst_control && !nocst) {
> + status = acpi_os_write_port(acpi_gbl_FADT.smi_command,
> + acpi_gbl_FADT.cst_control, 8);
> + if (ACPI_FAILURE(status))
> + ACPI_EXCEPTION((AE_INFO, status,
> + "Notifying BIOS of _CST ability failed"));
> + }
> +}
> +#else
> +
> +static inline int disabled_by_idle_boot_param(void) { return 0; }
> +static inline void acpi_processor_cstate_first_run_checks(void) { }
> +static int acpi_processor_get_power_info(struct acpi_processor *pr)
> +{
> + return -ENODEV;
> +}
> +
> +static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
> + struct cpuidle_device *dev)
> +{
> + return -EINVAL;
> +}
> +
> +static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr)
> +{
> + return -EINVAL;
> +}
> +
> +#endif
Add a comment indicating what #ifdef block ends here?
> +
> int acpi_processor_hotplug(struct acpi_processor *pr)
> {
> int ret = 0;
> @@ -1018,29 +1062,11 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> acpi_status status;
> int retval;
> struct cpuidle_device *dev;
> - static int first_run;
>
> if (disabled_by_idle_boot_param())
> return 0;
>
> - if (!first_run) {
> - dmi_check_system(processor_power_dmi_table);
> - max_cstate = acpi_processor_cstate_check(max_cstate);
> - if (max_cstate < ACPI_C_STATES_MAX)
> - printk(KERN_NOTICE
> - "ACPI: processor limited to max C-state %d\n",
> - max_cstate);
> - first_run++;
> - }
> -
> - if (acpi_gbl_FADT.cst_control && !nocst) {
> - status > - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8);
> - if (ACPI_FAILURE(status)) {
> - ACPI_EXCEPTION((AE_INFO, status,
> - "Notifying BIOS of _CST ability failed"));
> - }
> - }
> + acpi_processor_cstate_first_run_checks();
>
> acpi_processor_get_power_info(pr);
> pr->flags.power_setup_done = 1;
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 6f1805dd5d3c..50f2423d31fa 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -242,7 +242,8 @@ extern int acpi_processor_get_performance_info(struct acpi_processor *pr);
> DECLARE_PER_CPU(struct acpi_processor *, processors);
> extern struct acpi_processor_errata errata;
>
> -#ifdef ARCH_HAS_POWER_INIT
> +#if defined(ARCH_HAS_POWER_INIT) && \
> + defined(CONFIG_ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE)
> void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
> unsigned int cpu);
> int acpi_processor_ffh_cstate_probe(unsigned int cpu,
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 1/5] ACPI / processor_idle: introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
2016-05-09 23:59 ` Rafael J. Wysocki
@ 2016-05-11 15:07 ` Sudeep Holla
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2016-05-11 15:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sudeep Holla, linux-acpi, linux-kernel, linux-arm-kernel,
Lorenzo Pieralisi, Al Stone, Prashanth Prakash, Ashwin Chaugule,
x86, linux-ia64
On 10/05/16 01:02, Rafael J. Wysocki wrote:
> On Tuesday, April 19, 2016 01:30:09 PM Sudeep Holla wrote:
>> ACPI 6.0 adds a new method to specify the CPU idle states(C-states)
>> called Low Power Idle(LPI) states. Since new architectures like ARM64
>> use only LPIs, introduce ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE to
>> encapsulate all the code supporting the old style C-states(_CST)
>>
>> This patch will help to extend the processor_idle module to support
>> LPI.
>>
>> Cc: x86@kernel.org
>> Cc: linux-ia64@vger.kernel.org
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> arch/ia64/Kconfig | 1 +
>> arch/x86/Kconfig | 1 +
>> drivers/acpi/Kconfig | 3 ++
>> drivers/acpi/processor_idle.c | 74 +++++++++++++++++++++++++++++--------------
>> include/acpi/processor.h | 3 +-
>> 5 files changed, 57 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
>> index b534ebab36ea..717de2a146e2 100644
>> --- a/arch/ia64/Kconfig
>> +++ b/arch/ia64/Kconfig
>> @@ -16,6 +16,7 @@ config IA64
>> select PCI if (!IA64_HP_SIM)
>> select ACPI if (!IA64_HP_SIM)
>> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
>> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE 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 2dc18605831f..eb03fd0d63b9 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -37,6 +37,7 @@ config X86
>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
>> select ARCH_MIGHT_HAVE_PC_PARPORT
>> select ARCH_MIGHT_HAVE_PC_SERIO
>> + select ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE if ACPI
>> select ARCH_SUPPORTS_ATOMIC_RMW
>> select ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
>> select ARCH_SUPPORTS_INT128 if X86_64
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 82b96ee8624c..ec289078667c 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -48,6 +48,9 @@ config ACPI_LEGACY_TABLES_LOOKUP
>> config ARCH_MIGHT_HAVE_ACPI_PDC
>> bool
>>
>> +config ARCH_SUPPORTS_ACPI_PROCESSOR_CSTATE
>> + bool
>
> It might be better to do
>
> config ACPI_PROCESSOR_CST
> bool
> depends on ia64 || x86
>
Done
> Should it depend on ARCH_HAS_POWER_INIT too?
>
IIUC it's not defined for ia64, so better not to add that dependency
here IMO. include/acpi/processor.h encapsulates it. Let me know if I
have misunderstood it.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread