[PATCH v2 35/35] acpi: Add an acpi split command

[Wolfgang Wallner]

Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v2 35/35] acpi: Add an acpi split command

Nit: I find the commit message confusing. What does "split command" mean?
How about: "acpi: Add an acpi command to list/dump generated ACPI items"

> 
> Add a command that shows the individual blocks of data generated by each
> device. This can be helpful for debugging.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v1: None
> 
>  cmd/acpi.c          | 15 +++++++++++++--
>  drivers/core/acpi.c | 16 ++++++++++++++++
>  include/dm/acpi.h   | 10 ++++++++++
>  test/dm/acpi.c      | 39 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 78 insertions(+), 2 deletions(-)
> 
> diff --git a/cmd/acpi.c b/cmd/acpi.c
> index e9a9161a91..3204b2ec43 100644
> --- a/cmd/acpi.c
> +++ b/cmd/acpi.c
> @@ -153,6 +153,17 @@ static int do_acpi_list(struct cmd_tbl *cmdtp, int flag, int argc,
>  	return 0;
>  }
>  
> +static int do_acpi_items(struct cmd_tbl *cmdtp, int flag, int argc,
> +			 char *const argv[])
> +{
> +	bool dump;
> +
> +	dump = argc >= 2 && !strcmp("-d", argv[1]);
> +	acpi_dump_items(dump);
> +
> +	return 0;
> +}
> +
>  static int do_acpi_dump(struct cmd_tbl *cmdtp, int flag, int argc,
>  			char *const argv[])
>  {
> @@ -160,8 +171,6 @@ static int do_acpi_dump(struct cmd_tbl *cmdtp, int flag, int argc,
>  	char sig[ACPI_NAME_LEN];
>  	int ret;
>  
> -	if (argc < 2)
> -		return CMD_RET_USAGE;
>  	name = argv[1];
>  	if (strlen(name) != ACPI_NAME_LEN) {
>  		printf("Table name '%s' must be four characters\n", name);
> @@ -179,8 +188,10 @@ static int do_acpi_dump(struct cmd_tbl *cmdtp, int flag, int argc,
>  
>  static char acpi_help_text[] =
>  	"list - list ACPI tables\n"
> +	"acpi items [-d]  - List/dump each piece of ACPI data from devices\n"
>  	"acpi dump <name> - Dump ACPI table";
>  
>  U_BOOT_CMD_WITH_SUBCMDS(acpi, "ACPI tables", acpi_help_text,
>  	U_BOOT_SUBCMD_MKENT(list, 1, 1, do_acpi_list),
> +	U_BOOT_SUBCMD_MKENT(items, 2, 1, do_acpi_items),
>  	U_BOOT_SUBCMD_MKENT(dump, 2, 1, do_acpi_dump));
> diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> index ec70db3951..aa11001a8d 100644
> --- a/drivers/core/acpi.c
> +++ b/drivers/core/acpi.c
> @@ -119,6 +119,22 @@ static int acpi_add_item(struct acpi_ctx *ctx, struct udevice *dev,
>  	return 0;
>  }
>  
> +void acpi_dump_items(bool dump_contents)
> +{
> +	int i;
> +
> +	for (i = 0; i < item_count; i++) {
> +		struct acpi_item *item = &acpi_item[i];
> +
> +		printf("dev '%s', type %d, size %x\n", item->dev->name,
> +		       item->type, item->size);
> +		if (dump_contents) {
> +			print_buffer(0, item->buf, 1, item->size, 0);
> +			printf("\n");
> +		}
> +	}
> +}
> +
>  struct acpi_item *find_item(const char *devname)
>  {
>  	int i;
> diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> index 73bad2e763..678391ccfc 100644
> --- a/include/dm/acpi.h
> +++ b/include/dm/acpi.h
> @@ -162,6 +162,16 @@ int acpi_fill_ssdt(struct acpi_ctx *ctx);
>   */
>  int acpi_inject_dsdt(struct acpi_ctx *ctx);
>  
> +/**
> + * acpi_dump_items() - Dump out the collected ACPI items
> + *
> + * This lists the ACPI DSDT and SSDT items generated by the various U-Boot
> + * drivers.
> + *
> + * @dump_contents: true to dump the binary contents, false to just show the list
> + */
> +void acpi_dump_items(bool dump_contents);

Nit: IMHO an enum would be more readable instead of a boolean parameter.

> +
>  #endif /* __ACPI__ */
>  
>  #endif
> diff --git a/test/dm/acpi.c b/test/dm/acpi.c
> index bd4c66c471..4952256ce1 100644
> --- a/test/dm/acpi.c
> +++ b/test/dm/acpi.c
> @@ -525,3 +525,42 @@ static int dm_test_acpi_inject_dsdt(struct unit_test_state *uts)
>  	return 0;
>  }
>  DM_TEST(dm_test_acpi_inject_dsdt, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test 'acpi items' command */
> +static int dm_test_acpi_cmd_items(struct unit_test_state *uts)
> +{
> +	struct acpi_ctx ctx;
> +	void *buf;
> +
> +	buf = malloc(BUF_SIZE);
> +	ut_assertnonnull(buf);
> +
> +	ctx.current = buf;
> +	ut_assertok(acpi_fill_ssdt(&ctx));
> +	console_record_reset();
> +	run_command("acpi items", 0);
> +	ut_assert_nextline("dev 'acpi-test', type 1, size 2");
> +	ut_assert_nextline("dev 'acpi-test2', type 1, size 2");
> +	ut_assert_console_end();
> +
> +	ctx.current = buf;
> +	ut_assertok(acpi_inject_dsdt(&ctx));
> +	console_record_reset();
> +	run_command("acpi items", 0);
> +	ut_assert_nextline("dev 'acpi-test', type 2, size 2");
> +	ut_assert_nextline("dev 'acpi-test2', type 2, size 2");
> +	ut_assert_console_end();
> +
> +	console_record_reset();
> +	run_command("acpi items -d", 0);
> +	ut_assert_nextline("dev 'acpi-test', type 2, size 2");
> +	ut_assert_nextlines_are_dump(2);
> +	ut_assert_nextline("%s", "");
> +	ut_assert_nextline("dev 'acpi-test2', type 2, size 2");
> +	ut_assert_nextlines_are_dump(2);
> +	ut_assert_nextline("%s", "");
> +	ut_assert_console_end();
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_acpi_cmd_items, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> -- 
> 2.26.2.645.ge9eca65c58-goog

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