From a8ca7d46ea744381185c0bf70cbdf4db7dc734b7 Mon Sep 17 00:00:00 2001 From: Dmitry Rokosov Date: Thu, 17 Oct 2024 17:12:07 +0300 Subject: cmd: bcb: rework the command to U_BOOT_LONGHELP approach U_BOOT_LONGHELP and U_BOOT_CMD_WITH_SUBCMDS offer numerous advantages, including: - common argument restrictions checking - automatic subcommand matching - improved usage and help handling By utilizing the U_BOOT_LONGHELP approach, we can reduce the amount of command management code and describe commands more succinctly. Signed-off-by: Dmitry Rokosov Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek # vim3_android Link: https://lore.kernel.org/r/20241017-android_ab_master-v5-2-43bfcc096d95@salutedevices.com Signed-off-by: Mattijs Korpershoek --- cmd/bcb.c | 151 ++++++++++++++++++-------------------------------------------- 1 file changed, 43 insertions(+), 108 deletions(-) (limited to 'cmd') diff --git a/cmd/bcb.c b/cmd/bcb.c index 97a96c00964..98b2a71087a 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -16,15 +16,6 @@ #include #include -enum bcb_cmd { - BCB_CMD_LOAD, - BCB_CMD_FIELD_SET, - BCB_CMD_FIELD_CLEAR, - BCB_CMD_FIELD_TEST, - BCB_CMD_FIELD_DUMP, - BCB_CMD_STORE, -}; - static const char * const fields[] = { "command", "status", @@ -38,67 +29,9 @@ static struct disk_partition partition_data; static struct blk_desc *block; static struct disk_partition *partition = &partition_data; -static int bcb_cmd_get(char *cmd) -{ - if (!strcmp(cmd, "load")) - return BCB_CMD_LOAD; - if (!strcmp(cmd, "set")) - return BCB_CMD_FIELD_SET; - if (!strcmp(cmd, "clear")) - return BCB_CMD_FIELD_CLEAR; - if (!strcmp(cmd, "test")) - return BCB_CMD_FIELD_TEST; - if (!strcmp(cmd, "store")) - return BCB_CMD_STORE; - if (!strcmp(cmd, "dump")) - return BCB_CMD_FIELD_DUMP; - else - return -1; -} - -static int bcb_is_misused(int argc, char *const argv[]) +static int bcb_not_loaded(void) { - int cmd = bcb_cmd_get(argv[0]); - - switch (cmd) { - case BCB_CMD_LOAD: - if (argc != 3 && argc != 4) - goto err; - break; - case BCB_CMD_FIELD_SET: - if (argc != 3) - goto err; - break; - case BCB_CMD_FIELD_TEST: - if (argc != 4) - goto err; - break; - case BCB_CMD_FIELD_CLEAR: - if (argc != 1 && argc != 2) - goto err; - break; - case BCB_CMD_STORE: - if (argc != 1) - goto err; - break; - case BCB_CMD_FIELD_DUMP: - if (argc != 2) - goto err; - break; - default: - printf("Error: 'bcb %s' not supported\n", argv[0]); - return -1; - } - - if (cmd != BCB_CMD_LOAD && !block) { - printf("Error: Please, load BCB first!\n"); - return -1; - } - - return 0; -err: - printf("Error: Bad usage of 'bcb %s'\n", argv[0]); - + printf("Error: Please, load BCB first!\n"); return -1; } @@ -216,6 +149,9 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char *endp; char *iface = "mmc"; + if (argc < 3) + return CMD_RET_USAGE; + if (argc == 4) { iface = argv[1]; argc--; @@ -270,6 +206,12 @@ static int __bcb_set(const char *fieldp, const char *valp) static int do_bcb_set(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { + if (argc < 3) + return CMD_RET_USAGE; + + if (!block) + return bcb_not_loaded(); + return __bcb_set(argv[1], argv[2]); } @@ -279,6 +221,9 @@ static int do_bcb_clear(struct cmd_tbl *cmdtp, int flag, int argc, int size; char *field; + if (!block) + return bcb_not_loaded(); + if (argc == 1) { memset(&bcb, 0, sizeof(bcb)); return CMD_RET_SUCCESS; @@ -297,7 +242,15 @@ static int do_bcb_test(struct cmd_tbl *cmdtp, int flag, int argc, { int size; char *field; - char *op = argv[2]; + char *op; + + if (argc < 4) + return CMD_RET_USAGE; + + if (!block) + return bcb_not_loaded(); + + op = argv[2]; if (bcb_field_get(argv[1], &field, &size)) return CMD_RET_FAILURE; @@ -325,6 +278,12 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc, int size; char *field; + if (argc < 2) + return CMD_RET_USAGE; + + if (!block) + return bcb_not_loaded(); + if (bcb_field_get(argv[1], &field, &size)) return CMD_RET_FAILURE; @@ -356,6 +315,9 @@ err: static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { + if (!block) + return bcb_not_loaded(); + return __bcb_store(); } @@ -414,44 +376,7 @@ void bcb_reset(void) __bcb_reset(); } -static struct cmd_tbl cmd_bcb_sub[] = { - U_BOOT_CMD_MKENT(load, CONFIG_SYS_MAXARGS, 1, do_bcb_load, "", ""), - U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 1, do_bcb_set, "", ""), - U_BOOT_CMD_MKENT(clear, CONFIG_SYS_MAXARGS, 1, do_bcb_clear, "", ""), - U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_bcb_test, "", ""), - U_BOOT_CMD_MKENT(dump, CONFIG_SYS_MAXARGS, 1, do_bcb_dump, "", ""), - U_BOOT_CMD_MKENT(store, CONFIG_SYS_MAXARGS, 1, do_bcb_store, "", ""), -}; - -static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) -{ - struct cmd_tbl *c; - - if (argc < 2) - return CMD_RET_USAGE; - - argc--; - argv++; - - c = find_cmd_tbl(argv[0], cmd_bcb_sub, ARRAY_SIZE(cmd_bcb_sub)); - if (!c) - return CMD_RET_USAGE; - - if (bcb_is_misused(argc, argv)) { - /* - * We try to improve the user experience by reporting the - * root-cause of misusage, so don't return CMD_RET_USAGE, - * since the latter prints out the full-blown help text - */ - return CMD_RET_FAILURE; - } - - return c->cmd(cmdtp, flag, argc, argv); -} - -U_BOOT_CMD( - bcb, CONFIG_SYS_MAXARGS, 1, do_bcb, - "Load/set/clear/test/dump/store Android BCB fields", +U_BOOT_LONGHELP(bcb, "load - load BCB from :\n" "load - load BCB from mmc :\n" "bcb set - set BCB to \n" @@ -472,3 +397,13 @@ U_BOOT_CMD( " NOTE: any ':' character in will be replaced by line feed\n" " during 'bcb set' and used as separator by upper layers\n" ); + +U_BOOT_CMD_WITH_SUBCMDS(bcb, + "Load/set/clear/test/dump/store Android BCB fields", bcb_help_text, + U_BOOT_SUBCMD_MKENT(load, 4, 1, do_bcb_load), + U_BOOT_SUBCMD_MKENT(set, 3, 1, do_bcb_set), + U_BOOT_SUBCMD_MKENT(clear, 2, 1, do_bcb_clear), + U_BOOT_SUBCMD_MKENT(test, 4, 1, do_bcb_test), + U_BOOT_SUBCMD_MKENT(dump, 2, 1, do_bcb_dump), + U_BOOT_SUBCMD_MKENT(store, 1, 1, do_bcb_store), +); -- cgit v1.2.3 From b523b4d2c32f07ca0793bb0f926b02ecb0556cc6 Mon Sep 17 00:00:00 2001 From: Dmitry Rokosov Date: Thu, 17 Oct 2024 17:12:08 +0300 Subject: treewide: bcb: move ab_select command to bcb subcommands To enhance code organization, it is beneficial to consolidate all A/B BCB management routines into a single super-command. The 'bcb' command is an excellent candidate for this purpose. This patch integrates the separate 'ab_select' command into the 'bcb' group as the 'ab_select' subcommand, maintaining the same parameter list for consistency. Signed-off-by: Dmitry Rokosov Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek # vim3_android Link: https://lore.kernel.org/r/20241017-android_ab_master-v5-3-43bfcc096d95@salutedevices.com Signed-off-by: Mattijs Korpershoek --- cmd/Kconfig | 14 ------------ cmd/Makefile | 1 - cmd/ab_select.c | 66 --------------------------------------------------------- cmd/bcb.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 81 deletions(-) delete mode 100644 cmd/ab_select.c (limited to 'cmd') diff --git a/cmd/Kconfig b/cmd/Kconfig index 3ee70f31b14..40485be192f 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1782,20 +1782,6 @@ config CMD_XXD endmenu -menu "Android support commands" - -config CMD_AB_SELECT - bool "ab_select" - depends on ANDROID_AB - help - On Android devices with more than one boot slot (multiple copies of - the kernel and system images) this provides a command to select which - slot should be used to boot from and register the boot attempt. This - is used by the new A/B update model where one slot is updated in the - background while running from the other slot. - -endmenu - if NET || NET_LWIP menuconfig CMD_NET diff --git a/cmd/Makefile b/cmd/Makefile index 3c5bd56e912..7fe2044aab7 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -17,7 +17,6 @@ obj-$(CONFIG_CMD_2048) += 2048.o obj-$(CONFIG_CMD_ACPI) += acpi.o obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o obj-$(CONFIG_CMD_AES) += aes.o -obj-$(CONFIG_CMD_AB_SELECT) += ab_select.o obj-$(CONFIG_CMD_ADC) += adc.o obj-$(CONFIG_CMD_ARMFLASH) += armflash.o obj-$(CONFIG_BLK) += blk_common.o diff --git a/cmd/ab_select.c b/cmd/ab_select.c deleted file mode 100644 index 7c178c728ca..00000000000 --- a/cmd/ab_select.c +++ /dev/null @@ -1,66 +0,0 @@ -// SPDX-License-Identifier: BSD-2-Clause -/* - * Copyright (C) 2017 The Android Open Source Project - */ - -#include -#include -#include -#include - -static int do_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) -{ - int ret; - struct blk_desc *dev_desc; - struct disk_partition part_info; - char slot[2]; - bool dec_tries = true; - - if (argc < 4) - return CMD_RET_USAGE; - - for (int i = 4; i < argc; i++) { - if (strcmp(argv[i], "--no-dec") == 0) { - dec_tries = false; - } else { - return CMD_RET_USAGE; - } - } - - /* Lookup the "misc" partition from argv[2] and argv[3] */ - if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3], - &dev_desc, &part_info, - false) < 0) { - return CMD_RET_FAILURE; - } - - ret = ab_select_slot(dev_desc, &part_info, dec_tries); - if (ret < 0) { - printf("Android boot failed, error %d.\n", ret); - return CMD_RET_FAILURE; - } - - /* Android standard slot names are 'a', 'b', ... */ - slot[0] = BOOT_SLOT_NAME(ret); - slot[1] = '\0'; - env_set(argv[1], slot); - printf("ANDROID: Booting slot: %s\n", slot); - return CMD_RET_SUCCESS; -} - -U_BOOT_CMD(ab_select, 5, 0, do_ab_select, - "Select the slot used to boot from and register the boot attempt.", - " [--no-dec]\n" - " - Load the slot metadata from the partition 'part' on\n" - " device type 'interface' instance 'dev' and store the active\n" - " slot in the 'slot_var_name' variable. This also updates the\n" - " Android slot metadata with a boot attempt, which can cause\n" - " successive calls to this function to return a different result\n" - " if the returned slot runs out of boot attempts.\n" - " - If 'part_name' is passed, preceded with a # instead of :, the\n" - " partition name whose label is 'part_name' will be looked up in\n" - " the partition table. This is commonly the \"misc\" partition.\n" - " - If '--no-dec' is set, the number of tries remaining will not\n" - " decremented for the selected boot slot\n" -); diff --git a/cmd/bcb.c b/cmd/bcb.c index 98b2a71087a..2854408e566 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -376,6 +377,48 @@ void bcb_reset(void) __bcb_reset(); } +__maybe_unused static int do_bcb_ab_select(struct cmd_tbl *cmdtp, + int flag, int argc, + char * const argv[]) +{ + int ret; + struct blk_desc *dev_desc; + struct disk_partition part_info; + char slot[2]; + bool dec_tries = true; + + if (argc < 4) + return CMD_RET_USAGE; + + for (int i = 4; i < argc; i++) { + if (strcmp(argv[i], "--no-dec") == 0) + dec_tries = false; + else + return CMD_RET_USAGE; + } + + /* Lookup the "misc" partition from argv[2] and argv[3] */ + if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3], + &dev_desc, &part_info, + false) < 0) { + return CMD_RET_FAILURE; + } + + ret = ab_select_slot(dev_desc, &part_info, dec_tries); + if (ret < 0) { + printf("Android boot failed, error %d.\n", ret); + return CMD_RET_FAILURE; + } + + /* Android standard slot names are 'a', 'b', ... */ + slot[0] = BOOT_SLOT_NAME(ret); + slot[1] = '\0'; + env_set(argv[1], slot); + printf("ANDROID: Booting slot: %s\n", slot); + + return CMD_RET_SUCCESS; +} + U_BOOT_LONGHELP(bcb, "load - load BCB from :\n" "load - load BCB from mmc :\n" @@ -385,6 +428,23 @@ U_BOOT_LONGHELP(bcb, "bcb dump - dump BCB \n" "bcb store - store BCB back to \n" "\n" +#if IS_ENABLED(CONFIG_ANDROID_AB) + "bcb ab_select -\n" + " Select the slot used to boot from and register the boot attempt.\n" + " [--no-dec]\n" + " - Load the slot metadata from the partition 'part' on\n" + " device type 'interface' instance 'dev' and store the active\n" + " slot in the 'slot_var_name' variable. This also updates the\n" + " Android slot metadata with a boot attempt, which can cause\n" + " successive calls to this function to return a different result\n" + " if the returned slot runs out of boot attempts.\n" + " - If 'part_name' is passed, preceded with a # instead of :, the\n" + " partition name whose label is 'part_name' will be looked up in\n" + " the partition table. This is commonly the \"misc\" partition.\n" + " - If '--no-dec' is set, the number of tries remaining will not\n" + " decremented for the selected boot slot\n" + "\n" +#endif "Legend:\n" " - storage device interface (virtio, mmc, etc)\n" " - storage device index containing the BCB partition\n" @@ -406,4 +466,7 @@ U_BOOT_CMD_WITH_SUBCMDS(bcb, U_BOOT_SUBCMD_MKENT(test, 4, 1, do_bcb_test), U_BOOT_SUBCMD_MKENT(dump, 2, 1, do_bcb_dump), U_BOOT_SUBCMD_MKENT(store, 1, 1, do_bcb_store), +#if IS_ENABLED(CONFIG_ANDROID_AB) + U_BOOT_SUBCMD_MKENT(ab_select, 5, 1, do_bcb_ab_select), +#endif ); -- cgit v1.2.3 From b1bc9a2fc93da8ed25dfc4078e76220ac26eb6b0 Mon Sep 17 00:00:00 2001 From: Dmitry Rokosov Date: Thu, 17 Oct 2024 17:12:09 +0300 Subject: cmd: bcb: change strcmp() usage style in the do_bcb_ab_select() In the entire cmd/bcb.c file, the return value of strcmp() is not directly compared to 0. Therefore, it would be better to maintain this style in the new do_bcb_ab_select() function as well. Reviewed-by: Mattijs Korpershoek Reviewed-by: Simon Glass Tested-by: Guillaume La Roque Signed-off-by: Dmitry Rokosov Tested-by: Mattijs Korpershoek # vim3_android Link: https://lore.kernel.org/r/20241017-android_ab_master-v5-4-43bfcc096d95@salutedevices.com Signed-off-by: Mattijs Korpershoek --- cmd/bcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cmd') diff --git a/cmd/bcb.c b/cmd/bcb.c index 2854408e566..b33c046af03 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -391,7 +391,7 @@ __maybe_unused static int do_bcb_ab_select(struct cmd_tbl *cmdtp, return CMD_RET_USAGE; for (int i = 4; i < argc; i++) { - if (strcmp(argv[i], "--no-dec") == 0) + if (!strcmp(argv[i], "--no-dec")) dec_tries = false; else return CMD_RET_USAGE; -- cgit v1.2.3 From a995084beb6682748002878ccb4b8c4de9fc0136 Mon Sep 17 00:00:00 2001 From: Dmitry Rokosov Date: Thu, 17 Oct 2024 17:12:10 +0300 Subject: cmd: bcb: introduce 'ab_dump' command to print BCB block content It's really helpful to have the ability to dump BCB block for debugging A/B logic on the board supported this partition schema. Command 'bcb ab_dump' prints all fields of bootloader_control struct including slot_metadata for all presented slots. Output example: ===== > board# bcb ab_dump ubi 0#misc > Read 512 bytes from volume misc to 000000000bf07580 > Read 512 bytes from volume misc to 000000000bf42f40 > Bootloader Control: [misc] > Active Slot: _a > Magic Number: 0x42414342 > Version: 1 > Number of Slots: 2 > Recovery Tries Remaining: 0 > CRC: 0x2c8b50bc (Valid) > > Slot[0] Metadata: > - Priority: 15 > - Tries Remaining: 0 > - Successful Boot: 1 > - Verity Corrupted: 0 > > Slot[1] Metadata: > - Priority: 14 > - Tries Remaining: 7 > - Successful Boot: 0 > - Verity Corrupted: 0 ==== The ab_dump command allows you to display ABC data directly on the U-Boot console. During an A/B test execution, this test verifies the accuracy of each field within the ABC data. Signed-off-by: Dmitry Rokosov Reviewed-by: Mattijs Korpershoek Tested-by: Mattijs Korpershoek # vim3_android Link: https://lore.kernel.org/r/20241017-android_ab_master-v5-5-43bfcc096d95@salutedevices.com Signed-off-by: Mattijs Korpershoek --- cmd/bcb.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'cmd') diff --git a/cmd/bcb.c b/cmd/bcb.c index b33c046af03..16eabfe00f5 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -419,6 +419,32 @@ __maybe_unused static int do_bcb_ab_select(struct cmd_tbl *cmdtp, return CMD_RET_SUCCESS; } +__maybe_unused static int do_bcb_ab_dump(struct cmd_tbl *cmdtp, + int flag, int argc, + char *const argv[]) +{ + int ret; + struct blk_desc *dev_desc; + struct disk_partition part_info; + + if (argc < 3) + return CMD_RET_USAGE; + + if (part_get_info_by_dev_and_name_or_num(argv[1], argv[2], + &dev_desc, &part_info, + false) < 0) { + return CMD_RET_FAILURE; + } + + ret = ab_dump_abc(dev_desc, &part_info); + if (ret < 0) { + printf("Cannot dump ABC data, error %d.\n", ret); + return CMD_RET_FAILURE; + } + + return CMD_RET_SUCCESS; +} + U_BOOT_LONGHELP(bcb, "load - load BCB from :\n" "load - load BCB from mmc :\n" @@ -444,6 +470,10 @@ U_BOOT_LONGHELP(bcb, " - If '--no-dec' is set, the number of tries remaining will not\n" " decremented for the selected boot slot\n" "\n" + "bcb ab_dump -\n" + " Dump boot_control information from specific partition.\n" + " \n" + "\n" #endif "Legend:\n" " - storage device interface (virtio, mmc, etc)\n" @@ -468,5 +498,6 @@ U_BOOT_CMD_WITH_SUBCMDS(bcb, U_BOOT_SUBCMD_MKENT(store, 1, 1, do_bcb_store), #if IS_ENABLED(CONFIG_ANDROID_AB) U_BOOT_SUBCMD_MKENT(ab_select, 5, 1, do_bcb_ab_select), + U_BOOT_SUBCMD_MKENT(ab_dump, 3, 1, do_bcb_ab_dump), #endif ); -- cgit v1.2.3