Hi Simon,
-----"Simon Glass" sjg@chromium.org schrieb: -----
Betreff: Re: [PATCH v2 05/35] acpi: Support generation of ACPI code
Hi Wolfgang,
On Thu, 14 May 2020 at 02:32, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
-----"Simon Glass" sjg@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@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.
Ok, thanks for the explanation.
*(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?
No, I did not mean that.
I thought it was only big-endian but actually I don't know that. Do you have any details?
You are right that ACPI only supports a single endianess, but it is little-endian, not big-endian.
Section 5.2 "ACPI System Description Tables" of ACPI 6.3 states: "All numeric values in ACPI-defined tables, blocks, and structures are always encoded in little endian format."
and section 19.3.5 "ASL Data Types" tells us that an integer is "An n-bit little-endian unsigned integer".
The only exception to the everything-is-little-endian-rule that I could find is given in Section "5.2.6 System Description Table Header":
"Tables defined outside of the ACPI specification may define data value encodings in either little endian or big endian format. For the purpose of clarity, external table definition documents should include the endian-ness of their data value encodings."
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.
Yes, that is what I was worried about, but I got it wrong. As long as we only write, we only have to take care about the endianess of the target (ACPI).
The could above should work on both little-endian and big-endian architectures, so please ignore my comment.
+}
+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@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
regards, Wolfgang