[PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[Dmitry Rokosov]

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.com/

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

[PATCH v2 1/6] include/android_ab: move ab_select_slot() documentation to @ notation

[Dmitry Rokosov]

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,
-- 
2.43.0

Re: [PATCH v2 1/6] include/android_ab: move ab_select_slot() documentation to @ notation

[Simon Glass]

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>

Re: [PATCH v2 1/6] include/android_ab: move ab_select_slot() documentation to @ notation

[Mattijs Korpershoek]

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(-)
>

[PATCH v2 2/6] treewide: bcb: move ab_select command to bcb subcommands

[Dmitry Rokosov]

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
-- 
2.43.0

Re: [PATCH v2 2/6] treewide: bcb: move ab_select command to bcb subcommands

[Simon Glass]

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>

Re: [PATCH v2 2/6] treewide: bcb: move ab_select command to bcb subcommands

[Mattijs Korpershoek]

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 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"

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};" \
>  	"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
> -- 
> 2.43.0

Re: [PATCH v2 2/6] treewide: bcb: move ab_select command to bcb subcommands

[Dmitry Rokosov]

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 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"
> 
> 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};" \
> >  	"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
> > -- 
> > 2.43.0

-- 
Thank you,
Dmitry

[PATCH v2 3/6] cmd: bcb: change strcmp() usage style in the do_bcb_ab_select()

[Dmitry Rokosov]

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;
-- 
2.43.0

Re: [PATCH v2 3/6] cmd: bcb: change strcmp() usage style in the do_bcb_ab_select()

[Simon Glass]

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
>

Re: [PATCH v2 3/6] cmd: bcb: change strcmp() usage style in the do_bcb_ab_select()

[Mattijs Korpershoek]

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;
>  		else
>  			return CMD_RET_USAGE;
> -- 
> 2.43.0

[PATCH v2 4/6] cmd: bcb: introduce 'ab_dump' command to print BCB block content

[Dmitry Rokosov]

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 */
-- 
2.43.0

Re: [PATCH v2 4/6] cmd: bcb: introduce 'ab_dump' command to print BCB block content

[Simon Glass]

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>

Re: [PATCH v2 4/6] cmd: bcb: introduce 'ab_dump' command to print BCB block content

[Mattijs Korpershoek]

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(+)
>

[PATCH v2 5/6] common: android_ab: fix slot suffix for abc block

[Dmitry Rokosov]

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,
-- 
2.43.0

Re: [PATCH v2 5/6] common: android_ab: fix slot suffix for abc block

[Dmitry Rokosov]

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);
>  		if (memcmp(abc->slot_suffix, slot_suffix,
>  			   sizeof(slot_suffix))) {
>  			memcpy(abc->slot_suffix, slot_suffix,
> -- 
> 2.43.0
> 

-- 
Thank you,
Dmitry

[PATCH v2 6/6] test/py: introduce test for ab_dump command

[Dmitry Rokosov]

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')
-- 
2.43.0

Re: [PATCH v2 6/6] test/py: introduce test for ab_dump command

[Simon Glass]

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>

Re: [PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[Dmitry Rokosov]

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.com/
> 
> 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
> 

-- 
Thank you,
Dmitry

Re: [PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[Dmitry Rokosov]

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.

-- 
Thank you,
Dmitry

Re: [PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[Dmitry Rokosov]

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

-- 
Thank you,
Dmitry

Re: [PATCH v2 0/6] android_ab: introduce bcb ab_dump command and provide several bcb fixes

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]

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.
> 
> https://github.com/u-boot/u-boot/pull/625/checks

Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]