This patch set are some minor cleanups for ACPI processor driver to address the comments which raised by Rafael in ARM64 ACPI core patches, so this patch set is on top of ARM64 ACPI core patches the git tree is
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git branch for-next/acpi.
Rafael, I assume this patchset will be taken with your tree if it makes sense, any rebase work needed please let me know.
the last patch - ACPI / processor: Introduce invalid_phys_cpuid() will cut u64 mpidr to int, but I think it is ok for error values, correct me if I'm wrong.
Comments are welcomed.
Hanjun Guo (7): ACPI / processor: remove cpu_index in acpi_processor_get_info() ACPI / processor: remove phys_id in acpi_processor_get_info() ACPI / processor: Introduce invalid_logical_cpuid() Xen / ACPI / processor: use invalid_logical_cpuid() Xen / ACPI / processor: Remove unneeded NULL check in xen_acpi_processor_enable() ACPI / processor: return specific error instead of -1 ACPI / processor: Introduce invalid_phys_cpuid()
drivers/acpi/acpi_processor.c | 20 +++++++++----------- drivers/acpi/processor_core.c | 10 +++++----- drivers/acpi/processor_pdc.c | 5 +---- drivers/xen/xen-acpi-cpuhotplug.c | 12 +++--------- include/linux/acpi.h | 10 ++++++++++ 5 files changed, 28 insertions(+), 29 deletions(-)
Just use pr->id instead of cpu_index to simplify the code.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/acpi_processor.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 58f335c..d6ac918 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -216,7 +216,7 @@ static int acpi_processor_get_info(struct acpi_device *device) struct acpi_buffer buffer = { sizeof(union acpi_object), &object }; struct acpi_processor *pr = acpi_driver_data(device); phys_cpuid_t phys_id; - int cpu_index, device_declaration = 0; + int device_declaration = 0; acpi_status status = AE_OK; static int cpu0_initialized; unsigned long long value; @@ -268,17 +268,16 @@ static int acpi_processor_get_info(struct acpi_device *device) acpi_handle_debug(pr->handle, "failed to get CPU physical ID.\n"); pr->phys_id = phys_id;
- cpu_index = acpi_map_cpuid(pr->phys_id, pr->acpi_id); + pr->id = acpi_map_cpuid(pr->phys_id, pr->acpi_id); if (!cpu0_initialized && !acpi_has_cpu_in_madt()) { cpu0_initialized = 1; /* * Handle UP system running SMP kernel, with no CPU * entry in MADT */ - if ((cpu_index == -1) && (num_online_cpus() == 1)) - cpu_index = 0; + if ((pr->id == -1) && (num_online_cpus() == 1)) + pr->id = 0; } - pr->id = cpu_index;
/* * Extra Processor objects may be enumerated on MP systems with
Use pr->phys_id to replace phys_id to simplify the code.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/acpi_processor.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index d6ac918..95492d0 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -215,7 +215,6 @@ static int acpi_processor_get_info(struct acpi_device *device) union acpi_object object = { 0 }; struct acpi_buffer buffer = { sizeof(union acpi_object), &object }; struct acpi_processor *pr = acpi_driver_data(device); - phys_cpuid_t phys_id; int device_declaration = 0; acpi_status status = AE_OK; static int cpu0_initialized; @@ -263,10 +262,10 @@ static int acpi_processor_get_info(struct acpi_device *device) pr->acpi_id = value; }
- phys_id = acpi_get_phys_id(pr->handle, device_declaration, pr->acpi_id); - if (phys_id == PHYS_CPUID_INVALID) + pr->phys_id = acpi_get_phys_id(pr->handle, device_declaration, + pr->acpi_id); + if (pr->phys_id == PHYS_CPUID_INVALID) acpi_handle_debug(pr->handle, "failed to get CPU physical ID.\n"); - pr->phys_id = phys_id;
pr->id = acpi_map_cpuid(pr->phys_id, pr->acpi_id); if (!cpu0_initialized && !acpi_has_cpu_in_madt()) {
In ACPI processor drivers, we use direct comparisons of cpu logical id with -1 which are error prone in case logical cpuid is accidentally assinged an error code and prevents us from returning an error-encoding cpuid directly in some cases.
So introduce invalid_logical_cpuid() to identify cpu with invalid logical cpu num, then it will be used to replace the direct comparisons with -1.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/acpi_processor.c | 4 ++-- drivers/acpi/processor_pdc.c | 5 +---- include/linux/acpi.h | 5 +++++ 3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 95492d0..62c846b 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -274,7 +274,7 @@ static int acpi_processor_get_info(struct acpi_device *device) * Handle UP system running SMP kernel, with no CPU * entry in MADT */ - if ((pr->id == -1) && (num_online_cpus() == 1)) + if (invalid_logical_cpuid(pr->id) && (num_online_cpus() == 1)) pr->id = 0; }
@@ -283,7 +283,7 @@ static int acpi_processor_get_info(struct acpi_device *device) * less than the max # of CPUs. They should be ignored _iff * they are physically not present. */ - if (pr->id == -1) { + if (invalid_logical_cpuid(pr->id)) { int ret = acpi_processor_hotadd_init(pr); if (ret) return ret; diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c index e5dd808..6dd98d0 100644 --- a/drivers/acpi/processor_pdc.c +++ b/drivers/acpi/processor_pdc.c @@ -52,10 +52,7 @@ static bool __init processor_physically_present(acpi_handle handle) type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; cpuid = acpi_get_cpuid(handle, type, acpi_id);
- if (cpuid == -1) - return false; - - return true; + return invalid_logical_cpuid(cpuid) ? false : true; }
static void acpi_set_pdc_bits(u32 *buf) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index de4e86f..fd8a2a4 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -152,6 +152,11 @@ typedef u32 phys_cpuid_t; #define PHYS_CPUID_INVALID (phys_cpuid_t)(-1) #endif
+static inline bool invalid_logical_cpuid(u32 cpuid) +{ + return (int)cpuid < 0; +} + #ifdef CONFIG_ACPI_HOTPLUG_CPU /* Arch dependent functions for cpu hotplug support */ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu);
Use invalid_logical_cpuid(pr->id) instead of direct comparison of -1.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Boris Ostrovsky boris.ostrovsky@oracle.com CC: Stefano Stabellini stefano.stabellini@eu.citrix.com --- drivers/xen/xen-acpi-cpuhotplug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xen-acpi-cpuhotplug.c b/drivers/xen/xen-acpi-cpuhotplug.c index 3e62ee4..5a62aa0 100644 --- a/drivers/xen/xen-acpi-cpuhotplug.c +++ b/drivers/xen/xen-acpi-cpuhotplug.c @@ -77,7 +77,7 @@ static int xen_acpi_processor_enable(struct acpi_device *device)
pr->id = xen_pcpu_id(pr->acpi_id);
- if ((int)pr->id < 0) + if (invalid_logical_cpuid(pr->id)) /* This cpu is not presented at hypervisor, try to hotadd it */ if (ACPI_FAILURE(xen_acpi_cpu_hotadd(pr))) { pr_err(PREFIX "Hotadd CPU (acpi_id = %d) failed.\n", @@ -226,7 +226,7 @@ static acpi_status xen_acpi_cpu_hotadd(struct acpi_processor *pr) return AE_ERROR;
pr->id = xen_hotadd_cpu(pr); - if ((int)pr->id < 0) + if (invalid_logical_cpuid(pr->id)) return AE_ERROR;
/*
Before xen_acpi_processor_enable() is called, struct acpi_processor *pr is allocated in xen_acpi_processor_add() and checked if it's NULL, so no need to check again when passed to xen_acpi_processor_enable(), just remove it.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org CC: Boris Ostrovsky boris.ostrovsky@oracle.com CC: Stefano Stabellini stefano.stabellini@eu.citrix.com --- drivers/xen/xen-acpi-cpuhotplug.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/xen/xen-acpi-cpuhotplug.c b/drivers/xen/xen-acpi-cpuhotplug.c index 5a62aa0..f4a3694 100644 --- a/drivers/xen/xen-acpi-cpuhotplug.c +++ b/drivers/xen/xen-acpi-cpuhotplug.c @@ -46,13 +46,7 @@ static int xen_acpi_processor_enable(struct acpi_device *device) unsigned long long value; union acpi_object object = { 0 }; struct acpi_buffer buffer = { sizeof(union acpi_object), &object }; - struct acpi_processor *pr; - - pr = acpi_driver_data(device); - if (!pr) { - pr_err(PREFIX "Cannot find driver data\n"); - return -EINVAL; - } + struct acpi_processor *pr = acpi_driver_data(device);
if (!strcmp(acpi_device_hid(device), ACPI_PROCESSOR_OBJECT_HID)) { /* Declared with "Processor" statement; match ProcessorID */
Since invalid_logical_cpuid() can check error values, so return specific error instead of -1 for acpi_map_cpuid().
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/processor_core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index b1ec78b..fd4140d 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -215,12 +215,12 @@ int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id) * Ignores phys_id and always returns 0 for the processor * handle with acpi id 0 if nr_cpu_ids is 1. * This should be the case if SMP tables are not found. - * Return -1 for other CPU's handle. + * Return -EINVAL for other CPU's handle. */ if (nr_cpu_ids <= 1 && acpi_id == 0) return acpi_id; else - return -1; + return -EINVAL; }
#ifdef CONFIG_SMP @@ -233,7 +233,7 @@ int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id) if (phys_id == 0) return phys_id; #endif - return -1; + return -ENODEV; }
int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
Introduce invalid_phys_cpuid() to identify cpu with invalid physical ID, then used it as replacement of the direct comparisons with PHYS_CPUID_INVALID.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/acpi_processor.c | 4 ++-- drivers/acpi/processor_core.c | 4 ++-- include/linux/acpi.h | 5 +++++ 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 62c846b..92a5f73 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -170,7 +170,7 @@ static int acpi_processor_hotadd_init(struct acpi_processor *pr) acpi_status status; int ret;
- if (pr->phys_id == PHYS_CPUID_INVALID) + if (invalid_phys_cpuid(pr->phys_id)) return -ENODEV;
status = acpi_evaluate_integer(pr->handle, "_STA", NULL, &sta); @@ -264,7 +264,7 @@ static int acpi_processor_get_info(struct acpi_device *device)
pr->phys_id = acpi_get_phys_id(pr->handle, device_declaration, pr->acpi_id); - if (pr->phys_id == PHYS_CPUID_INVALID) + if (invalid_phys_cpuid(pr->phys_id)) acpi_handle_debug(pr->handle, "failed to get CPU physical ID.\n");
pr->id = acpi_map_cpuid(pr->phys_id, pr->acpi_id); diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index fd4140d..33a38d6 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -184,7 +184,7 @@ phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) phys_cpuid_t phys_id;
phys_id = map_mat_entry(handle, type, acpi_id); - if (phys_id == PHYS_CPUID_INVALID) + if (invalid_phys_cpuid(phys_id)) phys_id = map_madt_entry(type, acpi_id);
return phys_id; @@ -196,7 +196,7 @@ int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id) int i; #endif
- if (phys_id == PHYS_CPUID_INVALID) { + if (invalid_phys_cpuid(phys_id)) { /* * On UP processor, there is no _MAT or MADT table. * So above phys_id is always set to PHYS_CPUID_INVALID. diff --git a/include/linux/acpi.h b/include/linux/acpi.h index fd8a2a4..9a1f467 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -157,6 +157,11 @@ static inline bool invalid_logical_cpuid(u32 cpuid) return (int)cpuid < 0; }
+static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id) +{ + return (int)phys_id < 0; +} + #ifdef CONFIG_ACPI_HOTPLUG_CPU /* Arch dependent functions for cpu hotplug support */ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu);
On Friday, March 27, 2015 10:02:21 PM Hanjun Guo wrote:
This patch set are some minor cleanups for ACPI processor driver to address the comments which raised by Rafael in ARM64 ACPI core patches, so this patch set is on top of ARM64 ACPI core patches the git tree is
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git branch for-next/acpi.
Rafael, I assume this patchset will be taken with your tree if it makes sense, any rebase work needed please let me know.
the last patch - ACPI / processor: Introduce invalid_phys_cpuid() will cut u64 mpidr to int, but I think it is ok for error values, correct me if I'm wrong.
Comments are welcomed.
The patches look OK to me and all their dependencies should be met for 4.2, so can you please make sure that they still apply and possibly resend on top of the current Linus' tree?
Hi Rafael,
On 2015/5/5 7:50, Rafael J. Wysocki wrote:
On Friday, March 27, 2015 10:02:21 PM Hanjun Guo wrote:
This patch set are some minor cleanups for ACPI processor driver to address the comments which raised by Rafael in ARM64 ACPI core patches, so this patch set is on top of ARM64 ACPI core patches the git tree is
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git branch for-next/acpi.
Rafael, I assume this patchset will be taken with your tree if it makes sense, any rebase work needed please let me know.
the last patch - ACPI / processor: Introduce invalid_phys_cpuid() will cut u64 mpidr to int, but I think it is ok for error values, correct me if I'm wrong.
Comments are welcomed.
The patches look OK to me and all their dependencies should be met for 4.2, so can you please make sure that they still apply and possibly resend on top of the current Linus' tree?
I will rebase on top of current Linus' tree and send them out today.
Thanks Hanjun
-
Hanjun Guo -
Hanjun Guo -
Rafael J. Wysocki