Re: [Linaro-acpi] [PATCH] ACPI: Add error checks to ARM ACPI core code

On 2014年03月15日 05:38, Ashwin Chaugle wrote:

Hi Hanjun,

Hi Ashwin,

On 14 March 2014 17:34, Ashwin Chaugule <ashwin.chaugule@linaro.org mailto:ashwin.chaugule@linaro.org> wrote:

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org
<mailto:ashwin.chaugule@linaro.org>>
---
drivers/acpi/plat/arm-core.c | 57
+++++++++++++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/plat/arm-core.c
b/drivers/acpi/plat/arm-core.c
index 9704229..52865c9 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -224,24 +224,37 @@ static int __init acpi_parse_fadt(struct
acpi_table_header *table)

static void __init acpi_process_madt(void)
{
- int error;
-
- if (!acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt)) {
-
- /*
- * Parse MADT GIC cpu interface entries
- */
- error = acpi_parse_madt_gic_entries();
- if (!error) {
- /*
- * Parse MADT GIC distributor entries
- */
- acpi_parse_madt_gic_distributor_entries();
- }
+ int err;
+
+ err = acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt);
+ if (err) {
+ pr_err("Failed to parse MADT\n");
+ goto out_err;
+ }
+
+ /*
+ * Parse MADT GIC cpu interface entries
+ */
+ err = acpi_parse_madt_gic_entries();
+ if (err) {
+ pr_err("Failed to parse GIC entries from MADT\n");
+ goto out_err;
+ }
+
+ /*
+ * Parse MADT GIC distributor entries
+ */
+ err = acpi_parse_madt_gic_distributor_entries();
+ if (err) {
+ pr_err("Failed to find GIC Distributor entries from MADT\n");
+ goto out_err;
}

I think we can do better here. e.g. Roll up acpi_parse_madt_gic_entries() and acpi_parse_madt_gic_distributor_entries() into acpi_parse_madt(), since we already get the MADT table pointer in the callback. I'll send a follow up patch to that sometime later.

It makes sense to me too, please send it out for review.

Also, this is based off of your remotes/origin/arm64-acpi-core-v3 branch.

Ok, I will merge your patch into v3 patch set.

Thanks Hanjun