[PATCH v2 05/35] acpi: Support generation of ACPI code
Simon Glass
sjg at chromium.org
Tue Jun 9 23:14:11 CEST 2020
Hi Wolfgang,
On Thu, 14 May 2020 at 02:32, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon,
>
> -----"Simon Glass" <sjg at chromium.org> schrieb: -----
> > Betreff: [PATCH v2 05/35] acpi: Support generation of ACPI code
> >
> > 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.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2: None
> > Changes in v1: None
> >
> > include/acpi/acpigen.h | 49 +++++++++++++++++++++++++++++++
> > lib/acpi/Makefile | 1 +
> > lib/acpi/acpigen.c | 38 ++++++++++++++++++++++++
> > test/dm/Makefile | 1 +
> > test/dm/acpigen.c | 65 ++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 154 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/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/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)
> > +{
>
> As we expect exactly a byte, could data be of type uint8_t ?
> Similar for the functions below.
It could but I really try to avoid that. It means that the compiler
has to mask the values coming in in a register wihch adds to code
size.
>
> > + *(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);
>
> This function assumes little-endian host endianess. This works under
> x86 and probably most of ARM, and I'm not aware of other architectures
> using ACPI.
>
> Should it be made more portable anyway e.g. by using cpu_to_le16()?
Do you mean that ACPI can store the bytes in the other order? I
thought it was only big-endian but actually I don't know that. Do you
have any details?
Or are you asking about the shift? I believe that on big-endian
machines, shifting right still divides by 256, and so does what is
expected.
>
> > +}
> > +
> > +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);
> > +}
> > 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..68f2b73132
> > --- /dev/null
> > +++ b/test/dm/acpigen.c
> > @@ -0,0 +1,65 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Tests for ACPI code generation
> > + *
> > + * Copyright 2019 Google LLC
> > + * Written by Simon Glass <sjg at 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->current = malloc(150);
> > + if (!ctx->current)
>
> free(ctx)
>
> > + return -ENOMEM;
> > + *ctxp = ctx;
> > +
> > + return 0;
> > +}
> > +
> > +static void free_context(struct acpi_ctx **ctxp)
> > +{
>
> Should the memory that was initally allocated to ctx->current also be
> released or stay allocted?
It is normally allocated by the code that uses it, and generally just
points to memory (e.g. 0xf0000) rather than using malloc(). But for
the benefit of tests I can add a base value, and free it.
Regards,
Simon
More information about the U-Boot
mailing list