[U-Boot] [UBOOT PATCH v6 1/3] x86: Generate a valid ACPI table

[Bin Meng]

Hi Saket,

On Sat, Aug 22, 2015 at 2:50 PM, Saket Sinha <saket.sinha89 at gmail.com> wrote:
> Implement write_acpi_table() to create a minimal working ACPI table.
> This includes writing FACS, XSDT, RSDP, FADT, MCFG, MADT, DSDT & SSDT
> ACPI table entries.
>
> Use a Kconfig option GENERATE_ACPI_TABLE to tell U-Boot whether we need
> actually write the APCI table just like we did for PIRQ routing, MP table
> and SFI tables. With ACPI table existence, linux kernel gets control of
> power management, thermal management, configuration management and
> monitoring in hardware.
>
> Signed-off-by: Saket Sinha <saket.sinha89 at gmail.com>

Now I am more comfortable with this version, so

Reviewed-by: Bin Meng <bmeng.cn at gmail.com>

But still two nits below.

> ---
>
>  arch/x86/Kconfig                  |   9 +
>  arch/x86/include/asm/acpi_table.h | 390 ++++++++++++++++++++++++++++++++++
>  arch/x86/lib/Makefile             |   1 +
>  arch/x86/lib/acpi_table.c         | 436 ++++++++++++++++++++++++++++++++++++++
>  arch/x86/lib/tables.c             |   5 +
>  scripts/Makefile.lib              |  11 +
>  6 files changed, 852 insertions(+)
>  create mode 100644 arch/x86/include/asm/acpi_table.h
>  create mode 100644 arch/x86/lib/acpi_table.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 273f08f..5e42d7d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -349,6 +349,15 @@ config GENERATE_MP_TABLE
>           multiprocessing as well as symmetric I/O interrupt handling with
>           the local APIC and I/O APIC.
>
> +config GENERATE_ACPI_TABLE
> +       bool "Generate an ACPI (Advanced Configuration and Power Interface) table"
> +       default n
> +       help
> +         The Advanced Configuration and Power Interface (ACPI) specification
> +         provides an open standard for device configuration and management
> +         by the operating system. It defines platform-independent interfaces
> +         for configuration and power management monitoring.
> +
>  endmenu
>
>  config MAX_PIRQ_LINKS
> diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
> new file mode 100644
> index 0000000..cceed97
> --- /dev/null
> +++ b/arch/x86/include/asm/acpi_table.h
> @@ -0,0 +1,390 @@
> +/*
> + * Based on acpi.c from coreboot
> + *
> + * Copyright (C) 2015, Saket Sinha <saket.sinha89 at gmail.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <asm/post.h>
> +#include <linux/string.h>
> +
> +#define        RSDP_SIG        "RSD PTR "      /* RSDT pointer signature */
> +#define        ACPI_TABLE_CREATOR      "UBOOT   "      /* Must be 8 bytes long! */
> +#define        OEM_ID  "UBOOT "        /* Must be 6 bytes long! */
> +#define        ASLC    "INTL"  /* Must be exactly 4 bytes long! */

I think you misunderstood me. I was saying adding tab to make those
macros' values aligned on the same indention level. But no need to add
tab between 'defined' and the macro. Like this:

#define<space>RSDP_SIG<one or more tabs>"RSD PTR "

[snip]

> +/* FACS flags */
> +#define        ACPI_FACS_S4BIOS_F      (1 << 0)
> +#define        ACPI_FACS_64BIT_WAKE_F  (1 << 1)
> +/* Bits 31..2: reserved */
> +
> +/* These can be used by the target port. */

Nits: ending period is not needed for a single sentence.

[snip]

Regards,
Bin