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

Hi Simon,

-----"Simon Glass" sjg@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@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@br-automation.com