[PATCH v2 18/35] acpi: Support writing a UUID

[Wolfgang Wallner]

Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v2 18/35] acpi: Support writing a UUID
> 
> ACPI supports writing a UUID in a special format. Add a function to handle
> this.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v1: None
> 
>  include/acpi/acpigen.h | 13 +++++++++++++
>  lib/acpi/acpigen.c     | 39 +++++++++++++++++++++++++++++++++++++++
>  test/dm/acpigen.c      | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/include/acpi/acpigen.h b/include/acpi/acpigen.h
> index 0fecb7d57a..f35467f029 100644
> --- a/include/acpi/acpigen.h
> +++ b/include/acpi/acpigen.h
> @@ -24,6 +24,7 @@ enum {
>  	DWORD_PREFIX		= 0x0c,
>  	STRING_PREFIX		= 0x0d,
>  	QWORD_PREFIX		= 0x0e,
> +	BUFFER_OP		= 0x11,
>  	PACKAGE_OP		= 0x12,
>  	DUAL_NAME_PREFIX	= 0x2e,
>  	MULTI_NAME_PREFIX	= 0x2f,
> @@ -182,4 +183,16 @@ void acpigen_emit_namestring(struct acpi_ctx *ctx, const char *namepath);
>   * @namepath: Name / path to emit
>   */
>  void acpigen_write_name(struct acpi_ctx *ctx, const char *namepath);
> +
> +/**
> + * acpigen_write_uuid() - Write a UUID
> + *
> + * This writes out a UUID in the format used by ACPI, with a BUFFER_OP prefix.
> + *
> + * @ctx: ACPI context pointer
> + * @uuid: UUID to write in the form aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
> + * @return 0 if OK, -EINVAL if the format is incorrect
> + */
> +int acpigen_write_uuid(struct acpi_ctx *ctx, const char *uuid);
> +
>  #endif
> diff --git a/lib/acpi/acpigen.c b/lib/acpi/acpigen.c
> index eae2f605ed..f781ad4d87 100644
> --- a/lib/acpi/acpigen.c
> +++ b/lib/acpi/acpigen.c
> @@ -11,6 +11,7 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <log.h>
> +#include <uuid.h>
>  #include <acpi/acpigen.h>
>  #include <dm/acpi.h>
>  
> @@ -248,3 +249,41 @@ void acpigen_write_name(struct acpi_ctx *ctx, const char *namepath)
>  	acpigen_emit_byte(ctx, NAME_OP);
>  	acpigen_emit_namestring(ctx, namepath);
>  }
> +
> +/*
> + * ToUUID(uuid)
> + *
> + * ACPI 6.3 Section 19.6.142 table 19-438 defines a special output order for the
> + * bytes that make up a UUID Buffer object:
> + *
> + * UUID byte order for input to this function:
> + *   aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
> + *
> + * UUID byte order output by this function:
> + *   ddccbbaa-ffee-hhgg-iijj-kkllmmnnoopp
> + */
> +#define UUID_LEN 16

Nit: To me it looks strange that this is defined within the ACPI module.
Could this be moved to e.g. include/uuid.h?

It seems this define could be useful elsewhere, as I could not find an
existing define like this, but "grep -r -i -e uuid | grep -e 16" shows
several places where the fixed length of 16 bytes for UUIDs is hardcoded.

> +int acpigen_write_uuid(struct acpi_ctx *ctx, const char *uuid)
> +{
> +	u8 buf[UUID_LEN];
> +	int ret;
> +
> +	/* Parse UUID string into bytes */
> +	ret = uuid_str_to_bin(uuid, buf, UUID_STR_FORMAT_GUID);
> +	if (ret)
> +		return log_msg_ret("bad hex", -EINVAL);
> +
> +	/* BufferOp */
> +	acpigen_emit_byte(ctx, BUFFER_OP);
> +	acpigen_write_len_f(ctx);
> +
> +	/* Buffer length in bytes */
> +	acpigen_write_word(ctx, UUID_LEN);
> +
> +	/* Output UUID in expected order */
> +	acpigen_emit_stream(ctx, (char *)buf, UUID_LEN);
> +
> +	acpigen_pop_len(ctx);
> +
> +	return 0;
> +}
> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> index 0f56262a28..db8cad47d8 100644
> --- a/test/dm/acpigen.c
> +++ b/test/dm/acpigen.c
> @@ -588,3 +588,36 @@ static int dm_test_acpi_name(struct unit_test_state *uts)
>  	return 0;
>  }
>  DM_TEST(dm_test_acpi_name, 0);
> +
> +/* Test writing a UUID */
> +static int dm_test_acpi_uuid(struct unit_test_state *uts)
> +{
> +	struct acpi_ctx *ctx;
> +	u8 *ptr;
> +
> +	ut_assertok(alloc_context(&ctx));
> +
> +	ptr = acpigen_get_current(ctx);
> +
> +	ut_assertok(acpigen_write_uuid(ctx,
> +				       "dbb8e3e6-5886-4ba6-8795-1319f52a966b"));
> +	ut_asserteq(23, acpigen_get_current(ctx) - ptr);
> +	ut_asserteq(BUFFER_OP, ptr[0]);
> +	ut_asserteq(22, get_length(ptr + 1));
> +	ut_asserteq(0xdbb8e3e6, get_unaligned((u32 *)(ptr + 7)));
> +	ut_asserteq(0x5886, get_unaligned((u16 *)(ptr + 11)));
> +	ut_asserteq(0x4ba6, get_unaligned((u16 *)(ptr + 13)));
> +	ut_asserteq(0x9587, get_unaligned((u16 *)(ptr + 15)));
> +	ut_asserteq(0x2af51913, get_unaligned((u32 *)(ptr + 17)));
> +	ut_asserteq(0x6b96, get_unaligned((u16 *)(ptr + 21)));
> +
> +	/* Try a bad UUID */
> +	ut_asserteq(-EINVAL,
> +		    acpigen_write_uuid(ctx,
> +				       "dbb8e3e6-5886-4ba6x8795-1319f52a966b"));
> +
> +	free_context(&ctx);
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_acpi_uuid, 0);
> -- 
> 2.26.2.645.ge9eca65c58-goog

I would also be fine with keeping the UUID_LEN define where it is,
and the rest of the patch looks good to me:

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

regards, Wolfgang