[PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes
The patch series include changes: - move ab_select_slot() documentation to @ notation - move ab_select command to bcb subcommands - introduce the ab_dump command to print the content of the BCB block; it's useful for debugging A/B logic on supported boards - fix the slot suffix format in the ABC block to align with official Android BCB specifications - add a test for the ab_dump command to verify the accuracy of each field within the ABC data displayed, it's also useful for testing slot_suffix problem code paths
Changes v2 since v1 at [1]: - move ab_select_slot() documentation to @ notation - move ab_select command to bcb subcommands per Simon and Mattijs suggestions - redesign ab_dump as bcb subcommand - use spaces instead of tabs in the ab_dump command output - print hex values in the lowercase - add RvB tags
Links: [1] https://lore.kernel.org/all/20240725194716.32232-1-ddrokosov@salutedevices.c...
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Dmitry Rokosov (6): include/android_ab: move ab_select_slot() documentation to @ notation treewide: bcb: move ab_select command to bcb subcommands cmd: bcb: change strcmp() usage style in the do_bcb_ab_select() cmd: bcb: introduce 'ab_dump' command to print BCB block content common: android_ab: fix slot suffix for abc block test/py: introduce test for ab_dump command
MAINTAINERS | 1 - boot/android_ab.c | 116 ++++++++++++++++++---- cmd/Kconfig | 15 +-- cmd/Makefile | 1 - cmd/ab_select.c | 66 ------------ cmd/bcb.c | 108 ++++++++++++++++++-- configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 4 +- configs/sandbox_defconfig | 4 +- doc/android/ab.rst | 12 +-- include/android_ab.h | 17 +++- include/configs/khadas-vim3_android.h | 2 +- include/configs/khadas-vim3l_android.h | 2 +- include/configs/meson64_android.h | 4 +- include/configs/ti_omap5_common.h | 4 +- test/py/tests/test_android/test_ab.py | 31 +++++- 18 files changed, 251 insertions(+), 139 deletions(-) delete mode 100644 cmd/ab_select.c
There are new function documentation requirements in U-Boot, so apply these changes for android_ab.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- boot/android_ab.c | 43 ++++++++++++++++++++++++------------------- include/android_ab.h | 7 ++++--- 2 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/boot/android_ab.c b/boot/android_ab.c index 1196a189ed53..0045c8133a8e 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -13,13 +13,13 @@ #include <u-boot/crc.h>
/** - * Compute the CRC-32 of the bootloader control struct. + * ab_control_compute_crc() - Compute the CRC32 of the bootloader control. + * + * @abc: Bootloader control block * * Only the bytes up to the crc32_le field are considered for the CRC-32 * calculation. * - * @param[in] abc bootloader control block - * * Return: crc32 sum */ static uint32_t ab_control_compute_crc(struct bootloader_control *abc) @@ -28,14 +28,14 @@ static uint32_t ab_control_compute_crc(struct bootloader_control *abc) }
/** - * Initialize bootloader_control to the default value. + * ab_control_default() - Initialize bootloader_control to the default value. + * + * @abc: Bootloader control block * * It allows us to boot all slots in order from the first one. This value * should be used when the bootloader message is corrupted, but not when * a valid message indicates that all slots are unbootable. * - * @param[in] abc bootloader control block - * * Return: 0 on success and a negative on error */ static int ab_control_default(struct bootloader_control *abc) @@ -67,7 +67,13 @@ static int ab_control_default(struct bootloader_control *abc) }
/** - * Load the boot_control struct from disk into newly allocated memory. + * ab_control_create_from_disk() - Load the boot_control from disk into memory. + * + * @dev_desc: Device where to read the boot_control struct from + * @part_info: Partition in 'dev_desc' where to read from, normally + * the "misc" partition should be used + * @abc: pointer to pointer to bootloader_control data + * @offset: boot_control struct offset * * This function allocates and returns an integer number of disk blocks, * based on the block size of the passed device to help performing a @@ -75,10 +81,6 @@ static int ab_control_default(struct bootloader_control *abc) * The boot_control struct offset (2 KiB) must be a multiple of the device * block size, for simplicity. * - * @param[in] dev_desc Device where to read the boot_control struct from - * @param[in] part_info Partition in 'dev_desc' where to read from, normally - * the "misc" partition should be used - * @param[out] pointer to pointer to bootloader_control data * Return: 0 on success and a negative on error */ static int ab_control_create_from_disk(struct blk_desc *dev_desc, @@ -122,15 +124,17 @@ static int ab_control_create_from_disk(struct blk_desc *dev_desc, }
/** - * Store the loaded boot_control block. + * ab_control_store() - Store the loaded boot_control block. + * + * @dev_desc: Device where we should write the boot_control struct + * @part_info: Partition on the 'dev_desc' where to write + * @abc Pointer to the boot control struct and the extra bytes after + * it up to the nearest block boundary + * @offset: boot_control struct offset * * Store back to the same location it was read from with * ab_control_create_from_misc(). * - * @param[in] dev_desc Device where we should write the boot_control struct - * @param[in] part_info Partition on the 'dev_desc' where to write - * @param[in] abc Pointer to the boot control struct and the extra bytes after - * it up to the nearest block boundary * Return: 0 on success and a negative on error */ static int ab_control_store(struct blk_desc *dev_desc, @@ -160,12 +164,13 @@ static int ab_control_store(struct blk_desc *dev_desc, }
/** - * Compare two slots. + * ab_compare_slots() - Compare two slots. + * + * @a: The first bootable slot metadata + * @b: The second bootable slot metadata * * The function determines slot which is should we boot from among the two. * - * @param[in] a The first bootable slot metadata - * @param[in] b The second bootable slot metadata * Return: Negative if the slot "a" is better, positive of the slot "b" is * better or 0 if they are equally good. */ diff --git a/include/android_ab.h b/include/android_ab.h index dbf20343da62..1e53879a25f1 100644 --- a/include/android_ab.h +++ b/include/android_ab.h @@ -18,7 +18,10 @@ struct disk_partition; #define NUM_SLOTS 2
/** - * Select the slot where to boot from. + * ab_select_slot() - Select the slot where to boot from. + * + * @dev_desc: Place to store the device description pointer + * @part_info: Place to store the partition information * * On Android devices with more than one boot slot (multiple copies of the * kernel and system images) selects which slot should be used to boot from and @@ -28,8 +31,6 @@ struct disk_partition; * registered before returning from this function so it isn't selected * indefinitely. * - * @param[in] dev_desc Place to store the device description pointer - * @param[in] part_info Place to store the partition information * Return: The slot number (>= 0) on success, or a negative on error */ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
On Wed, 11 Sept 2024 at 15:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
There are new function documentation requirements in U-Boot, so apply these changes for android_ab.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/android_ab.c | 43 ++++++++++++++++++++++++------------------- include/android_ab.h | 7 ++++--- 2 files changed, 28 insertions(+), 22 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Hi Dmitry,
Thank you for the patch.
On jeu., sept. 12, 2024 at 00:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
There are new function documentation requirements in U-Boot, so apply these changes for android_ab.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
boot/android_ab.c | 43 ++++++++++++++++++++++++------------------- include/android_ab.h | 7 ++++--- 2 files changed, 28 insertions(+), 22 deletions(-)
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 ddrokosov@salutedevices.com --- MAINTAINERS | 1 - cmd/Kconfig | 15 +---- cmd/Makefile | 1 - cmd/ab_select.c | 66 -------------------- cmd/bcb.c | 73 +++++++++++++++++++---- configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 4 +- configs/sandbox_defconfig | 4 +- doc/android/ab.rst | 12 ++-- include/configs/khadas-vim3_android.h | 2 +- include/configs/khadas-vim3l_android.h | 2 +- include/configs/meson64_android.h | 4 +- include/configs/ti_omap5_common.h | 4 +- test/py/tests/test_android/test_ab.py | 8 +-- 16 files changed, 84 insertions(+), 115 deletions(-) delete mode 100644 cmd/ab_select.c
diff --git a/MAINTAINERS b/MAINTAINERS index 2050ae24df82..876330d2d750 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -65,7 +65,6 @@ R: Sam Protsenko semen.protsenko@linaro.org S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: boot/android_ab.c -F: cmd/ab_select.c F: doc/android/ab.rst F: include/android_ab.h F: test/py/tests/test_android/test_ab.py diff --git a/cmd/Kconfig b/cmd/Kconfig index 978f44eda426..eb0fb07b426a 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1053,6 +1053,7 @@ config CMD_ADC config CMD_BCB bool "bcb" depends on PARTITIONS + depends on ANDROID_AB help Read/modify/write the fields of Bootloader Control Block, usually stored on the flash "misc" partition with its structure defined in: @@ -1766,20 +1767,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
menuconfig CMD_NET diff --git a/cmd/Makefile b/cmd/Makefile index 87133cc27a8a..281c13220e95 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 7c178c728ca4..000000000000 --- 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 <android_ab.h> -#include <command.h> -#include <env.h> -#include <part.h> - -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.", - "<slot_var_name> <interface> <dev[:part|#part_name]> [--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 97a96c009641..a56535a743c0 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -8,6 +8,7 @@ #include <android_bootloader_message.h> #include <bcb.h> #include <command.h> +#include <android_ab.h> #include <display_options.h> #include <log.h> #include <part.h> @@ -23,6 +24,7 @@ enum bcb_cmd { BCB_CMD_FIELD_TEST, BCB_CMD_FIELD_DUMP, BCB_CMD_STORE, + BCB_CMD_AB_SELECT, };
static const char * const fields[] = { @@ -52,6 +54,8 @@ static int bcb_cmd_get(char *cmd) return BCB_CMD_STORE; if (!strcmp(cmd, "dump")) return BCB_CMD_FIELD_DUMP; + if (!strcmp(cmd, "ab_select")) + return BCB_CMD_AB_SELECT; else return -1; } @@ -85,6 +89,10 @@ static int bcb_is_misused(int argc, char *const argv[]) if (argc != 2) goto err; break; + case BCB_CMD_AB_SELECT: + if (argc != 4 && argc != 5) + goto err; + return 0; default: printf("Error: 'bcb %s' not supported\n", argv[0]); return -1; @@ -414,6 +422,44 @@ void bcb_reset(void) __bcb_reset(); }
+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; + + 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; +} + 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, "", ""), @@ -421,6 +467,8 @@ static struct cmd_tbl cmd_bcb_sub[] = { 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, "", ""), + U_BOOT_CMD_MKENT(ab_select, CONFIG_SYS_MAXARGS, 1, + do_bcb_ab_select, "", ""), };
static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -460,15 +508,18 @@ U_BOOT_CMD( "bcb dump <field> - dump BCB <field>\n" "bcb store - store BCB back to <interface>\n" "\n" - "Legend:\n" - "<interface> - storage device interface (virtio, mmc, etc)\n" - "<dev> - storage device index containing the BCB partition\n" - "<part> - partition index or name containing the BCB\n" - "<field> - one of {command,status,recovery,stage,reserved}\n" - "<op> - the binary operator used in 'bcb test':\n" - " '=' returns true if <val> matches the string stored in <field>\n" - " '~' returns true if <val> matches a subset of <field>'s string\n" - "<val> - string/text provided as input to bcb {set,test}\n" - " NOTE: any ':' character in <val> will be replaced by line feed\n" - " during 'bcb set' and used as separator by upper layers\n" + "bcb ab_select -\n" + " Select the slot used to boot from and register the boot attempt.\n" + " <slot_var_name> <interface> <dev[:part|#part_name]> [--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/configs/am57xx_hs_evm_usb_defconfig b/configs/am57xx_hs_evm_usb_defconfig index 807e1d66a6d7..6d822b021768 100644 --- a/configs/am57xx_hs_evm_usb_defconfig +++ b/configs/am57xx_hs_evm_usb_defconfig @@ -50,7 +50,6 @@ CONFIG_CMD_ADTIMG=y CONFIG_CMD_ABOOTIMG=y CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2 CONFIG_CMD_BCB=y -CONFIG_CMD_AB_SELECT=y CONFIG_BOOTP_DNS2=y # CONFIG_CMD_PMIC is not set CONFIG_CMD_AVB=y diff --git a/configs/khadas-vim3_android_ab_defconfig b/configs/khadas-vim3_android_ab_defconfig index 37b8d6a9256b..5281c426cb73 100644 --- a/configs/khadas-vim3_android_ab_defconfig +++ b/configs/khadas-vim3_android_ab_defconfig @@ -47,7 +47,6 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_AVB=y CONFIG_OF_CONTROL=y diff --git a/configs/khadas-vim3l_android_ab_defconfig b/configs/khadas-vim3l_android_ab_defconfig index 95e70275cc4f..ff3001b78fa2 100644 --- a/configs/khadas-vim3l_android_ab_defconfig +++ b/configs/khadas-vim3l_android_ab_defconfig @@ -47,7 +47,6 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_AVB=y CONFIG_OF_CONTROL=y diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index dd0582d2a0c0..635753f00b73 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -25,6 +25,7 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_ANDROID_AB=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y @@ -44,6 +45,7 @@ CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPT=y @@ -167,8 +169,8 @@ CONFIG_PWRSEQ=y CONFIG_I2C_EEPROM=y CONFIG_MMC_SANDBOX=y CONFIG_DM_MTD=y -CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 +CONFIG_MTD_RAW_NAND=y CONFIG_SYS_NAND_USE_FLASH_BBT=y CONFIG_NAND_SANDBOX=y CONFIG_SYS_NAND_ONFI_DETECTION=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index dc5fcdbd1c9e..cb69554a7306 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -66,6 +66,7 @@ CONFIG_CMD_MEMINFO=y CONFIG_CMD_MEM_SEARCH=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPIO_READ=y @@ -93,7 +94,6 @@ CONFIG_CMD_AXI=y CONFIG_CMD_CAT=y CONFIG_CMD_SETEXPR_FMT=y CONFIG_CMD_XXD=y -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_DHCP6=y CONFIG_BOOTP_DNS2=y CONFIG_CMD_PCAP=y @@ -219,8 +219,8 @@ CONFIG_MMC_PCI=y CONFIG_MMC_SANDBOX=y CONFIG_MMC_SDHCI=y CONFIG_DM_MTD=y -CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 +CONFIG_MTD_RAW_NAND=y CONFIG_SYS_NAND_USE_FLASH_BBT=y CONFIG_NAND_SANDBOX=y CONFIG_SYS_NAND_ONFI_DETECTION=y diff --git a/doc/android/ab.rst b/doc/android/ab.rst index 2adf88781d60..7fd4aeb6a724 100644 --- a/doc/android/ab.rst +++ b/doc/android/ab.rst @@ -18,7 +18,7 @@ The A/B updates support can be activated by specifying next options in your board configuration file::
CONFIG_ANDROID_AB=y - CONFIG_CMD_AB_SELECT=y + CONFIG_CMD_BCB=y
The disk space on target device must be partitioned in a way so that each partition which needs to be updated has two or more instances. The name of @@ -26,8 +26,8 @@ each instance must be formed by adding suffixes: ``_a``, ``_b``, ``_c``, etc. For example: ``boot_a``, ``boot_b``, ``system_a``, ``system_b``, ``vendor_a``, ``vendor_b``.
-As a result you can use ``ab_select`` command to ensure A/B boot process in your -boot script. This command analyzes and processes A/B metadata stored on a +As a result you can use ``bcb ab_select`` command to ensure A/B boot process in +your boot script. This command analyzes and processes A/B metadata stored on a special partition (e.g. ``misc``) and determines which slot should be used for booting up.
@@ -42,15 +42,15 @@ Command usage
.. code-block:: none
- ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]> + bcb ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
for example::
- => ab_select slot_name mmc 1:4 + => bcb ab_select slot_name mmc 1:4
or::
- => ab_select slot_name mmc 1#misc + => bcb ab_select slot_name mmc 1#misc
Result::
diff --git a/include/configs/khadas-vim3_android.h b/include/configs/khadas-vim3_android.h index da6adf6c413a..4a8348914035 100644 --- a/include/configs/khadas-vim3_android.h +++ b/include/configs/khadas-vim3_android.h @@ -12,7 +12,7 @@ #define LOGO_UUID "43a3305d-150f-4cc9-bd3b-38fca8693846;" #define ROOT_UUID "ddb8c3f6-d94d-4394-b633-3134139cc2e0;"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define PARTS_DEFAULT \ "uuid_disk=${uuid_gpt_disk};" \ "name=logo,start=512K,size=2M,uuid=" LOGO_UUID \ diff --git a/include/configs/khadas-vim3l_android.h b/include/configs/khadas-vim3l_android.h index b1768e2d8211..bf1c831c0bc3 100644 --- a/include/configs/khadas-vim3l_android.h +++ b/include/configs/khadas-vim3l_android.h @@ -12,7 +12,7 @@ #define LOGO_UUID "43a3305d-150f-4cc9-bd3b-38fca8693846;" #define ROOT_UUID "ddb8c3f6-d94d-4394-b633-3134139cc2e0;"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define PARTS_DEFAULT \ "uuid_disk=${uuid_gpt_disk};" \ "name=logo,start=512K,size=2M,uuid=" LOGO_UUID \ diff --git a/include/configs/meson64_android.h b/include/configs/meson64_android.h index c0e977abb01f..93f9e191a165 100644 --- a/include/configs/meson64_android.h +++ b/include/configs/meson64_android.h @@ -47,13 +47,13 @@ #define AVB_VERIFY_CMD "" #endif
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define ANDROIDBOOT_GET_CURRENT_SLOT_CMD "get_current_slot=" \ "if part number mmc ${mmcdev} " CONTROL_PARTITION " control_part_number; " \ "then " \ "echo " CONTROL_PARTITION \ " partition number:${control_part_number};" \ - "ab_select current_slot mmc ${mmcdev}:${control_part_number};" \ + "bcb ab_select current_slot mmc ${mmcdev}:${control_part_number};" \ "else " \ "echo " CONTROL_PARTITION " partition not found;" \ "fi;\0" diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h index 26494ae98010..5a2ea8c4ddcc 100644 --- a/include/configs/ti_omap5_common.h +++ b/include/configs/ti_omap5_common.h @@ -93,13 +93,13 @@
#define CONTROL_PARTITION "misc"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define AB_SELECT_SLOT \ "if part number mmc 1 " CONTROL_PARTITION " control_part_number; " \ "then " \ "echo " CONTROL_PARTITION \ " partition number:${control_part_number};" \ - "ab_select slot_name mmc ${mmcdev}:${control_part_number};" \ + "bcb ab_select slot_name mmc ${mmcdev}:${control_part_number};" \ "else " \ "echo " CONTROL_PARTITION " partition not found;" \ "exit;" \ diff --git a/test/py/tests/test_android/test_ab.py b/test/py/tests/test_android/test_ab.py index c79cb07fda35..0d7b7995a9fa 100644 --- a/test/py/tests/test_android/test_ab.py +++ b/test/py/tests/test_android/test_ab.py @@ -56,20 +56,20 @@ def ab_disk_image(u_boot_console):
@pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('android_ab') -@pytest.mark.buildconfigspec('cmd_ab_select') +@pytest.mark.buildconfigspec('cmd_bcb') @pytest.mark.requiredtool('sgdisk') def test_ab(ab_disk_image, u_boot_console): - """Test the 'ab_select' command.""" + """Test the 'bcb ab_select' command."""
u_boot_console.run_command('host bind 0 ' + ab_disk_image.path)
- output = u_boot_console.run_command('ab_select slot_name host 0#misc') + output = u_boot_console.run_command('bcb ab_select slot_name host 0#misc') assert 're-initializing A/B metadata' in output assert 'Attempting slot a, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=a' in output
- output = u_boot_console.run_command('ab_select slot_name host 0:1') + output = u_boot_console.run_command('bcb ab_select slot_name host 0:1') assert 'Attempting slot b, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=b' in output
On Wed, 11 Sept 2024 at 15:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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 ddrokosov@salutedevices.com
MAINTAINERS | 1 - cmd/Kconfig | 15 +---- cmd/Makefile | 1 - cmd/ab_select.c | 66 -------------------- cmd/bcb.c | 73 +++++++++++++++++++---- configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 4 +- configs/sandbox_defconfig | 4 +- doc/android/ab.rst | 12 ++-- include/configs/khadas-vim3_android.h | 2 +- include/configs/khadas-vim3l_android.h | 2 +- include/configs/meson64_android.h | 4 +- include/configs/ti_omap5_common.h | 4 +- test/py/tests/test_android/test_ab.py | 8 +-- 16 files changed, 84 insertions(+), 115 deletions(-) delete mode 100644 cmd/ab_select.c
Reviewed-by: Simon Glass sjg@chromium.org
Hi Dmitry,
Thank you for the patch.
On jeu., sept. 12, 2024 at 00:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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 ddrokosov@salutedevices.com
MAINTAINERS | 1 - cmd/Kconfig | 15 +---- cmd/Makefile | 1 - cmd/ab_select.c | 66 -------------------- cmd/bcb.c | 73 +++++++++++++++++++---- configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 4 +- configs/sandbox_defconfig | 4 +- doc/android/ab.rst | 12 ++-- include/configs/khadas-vim3_android.h | 2 +- include/configs/khadas-vim3l_android.h | 2 +- include/configs/meson64_android.h | 4 +- include/configs/ti_omap5_common.h | 4 +- test/py/tests/test_android/test_ab.py | 8 +-- 16 files changed, 84 insertions(+), 115 deletions(-) delete mode 100644 cmd/ab_select.c
diff --git a/MAINTAINERS b/MAINTAINERS index 2050ae24df82..876330d2d750 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -65,7 +65,6 @@ R: Sam Protsenko semen.protsenko@linaro.org S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: boot/android_ab.c -F: cmd/ab_select.c F: doc/android/ab.rst F: include/android_ab.h F: test/py/tests/test_android/test_ab.py diff --git a/cmd/Kconfig b/cmd/Kconfig index 978f44eda426..eb0fb07b426a 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1053,6 +1053,7 @@ config CMD_ADC config CMD_BCB bool "bcb" depends on PARTITIONS
- depends on ANDROID_AB help Read/modify/write the fields of Bootloader Control Block, usually stored on the flash "misc" partition with its structure defined in:
@@ -1766,20 +1767,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 ofthe kernel and system images) this provides a command to select whichslot should be used to boot from and register the boot attempt. Thisis used by the new A/B update model where one slot is updated in thebackground while running from the other slot.-endmenu
if NET
menuconfig CMD_NET diff --git a/cmd/Makefile b/cmd/Makefile index 87133cc27a8a..281c13220e95 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 7c178c728ca4..000000000000 --- 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 <android_ab.h> -#include <command.h> -#include <env.h> -#include <part.h>
-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.","<slot_var_name> <interface> <dev[:part|#part_name]> [--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 97a96c009641..a56535a743c0 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -8,6 +8,7 @@ #include <android_bootloader_message.h> #include <bcb.h> #include <command.h> +#include <android_ab.h> #include <display_options.h> #include <log.h> #include <part.h> @@ -23,6 +24,7 @@ enum bcb_cmd { BCB_CMD_FIELD_TEST, BCB_CMD_FIELD_DUMP, BCB_CMD_STORE,
- BCB_CMD_AB_SELECT,
};
static const char * const fields[] = { @@ -52,6 +54,8 @@ static int bcb_cmd_get(char *cmd) return BCB_CMD_STORE; if (!strcmp(cmd, "dump")) return BCB_CMD_FIELD_DUMP;
- if (!strcmp(cmd, "ab_select"))
return BCB_CMD_AB_SELECT;} @@ -85,6 +89,10 @@ static int bcb_is_misused(int argc, char *const argv[]) if (argc != 2) goto err; break;
- case BCB_CMD_AB_SELECT:
if (argc != 4 && argc != 5)goto err;return 0;@@ -414,6 +422,44 @@ void bcb_reset(void) __bcb_reset(); }
+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;
- for (int i = 4; i < argc; i++) {
if (strcmp(argv[i], "--no-dec") == 0)dec_tries = false;elsereturn 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;
+}
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, "", ""), @@ -421,6 +467,8 @@ static struct cmd_tbl cmd_bcb_sub[] = { 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, "", ""),
- U_BOOT_CMD_MKENT(ab_select, CONFIG_SYS_MAXARGS, 1,
do_bcb_ab_select, "", ""),};
static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -460,15 +508,18 @@ U_BOOT_CMD( "bcb dump <field> - dump BCB <field>\n" "bcb store - store BCB back to <interface>\n" "\n"
- "Legend:\n"
- "<interface> - storage device interface (virtio, mmc, etc)\n"
- "<dev> - storage device index containing the BCB partition\n"
- "<part> - partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in <field>\n"
- " '~' returns true if <val> matches a subset of <field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by line feed\n"
- " during 'bcb set' and used as separator by upper layers\n"
Why does the "Legend:" block gets removed? Is it no longer relevant? To me, we should keep this block, moving it below "bcb ab_select"
- "bcb ab_select -\n"
- " Select the slot used to boot from and register the boot attempt.\n"
- " <slot_var_name> <interface> <dev[:part|#part_name]> [--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/configs/am57xx_hs_evm_usb_defconfig b/configs/am57xx_hs_evm_usb_defconfig index 807e1d66a6d7..6d822b021768 100644 --- a/configs/am57xx_hs_evm_usb_defconfig +++ b/configs/am57xx_hs_evm_usb_defconfig @@ -50,7 +50,6 @@ CONFIG_CMD_ADTIMG=y CONFIG_CMD_ABOOTIMG=y CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2 CONFIG_CMD_BCB=y -CONFIG_CMD_AB_SELECT=y CONFIG_BOOTP_DNS2=y # CONFIG_CMD_PMIC is not set CONFIG_CMD_AVB=y diff --git a/configs/khadas-vim3_android_ab_defconfig b/configs/khadas-vim3_android_ab_defconfig index 37b8d6a9256b..5281c426cb73 100644 --- a/configs/khadas-vim3_android_ab_defconfig +++ b/configs/khadas-vim3_android_ab_defconfig @@ -47,7 +47,6 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_AVB=y CONFIG_OF_CONTROL=y diff --git a/configs/khadas-vim3l_android_ab_defconfig b/configs/khadas-vim3l_android_ab_defconfig index 95e70275cc4f..ff3001b78fa2 100644 --- a/configs/khadas-vim3l_android_ab_defconfig +++ b/configs/khadas-vim3l_android_ab_defconfig @@ -47,7 +47,6 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_AVB=y CONFIG_OF_CONTROL=y diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index dd0582d2a0c0..635753f00b73 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -25,6 +25,7 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_ANDROID_AB=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y @@ -44,6 +45,7 @@ CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPT=y @@ -167,8 +169,8 @@ CONFIG_PWRSEQ=y CONFIG_I2C_EEPROM=y CONFIG_MMC_SANDBOX=y CONFIG_DM_MTD=y -CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 +CONFIG_MTD_RAW_NAND=y CONFIG_SYS_NAND_USE_FLASH_BBT=y CONFIG_NAND_SANDBOX=y CONFIG_SYS_NAND_ONFI_DETECTION=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index dc5fcdbd1c9e..cb69554a7306 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -66,6 +66,7 @@ CONFIG_CMD_MEMINFO=y CONFIG_CMD_MEM_SEARCH=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPIO_READ=y @@ -93,7 +94,6 @@ CONFIG_CMD_AXI=y CONFIG_CMD_CAT=y CONFIG_CMD_SETEXPR_FMT=y CONFIG_CMD_XXD=y -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_DHCP6=y CONFIG_BOOTP_DNS2=y CONFIG_CMD_PCAP=y @@ -219,8 +219,8 @@ CONFIG_MMC_PCI=y CONFIG_MMC_SANDBOX=y CONFIG_MMC_SDHCI=y CONFIG_DM_MTD=y -CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 +CONFIG_MTD_RAW_NAND=y CONFIG_SYS_NAND_USE_FLASH_BBT=y CONFIG_NAND_SANDBOX=y CONFIG_SYS_NAND_ONFI_DETECTION=y diff --git a/doc/android/ab.rst b/doc/android/ab.rst index 2adf88781d60..7fd4aeb6a724 100644 --- a/doc/android/ab.rst +++ b/doc/android/ab.rst @@ -18,7 +18,7 @@ The A/B updates support can be activated by specifying next options in your board configuration file::
CONFIG_ANDROID_AB=y
- CONFIG_CMD_AB_SELECT=y
- CONFIG_CMD_BCB=y
The disk space on target device must be partitioned in a way so that each partition which needs to be updated has two or more instances. The name of @@ -26,8 +26,8 @@ each instance must be formed by adding suffixes: ``_a``, ``_b``, ``_c``, etc. For example: ``boot_a``, ``boot_b``, ``system_a``, ``system_b``, ``vendor_a``, ``vendor_b``.
-As a result you can use ``ab_select`` command to ensure A/B boot process in your -boot script. This command analyzes and processes A/B metadata stored on a +As a result you can use ``bcb ab_select`` command to ensure A/B boot process in +your boot script. This command analyzes and processes A/B metadata stored on a special partition (e.g. ``misc``) and determines which slot should be used for booting up.
@@ -42,15 +42,15 @@ Command usage
.. code-block:: none
- ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
- bcb ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
for example::
- => ab_select slot_name mmc 1:4
- => bcb ab_select slot_name mmc 1:4
or::
- => ab_select slot_name mmc 1#misc
- => bcb ab_select slot_name mmc 1#misc
Result::
diff --git a/include/configs/khadas-vim3_android.h b/include/configs/khadas-vim3_android.h index da6adf6c413a..4a8348914035 100644 --- a/include/configs/khadas-vim3_android.h +++ b/include/configs/khadas-vim3_android.h @@ -12,7 +12,7 @@ #define LOGO_UUID "43a3305d-150f-4cc9-bd3b-38fca8693846;" #define ROOT_UUID "ddb8c3f6-d94d-4394-b633-3134139cc2e0;"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define PARTS_DEFAULT \ "uuid_disk=${uuid_gpt_disk};" \ "name=logo,start=512K,size=2M,uuid=" LOGO_UUID \ diff --git a/include/configs/khadas-vim3l_android.h b/include/configs/khadas-vim3l_android.h index b1768e2d8211..bf1c831c0bc3 100644 --- a/include/configs/khadas-vim3l_android.h +++ b/include/configs/khadas-vim3l_android.h @@ -12,7 +12,7 @@ #define LOGO_UUID "43a3305d-150f-4cc9-bd3b-38fca8693846;" #define ROOT_UUID "ddb8c3f6-d94d-4394-b633-3134139cc2e0;"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB)
This part should only be executed when our partitioning scheme is A/B. I think this diff will break because for both:
configs/khadas-vim3l_android_ab_defconfig configs/khadas-vim3_android_defconfig
We have CONFIG_CMD_BCB=y
How can we differentiate, at build time, boards wanting A/B support and boards that don't?
Note that non A/B is being deprecated so this might not be an issue in the future.
#define PARTS_DEFAULT \ "uuid_disk=${uuid_gpt_disk};" \ "name=logo,start=512K,size=2M,uuid=" LOGO_UUID \ diff --git a/include/configs/meson64_android.h b/include/configs/meson64_android.h index c0e977abb01f..93f9e191a165 100644 --- a/include/configs/meson64_android.h +++ b/include/configs/meson64_android.h @@ -47,13 +47,13 @@ #define AVB_VERIFY_CMD "" #endif
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define ANDROIDBOOT_GET_CURRENT_SLOT_CMD "get_current_slot=" \ "if part number mmc ${mmcdev} " CONTROL_PARTITION " control_part_number; " \ "then " \ "echo " CONTROL_PARTITION \ " partition number:${control_part_number};" \
"ab_select current_slot mmc ${mmcdev}:${control_part_number};" \
"bcb ab_select current_slot mmc ${mmcdev}:${control_part_number};" \diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h index 26494ae98010..5a2ea8c4ddcc 100644 --- a/include/configs/ti_omap5_common.h +++ b/include/configs/ti_omap5_common.h @@ -93,13 +93,13 @@
#define CONTROL_PARTITION "misc"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define AB_SELECT_SLOT \ "if part number mmc 1 " CONTROL_PARTITION " control_part_number; " \ "then " \ "echo " CONTROL_PARTITION \ " partition number:${control_part_number};" \
"ab_select slot_name mmc ${mmcdev}:${control_part_number};" \
"bcb ab_select slot_name mmc ${mmcdev}:${control_part_number};" \diff --git a/test/py/tests/test_android/test_ab.py b/test/py/tests/test_android/test_ab.py index c79cb07fda35..0d7b7995a9fa 100644 --- a/test/py/tests/test_android/test_ab.py +++ b/test/py/tests/test_android/test_ab.py @@ -56,20 +56,20 @@ def ab_disk_image(u_boot_console):
@pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('android_ab') -@pytest.mark.buildconfigspec('cmd_ab_select') +@pytest.mark.buildconfigspec('cmd_bcb') @pytest.mark.requiredtool('sgdisk') def test_ab(ab_disk_image, u_boot_console):
- """Test the 'ab_select' command."""
"""Test the 'bcb ab_select' command."""
u_boot_console.run_command('host bind 0 ' + ab_disk_image.path)
- output = u_boot_console.run_command('ab_select slot_name host 0#misc')
- output = u_boot_console.run_command('bcb ab_select slot_name host 0#misc') assert 're-initializing A/B metadata' in output assert 'Attempting slot a, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=a' in output
- output = u_boot_console.run_command('ab_select slot_name host 0:1')
- output = u_boot_console.run_command('bcb ab_select slot_name host 0:1') assert 'Attempting slot b, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=b' in output
-- 2.43.0
On Mon, Sep 30, 2024 at 01:24:21PM +0200, Mattijs Korpershoek wrote:
Hi Dmitry,
Thank you for the patch.
On jeu., sept. 12, 2024 at 00:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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 ddrokosov@salutedevices.com
MAINTAINERS | 1 - cmd/Kconfig | 15 +---- cmd/Makefile | 1 - cmd/ab_select.c | 66 -------------------- cmd/bcb.c | 73 +++++++++++++++++++---- configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 4 +- configs/sandbox_defconfig | 4 +- doc/android/ab.rst | 12 ++-- include/configs/khadas-vim3_android.h | 2 +- include/configs/khadas-vim3l_android.h | 2 +- include/configs/meson64_android.h | 4 +- include/configs/ti_omap5_common.h | 4 +- test/py/tests/test_android/test_ab.py | 8 +-- 16 files changed, 84 insertions(+), 115 deletions(-) delete mode 100644 cmd/ab_select.c
diff --git a/MAINTAINERS b/MAINTAINERS index 2050ae24df82..876330d2d750 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -65,7 +65,6 @@ R: Sam Protsenko semen.protsenko@linaro.org S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: boot/android_ab.c -F: cmd/ab_select.c F: doc/android/ab.rst F: include/android_ab.h F: test/py/tests/test_android/test_ab.py diff --git a/cmd/Kconfig b/cmd/Kconfig index 978f44eda426..eb0fb07b426a 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1053,6 +1053,7 @@ config CMD_ADC config CMD_BCB bool "bcb" depends on PARTITIONS
- depends on ANDROID_AB help Read/modify/write the fields of Bootloader Control Block, usually stored on the flash "misc" partition with its structure defined in:
@@ -1766,20 +1767,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 ofthe kernel and system images) this provides a command to select whichslot should be used to boot from and register the boot attempt. Thisis used by the new A/B update model where one slot is updated in thebackground while running from the other slot.-endmenu
if NET
menuconfig CMD_NET diff --git a/cmd/Makefile b/cmd/Makefile index 87133cc27a8a..281c13220e95 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 7c178c728ca4..000000000000 --- 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 <android_ab.h> -#include <command.h> -#include <env.h> -#include <part.h>
-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.","<slot_var_name> <interface> <dev[:part|#part_name]> [--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 97a96c009641..a56535a743c0 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -8,6 +8,7 @@ #include <android_bootloader_message.h> #include <bcb.h> #include <command.h> +#include <android_ab.h> #include <display_options.h> #include <log.h> #include <part.h> @@ -23,6 +24,7 @@ enum bcb_cmd { BCB_CMD_FIELD_TEST, BCB_CMD_FIELD_DUMP, BCB_CMD_STORE,
- BCB_CMD_AB_SELECT,
};
static const char * const fields[] = { @@ -52,6 +54,8 @@ static int bcb_cmd_get(char *cmd) return BCB_CMD_STORE; if (!strcmp(cmd, "dump")) return BCB_CMD_FIELD_DUMP;
- if (!strcmp(cmd, "ab_select"))
return BCB_CMD_AB_SELECT;} @@ -85,6 +89,10 @@ static int bcb_is_misused(int argc, char *const argv[]) if (argc != 2) goto err; break;
- case BCB_CMD_AB_SELECT:
if (argc != 4 && argc != 5)goto err;return 0;@@ -414,6 +422,44 @@ void bcb_reset(void) __bcb_reset(); }
+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;
- for (int i = 4; i < argc; i++) {
if (strcmp(argv[i], "--no-dec") == 0)dec_tries = false;elsereturn 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;
+}
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, "", ""), @@ -421,6 +467,8 @@ static struct cmd_tbl cmd_bcb_sub[] = { 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, "", ""),
- U_BOOT_CMD_MKENT(ab_select, CONFIG_SYS_MAXARGS, 1,
do_bcb_ab_select, "", ""),};
static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -460,15 +508,18 @@ U_BOOT_CMD( "bcb dump <field> - dump BCB <field>\n" "bcb store - store BCB back to <interface>\n" "\n"
- "Legend:\n"
- "<interface> - storage device interface (virtio, mmc, etc)\n"
- "<dev> - storage device index containing the BCB partition\n"
- "<part> - partition index or name containing the BCB\n"
- "<field> - one of {command,status,recovery,stage,reserved}\n"
- "<op> - the binary operator used in 'bcb test':\n"
- " '=' returns true if <val> matches the string stored in <field>\n"
- " '~' returns true if <val> matches a subset of <field>'s string\n"
- "<val> - string/text provided as input to bcb {set,test}\n"
- " NOTE: any ':' character in <val> will be replaced by line feed\n"
- " during 'bcb set' and used as separator by upper layers\n"
Why does the "Legend:" block gets removed? Is it no longer relevant? To me, we should keep this block, moving it below "bcb ab_select"
Oh, my apologies! I'll make sure to fix it in the next version.
- "bcb ab_select -\n"
- " Select the slot used to boot from and register the boot attempt.\n"
- " <slot_var_name> <interface> <dev[:part|#part_name]> [--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/configs/am57xx_hs_evm_usb_defconfig b/configs/am57xx_hs_evm_usb_defconfig index 807e1d66a6d7..6d822b021768 100644 --- a/configs/am57xx_hs_evm_usb_defconfig +++ b/configs/am57xx_hs_evm_usb_defconfig @@ -50,7 +50,6 @@ CONFIG_CMD_ADTIMG=y CONFIG_CMD_ABOOTIMG=y CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2 CONFIG_CMD_BCB=y -CONFIG_CMD_AB_SELECT=y CONFIG_BOOTP_DNS2=y # CONFIG_CMD_PMIC is not set CONFIG_CMD_AVB=y diff --git a/configs/khadas-vim3_android_ab_defconfig b/configs/khadas-vim3_android_ab_defconfig index 37b8d6a9256b..5281c426cb73 100644 --- a/configs/khadas-vim3_android_ab_defconfig +++ b/configs/khadas-vim3_android_ab_defconfig @@ -47,7 +47,6 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_AVB=y CONFIG_OF_CONTROL=y diff --git a/configs/khadas-vim3l_android_ab_defconfig b/configs/khadas-vim3l_android_ab_defconfig index 95e70275cc4f..ff3001b78fa2 100644 --- a/configs/khadas-vim3l_android_ab_defconfig +++ b/configs/khadas-vim3l_android_ab_defconfig @@ -47,7 +47,6 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_REGULATOR=y CONFIG_CMD_AVB=y CONFIG_OF_CONTROL=y diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index dd0582d2a0c0..635753f00b73 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -25,6 +25,7 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_DISPLAY_BOARDINFO_LATE=y +CONFIG_ANDROID_AB=y CONFIG_CMD_CPU=y CONFIG_CMD_LICENSE=y CONFIG_CMD_BOOTZ=y @@ -44,6 +45,7 @@ CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPT=y @@ -167,8 +169,8 @@ CONFIG_PWRSEQ=y CONFIG_I2C_EEPROM=y CONFIG_MMC_SANDBOX=y CONFIG_DM_MTD=y -CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 +CONFIG_MTD_RAW_NAND=y CONFIG_SYS_NAND_USE_FLASH_BBT=y CONFIG_NAND_SANDBOX=y CONFIG_SYS_NAND_ONFI_DETECTION=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index dc5fcdbd1c9e..cb69554a7306 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -66,6 +66,7 @@ CONFIG_CMD_MEMINFO=y CONFIG_CMD_MEM_SEARCH=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPIO_READ=y @@ -93,7 +94,6 @@ CONFIG_CMD_AXI=y CONFIG_CMD_CAT=y CONFIG_CMD_SETEXPR_FMT=y CONFIG_CMD_XXD=y -CONFIG_CMD_AB_SELECT=y CONFIG_CMD_DHCP6=y CONFIG_BOOTP_DNS2=y CONFIG_CMD_PCAP=y @@ -219,8 +219,8 @@ CONFIG_MMC_PCI=y CONFIG_MMC_SANDBOX=y CONFIG_MMC_SDHCI=y CONFIG_DM_MTD=y -CONFIG_MTD_RAW_NAND=y CONFIG_SYS_MAX_NAND_DEVICE=8 +CONFIG_MTD_RAW_NAND=y CONFIG_SYS_NAND_USE_FLASH_BBT=y CONFIG_NAND_SANDBOX=y CONFIG_SYS_NAND_ONFI_DETECTION=y diff --git a/doc/android/ab.rst b/doc/android/ab.rst index 2adf88781d60..7fd4aeb6a724 100644 --- a/doc/android/ab.rst +++ b/doc/android/ab.rst @@ -18,7 +18,7 @@ The A/B updates support can be activated by specifying next options in your board configuration file::
CONFIG_ANDROID_AB=y
- CONFIG_CMD_AB_SELECT=y
- CONFIG_CMD_BCB=y
The disk space on target device must be partitioned in a way so that each partition which needs to be updated has two or more instances. The name of @@ -26,8 +26,8 @@ each instance must be formed by adding suffixes: ``_a``, ``_b``, ``_c``, etc. For example: ``boot_a``, ``boot_b``, ``system_a``, ``system_b``, ``vendor_a``, ``vendor_b``.
-As a result you can use ``ab_select`` command to ensure A/B boot process in your -boot script. This command analyzes and processes A/B metadata stored on a +As a result you can use ``bcb ab_select`` command to ensure A/B boot process in +your boot script. This command analyzes and processes A/B metadata stored on a special partition (e.g. ``misc``) and determines which slot should be used for booting up.
@@ -42,15 +42,15 @@ Command usage
.. code-block:: none
- ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
- bcb ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
for example::
- => ab_select slot_name mmc 1:4
- => bcb ab_select slot_name mmc 1:4
or::
- => ab_select slot_name mmc 1#misc
- => bcb ab_select slot_name mmc 1#misc
Result::
diff --git a/include/configs/khadas-vim3_android.h b/include/configs/khadas-vim3_android.h index da6adf6c413a..4a8348914035 100644 --- a/include/configs/khadas-vim3_android.h +++ b/include/configs/khadas-vim3_android.h @@ -12,7 +12,7 @@ #define LOGO_UUID "43a3305d-150f-4cc9-bd3b-38fca8693846;" #define ROOT_UUID "ddb8c3f6-d94d-4394-b633-3134139cc2e0;"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define PARTS_DEFAULT \ "uuid_disk=${uuid_gpt_disk};" \ "name=logo,start=512K,size=2M,uuid=" LOGO_UUID \ diff --git a/include/configs/khadas-vim3l_android.h b/include/configs/khadas-vim3l_android.h index b1768e2d8211..bf1c831c0bc3 100644 --- a/include/configs/khadas-vim3l_android.h +++ b/include/configs/khadas-vim3l_android.h @@ -12,7 +12,7 @@ #define LOGO_UUID "43a3305d-150f-4cc9-bd3b-38fca8693846;" #define ROOT_UUID "ddb8c3f6-d94d-4394-b633-3134139cc2e0;"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB)
This part should only be executed when our partitioning scheme is A/B. I think this diff will break because for both:
configs/khadas-vim3l_android_ab_defconfig configs/khadas-vim3_android_defconfig
We have CONFIG_CMD_BCB=y
How can we differentiate, at build time, boards wanting A/B support and boards that don't?
Note that non A/B is being deprecated so this might not be an issue in the future.
I believe it's better to check both conditions as shown below:
``` #if defined(CONFIG_CMD_BCB) && defined(CONFIG_ANDROID_AB) ```
#define PARTS_DEFAULT \ "uuid_disk=${uuid_gpt_disk};" \ "name=logo,start=512K,size=2M,uuid=" LOGO_UUID \ diff --git a/include/configs/meson64_android.h b/include/configs/meson64_android.h index c0e977abb01f..93f9e191a165 100644 --- a/include/configs/meson64_android.h +++ b/include/configs/meson64_android.h @@ -47,13 +47,13 @@ #define AVB_VERIFY_CMD "" #endif
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define ANDROIDBOOT_GET_CURRENT_SLOT_CMD "get_current_slot=" \ "if part number mmc ${mmcdev} " CONTROL_PARTITION " control_part_number; " \ "then " \ "echo " CONTROL_PARTITION \ " partition number:${control_part_number};" \
"ab_select current_slot mmc ${mmcdev}:${control_part_number};" \
"bcb ab_select current_slot mmc ${mmcdev}:${control_part_number};" \diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h index 26494ae98010..5a2ea8c4ddcc 100644 --- a/include/configs/ti_omap5_common.h +++ b/include/configs/ti_omap5_common.h @@ -93,13 +93,13 @@
#define CONTROL_PARTITION "misc"
-#if defined(CONFIG_CMD_AB_SELECT) +#if defined(CONFIG_CMD_BCB) #define AB_SELECT_SLOT \ "if part number mmc 1 " CONTROL_PARTITION " control_part_number; " \ "then " \ "echo " CONTROL_PARTITION \ " partition number:${control_part_number};" \
"ab_select slot_name mmc ${mmcdev}:${control_part_number};" \
"bcb ab_select slot_name mmc ${mmcdev}:${control_part_number};" \diff --git a/test/py/tests/test_android/test_ab.py b/test/py/tests/test_android/test_ab.py index c79cb07fda35..0d7b7995a9fa 100644 --- a/test/py/tests/test_android/test_ab.py +++ b/test/py/tests/test_android/test_ab.py @@ -56,20 +56,20 @@ def ab_disk_image(u_boot_console):
@pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('android_ab') -@pytest.mark.buildconfigspec('cmd_ab_select') +@pytest.mark.buildconfigspec('cmd_bcb') @pytest.mark.requiredtool('sgdisk') def test_ab(ab_disk_image, u_boot_console):
- """Test the 'ab_select' command."""
"""Test the 'bcb ab_select' command."""
u_boot_console.run_command('host bind 0 ' + ab_disk_image.path)
- output = u_boot_console.run_command('ab_select slot_name host 0#misc')
- output = u_boot_console.run_command('bcb ab_select slot_name host 0#misc') assert 're-initializing A/B metadata' in output assert 'Attempting slot a, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=a' in output
- output = u_boot_console.run_command('ab_select slot_name host 0:1')
- output = u_boot_console.run_command('bcb ab_select slot_name host 0:1') assert 'Attempting slot b, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=b' in output
-- 2.43.0
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.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- cmd/bcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/bcb.c b/cmd/bcb.c index a56535a743c0..a888549eed3a 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -432,7 +432,7 @@ static int do_bcb_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, bool dec_tries = true;
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;
On Wed, 11 Sept 2024 at 15:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
cmd/bcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
And throughout U-Boot
diff --git a/cmd/bcb.c b/cmd/bcb.c index a56535a743c0..a888549eed3a 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -432,7 +432,7 @@ static int do_bcb_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, bool dec_tries = true;
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;-- 2.43.0
Hi Dmitry,
Thank you for the patch.
On jeu., sept. 12, 2024 at 00:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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.
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
cmd/bcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/bcb.c b/cmd/bcb.c index a56535a743c0..a888549eed3a 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -432,7 +432,7 @@ static int do_bcb_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, bool dec_tries = true;
for (int i = 4; i < argc; i++) {
if (strcmp(argv[i], "--no-dec") == 0)
if (!strcmp(argv[i], "--no-dec")) dec_tries = false;-- 2.43.0
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
====
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com --- boot/android_ab.c | 68 ++++++++++++++++++++++++++++++++++++++++++++ cmd/bcb.c | 35 +++++++++++++++++++++++ include/android_ab.h | 10 +++++++ 3 files changed, 113 insertions(+)
diff --git a/boot/android_ab.c b/boot/android_ab.c index 0045c8133a8e..c93e51541019 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -372,3 +372,71 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
return slot; } + +int ab_dump_abc(struct blk_desc *dev_desc, struct disk_partition *part_info) +{ + struct bootloader_control *abc; + u32 crc32_le; + int i, ret; + struct slot_metadata *slot; + + if (!dev_desc || !part_info) { + log_err("ANDROID: Empty device descriptor or partition info\n"); + return -EINVAL; + } + + ret = ab_control_create_from_disk(dev_desc, part_info, &abc, 0); + if (ret < 0) { + log_err("ANDROID: Cannot create bcb from disk %d\n", ret); + return ret; + } + + if (abc->magic != BOOT_CTRL_MAGIC) { + log_err("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic); + ret = -ENODATA; + goto error; + } + + if (abc->version > BOOT_CTRL_VERSION) { + log_err("ANDROID: Unsupported A/B metadata version: %.8x\n", + abc->version); + ret = -ENODATA; + goto error; + } + + if (abc->nb_slot > ARRAY_SIZE(abc->slot_info)) { + log_err("ANDROID: Wrong number of slots %u, expected %zu\n", + abc->nb_slot, ARRAY_SIZE(abc->slot_info)); + ret = -ENODATA; + goto error; + } + + printf("Bootloader Control: [%s]\n", part_info->name); + printf("Active Slot: %s\n", abc->slot_suffix); + printf("Magic Number: 0x%x\n", abc->magic); + printf("Version: %u\n", abc->version); + printf("Number of Slots: %u\n", abc->nb_slot); + printf("Recovery Tries Remaining: %u\n", abc->recovery_tries_remaining); + + printf("CRC: 0x%.8x", abc->crc32_le); + + crc32_le = ab_control_compute_crc(abc); + if (abc->crc32_le != crc32_le) + printf(" (Invalid, Expected: 0x%.8x)\n", crc32_le); + else + printf(" (Valid)\n"); + + for (i = 0; i < abc->nb_slot; ++i) { + slot = &abc->slot_info[i]; + printf("\nSlot[%d] Metadata:\n", i); + printf("\t- Priority: %u\n", slot->priority); + printf("\t- Tries Remaining: %u\n", slot->tries_remaining); + printf("\t- Successful Boot: %u\n", slot->successful_boot); + printf("\t- Verity Corrupted: %u\n", slot->verity_corrupted); + } + +error: + free(abc); + + return ret; +} diff --git a/cmd/bcb.c b/cmd/bcb.c index a888549eed3a..d69f6df05e15 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -25,6 +25,7 @@ enum bcb_cmd { BCB_CMD_FIELD_DUMP, BCB_CMD_STORE, BCB_CMD_AB_SELECT, + BCB_CMD_AB_DUMP, };
static const char * const fields[] = { @@ -56,6 +57,8 @@ static int bcb_cmd_get(char *cmd) return BCB_CMD_FIELD_DUMP; if (!strcmp(cmd, "ab_select")) return BCB_CMD_AB_SELECT; + if (!strcmp(cmd, "ab_dump")) + return BCB_CMD_AB_DUMP; else return -1; } @@ -93,6 +96,10 @@ static int bcb_is_misused(int argc, char *const argv[]) if (argc != 4 && argc != 5) goto err; return 0; + case BCB_CMD_AB_DUMP: + if (argc != 3) + goto err; + return 0; default: printf("Error: 'bcb %s' not supported\n", argv[0]); return -1; @@ -460,6 +467,28 @@ static int do_bcb_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_SUCCESS; }
+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 (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; +} + 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, "", ""), @@ -469,6 +498,8 @@ static struct cmd_tbl cmd_bcb_sub[] = { U_BOOT_CMD_MKENT(store, CONFIG_SYS_MAXARGS, 1, do_bcb_store, "", ""), U_BOOT_CMD_MKENT(ab_select, CONFIG_SYS_MAXARGS, 1, do_bcb_ab_select, "", ""), + U_BOOT_CMD_MKENT(ab_dump, CONFIG_SYS_MAXARGS, 1, + do_bcb_ab_dump, "", ""), };
static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -522,4 +553,8 @@ U_BOOT_CMD( " 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" + "bcb ab_dump -\n" + " Dump boot_control information from specific partition.\n" + " <interface> <dev[:part|#part_name]>\n" ); diff --git a/include/android_ab.h b/include/android_ab.h index 1e53879a25f1..838230e06f8c 100644 --- a/include/android_ab.h +++ b/include/android_ab.h @@ -36,4 +36,14 @@ struct disk_partition; int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, bool dec_tries);
+/** + * ab_dump_abc() - Dump ABC information for specific partition. + * + * @dev_desc: Device description pointer + * @part_info: Partition information + * + * Return: 0 on success, or a negative on error + */ +int ab_dump_abc(struct blk_desc *dev_desc, struct disk_partition *part_info); + #endif /* __ANDROID_AB_H */
On Wed, 11 Sept 2024 at 15:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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
====
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
boot/android_ab.c | 68 ++++++++++++++++++++++++++++++++++++++++++++ cmd/bcb.c | 35 +++++++++++++++++++++++ include/android_ab.h | 10 +++++++ 3 files changed, 113 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Hi Dmitry,
Thank you for the patch.
On jeu., sept. 12, 2024 at 00:49, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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
====
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
boot/android_ab.c | 68 ++++++++++++++++++++++++++++++++++++++++++++ cmd/bcb.c | 35 +++++++++++++++++++++++ include/android_ab.h | 10 +++++++ 3 files changed, 113 insertions(+)
To align with the official Android BCB (Bootloader Control Block) specifications, it's important to note that the slot_suffix should start with an underscore symbol.
For a comprehensive understanding of the expected slot_suffix format in userspace, please refer to the provided reference [1].
Links: [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots
Based-on: https://android-review.googlesource.com/c/platform/external/u-boot/+/1446439 Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- boot/android_ab.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/boot/android_ab.c b/boot/android_ab.c index c93e51541019..a287eac04fe8 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -52,7 +52,7 @@ static int ab_control_default(struct bootloader_control *abc) if (!abc) return -EFAULT;
- memcpy(abc->slot_suffix, "a\0\0\0", 4); + memcpy(abc->slot_suffix, "_a\0\0", 4); abc->magic = BOOT_CTRL_MAGIC; abc->version = BOOT_CTRL_VERSION; abc->nb_slot = NUM_SLOTS; @@ -328,7 +328,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, * or the device tree. */ memset(slot_suffix, 0, sizeof(slot_suffix)); - slot_suffix[0] = BOOT_SLOT_NAME(slot); + slot_suffix[0] = '_'; + slot_suffix[1] = BOOT_SLOT_NAME(slot); if (memcmp(abc->slot_suffix, slot_suffix, sizeof(slot_suffix))) { memcpy(abc->slot_suffix, slot_suffix,
Hello Simon and Mattijs!
In the v1 patch series, you suggested creating a test case for the changes in this patchset. However, the bcb ab_dump test in the current patch series already covers all code paths involving slot_suffix generation. Therefore, an additional test case for that is not necessary.
On Thu, Sep 12, 2024 at 12:49:13AM +0300, Dmitry Rokosov wrote:
To align with the official Android BCB (Bootloader Control Block) specifications, it's important to note that the slot_suffix should start with an underscore symbol.
For a comprehensive understanding of the expected slot_suffix format in userspace, please refer to the provided reference [1].
Links: [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots
Based-on: https://android-review.googlesource.com/c/platform/external/u-boot/+/1446439 Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
boot/android_ab.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/boot/android_ab.c b/boot/android_ab.c index c93e51541019..a287eac04fe8 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -52,7 +52,7 @@ static int ab_control_default(struct bootloader_control *abc) if (!abc) return -EFAULT;
- memcpy(abc->slot_suffix, "a\0\0\0", 4);
- memcpy(abc->slot_suffix, "_a\0\0", 4); abc->magic = BOOT_CTRL_MAGIC; abc->version = BOOT_CTRL_VERSION; abc->nb_slot = NUM_SLOTS;
@@ -328,7 +328,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, * or the device tree. */ memset(slot_suffix, 0, sizeof(slot_suffix));
slot_suffix[0] = BOOT_SLOT_NAME(slot);
slot_suffix[0] = '_';slot_suffix[1] = BOOT_SLOT_NAME(slot);-- 2.43.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 ddrokosov@salutedevices.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com --- test/py/tests/test_android/test_ab.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/test/py/tests/test_android/test_ab.py b/test/py/tests/test_android/test_ab.py index 0d7b7995a9fa..9bf1a0eb00a6 100644 --- a/test/py/tests/test_android/test_ab.py +++ b/test/py/tests/test_android/test_ab.py @@ -54,6 +54,27 @@ def ab_disk_image(u_boot_console): di = ABTestDiskImage(u_boot_console) return di
+def ab_dump(u_boot_console, slot_num, crc): + output = u_boot_console.run_command('bcb ab_dump host 0#misc') + header, slot0, slot1 = output.split('\r\r\n\r\r\n') + slots = [slot0, slot1] + slot_suffixes = ['_a', '_b'] + + header = dict(map(lambda x: map(str.strip, x.split(':')), header.split('\r\r\n'))) + assert header['Bootloader Control'] == '[misc]' + assert header['Active Slot'] == slot_suffixes[slot_num] + assert header['Magic Number'] == '0x42414342' + assert header['Version'] == '1' + assert header['Number of Slots'] == '2' + assert header['Recovery Tries Remaining'] == '0' + assert header['CRC'] == '{} (Valid)'.format(crc) + + slot = dict(map(lambda x: map(str.strip, x.split(':')), slots[slot_num].split('\r\r\n\t- ')[1:])) + assert slot['Priority'] == '15' + assert slot['Tries Remaining'] == '6' + assert slot['Successful Boot'] == '0' + assert slot['Verity Corrupted'] == '0' + @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('android_ab') @pytest.mark.buildconfigspec('cmd_bcb') @@ -68,8 +89,10 @@ def test_ab(ab_disk_image, u_boot_console): assert 'Attempting slot a, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=a' in output + ab_dump(u_boot_console, 0, '0xd438d1b9')
output = u_boot_console.run_command('bcb ab_select slot_name host 0:1') assert 'Attempting slot b, tries remaining 7' in output output = u_boot_console.run_command('printenv slot_name') assert 'slot_name=b' in output + ab_dump(u_boot_console, 1, '0x011ec016')
On Wed, 11 Sept 2024 at 15:50, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
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 ddrokosov@salutedevices.com Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
test/py/tests/test_android/test_ab.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
There are Pipeline results. One test was failed. I suppose it's not related to my patch series:
https://github.com/u-boot/u-boot/pull/625/checks?check_run_id=30029925059
=================================== FAILURES =================================== _______________________ test_ut[ut_dm_dm_test_rtc_dual] ________________________ test/py/tests/test_ut.py:590: in test_ut assert output.endswith('Failures: 0') E AssertionError: assert False E + where False = <built-in method endswith of str object at 0x7fb0835c50d0>('Failures: 0') E + where <built-in method endswith of str object at 0x7fb0835c50d0> = 'Test: dm_test_rtc_dual: rtc.c\r\r\nTest: dm_test_rtc_dual: rtc.c (flat tree)\r\r\ntest/dm/rtc.c:307, dm_test_rtc_dual...&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)\r\r\nTest dm_test_rtc_dual failed 1 times\r\r\nFailures: 1'.endswith ----------------------------- Captured stdout call ----------------------------- => ut dm dm_test_rtc_dual
Test: dm_test_rtc_dual: rtc.c
Test: dm_test_rtc_dual: rtc.c (flat tree)
test/dm/rtc.c:307, dm_test_rtc_dual(): -EINVAL == cmp_times(&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)
Test dm_test_rtc_dual failed 1 times
Failures: 1
=> =================================== FAILURES ===================================
On Thu, Sep 12, 2024 at 12:49:08AM +0300, Dmitry Rokosov wrote:
The patch series include changes: - move ab_select_slot() documentation to @ notation - move ab_select command to bcb subcommands - introduce the ab_dump command to print the content of the BCB block; it's useful for debugging A/B logic on supported boards - fix the slot suffix format in the ABC block to align with official Android BCB specifications - add a test for the ab_dump command to verify the accuracy of each field within the ABC data displayed, it's also useful for testing slot_suffix problem code paths
Changes v2 since v1 at [1]: - move ab_select_slot() documentation to @ notation - move ab_select command to bcb subcommands per Simon and Mattijs suggestions - redesign ab_dump as bcb subcommand - use spaces instead of tabs in the ab_dump command output - print hex values in the lowercase - add RvB tags
Links: [1] https://lore.kernel.org/all/20240725194716.32232-1-ddrokosov@salutedevices.c...
Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com
Dmitry Rokosov (6): include/android_ab: move ab_select_slot() documentation to @ notation treewide: bcb: move ab_select command to bcb subcommands cmd: bcb: change strcmp() usage style in the do_bcb_ab_select() cmd: bcb: introduce 'ab_dump' command to print BCB block content common: android_ab: fix slot suffix for abc block test/py: introduce test for ab_dump command
MAINTAINERS | 1 - boot/android_ab.c | 116 ++++++++++++++++++---- cmd/Kconfig | 15 +-- cmd/Makefile | 1 - cmd/ab_select.c | 66 ------------ cmd/bcb.c | 108 ++++++++++++++++++-- configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 4 +- configs/sandbox_defconfig | 4 +- doc/android/ab.rst | 12 +-- include/android_ab.h | 17 +++- include/configs/khadas-vim3_android.h | 2 +- include/configs/khadas-vim3l_android.h | 2 +- include/configs/meson64_android.h | 4 +- include/configs/ti_omap5_common.h | 4 +- test/py/tests/test_android/test_ab.py | 31 +++++- 18 files changed, 251 insertions(+), 139 deletions(-) delete mode 100644 cmd/ab_select.c
-- 2.43.0
On Thu, Sep 12, 2024 at 10:10:53AM +0300, Dmitry Rokosov wrote:
There are Pipeline results. One test was failed. I suppose it's not related to my patch series:
https://github.com/u-boot/u-boot/pull/625/checks?check_run_id=30029925059
=================================== FAILURES =================================== _______________________ test_ut[ut_dm_dm_test_rtc_dual] ________________________ test/py/tests/test_ut.py:590: in test_ut assert output.endswith('Failures: 0') E AssertionError: assert False E + where False = <built-in method endswith of str object at 0x7fb0835c50d0>('Failures: 0') E + where <built-in method endswith of str object at 0x7fb0835c50d0> = 'Test: dm_test_rtc_dual: rtc.c\r\r\nTest: dm_test_rtc_dual: rtc.c (flat tree)\r\r\ntest/dm/rtc.c:307, dm_test_rtc_dual...&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)\r\r\nTest dm_test_rtc_dual failed 1 times\r\r\nFailures: 1'.endswith ----------------------------- Captured stdout call ----------------------------- => ut dm dm_test_rtc_dual
Test: dm_test_rtc_dual: rtc.c
Test: dm_test_rtc_dual: rtc.c (flat tree)
test/dm/rtc.c:307, dm_test_rtc_dual(): -EINVAL == cmp_times(&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)
Test dm_test_rtc_dual failed 1 times
Failures: 1
=> =================================== FAILURES ===================================
Yes, sometimes that just fails due to the underlying hardware (even with retries). Please tell it to try again so that the world build tests run. Sorry for the noise here.
Hello Tom,
On Thu, Sep 12, 2024 at 11:45:01AM -0600, Tom Rini wrote:
On Thu, Sep 12, 2024 at 10:10:53AM +0300, Dmitry Rokosov wrote:
There are Pipeline results. One test was failed. I suppose it's not related to my patch series:
https://github.com/u-boot/u-boot/pull/625/checks?check_run_id=30029925059
=================================== FAILURES =================================== _______________________ test_ut[ut_dm_dm_test_rtc_dual] ________________________ test/py/tests/test_ut.py:590: in test_ut assert output.endswith('Failures: 0') E AssertionError: assert False E + where False = <built-in method endswith of str object at 0x7fb0835c50d0>('Failures: 0') E + where <built-in method endswith of str object at 0x7fb0835c50d0> = 'Test: dm_test_rtc_dual: rtc.c\r\r\nTest: dm_test_rtc_dual: rtc.c (flat tree)\r\r\ntest/dm/rtc.c:307, dm_test_rtc_dual...&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)\r\r\nTest dm_test_rtc_dual failed 1 times\r\r\nFailures: 1'.endswith ----------------------------- Captured stdout call ----------------------------- => ut dm dm_test_rtc_dual
Test: dm_test_rtc_dual: rtc.c
Test: dm_test_rtc_dual: rtc.c (flat tree)
test/dm/rtc.c:307, dm_test_rtc_dual(): -EINVAL == cmp_times(&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)
Test dm_test_rtc_dual failed 1 times
Failures: 1
=> =================================== FAILURES ===================================
Yes, sometimes that just fails due to the underlying hardware (even with retries). Please tell it to try again so that the world build tests run. Sorry for the noise here.
Thank you for the comments. I have re-run the pipeline. Once it is finished, I will provide the results here.
On Thu, Sep 12, 2024 at 10:10:53AM +0300, Dmitry Rokosov wrote:
There are Pipeline results. One test was failed. I suppose it's not related to my patch series:
https://github.com/u-boot/u-boot/pull/625/checks?check_run_id=30029925059
=================================== FAILURES =================================== _______________________ test_ut[ut_dm_dm_test_rtc_dual] ________________________ test/py/tests/test_ut.py:590: in test_ut assert output.endswith('Failures: 0') E AssertionError: assert False E + where False = <built-in method endswith of str object at 0x7fb0835c50d0>('Failures: 0') E + where <built-in method endswith of str object at 0x7fb0835c50d0> = 'Test: dm_test_rtc_dual: rtc.c\r\r\nTest: dm_test_rtc_dual: rtc.c (flat tree)\r\r\ntest/dm/rtc.c:307, dm_test_rtc_dual...&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)\r\r\nTest dm_test_rtc_dual failed 1 times\r\r\nFailures: 1'.endswith ----------------------------- Captured stdout call ----------------------------- => ut dm dm_test_rtc_dual
Test: dm_test_rtc_dual: rtc.c
Test: dm_test_rtc_dual: rtc.c (flat tree)
test/dm/rtc.c:307, dm_test_rtc_dual(): -EINVAL == cmp_times(&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)
Test dm_test_rtc_dual failed 1 times
Failures: 1
=> =================================== FAILURES ===================================
I've re-run the pipeline. This time it was finished successfully.
https://github.com/u-boot/u-boot/pull/625/checks
On Fri, Sep 13, 2024 at 07:27:38PM +0300, Dmitry Rokosov wrote:
On Thu, Sep 12, 2024 at 10:10:53AM +0300, Dmitry Rokosov wrote:
There are Pipeline results. One test was failed. I suppose it's not related to my patch series:
https://github.com/u-boot/u-boot/pull/625/checks?check_run_id=30029925059
=================================== FAILURES =================================== _______________________ test_ut[ut_dm_dm_test_rtc_dual] ________________________ test/py/tests/test_ut.py:590: in test_ut assert output.endswith('Failures: 0') E AssertionError: assert False E + where False = <built-in method endswith of str object at 0x7fb0835c50d0>('Failures: 0') E + where <built-in method endswith of str object at 0x7fb0835c50d0> = 'Test: dm_test_rtc_dual: rtc.c\r\r\nTest: dm_test_rtc_dual: rtc.c (flat tree)\r\r\ntest/dm/rtc.c:307, dm_test_rtc_dual...&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)\r\r\nTest dm_test_rtc_dual failed 1 times\r\r\nFailures: 1'.endswith ----------------------------- Captured stdout call ----------------------------- => ut dm dm_test_rtc_dual
Test: dm_test_rtc_dual: rtc.c
Test: dm_test_rtc_dual: rtc.c (flat tree)
test/dm/rtc.c:307, dm_test_rtc_dual(): -EINVAL == cmp_times(&now1, &cmp, false): Expected 0xffffffea (-22), got 0x0 (0)
Test dm_test_rtc_dual failed 1 times
Failures: 1
=> =================================== FAILURES ===================================
I've re-run the pipeline. This time it was finished successfully.
Thanks!
participants (4)
-
Dmitry Rokosov -
Mattijs Korpershoek -
Simon Glass -
Tom Rini