RE: [PATCH v2] ACPI / tables: Fix DSDT override mechanism

[Zheng, Lv]

Hi,

I also noticed this and was about to write a fix patch.
The build issue wasn't caught by LKP, I noticed it during ACPICA release test.
Thanks for doing this. :-)

> From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Rafael J. Wysocki
> Subject: [PATCH v2] ACPI / tables: Fix DSDT override mechanism
> 
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> 
> Commit 5ae74f2cc2f1 (ACPI / tables: Move table override mechanisms to
> tables.c) forgot to move the CONFIG_ACPI_CUSTOM_DSDT_FILE inclusion
> directive from osl.c to tables.c.  Fix that.
> 
> Fixes: 5ae74f2cc2f1 (ACPI / tables: Move table override mechanisms to tables.c)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
> 
> A cosmetic change.
> 
> v1 -> v2: Move the new #include closer to the other #include directives in
>           tables.c.
> 
> ---
>  drivers/acpi/osl.c    |    4 ----
>  drivers/acpi/tables.c |    4 ++++
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/drivers/acpi/tables.c
> ================================================================
> ===
> --- linux-pm.orig/drivers/acpi/tables.c
> +++ linux-pm/drivers/acpi/tables.c
> @@ -36,6 +36,10 @@
>  #include <linux/memblock.h>
>  #include "internal.h"
> 
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT
> +#include CONFIG_ACPI_CUSTOM_DSDT_FILE
> +#endif
> +
[Lv Zheng] 
IMO, we could move this closer to acpi_os_table_override.
And move CONFIG_ACPI_CUSTOM_DSDT out of this function to form the following style:

#ifdef CONFIG_ACPI_CUSTOM_DSDT
#include CONFIG_ACPI_CUSTOM_DSDT_FILE

acpi_status acpi_os_table_override(...)
{
}
#else
acpi_status acpi_os_table_override(...)
{
	return AE_SUPPORT;
}
#endif

We can return AE_SUPPORT from acpi_os_table_override() here, please refer to drivers/acpi/acpica/tbinstal.c.
By doing so, we can:
1. make acpi_os_table_override clearer, and
2. make #ifdef maintained at function boundary, and
3. remove additional #ifdef CONFIG_ACPI_CUSTOM_DSDT, and
4. help to avoid same future mistakes.

Thanks and best regards
-Lv

>  #define ACPI_MAX_TABLES		128
> 
>  static char *mps_inti_flags_polarity[] = { "dfl", "high", "res", "low" };
> Index: linux-pm/drivers/acpi/osl.c
> ================================================================
> ===
> --- linux-pm.orig/drivers/acpi/osl.c
> +++ linux-pm/drivers/acpi/osl.c
> @@ -56,10 +56,6 @@ struct acpi_os_dpc {
>  	struct work_struct work;
>  };
> 
> -#ifdef CONFIG_ACPI_CUSTOM_DSDT
> -#include CONFIG_ACPI_CUSTOM_DSDT_FILE
> -#endif
> -
>  #ifdef ENABLE_DEBUGGER
>  #include <linux/kdb.h>
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html