[PATCH v2 14/35] acpigen: Support writing a package

[Wolfgang Wallner]

Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v2 14/35] acpigen: Support writing a package
> 
> A package collects together several elements. Add an easy way of writing
> a package header and updating its length later.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v1: None
> 
>  include/acpi/acpigen.h | 62 ++++++++++++++++++++++++++++++++++++++++++
>  lib/acpi/acpigen.c     | 12 ++++++++
>  test/dm/acpigen.c      | 27 ++++++++++++++++++
>  3 files changed, 101 insertions(+)
> 
> diff --git a/include/acpi/acpigen.h b/include/acpi/acpigen.h
> index 31366f5e34..52e0b75128 100644
> --- a/include/acpi/acpigen.h
> +++ b/include/acpi/acpigen.h
> @@ -14,6 +14,11 @@
>  
>  struct acpi_ctx;
>  
> +/* ACPI Op/Prefix codes */
> +enum {
> +	PACKAGE_OP		= 0x12,
> +};
> +
>  /**
>   * acpigen_get_current() - Get the current ACPI code output pointer
>   *
> @@ -65,7 +70,64 @@ void acpigen_emit_stream(struct acpi_ctx *ctx, const char *data, int size);
>   */
>  void acpigen_emit_string(struct acpi_ctx *ctx, const char *str);
>  
> +/**
> + * acpigen_write_len_f() - Write a 'forward' length placeholder
> + *
> + * This adds space for a length value in the ACPI stream and pushes the current
> + * position (before the length) on the stack. After calling this you can write
> + * some data and then call acpigen_pop_len() to update the length value.
> + *
> + * Usage:
> + *
> + *    acpigen_write_len_f() ------\
> + *    acpigen_write...()          |
> + *    acpigen_write...()          |
> + *      acpigen_write_len_f() --\ |
> + *      acpigen_write...()      | |
> + *      acpigen_write...()      | |
> + *      acpigen_pop_len() ------/ |
> + *    acpigen_write...()          |
> + *    acpigen_pop_len() ----------/
> + *
> + * @ctx: ACPI context pointer
> + */

Nit: This description should be part of the previous patch.

>  void acpigen_write_len_f(struct acpi_ctx *ctx);
> +
> +/**
> + * acpigen_pop_len() - Update the previously stacked length placeholder
> + *
> + * Call this after the data for the block gas been written. It updates the
> + * top length value in the stack and pops it off.
> + *
> + * @ctx: ACPI context pointer
> + */

Nit: This description should be part of the previous patch.

>  void acpigen_pop_len(struct acpi_ctx *ctx);
>  
> +/**
> + * acpigen_write_package() - Start writing a package
> + *
> + * A package collects together a number of elements in the ACPI code. To write
> + * a package use:
> + *
> + * acpigen_write_package(ctx, 3);
> + * ...write things
> + * acpigen_pop_len()
> + *
> + * If you don't know the number of elements in advance, acpigen_write_package()
> + * returns a pointer to the value so you can update it later:
> + *
> + * char *num_elements = acpigen_write_package(ctx, 0);
> + * ...write things
> + * *num_elements += 1;
> + * ...write things
> + * *num_elements += 1;
> + * acpigen_pop_len()
> + *
> + * @ctx: ACPI context pointer
> + * @nr_el: Number of elements (0 if not known)
> + * @returns pointer to the number of elements, which can be updated by the
> + *	caller if needed
> + */
> +char *acpigen_write_package(struct acpi_ctx *ctx, int nr_el);
> +
>  #endif
> diff --git a/lib/acpi/acpigen.c b/lib/acpi/acpigen.c
> index bd1fa24fb6..e1fd1f6b6a 100644
> --- a/lib/acpi/acpigen.c
> +++ b/lib/acpi/acpigen.c
> @@ -70,6 +70,18 @@ void acpigen_pop_len(struct acpi_ctx *ctx)
>  	p[2] = (len >> 12 & 0xff);
>  }
>  
> +char *acpigen_write_package(struct acpi_ctx *ctx, int nr_el)
> +{
> +	char *p;
> +
> +	acpigen_emit_byte(ctx, PACKAGE_OP);
> +	acpigen_write_len_f(ctx);
> +	p = ctx->current;
> +	acpigen_emit_byte(ctx, nr_el);
> +
> +	return p;
> +}
> +
>  void acpigen_emit_stream(struct acpi_ctx *ctx, const char *data, int size)
>  {
>  	int i;
> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> index f9c15b7503..be81068759 100644
> --- a/test/dm/acpigen.c
> +++ b/test/dm/acpigen.c
> @@ -394,3 +394,30 @@ static int dm_test_acpi_len(struct unit_test_state *uts)
>  	return 0;
>  }
>  DM_TEST(dm_test_acpi_len, 0);
> +
> +/* Test emitting a package */
> +static int dm_test_acpi_package(struct unit_test_state *uts)
> +{
> +	struct acpi_ctx *ctx;
> +	char *num_elements;
> +	u8 *ptr;
> +
> +	ut_assertok(alloc_context(&ctx));
> +
> +	ptr = acpigen_get_current(ctx);
> +
> +	num_elements = acpigen_write_package(ctx, 3);
> +	ut_asserteq_ptr(num_elements, ptr + 4);
> +
> +	/* For easy of testing, just emit a byte, not valid package contents */

I'm not sure, but shouldn't this be "ease" of testing?

> +	acpigen_emit_byte(ctx, 0x23);
> +	acpigen_pop_len(ctx);
> +	ut_asserteq(PACKAGE_OP, ptr[0]);
> +	ut_asserteq(5, get_length(ptr + 1));
> +	ut_asserteq(3, ptr[4]);
> +
> +	free_context(&ctx);
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_acpi_package, 0);
> -- 
> 2.26.2.645.ge9eca65c58-goog

Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>