From: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
To: u-boot@lists.denx.de
Subject: Antwort: [PATCH v2 21/39] acpi: Convert part of acpi_table to use acpi_ctx
Date: Wed, 11 Mar 2020 13:58:48 +0100 [thread overview]
Message-ID: <OFD40C4652.9A011211-ONC1258528.003738E9-C1258528.00474D76@br-automation.com> (raw)
In-Reply-To: <20200308214442.v2.21.I93e1e33891714417335e1dd517982b18bf9f882f@changeid>
Hi Simon,
-----"Simon Glass" <sjg@chromium.org> schrieb: -----
>
> The current code uses an address but a pointer would result in fewer
> casts. Also it repeats the alignment code in a lot of places so this would
> be better done in a helper function.
>
> Update write_acpi_tables() to make use of the new acpi_ctx structure,
> adding a few helpers to clean things up.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2: None
>
> arch/x86/lib/acpi_table.c | 88 +++++++++++++++++++--------------------
> include/acpi_table.h | 36 ++++++++++++++++
> lib/acpi/acpi_table.c | 22 ++++++++++
> test/dm/acpi.c | 28 +++++++++++++
> 4 files changed, 129 insertions(+), 45 deletions(-)
>
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 487fef87f2..8e13d6a3e6 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -10,6 +10,7 @@
> #include <cpu.h>
> #include <dm.h>
> #include <dm/uclass-internal.h>
> +#include <mapmem.h>
> #include <serial.h>
> #include <version.h>
> #include <asm/acpi/global_nvs.h>
> @@ -19,6 +20,7 @@
> #include <asm/mpspec.h>
> #include <asm/tables.h>
> #include <asm/arch/global_nvs.h>
> +#include <dm/acpi.h>
>
> /*
> * IASL compiles the dsdt entries and writes the hex values
> @@ -468,9 +470,9 @@ static void acpi_create_spcr(struct acpi_spcr *spcr)
> /*
> * QEMU's version of write_acpi_tables is defined in drivers/misc/qfw.c
> */
> -ulong write_acpi_tables(ulong start)
> +ulong write_acpi_tables(ulong start_addr)
> {
> - u32 current;
> + struct acpi_ctx sctx, *ctx = &sctx;
> struct acpi_rsdp *rsdp;
> struct acpi_rsdt *rsdt;
> struct acpi_xsdt *xsdt;
> @@ -481,60 +483,61 @@ ulong write_acpi_tables(ulong start)
> struct acpi_madt *madt;
> struct acpi_csrt *csrt;
> struct acpi_spcr *spcr;
> + void *start;
> + ulong addr;
> int i;
>
> - current = start;
> + start = map_sysmem(start_addr, 0);
> + ctx->current = start;
>
> /* Align ACPI tables to 16 byte */
> - current = ALIGN(current, 16);
> + acpi_align(ctx);
>
> - debug("ACPI: Writing ACPI tables at %lx\n", start);
> + debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
>
> /* We need at least an RSDP and an RSDT Table */
> - rsdp = (struct acpi_rsdp *)current;
> - current += sizeof(struct acpi_rsdp);
> - current = ALIGN(current, 16);
> - rsdt = (struct acpi_rsdt *)current;
> - current += sizeof(struct acpi_rsdt);
> - current = ALIGN(current, 16);
> - xsdt = (struct acpi_xsdt *)current;
> - current += sizeof(struct acpi_xsdt);
> + rsdp = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
> + rsdt = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
> + xsdt = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
> /*
> * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> * boundary (Windows checks this, but Linux does not).
> */
> - current = ALIGN(current, 64);
> + acpi_align_large(ctx);
>
> /* clear all table memory */
> - memset((void *)start, 0, current - start);
> + memset((void *)start, 0, ctx->current - start);
>
> acpi_write_rsdp(rsdp, rsdt, xsdt);
> acpi_write_rsdt(rsdt);
> acpi_write_xsdt(xsdt);
>
> debug("ACPI: * FACS\n");
> - facs = (struct acpi_facs *)current;
> - current += sizeof(struct acpi_facs);
> - current = ALIGN(current, 16);
> + facs = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_facs));
>
> acpi_create_facs(facs);
>
> debug("ACPI: * DSDT\n");
> - dsdt = (struct acpi_table_header *)current;
> + dsdt = ctx->current;
> memcpy(dsdt, &AmlCode, sizeof(struct acpi_table_header));
> - current += sizeof(struct acpi_table_header);
> - memcpy((char *)current,
> + acpi_inc(ctx, sizeof(struct acpi_table_header));
> + memcpy(ctx->current,
> (char *)&AmlCode + sizeof(struct acpi_table_header),
> dsdt->length - sizeof(struct acpi_table_header));
> - current += dsdt->length - sizeof(struct acpi_table_header);
> - current = ALIGN(current, 16);
> + acpi_inc_align(ctx, dsdt->length - sizeof(struct acpi_table_header));
>
> /* Pack GNVS into the ACPI table area */
> for (i = 0; i < dsdt->length; i++) {
> u32 *gnvs = (u32 *)((u32)dsdt + i);
> if (*gnvs == ACPI_GNVS_ADDR) {
> - debug("Fix up global NVS in DSDT to 0x%08x\n", current);
> - *gnvs = current;
> + ulong addr = (ulong)map_to_sysmem(ctx->current);
> +
> + debug("Fix up global NVS in DSDT to %#08lx\n", addr);
> + *gnvs = addr;
> break;
> }
> }
> @@ -544,51 +547,46 @@ ulong write_acpi_tables(ulong start)
> dsdt->checksum = table_compute_checksum((void *)dsdt, dsdt->length);
>
> /* Fill in platform-specific global NVS variables */
> - acpi_create_gnvs((struct acpi_global_nvs *)current);
> - current += sizeof(struct acpi_global_nvs);
> - current = ALIGN(current, 16);
> + acpi_create_gnvs(ctx->current);
> + acpi_inc_align(ctx, sizeof(struct acpi_global_nvs));
>
> debug("ACPI: * FADT\n");
> - fadt = (struct acpi_fadt *)current;
> - current += sizeof(struct acpi_fadt);
> - current = ALIGN(current, 16);
> + fadt = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_fadt));
> acpi_create_fadt(fadt, facs, dsdt);
> acpi_add_table(rsdp, fadt);
>
> debug("ACPI: * MADT\n");
> - madt = (struct acpi_madt *)current;
> + madt = ctx->current;
> acpi_create_madt(madt);
> - current += madt->header.length;
> + acpi_inc_align(ctx, madt->header.length);
> acpi_add_table(rsdp, madt);
> - current = ALIGN(current, 16);
>
> debug("ACPI: * MCFG\n");
> - mcfg = (struct acpi_mcfg *)current;
> + mcfg = ctx->current;
> acpi_create_mcfg(mcfg);
> - current += mcfg->header.length;
> + acpi_inc_align(ctx, mcfg->header.length);
> acpi_add_table(rsdp, mcfg);
> - current = ALIGN(current, 16);
>
> debug("ACPI: * CSRT\n");
> - csrt = (struct acpi_csrt *)current;
> + csrt = ctx->current;
> acpi_create_csrt(csrt);
> - current += csrt->header.length;
> + acpi_inc_align(ctx, csrt->header.length);
> acpi_add_table(rsdp, csrt);
> - current = ALIGN(current, 16);
>
> debug("ACPI: * SPCR\n");
> - spcr = (struct acpi_spcr *)current;
> + spcr = ctx->current;
> acpi_create_spcr(spcr);
> - current += spcr->header.length;
> + acpi_inc_align(ctx, spcr->header.length);
> acpi_add_table(rsdp, spcr);
> - current = ALIGN(current, 16);
>
> - debug("current = %x\n", current);
> + addr = map_to_sysmem(ctx->current);
> + debug("current = %lx\n", addr);
>
> acpi_rsdp_addr = (unsigned long)rsdp;
> debug("ACPI: done\n");
>
> - return current;
> + return addr;
> }
>
> ulong acpi_get_rsdp_addr(void)
> diff --git a/include/acpi_table.h b/include/acpi_table.h
> index 3fd2ef16b0..5fd0fa71a6 100644
> --- a/include/acpi_table.h
> +++ b/include/acpi_table.h
> @@ -26,6 +26,8 @@
>
> #if !defined(__ACPI__)
>
> +struct acpi_ctx;
> +
> /*
> * RSDP (Root System Description Pointer)
> * Note: ACPI 1.0 didn't have length, xsdt_address, and ext_checksum
> @@ -519,6 +521,40 @@ int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags);
> */
> void acpi_fill_header(struct acpi_table_header *header, char *signature);
>
> +/**
> + * acpi_align() - Align the ACPI output pointer to a 16-byte boundary
> + *
> + * @ctx: ACPI context
> + */
> +void acpi_align(struct acpi_ctx *ctx);
Nit: The function names acpi_align() and acpi_align_large() are both vague
on the exact alignment that is used.
How about acpi_align16() and acpi_align64() ?
> +
> +/**
> + * acpi_align_large() - Align the ACPI output pointer to a 64-byte boundary
> + *
> + * @ctx: ACPI context
> + */
> +void acpi_align_large(struct acpi_ctx *ctx);
> +
> +/**
> + * acpi_inc() - Increment the ACPI output pointer by a bit
> + *
> + * The pointer is NOT aligned afterwards.
> + *
> + * @ctx: ACPI context
> + * @amount: Amount to increment by
> + */
> +void acpi_inc(struct acpi_ctx *ctx, uint amount);
> +
> +/**
> + * acpi_inc_align() - Increment the ACPI output pointer by a bit and align
> + *
> + * The pointer is aligned afterwards.
> + *
> + * @ctx: ACPI context
> + * @amount: Amount to increment by
> + */
> +void acpi_inc_align(struct acpi_ctx *ctx, uint amount);
Similar nit as above: acpi_inc_align16() ?
> +
> #endif /* !__ACPI__*/
>
> #include <asm/acpi_table.h>
> diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> index a86bfa6187..3d24cc26b6 100644
> --- a/lib/acpi/acpi_table.c
> +++ b/lib/acpi/acpi_table.c
> @@ -10,6 +10,7 @@
> #include <acpi_table.h>
> #include <cpu.h>
> #include <version.h>
> +#include <dm/acpi.h>
>
> int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags)
> {
> @@ -94,3 +95,24 @@ void acpi_fill_header(struct acpi_table_header *header, char *signature)
> header->oem_revision = U_BOOT_BUILD_DATE;
> memcpy(header->aslc_id, ASLC_ID, 4);
> }
> +
> +void acpi_align(struct acpi_ctx *ctx)
> +{
> + ctx->current = (void *)ALIGN((ulong)ctx->current, 16);
> +}
> +
> +void acpi_align_large(struct acpi_ctx *ctx)
> +{
> + ctx->current = (void *)ALIGN((ulong)ctx->current, 64);
> +}
> +
> +void acpi_inc(struct acpi_ctx *ctx, uint amount)
> +{
> + ctx->current += amount;
> +}
> +
> +void acpi_inc_align(struct acpi_ctx *ctx, uint amount)
> +{
> + ctx->current += amount;
> + acpi_align(ctx);
> +}
> diff --git a/test/dm/acpi.c b/test/dm/acpi.c
> index b87fbd16b0..0bd7e51ac9 100644
> --- a/test/dm/acpi.c
> +++ b/test/dm/acpi.c
> @@ -152,3 +152,31 @@ static int dm_test_acpi_write_tables(struct unit_test_state *uts)
> return 0;
> }
> DM_TEST(dm_test_acpi_write_tables, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test basic ACPI functions */
> +static int dm_test_acpi_basic(struct unit_test_state *uts)
> +{
> + struct acpi_ctx ctx;
> +
> + /* Check align works */
> + ctx.current = (void *)5;
> + acpi_align(&ctx);
> + ut_asserteq_ptr((void *)16, ctx.current);
> +
> + /* Check that align does nothing if already aligned */
> + acpi_align(&ctx);
> + ut_asserteq_ptr((void *)16, ctx.current);
> + acpi_align_large(&ctx);
> + ut_asserteq_ptr((void *)64, ctx.current);
> + acpi_align_large(&ctx);
> + ut_asserteq_ptr((void *)64, ctx.current);
> +
> + /* Check incrementing */
> + acpi_inc(&ctx, 3);
> + ut_asserteq_ptr((void *)67, ctx.current);
> + acpi_inc_align(&ctx, 3);
> + ut_asserteq_ptr((void *)80, ctx.current);
> +
> + return 0;
> +}
> +DM_TEST(dm_test_acpi_basic, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> --
> 2.25.1.481.gfbce0eb801-goog
Apart from the nits above:
Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
next prev parent reply other threads:[~2020-03-11 12:58 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-09 3:44 [PATCH v2 00/39] dm: Add programmatic generation of ACPI tables (part A) Simon Glass
2020-03-09 3:44 ` [PATCH v2 01/39] cpu: Support querying the address width Simon Glass
2020-03-09 3:44 ` [PATCH v2 02/39] spi: Add SPI mode enums Simon Glass
2020-03-09 7:41 ` Andy Shevchenko
2020-03-09 3:44 ` [PATCH v2 03/39] tpm: cr50: Release locality on exit Simon Glass
2020-03-09 3:44 ` [PATCH v2 04/39] tpm: cr50: Add a comment for cr50_priv Simon Glass
2020-03-09 3:44 ` [PATCH v2 05/39] tpm: cr50: Use the correct GPIO binding Simon Glass
2020-03-09 3:44 ` [PATCH v2 06/39] tpm: Don't cleanup unless an error happens Simon Glass
2020-03-09 3:44 ` [PATCH v2 07/39] dm: pci: Allow disabling auto-config for a device Simon Glass
2020-03-09 7:43 ` Andy Shevchenko
2020-03-09 3:44 ` [PATCH v2 08/39] x86: Correct wording of coreboot source code Simon Glass
2020-03-09 7:44 ` Andy Shevchenko
2020-03-10 23:22 ` Simon Glass
2020-03-09 3:44 ` [PATCH v2 09/39] x86: apl: Move p2sb ofdata reading to the correct method Simon Glass
2020-03-10 14:39 ` Andy Shevchenko
2020-03-11 12:17 ` Simon Glass
2020-03-11 13:06 ` Andy Shevchenko
2020-03-09 3:44 ` [PATCH v2 10/39] pci: Adjust dm_pci_read_bar32() to return errors correctly Simon Glass
2020-03-09 3:44 ` [PATCH v2 11/39] x86: apl: Add Global NVS table header Simon Glass
2020-03-09 3:44 ` [PATCH v2 12/39] dm: core: Add basic ACPI support Simon Glass
2020-03-10 14:46 ` Andy Shevchenko
2020-03-11 12:17 ` Simon Glass
2020-03-09 3:44 ` [PATCH v2 13/39] acpi: Add a binding for ACPI settings in the device tree Simon Glass
2020-03-10 14:50 ` Andy Shevchenko
2020-03-12 3:22 ` Simon Glass
2020-03-09 3:44 ` [PATCH v2 14/39] acpi: Add a simple sandbox test Simon Glass
2020-03-09 3:44 ` [PATCH v2 15/39] x86: Move acpi_table header to main include/ directory Simon Glass
2020-03-09 3:44 ` [PATCH v2 16/39] acpi: Add an __ACPI__ preprocessor symbol Simon Glass
2020-03-09 3:44 ` [PATCH v2 17/39] acpi: Add a central location for table version numbers Simon Glass
2020-03-09 3:44 ` [PATCH v2 18/39] acpi: Add support for DMAR Simon Glass
2020-03-09 3:44 ` [PATCH v2 19/39] acpi: Move acpi_fill_header() to generic code Simon Glass
2020-03-09 3:44 ` [PATCH v2 20/39] acpi: Add a method to write tables for a device Simon Glass
2020-03-09 3:44 ` [PATCH v2 21/39] acpi: Convert part of acpi_table to use acpi_ctx Simon Glass
2020-03-09 3:44 ` [PATCH v2 22/39] x86: Allow devices to write ACPI tables Simon Glass
2020-03-09 3:44 ` [PATCH v2 23/39] acpi: Drop code for missing XSDT from acpi_write_rsdp() Simon Glass
2020-03-09 3:44 ` [PATCH v2 24/39] acpi: Move acpi_add_table() to generic code Simon Glass
2020-03-09 3:44 ` [PATCH v2 25/39] acpi: Put table-setup code in its own function Simon Glass
2020-03-09 3:44 ` [PATCH v2 26/39] acpi: Move the xsdt pointer to acpi_ctx Simon Glass
2020-03-09 3:44 ` [PATCH v2 27/39] acpi: Add an acpi command Simon Glass
2020-03-09 3:44 ` [PATCH v2 28/39] acpi: Add some tables required by the generation code Simon Glass
2020-03-09 3:44 ` [PATCH v2 29/39] acpi: Add generation code for devices Simon Glass
2020-03-09 3:44 ` [PATCH v2 30/39] acpi: Add functions to generate ACPI code Simon Glass
2020-03-09 3:44 ` [PATCH v2 31/39] gpio: Add a method to convert a GPIO to ACPI Simon Glass
2020-03-09 3:44 ` [PATCH v2 32/39] irq: Add a method to convert an interrupt " Simon Glass
2020-03-18 16:17 ` Antwort: " Wolfgang Wallner
2020-03-18 16:20 ` Wolfgang Wallner
2020-03-19 16:18 ` Simon Glass
2020-03-09 3:44 ` [PATCH v2 33/39] acpi: Add support for SSDT generation Simon Glass
2020-03-18 16:48 ` Antwort: " Wolfgang Wallner
2020-03-19 7:39 ` Wolfgang Wallner
2020-03-09 3:44 ` [PATCH v2 34/39] x86: acpi: Move MADT up a bit Simon Glass
2020-03-09 3:44 ` [PATCH v2 35/39] acpi: Record the items added to SSDT Simon Glass
2020-03-09 3:45 ` [PATCH v2 36/39] acpi: Support ordering SSDT data by device Simon Glass
2020-03-09 3:45 ` [PATCH v2 37/39] x86: Allow devices to write an SSDT Simon Glass
2020-03-09 3:45 ` [PATCH v2 38/39] acpi: Add support for DSDT generation Simon Glass
2020-03-09 3:45 ` [PATCH v2 39/39] x86: Allow devices to write to DSDT Simon Glass
2020-03-09 7:38 ` Antwort: [PATCH v2 02/39] spi: Add SPI mode enums Wolfgang Wallner
2020-03-09 7:38 ` Antwort: [PATCH v2 08/39] x86: Correct wording of coreboot source code Wolfgang Wallner
2020-03-09 7:38 ` Antwort: [PATCH v2 10/39] pci: Adjust dm_pci_read_bar32() to return errors correctly Wolfgang Wallner
2020-03-09 7:38 ` Antwort: [PATCH v2 11/39] x86: apl: Add Global NVS table header Wolfgang Wallner
2020-03-09 9:07 ` Antwort: [PATCH v2 12/39] dm: core: Add basic ACPI support Wolfgang Wallner
2020-03-10 9:15 ` Antwort: [PATCH v2 13/39] acpi: Add a binding for ACPI settings in the device tree Wolfgang Wallner
2020-03-12 3:24 ` Simon Glass
2020-03-12 12:44 ` Antwort: " Wolfgang Wallner
2020-03-13 0:36 ` Simon Glass
2020-03-10 9:16 ` Antwort: [PATCH v2 15/39] x86: Move acpi_table header to main include/ directory Wolfgang Wallner
2020-03-10 9:17 ` Antwort: [PATCH v2 16/39] acpi: Add an __ACPI__ preprocessor symbol Wolfgang Wallner
2020-03-10 9:26 ` Antwort: [PATCH v2 17/39] acpi: Add a central location for table version numbers Wolfgang Wallner
2020-03-12 3:22 ` Simon Glass
2020-03-10 12:32 ` Antwort: [PATCH v2 18/39] acpi: Add support for DMAR Wolfgang Wallner
2020-03-10 12:33 ` Antwort: [PATCH v2 19/39] acpi: Move acpi_fill_header() to generic code Wolfgang Wallner
2020-03-10 12:53 ` Antwort: [PATCH v2 14/39] acpi: Add a simple sandbox test Wolfgang Wallner
2020-03-11 9:04 ` Antwort: [PATCH v2 19/39] acpi: Move acpi_fill_header() to generic code Wolfgang Wallner
2020-03-11 11:36 ` Antwort: [PATCH v2 20/39] acpi: Add a method to write tables for a device Wolfgang Wallner
2020-03-11 11:37 ` Antwort: [PATCH v2 22/39] x86: Allow devices to write ACPI tables Wolfgang Wallner
2020-03-11 12:58 ` Wolfgang Wallner [this message]
2020-03-12 3:23 ` [PATCH v2 21/39] acpi: Convert part of acpi_table to use acpi_ctx Simon Glass
2020-03-12 13:03 ` Antwort: " Wolfgang Wallner
2020-03-11 13:43 ` Antwort: [PATCH v2 24/39] acpi: Move acpi_add_table() to generic code Wolfgang Wallner
2020-03-11 14:33 ` Antwort: [PATCH v2 25/39] acpi: Put table-setup code in its own function Wolfgang Wallner
2020-03-11 14:34 ` Antwort: [PATCH v2 23/39] acpi: Drop code for missing XSDT from acpi_write_rsdp() Wolfgang Wallner
2020-03-11 14:57 ` Antwort: [PATCH v2 26/39] acpi: Move the xsdt pointer to acpi_ctx Wolfgang Wallner
2020-03-12 14:30 ` Antwort: [PATCH v2 27/39] acpi: Add an acpi command Wolfgang Wallner
2020-03-13 9:55 ` Antwort: [PATCH v2 30/39] acpi: Add functions to generate ACPI code Wolfgang Wallner
2020-03-14 20:34 ` Simon Glass
2020-03-13 11:16 ` Antwort: [PATCH v2 31/39] gpio: Add a method to convert a GPIO to ACPI Wolfgang Wallner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=OFD40C4652.9A011211-ONC1258528.003738E9-C1258528.00474D76@br-automation.com \
--to=wolfgang.wallner@br-automation.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.