| Message ID | 1486149538-20432-5-git-send-email-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Fix and clean-up for ACPI and EFI | expand |
On 03/02/17 19:18, Julien Grall wrote: > @@ -260,16 +257,22 @@ int __init acpi_boot_table_init(void) > error = acpi_table_init(); > if ( error ) > { > - disable_acpi(); > - return error; > + printk("%s: Unable to initialize table parser (%d)\n", > + __FUNCTION__, error); As a nit, please use __func__. It is standard C, whereas __FUNCTION__ is a GCCism. (On that note, there are very few remaining uses in the hypervisor - let me submit a patch killing the remaining uses.) ~Andrew
Hi Andrew, On 03/02/17 20:44, Andrew Cooper wrote: > On 03/02/17 19:18, Julien Grall wrote: >> @@ -260,16 +257,22 @@ int __init acpi_boot_table_init(void) >> error = acpi_table_init(); >> if ( error ) >> { >> - disable_acpi(); >> - return error; >> + printk("%s: Unable to initialize table parser (%d)\n", >> + __FUNCTION__, error); > > As a nit, please use __func__. > > It is standard C, whereas __FUNCTION__ is a GCCism. > > (On that note, there are very few remaining uses in the hypervisor - let > me submit a patch killing the remaining uses.) I will fix it and try to remember for next time. Cheers,
On Fri, 3 Feb 2017, Julien Grall wrote: > There are multiple path disable ACPI on error. Consolidate in a single > place, this will help in a follow-up patch to add more code on the error > path. > > Signed-off-by: Julien Grall <julien.grall@arm.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/acpi/boot.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c > index c3242a0..889208a 100644 > --- a/xen/arch/arm/acpi/boot.c > +++ b/xen/arch/arm/acpi/boot.c > @@ -234,7 +234,7 @@ static int __init dt_scan_depth1_nodes(const void *fdt, int node, > */ > int __init acpi_boot_table_init(void) > { > - int error; > + int error = 0; > > /* > * Enable ACPI instead of device tree unless > @@ -245,10 +245,7 @@ int __init acpi_boot_table_init(void) > if ( param_acpi_off || ( !param_acpi_force > && device_tree_for_each_node(device_tree_flattened, > dt_scan_depth1_nodes, NULL))) > - { > - disable_acpi(); > - return 0; > - } > + goto disable; > > /* > * ACPI is disabled at this point. Enable it in order to parse > @@ -260,16 +257,22 @@ int __init acpi_boot_table_init(void) > error = acpi_table_init(); > if ( error ) > { > - disable_acpi(); > - return error; > + printk("%s: Unable to initialize table parser (%d)\n", > + __FUNCTION__, error); > + goto disable; > } > > - if ( acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt) ) > + error = acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt); > + if ( error ) > { > - /* disable ACPI if no FADT is found */ > - disable_acpi(); > - printk("Can't find FADT\n"); > + printk("%s: FADT not found (%d)\n", __FUNCTION__, error); > + goto disable; > } > > return 0; > + > +disable: > + disable_acpi(); > + > + return error; > } > -- > 1.9.1 >
diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c index c3242a0..889208a 100644 --- a/xen/arch/arm/acpi/boot.c +++ b/xen/arch/arm/acpi/boot.c @@ -234,7 +234,7 @@ static int __init dt_scan_depth1_nodes(const void *fdt, int node, */ int __init acpi_boot_table_init(void) { - int error; + int error = 0; /* * Enable ACPI instead of device tree unless @@ -245,10 +245,7 @@ int __init acpi_boot_table_init(void) if ( param_acpi_off || ( !param_acpi_force && device_tree_for_each_node(device_tree_flattened, dt_scan_depth1_nodes, NULL))) - { - disable_acpi(); - return 0; - } + goto disable; /* * ACPI is disabled at this point. Enable it in order to parse @@ -260,16 +257,22 @@ int __init acpi_boot_table_init(void) error = acpi_table_init(); if ( error ) { - disable_acpi(); - return error; + printk("%s: Unable to initialize table parser (%d)\n", + __FUNCTION__, error); + goto disable; } - if ( acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt) ) + error = acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt); + if ( error ) { - /* disable ACPI if no FADT is found */ - disable_acpi(); - printk("Can't find FADT\n"); + printk("%s: FADT not found (%d)\n", __FUNCTION__, error); + goto disable; } return 0; + +disable: + disable_acpi(); + + return error; }
There are multiple path disable ACPI on error. Consolidate in a single place, this will help in a follow-up patch to add more code on the error path. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/acpi/boot.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)