[PATCH 1/2] ARM64 / ACPI: Ignore the return error value of acpi_map_gic_cpu_interface()

[PATCH 1/2] ARM64 / ACPI: Ignore the return error value of acpi_map_gic_cpu_interface()

[Hanjun Guo]

MADT table scannig will stopped once it gets the errors
returned by the handler, which is acpi_map_gic_cpu_interface()
in for ARM64, so Ignore the return error value to search for
all enabled CPUs for SMP init.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 arch/arm64/kernel/acpi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 07649e4..c263cba 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -181,7 +181,8 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
 		return -EINVAL;
 
 	acpi_table_print_madt_entry(header);
-	return acpi_map_gic_cpu_interface(processor);
+	acpi_map_gic_cpu_interface(processor);
+	return 0;
 }
 
 /* Parse GIC cpu interface entries in MADT for SMP init */
-- 
1.9.1

[PATCH 2/2] ARM64 / ACPI: make acpi_map_gic_cpu_interface() as void function

[Hanjun Guo]

Since the only caller of acpi_parse_gic_cpu_interface() doesn't
need the return value, make it have a void return type to avoid
introducing subtle bugs, and update the comments of the function
accordingly.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 arch/arm64/kernel/acpi.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index c263cba..8b83955 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -98,12 +98,8 @@ void __init __acpi_unmap_table(char *map, unsigned long size)
 /**
  * acpi_map_gic_cpu_interface - generates a logical cpu number
  * and map to MPIDR represented by GICC structure
- * @mpidr: CPU's hardware id to register, MPIDR represented in MADT
- * @enabled: this cpu is enabled or not
- *
- * Returns the logical cpu number which maps to MPIDR
  */
-static int __init
+static void __init
 acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 {
 	int i;
@@ -112,17 +108,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 
 	if (mpidr == INVALID_HWID) {
 		pr_info("Skip MADT cpu entry with invalid MPIDR\n");
-		return -EINVAL;
+		return;
 	}
 
 	total_cpus++;
 	if (!enabled)
-		return -EINVAL;
+		return;
 
 	if (enabled_cpus >=  NR_CPUS) {
 		pr_warn("NR_CPUS limit of %d reached, Processor %d/0x%llx ignored.\n",
 			NR_CPUS, total_cpus, mpidr);
-		return -EINVAL;
+		return;
 	}
 
 	/* Check if GICC structure of boot CPU is available in the MADT */
@@ -130,7 +126,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 		if (bootcpu_valid) {
 			pr_err("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",
 			       mpidr);
-			return -EINVAL;
+			return;
 		}
 
 		bootcpu_valid = true;
@@ -145,28 +141,27 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 		if (cpu_logical_map(i) == mpidr) {
 			pr_err("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",
 			       mpidr);
-			return -EINVAL;
+			return;
 		}
 	}
 
 	if (!acpi_psci_present())
-		return -EOPNOTSUPP;
+		return;
 
 	cpu_ops[enabled_cpus] = cpu_get_ops("psci");
 	/* CPU 0 was already initialized */
 	if (enabled_cpus) {
 		if (!cpu_ops[enabled_cpus])
-			return -EINVAL;
+			return;
 
 		if (cpu_ops[enabled_cpus]->cpu_init(NULL, enabled_cpus))
-			return -EOPNOTSUPP;
+			return;
 
 		/* map the logical cpu id to cpu MPIDR */
 		cpu_logical_map(enabled_cpus) = mpidr;
 	}
 
 	enabled_cpus++;
-	return enabled_cpus;
 }
 
 static int __init
-- 
1.9.1

Re: [Linaro-acpi] [PATCH 1/2] ARM64 / ACPI: Ignore the return error value of acpi_map_gic_cpu_interface()

[Al Stone]

On 03/27/2015 06:14 AM, Hanjun Guo wrote:
> MADT table scannig will stopped once it gets the errors
> returned by the handler, which is acpi_map_gic_cpu_interface()
> in for ARM64, so Ignore the return error value to search for
> all enabled CPUs for SMP init.

I think the comment should say something like:

MADT scanning will stop when it gets an error from the handler,
acpi_map_gic_cpu_interface(), on arm64.  However, we need to
find all of the enabled CPUs so that SMP initialization can work
properly.  So, if an error occurs in this case, ignore it for
now so that we can find all of the enabled CPUs.

Is that what you meant, Hanjun?

> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  arch/arm64/kernel/acpi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 07649e4..c263cba 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -181,7 +181,8 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
>  		return -EINVAL;
>  
>  	acpi_table_print_madt_entry(header);
> -	return acpi_map_gic_cpu_interface(processor);
> +	acpi_map_gic_cpu_interface(processor);
> +	return 0;
>  }
>  
>  /* Parse GIC cpu interface entries in MADT for SMP init */
> 


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------

Re: [Linaro-acpi] [PATCH 1/2] ARM64 / ACPI: Ignore the return error value of acpi_map_gic_cpu_interface()

[Hanjun Guo]

On 2015/3/31 5:53, Al Stone wrote:
> On 03/27/2015 06:14 AM, Hanjun Guo wrote:
>> MADT table scannig will stopped once it gets the errors
>> returned by the handler, which is acpi_map_gic_cpu_interface()
>> in for ARM64, so Ignore the return error value to search for
>> all enabled CPUs for SMP init.
> I think the comment should say something like:
>
> MADT scanning will stop when it gets an error from the handler,
> acpi_map_gic_cpu_interface(), on arm64.  However, we need to
> find all of the enabled CPUs so that SMP initialization can work
> properly.  So, if an error occurs in this case, ignore it for
> now so that we can find all of the enabled CPUs.
>
> Is that what you meant, Hanjun?

That's exactly what I mean, you describe it much clearer :)

Thanks
Hanjun