| Message ID | 20200308214442.v2.21.I93e1e33891714417335e1dd517982b18bf9f882f@changeid |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dm: Add programmatic generation of ACPI tables (part A) | expand |
Hi Simon, -----"Simon Glass" <sjg at 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 at 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 at br-automation.com>
Hi Wolfgang, On Wed, 11 Mar 2020 at 06:58, Wolfgang Wallner < wolfgang.wallner at br-automation.com> wrote: > > Hi Simon, > > -----"Simon Glass" <sjg at 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 at 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(-) > > [..] > > +/** > > + * 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() ? There is I think only one case where we use 64. Most of the time it is 16. So I thought it was a bit silly to put 16 in the function name - it is the standard alignment. Perhaps I should use align() and align64()? Regards, Simon
Hi Simon, -----"Simon Glass" <sjg at chromium.org> schrieb: ----- > Hi Wolfgang, > > On Wed, 11 Mar 2020 at 06:58, Wolfgang Wallner <wolfgang.wallner at br-automation.com> wrote: > > > > Hi Simon, > > > > -----"Simon Glass" <sjg at 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 at 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(-) > > > > > [..] > > > > +/** > > > + * 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() ? > > There is I think only one case where we use 64. Most of the time it is 16. So I thought it was a bit silly to put 16 in the function name - it is the standard alignment. > > Perhaps I should use align() and align64()? Yes, I'm fine with that. regards, Wolfgang
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); + +/** + * 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); + #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);
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 at 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(-)