Hi Simon, -----"Simon Glass" <[email protected]> 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 <[email protected]> > --- > > 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 <[email protected]>
