Hi Simon,
On Sun, Jun 14, 2020 at 10:55 AM Simon Glass <s...@chromium.org> wrote:
>
> Add a new file to handle generating ACPI code programatically. This is
> used when information must be dynamically added to the tables, e.g. the
> SSDT.
>
> Initial support is just for writing simple values. Also add a 'base' value
> so that the table can be freed. This likely doesn't happen in normal code,
> but is nice to do in tests.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Update to add a new 'base' field to struct acpi_ctx
> - Free the memory allocated to the table and context
>
> include/acpi/acpigen.h | 49 ++++++++++++++++++++++++++++++
> include/dm/acpi.h | 2 ++
> lib/acpi/Makefile | 1 +
> lib/acpi/acpi_table.c | 1 +
> lib/acpi/acpigen.c | 38 +++++++++++++++++++++++
> test/dm/Makefile | 1 +
> test/dm/acpigen.c | 69 ++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 161 insertions(+)
> create mode 100644 include/acpi/acpigen.h
> create mode 100644 lib/acpi/acpigen.c
> create mode 100644 test/dm/acpigen.c
>
> diff --git a/include/acpi/acpigen.h b/include/acpi/acpigen.h
> new file mode 100644
> index 0000000000..8809cdb4e1
> --- /dev/null
> +++ b/include/acpi/acpigen.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Core ACPI (Advanced Configuration and Power Interface) support
> + *
> + * Copyright 2019 Google LLC
> + *
> + * Modified from coreboot file acpigen.h
> + */
> +
> +#ifndef __ACPI_ACPIGEN_H
> +#define __ACPI_ACPIGEN_H
> +
> +#include <linux/types.h>
> +
> +struct acpi_ctx;
> +
> +/**
> + * acpigen_get_current() - Get the current ACPI code output pointer
> + *
> + * @ctx: ACPI context pointer
> + * @return output pointer
> + */
> +u8 *acpigen_get_current(struct acpi_ctx *ctx);
> +
> +/**
> + * acpigen_emit_byte() - Emit a byte to the ACPI code
> + *
> + * @ctx: ACPI context pointer
> + * @data: Value to output
> + */
> +void acpigen_emit_byte(struct acpi_ctx *ctx, uint data);
> +
> +/**
> + * acpigen_emit_word() - Emit a 16-bit word to the ACPI code
> + *
> + * @ctx: ACPI context pointer
> + * @data: Value to output
> + */
> +void acpigen_emit_word(struct acpi_ctx *ctx, uint data);
> +
> +/**
> + * acpigen_emit_dword() - Emit a 32-bit 'double word' to the ACPI code
> + *
> + * @ctx: ACPI context pointer
> + * @data: Value to output
> + */
> +void acpigen_emit_dword(struct acpi_ctx *ctx, uint data);
> +
> +#endif
> diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> index 7563a4c60a..696b1a96a0 100644
> --- a/include/dm/acpi.h
> +++ b/include/dm/acpi.h
> @@ -29,6 +29,7 @@
> *
> * This contains a few useful pieces of information used when writing
> *
> + * @base: Base address of ACPI tables
> * @current: Current address for writing
> * @rsdp: Pointer to the Root System Description Pointer, typically used when
> * adding a new table. The RSDP holds pointers to the RSDT and XSDT.
> @@ -36,6 +37,7 @@
> * @xsdt: Pointer to the Extended System Description Table
> */
> struct acpi_ctx {
> + void *base;
> void *current;
> struct acpi_rsdp *rsdp;
> struct acpi_rsdt *rsdt;
> diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile
> index caae6c01bd..85a1f774ad 100644
> --- a/lib/acpi/Makefile
> +++ b/lib/acpi/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0+
> #
>
> +obj-y += acpigen.o
> obj-y += acpi_device.o
> obj-y += acpi_table.o
> diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> index 431776666e..17d5258438 100644
> --- a/lib/acpi/acpi_table.c
> +++ b/lib/acpi/acpi_table.c
> @@ -238,6 +238,7 @@ static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
> void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> {
> ctx->current = start;
> + ctx->current = start;
This seems to be unnecessary.
>
> /* Align ACPI tables to 16 byte */
> acpi_align(ctx);
> diff --git a/lib/acpi/acpigen.c b/lib/acpi/acpigen.c
> new file mode 100644
> index 0000000000..59bd3af0b7
> --- /dev/null
> +++ b/lib/acpi/acpigen.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generation of ACPI (Advanced Configuration and Power Interface) tables
> + *
> + * Copyright 2019 Google LLC
> + * Mostly taken from coreboot
> + */
> +
> +#define LOG_CATEGORY LOGC_ACPI
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <acpi/acpigen.h>
> +#include <dm/acpi.h>
> +
> +u8 *acpigen_get_current(struct acpi_ctx *ctx)
> +{
> + return ctx->current;
> +}
> +
> +void acpigen_emit_byte(struct acpi_ctx *ctx, uint data)
> +{
> + *(u8 *)ctx->current++ = data;
> +}
> +
> +void acpigen_emit_word(struct acpi_ctx *ctx, uint data)
> +{
> + acpigen_emit_byte(ctx, data & 0xff);
> + acpigen_emit_byte(ctx, (data >> 8) & 0xff);
> +}
> +
> +void acpigen_emit_dword(struct acpi_ctx *ctx, uint data)
> +{
> + acpigen_emit_byte(ctx, data & 0xff);
> + acpigen_emit_byte(ctx, (data >> 8) & 0xff);
> + acpigen_emit_byte(ctx, (data >> 16) & 0xff);
> + acpigen_emit_byte(ctx, (data >> 24) & 0xff);
It would be better to document the endianness which is little-endian.
> +}
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 6c18fd04ce..e3e0cccf01 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
> obj-$(CONFIG_UT_DM) += core.o
> ifneq ($(CONFIG_SANDBOX),)
> obj-$(CONFIG_ACPIGEN) += acpi.o
> +obj-$(CONFIG_ACPIGEN) += acpigen.o
> obj-$(CONFIG_SOUND) += audio.o
> obj-$(CONFIG_BLK) += blk.o
> obj-$(CONFIG_BOARD) += board.o
> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> new file mode 100644
> index 0000000000..c353408021
> --- /dev/null
> +++ b/test/dm/acpigen.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Tests for ACPI code generation
> + *
> + * Copyright 2019 Google LLC
> + * Written by Simon Glass <s...@chromium.org>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <malloc.h>
> +#include <acpi/acpigen.h>
> +#include <asm/unaligned.h>
> +#include <dm/acpi.h>
> +#include <dm/test.h>
> +#include <test/ut.h>
> +
> +static int alloc_context(struct acpi_ctx **ctxp)
> +{
> + struct acpi_ctx *ctx;
> +
> + *ctxp = NULL;
> + ctx = malloc(sizeof(*ctx));
> + if (!ctx)
> + return -ENOMEM;
> + ctx->base = malloc(150);
nits: avoid magic number
> + if (!ctx->base) {
> + free(ctx);
> + return -ENOMEM;
> + }
> + ctx->current = ctx->base;
> + *ctxp = ctx;
> +
> + return 0;
> +}
> +
> +static void free_context(struct acpi_ctx **ctxp)
> +{
> + free((*ctxp)->base);
> + free(*ctxp);
> + *ctxp = NULL;
> +}
> +
> +/* Test emitting simple types and acpigen_get_current() */
> +static int dm_test_acpi_emit_simple(struct unit_test_state *uts)
> +{
> + struct acpi_ctx *ctx;
> + u8 *ptr;
> +
> + ut_assertok(alloc_context(&ctx));
> +
> + ptr = acpigen_get_current(ctx);
> + acpigen_emit_byte(ctx, 0x23);
> + ut_asserteq(1, acpigen_get_current(ctx) - ptr);
> + ut_asserteq(0x23, *(u8 *)ptr);
> +
> + acpigen_emit_word(ctx, 0x1234);
> + ut_asserteq(3, acpigen_get_current(ctx) - ptr);
> + ut_asserteq(0x1234, get_unaligned((u16 *)(ptr + 1)));
> +
> + acpigen_emit_dword(ctx, 0x87654321);
> + ut_asserteq(7, acpigen_get_current(ctx) - ptr);
> + ut_asserteq(0x87654321, get_unaligned((u32 *)(ptr + 3)));
> +
> + free_context(&ctx);
> +
> + return 0;
> +}
> +DM_TEST(dm_test_acpi_emit_simple, 0);
> --
Regards,
Bin
