Hi Dmitry,

Thank you for the patch.

On jeu., oct. 17, 2024 at 17:12, Dmitry Rokosov ddrokosov@salutedevices.com wrote:

U_BOOT_LONGHELP and U_BOOT_CMD_WITH_SUBCMDS offer numerous advantages, including:

• common argument restrictions checking
• automatic subcommand matching
• improved usage and help handling

By utilizing the U_BOOT_LONGHELP approach, we can reduce the amount of command management code and describe commands more succinctly.

Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com

cmd/bcb.c | 151 ++++++++++++++++++-------------------------------------------- 1 file changed, 43 insertions(+), 108 deletions(-)

Nice diffstat, always great to see code removed!

Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

diff --git a/cmd/bcb.c b/cmd/bcb.c index 97a96c009641cc094645607ef833575f3c03fe4b..98b2a71087a27b1721f4bed4160095d65ec75402 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -16,15 +16,6 @@ #include <vsprintf.h> #include <linux/err.h>

-enum bcb_cmd {

• BCB_CMD_LOAD,
• BCB_CMD_FIELD_SET,
• BCB_CMD_FIELD_CLEAR,
• BCB_CMD_FIELD_TEST,
• BCB_CMD_FIELD_DUMP,
• BCB_CMD_STORE,

-};

static const char * const fields[] = { "command", "status", @@ -38,67 +29,9 @@ static struct disk_partition partition_data; static struct blk_desc *block; static struct disk_partition *partition = &partition_data;

-static int bcb_cmd_get(char *cmd) -{

• if (!strcmp(cmd, "load"))

• return BCB_CMD_LOAD;
  

• if (!strcmp(cmd, "set"))

• return BCB_CMD_FIELD_SET;
  

• if (!strcmp(cmd, "clear"))

• return BCB_CMD_FIELD_CLEAR;
  

• if (!strcmp(cmd, "test"))

• return BCB_CMD_FIELD_TEST;
  

• if (!strcmp(cmd, "store"))

• return BCB_CMD_STORE;
  

• if (!strcmp(cmd, "dump"))

• return BCB_CMD_FIELD_DUMP;
  

• else

• return -1;
  

-}

-static int bcb_is_misused(int argc, char *const argv[]) +static int bcb_not_loaded(void) {

• int cmd = bcb_cmd_get(argv[0]);
• switch (cmd) {
• case BCB_CMD_LOAD:

• if (argc != 3 && argc != 4)
  

• 	goto err;
  

• break;
  

• case BCB_CMD_FIELD_SET:

• if (argc != 3)
  

• 	goto err;
  

• break;
  

• case BCB_CMD_FIELD_TEST:

• if (argc != 4)
  

• 	goto err;
  

• break;
  

• case BCB_CMD_FIELD_CLEAR:

• if (argc != 1 && argc != 2)
  

• 	goto err;
  

• break;
  

• case BCB_CMD_STORE:

• if (argc != 1)
  

• 	goto err;
  

• break;
  

• case BCB_CMD_FIELD_DUMP:

• if (argc != 2)
  

• 	goto err;
  

• break;
  

• default:

• printf("Error: 'bcb %s' not supported\n", argv[0]);
  

• return -1;
  

• }
• if (cmd != BCB_CMD_LOAD && !block) {

• printf("Error: Please, load BCB first!\n");
  

• return -1;
  

• }
• return 0;

-err:

• printf("Error: Bad usage of 'bcb %s'\n", argv[0]);

• printf("Error: Please, load BCB first!\n"); return -1;

}

@@ -216,6 +149,9 @@ static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc, char *endp; char *iface = "mmc";

• if (argc < 3)

• return CMD_RET_USAGE;
  

• if (argc == 4) { iface = argv[1]; argc--;

@@ -270,6 +206,12 @@ static int __bcb_set(const char *fieldp, const char *valp) static int do_bcb_set(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {

• if (argc < 3)

• return CMD_RET_USAGE;
  

• if (!block)

• return bcb_not_loaded();
  

• return __bcb_set(argv[1], argv[2]);

}

@@ -279,6 +221,9 @@ static int do_bcb_clear(struct cmd_tbl *cmdtp, int flag, int argc, int size; char *field;

• if (!block)

• return bcb_not_loaded();
  

• if (argc == 1) { memset(&bcb, 0, sizeof(bcb)); return CMD_RET_SUCCESS;

@@ -297,7 +242,15 @@ static int do_bcb_test(struct cmd_tbl *cmdtp, int flag, int argc, { int size; char *field;

• char *op = argv[2];

• char *op;

• if (argc < 4)

• return CMD_RET_USAGE;
  

• if (!block)

• return bcb_not_loaded();
  

• op = argv[2];

  if (bcb_field_get(argv[1], &field, &size)) return CMD_RET_FAILURE;

@@ -325,6 +278,12 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc, int size; char *field;

• if (argc < 2)

• return CMD_RET_USAGE;
  

• if (!block)

• return bcb_not_loaded();
  

• if (bcb_field_get(argv[1], &field, &size)) return CMD_RET_FAILURE;

@@ -356,6 +315,9 @@ err: static int do_bcb_store(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) {

• if (!block)

• return bcb_not_loaded();
  

• return __bcb_store();

}

@@ -414,44 +376,7 @@ void bcb_reset(void) __bcb_reset(); }

-static struct cmd_tbl cmd_bcb_sub[] = {

• U_BOOT_CMD_MKENT(load, CONFIG_SYS_MAXARGS, 1, do_bcb_load, "", ""),
• U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 1, do_bcb_set, "", ""),
• U_BOOT_CMD_MKENT(clear, CONFIG_SYS_MAXARGS, 1, do_bcb_clear, "", ""),
• U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_bcb_test, "", ""),
• U_BOOT_CMD_MKENT(dump, CONFIG_SYS_MAXARGS, 1, do_bcb_dump, "", ""),
• U_BOOT_CMD_MKENT(store, CONFIG_SYS_MAXARGS, 1, do_bcb_store, "", ""),

-};

-static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) -{

• struct cmd_tbl *c;
• if (argc < 2)

• return CMD_RET_USAGE;
  

• argc--;
• argv++;
• c = find_cmd_tbl(argv[0], cmd_bcb_sub, ARRAY_SIZE(cmd_bcb_sub));
• if (!c)

• return CMD_RET_USAGE;
  

• if (bcb_is_misused(argc, argv)) {

• /*
  

•  * We try to improve the user experience by reporting the
  

•  * root-cause of misusage, so don't return CMD_RET_USAGE,
  

•  * since the latter prints out the full-blown help text
  

•  */
  

• return CMD_RET_FAILURE;
  

• }
• return c->cmd(cmdtp, flag, argc, argv);

-}

-U_BOOT_CMD(

• bcb, CONFIG_SYS_MAXARGS, 1, do_bcb,
• "Load/set/clear/test/dump/store Android BCB fields",

+U_BOOT_LONGHELP(bcb, "load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>\n" "load <dev> <part> - load BCB from mmc <dev>:<part>\n" "bcb set <field> <val> - set BCB <field> to <val>\n" @@ -472,3 +397,13 @@ U_BOOT_CMD( " NOTE: any ':' character in <val> will be replaced by line feed\n" " during 'bcb set' and used as separator by upper layers\n" );

+U_BOOT_CMD_WITH_SUBCMDS(bcb,

• "Load/set/clear/test/dump/store Android BCB fields", bcb_help_text,
• U_BOOT_SUBCMD_MKENT(load, 4, 1, do_bcb_load),
• U_BOOT_SUBCMD_MKENT(set, 3, 1, do_bcb_set),
• U_BOOT_SUBCMD_MKENT(clear, 2, 1, do_bcb_clear),
• U_BOOT_SUBCMD_MKENT(test, 4, 1, do_bcb_test),
• U_BOOT_SUBCMD_MKENT(dump, 2, 1, do_bcb_dump),
• U_BOOT_SUBCMD_MKENT(store, 1, 1, do_bcb_store),

+);

-- 2.43.0

Hi Dmitry,

Thank you for the patch.

On jeu., oct. 17, 2024 at 17:12, 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

Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

---

MAINTAINERS | 1 - cmd/Kconfig | 14 ------- cmd/Makefile | 1 - cmd/ab_select.c | 66 ------------------------------- cmd/bcb.c | 63 +++++++++++++++++++++++++++++ configs/am57xx_evm_defconfig | 1 - configs/am57xx_hs_evm_defconfig | 1 - configs/am57xx_hs_evm_usb_defconfig | 1 - configs/khadas-vim3_android_ab_defconfig | 1 - configs/khadas-vim3l_android_ab_defconfig | 1 - configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 1 - 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 ++-- 18 files changed, 81 insertions(+), 104 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS index 7c6c368285e729dee11bb116cd7dd30d369b1a87..e0c6a6b4b43774258ce1e20100e4a2f01cbeaab0 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 8c677b1e4864164d985914ea6ac4a65c6caee099..82b5852d47c1057ca9d5f13d078e428bb4a3b539 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1775,20 +1775,6 @@ config CMD_XXD

endmenu

-menu "Android support commands"

-config CMD_AB_SELECT

• bool "ab_select"
• depends on ANDROID_AB
• help

•  On Android devices with more than one boot slot (multiple copies of
  

•  the kernel and system images) this provides a command to select which
  

•  slot should be used to boot from and register the boot attempt. This
  

•  is used by the new A/B update model where one slot is updated in the
  

•  background while running from the other slot.
  

-endmenu

if NET || NET_LWIP

menuconfig CMD_NET diff --git a/cmd/Makefile b/cmd/Makefile index f30914053721d7e8ac17c828aef4d379cf3ecdb5..d1bb9d0e57d80855dd2da8af0cb510dbad65933e 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 7c178c728ca4c8b5bcba02a04eef2d6a7c86afb6..0000000000000000000000000000000000000000 --- 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 98b2a71087a27b1721f4bed4160095d65ec75402..2854408e5669b0d7d2a06073fa81158b21834b99 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> @@ -376,6 +377,48 @@ void bcb_reset(void) __bcb_reset(); }

+__maybe_unused static int do_bcb_ab_select(struct cmd_tbl *cmdtp,

• 			   int flag, int argc,
  

• 			   char * const argv[])
  

+{

• int ret;
• struct blk_desc *dev_desc;
• struct disk_partition part_info;
• char slot[2];
• bool dec_tries = true;
• if (argc < 4)

• return CMD_RET_USAGE;
  

• for (int i = 4; i < argc; i++) {

• if (strcmp(argv[i], "--no-dec") == 0)
  

• 	dec_tries = false;
  

• else
  

• 	return CMD_RET_USAGE;
  

• }
• /* Lookup the "misc" partition from argv[2] and argv[3] */
• if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3],

• 				 &dev_desc, &part_info,
  

• 				 false) < 0) {
  

• return CMD_RET_FAILURE;
  

• }
• ret = ab_select_slot(dev_desc, &part_info, dec_tries);
• if (ret < 0) {

• printf("Android boot failed, error %d.\n", ret);
  

• return CMD_RET_FAILURE;
  

• }
• /* Android standard slot names are 'a', 'b', ... */
• slot[0] = BOOT_SLOT_NAME(ret);
• slot[1] = '\0';
• env_set(argv[1], slot);
• printf("ANDROID: Booting slot: %s\n", slot);
• return CMD_RET_SUCCESS;

+}

U_BOOT_LONGHELP(bcb, "load <interface> <dev> <part> - load BCB from <interface> <dev>:<part>\n" "load <dev> <part> - load BCB from mmc <dev>:<part>\n" @@ -385,6 +428,23 @@ U_BOOT_LONGHELP(bcb, "bcb dump <field> - dump BCB <field>\n" "bcb store - store BCB back to <interface>\n" "\n" +#if IS_ENABLED(CONFIG_ANDROID_AB)

• "bcb ab_select -\n"
• " Select the slot used to boot from and register the boot attempt.\n"
• " <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"
• "\n"

+#endif "Legend:\n" "<interface> - storage device interface (virtio, mmc, etc)\n" "<dev> - storage device index containing the BCB partition\n" @@ -406,4 +466,7 @@ U_BOOT_CMD_WITH_SUBCMDS(bcb, U_BOOT_SUBCMD_MKENT(test, 4, 1, do_bcb_test), U_BOOT_SUBCMD_MKENT(dump, 2, 1, do_bcb_dump), U_BOOT_SUBCMD_MKENT(store, 1, 1, do_bcb_store), +#if IS_ENABLED(CONFIG_ANDROID_AB)

• U_BOOT_SUBCMD_MKENT(ab_select, 5, 1, do_bcb_ab_select),

+#endif ); diff --git a/configs/am57xx_evm_defconfig b/configs/am57xx_evm_defconfig index efc154eda043ff3ea08194288e33792ce48282f9..b793f00babe474ea3f15292fb4015a7120401238 100644 --- a/configs/am57xx_evm_defconfig +++ b/configs/am57xx_evm_defconfig @@ -48,7 +48,6 @@ CONFIG_CMD_SPL=y CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2 CONFIG_CMD_BCB=y # CONFIG_CMD_SETEXPR is not set -CONFIG_CMD_AB_SELECT=y CONFIG_BOOTP_DNS2=y # CONFIG_CMD_PMIC is not set CONFIG_CMD_AVB=y diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig index 0f8533e15dbd7c0186a513b27b46a0407b6f79f1..5cacd7f9cc53d338d52120186b16684add93fd21 100644 --- a/configs/am57xx_hs_evm_defconfig +++ b/configs/am57xx_hs_evm_defconfig @@ -44,7 +44,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/am57xx_hs_evm_usb_defconfig b/configs/am57xx_hs_evm_usb_defconfig index 81a938339d5934605cb7defa04ea92f76468b21a..2d8068ecdc79c01c1281ab3873fc892aa4c96be7 100644 --- a/configs/am57xx_hs_evm_usb_defconfig +++ b/configs/am57xx_hs_evm_usb_defconfig @@ -46,7 +46,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 510fe4f8928fe39a040a615636fa550b3e0dc5db..de5357c45cbfe4742d9491a29386850570acc235 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 d2da8ff2a69b209b8fb22a48be537bd4dd17a3bb..4d7b90f23002e464d7dc40516bcd3161b0f59439 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 1b3b8c6e788cd6845b61e62a06b730da28831edc..b5f80b8572ad32b2a92d4fe82c9068c453c11dfd 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -27,6 +27,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 @@ -46,6 +47,7 @@ CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_MEMTEST=y +CONFIG_CMD_BCB=y CONFIG_CMD_CLK=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index f596f1cc55a45ade4862122a6944d109c6c29238..d111858082d5a8ed3725c95462b2c81a406b57d2 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -103,7 +103,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 diff --git a/doc/android/ab.rst b/doc/android/ab.rst index 2adf88781d60b61d1b3c74efe976a684b590c813..7fd4aeb6a724b839de9be5e9a8843ade2ad3667e 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 b76e049f09cc014837981e914a3792c829f42cdc..fc89efb4c36e5216a1344bb2d758300eec3c8f44 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) && 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/khadas-vim3l_android.h b/include/configs/khadas-vim3l_android.h index 0ab8ffd372a4ae44804fca41c11ee930aaa72460..5b2aed1cf62af213f0ea6688fde6d07267aa4f55 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) && 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 fa520265800ccf081862bc97643b6a8e28b13327..77364bbf9cf0d1b68d413773902ea9d18b720928 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) && defined(CONFIG_ANDROID_AB) #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 26494ae980108ad84eb173a0deaa7b701a5309d4..26b6c1cd188c05371c6455cd6247f07a1773d885 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) && defined(CONFIG_ANDROID_AB) #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 c79cb07fda35dfeb608ac7345cd3f22744e2e491..0d7b7995a9fab6e3daad748721818b9e4cfac452 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

Hi Igor,

Thank you for the review.

On dim., nov. 03, 2024 at 10:43, Igor Opaniuk igor.opaniuk@gmail.com wrote:

Hi Dmitry,

On Thu, Oct 17, 2024 at 4:12 PM Dmitry Rokosov ddrokosov@salutedevices.com 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 Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com Reviewed-by: Simon Glass sjg@chromium.org Tested-by: Guillaume La Roque glaroque@baylibre.com Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.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 c93e51541019d0fe793303c4b3d5286df061906f..a287eac04fe88ad08bdcf1b1b1d6e564d503d800 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

Reviewed-by: Igor Opaniuk igor.opaniuk@gmail.com

Sorry, I can't apply that R-b tag because this series has been merged into master already:

https://lore.kernel.org/r/all/20241025175409.GB4959@bill-the-cat/

-- Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk

Hi Sam,

Thank you for the review.

On lun., nov. 04, 2024 at 17:06, Sam Protsenko semen.protsenko@linaro.org wrote:

On Thu, Oct 17, 2024 at 9:12 AM Dmitry Rokosov ddrokosov@salutedevices.com 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 Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com Reviewed-by: Simon Glass sjg@chromium.org Tested-by: Guillaume La Roque glaroque@baylibre.com Signed-off-by: Dmitry Rokosov ddrokosov@salutedevices.com

Would be nice to add "Fixes:" tag here, pointing to the corresponding commit where the issue was introduced (see kernel docs for details). It could be quite useful for possible stable branches and other purposes, I'd recommend to add that tag for all fixes if you have more in this series.

I agree. Unfortunately, this series has already been merged here:

https://lore.kernel.org/r/all/20241025175409.GB4959@bill-the-cat/

...

---

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 c93e51541019d0fe793303c4b3d5286df061906f..a287eac04fe88ad08bdcf1b1b1d6e564d503d800 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);
  

AFAIU, this changes the behavior of two above functions, and consequently of "bcb ab_select" command? If so, just to double check: were all users of those reworked correspondingly? I can see next occurrences (there may be more):

$ grep -sIrHn '"_' boot/bootmeth_android.c

I thought the same when first reviewing the patch. Looking a bit closer...

boot/bootmeth_android.c:74: sprintf(partname, BOOT_PART_NAME "_%s", priv->slot); boot/bootmeth_android.c:111: sprintf(partname, VENDOR_BOOT_PART_NAME "_%s", priv->slot); boot/bootmeth_android.c:156: sprintf(slot_suffix, "_%s", priv->slot); boot/bootmeth_android.c:397: sprintf(slot_suffix, "_%s", priv->slot);

... We can see that ab_select_slot() returns an integer That integer is used later on to initialize priv->slot:

""" priv->slot[0] = BOOT_SLOT_NAME(ret); priv->slot[1] = '\0'; """

The change from Dmitry only changes what we **write** to the BCB (into the misc partition), not what is returned by ab_select_slot().

ab_select_slot() still returns an integer which needs to be converted via the BOOT_SLOT_NAME() macro.

$ grep -sIrHn 'slot_suffix _' include/configs/ include/configs/ti_omap5_common.h:107: "setenv slot_suffix _${slot_name};" include/configs/meson64_android.h:65: "setenv slot_suffix _${current_slot}; " \

Same goes for these 2 examples, we use: The "bcb ab_select current_slot" command to store the slot into the "current_slot" environment variable. Looking at cmd/bcb.c we can see:

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

So I think this is fine.

...

            if (memcmp(abc->slot_suffix, slot_suffix,
                       sizeof(slot_suffix))) {
                    memcpy(abc->slot_suffix, slot_suffix,

-- 2.43.0

Hi Sam,

On mar., nov. 05, 2024 at 18:58, Sam Protsenko semen.protsenko@linaro.org wrote:

On Tue, Nov 5, 2024 at 9:06 AM Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

...

Hi Sam,

Hey Mattijs,

[snip]

...

...

...

@@ -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);
  

AFAIU, this changes the behavior of two above functions, and consequently of "bcb ab_select" command? If so, just to double check: were all users of those reworked correspondingly? I can see next occurrences (there may be more):

$ grep -sIrHn '"_' boot/bootmeth_android.c

I thought the same when first reviewing the patch. Looking a bit closer...

...

boot/bootmeth_android.c:74: sprintf(partname, BOOT_PART_NAME "_%s", priv->slot); boot/bootmeth_android.c:111: sprintf(partname, VENDOR_BOOT_PART_NAME "_%s", priv->slot); boot/bootmeth_android.c:156: sprintf(slot_suffix, "_%s", priv->slot); boot/bootmeth_android.c:397: sprintf(slot_suffix, "_%s", priv->slot);

... We can see that ab_select_slot() returns an integer That integer is used later on to initialize priv->slot:

""" priv->slot[0] = BOOT_SLOT_NAME(ret); priv->slot[1] = '\0'; """

The change from Dmitry only changes what we **write** to the BCB (into the misc partition), not what is returned by ab_select_slot().

Sure. Just wanted to double check that the behavior is not changed in any related parts, as the commit message doesn't mention that. Btw, BCB is an interface between the bootloader and AOSP, so if this patch changes what's being written into BCB, does it affect AOSP part of it somehow? Especially for already existing devices with particular BCB data, in case U-Boot gets updated there.

Those are valid concerns.

Per my understanding, on recent Android versions the slot suffix is not read from BCB, but from the ro.boot.slot_suffix property:

""" // Initialize the current_slot from the read-only property. If the property // was not set (from either the command line or the device tree), we can later // initialize it from the bootloader_control struct. std::string suffix_prop = android::base::GetProperty("ro.boot.slot_suffix", ""); if (suffix_prop.empty()) { LOG(ERROR) << "Slot suffix property is not set"; return false; } current_slot_ = SlotSuffixToIndex(suffix_prop.c_str()); """

See: https://cs.android.com/android/platform/superproject/main/+/main:hardware/in...

This has been the case since 2019.

If we look at earlier implementations of libboot_control (which was in bootable/recovery) https://android-review.googlesource.com/c/platform/bootable/recovery/+/11915...

So implementations before 2019 that do not have this patch: https://android-review.googlesource.com/c/platform/bootable/recovery/+/11118...

Will get the slot suffix from the BCB (not from the commandline)

For these older implementations, we will go through the following: BootControl::Init() LoadBootloaderControl(device.c_str(), &boot_ctrl) android::base::ReadFully(fd.get(), buffer, sizeof(bootloader_control)

And struct bootloader_control has:

struct bootloader_control { // NUL terminated active slot suffix. char slot_suffix[4];

And if we look at how the BCB is initialized from userspace in: https://cs.android.com/android/platform/superproject/main/+/main:hardware/in...

We can see that we copy _a, not a (for example, if slot == 0).

So I think this is fine.

If needed, I can dig more for behaviour on older devices (<2019), let me know!

...

ab_select_slot() still returns an integer which needs to be converted via the BOOT_SLOT_NAME() macro.

...

$ grep -sIrHn 'slot_suffix _' include/configs/ include/configs/ti_omap5_common.h:107: "setenv slot_suffix _${slot_name};" include/configs/meson64_android.h:65: "setenv slot_suffix _${current_slot}; " \

Same goes for these 2 examples, we use: The "bcb ab_select current_slot" command to store the slot into the "current_slot" environment variable. Looking at cmd/bcb.c we can see:

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

"""

So I think this is fine.

[snip]

Hi Sam,

On mer., nov. 06, 2024 at 21:17, Sam Protsenko semen.protsenko@linaro.org wrote:

On Wed, Nov 6, 2024 at 4:02 AM Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

...

Hi Sam,

On mar., nov. 05, 2024 at 18:58, Sam Protsenko semen.protsenko@linaro.org wrote:

...

On Tue, Nov 5, 2024 at 9:06 AM Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

...

Hi Sam,

Hey Mattijs,

[snip]

...

...

...

@@ -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);
  

AFAIU, this changes the behavior of two above functions, and consequently of "bcb ab_select" command? If so, just to double check: were all users of those reworked correspondingly? I can see next occurrences (there may be more):

$ grep -sIrHn '"_' boot/bootmeth_android.c

I thought the same when first reviewing the patch. Looking a bit closer...

...

boot/bootmeth_android.c:74: sprintf(partname, BOOT_PART_NAME "_%s", priv->slot); boot/bootmeth_android.c:111: sprintf(partname, VENDOR_BOOT_PART_NAME "_%s", priv->slot); boot/bootmeth_android.c:156: sprintf(slot_suffix, "_%s", priv->slot); boot/bootmeth_android.c:397: sprintf(slot_suffix, "_%s", priv->slot);

... We can see that ab_select_slot() returns an integer That integer is used later on to initialize priv->slot:

""" priv->slot[0] = BOOT_SLOT_NAME(ret); priv->slot[1] = '\0'; """

The change from Dmitry only changes what we **write** to the BCB (into the misc partition), not what is returned by ab_select_slot().

Sure. Just wanted to double check that the behavior is not changed in any related parts, as the commit message doesn't mention that. Btw, BCB is an interface between the bootloader and AOSP, so if this patch changes what's being written into BCB, does it affect AOSP part of it somehow? Especially for already existing devices with particular BCB data, in case U-Boot gets updated there.

Those are valid concerns.

Per my understanding, on recent Android versions the slot suffix is not read from BCB, but from the ro.boot.slot_suffix property:

That probably still leaves the possible combination of some devices running new U-Boot versions (with this patch applied) together with older Android versions. E.g. in case when U-Boot is updated but Android isn't, may be especially relevant for some dev boards out there.

Yes, you are right. Boards with Android older than 2019 and U-Boot v2025.01+

My experience is that most of the time, it's the bootloader that stays out of date and the Android that gets updated. But that might not be the general case.

...

""" // Initialize the current_slot from the read-only property. If the property // was not set (from either the command line or the device tree), we can later // initialize it from the bootloader_control struct.

So even in recent Android versions it's being initialized from BCB in case the property is not set.

Nope, the comment is wrong. If look at the lines below you can see that the function exits early when the property is not set.

So in recent Android versions if the propery is not set, we just fail to initialize.

...

std::string suffix_prop = android::base::GetProperty("ro.boot.slot_suffix", ""); if (suffix_prop.empty()) { LOG(ERROR) << "Slot suffix property is not set"; return false; } current_slot_ = SlotSuffixToIndex(suffix_prop.c_str()); """

See: https://cs.android.com/android/platform/superproject/main/+/main:hardware/in...

This has been the case since 2019.

If we look at earlier implementations of libboot_control (which was in bootable/recovery) https://android-review.googlesource.com/c/platform/bootable/recovery/+/11915...

So implementations before 2019 that do not have this patch: https://android-review.googlesource.com/c/platform/bootable/recovery/+/11118...

Will get the slot suffix from the BCB (not from the commandline)

For these older implementations, we will go through the following: BootControl::Init() LoadBootloaderControl(device.c_str(), &boot_ctrl) android::base::ReadFully(fd.get(), buffer, sizeof(bootloader_control)

And struct bootloader_control has:

struct bootloader_control { // NUL terminated active slot suffix. char slot_suffix[4];

And if we look at how the BCB is initialized from userspace in: https://cs.android.com/android/platform/superproject/main/+/main:hardware/in...

We can see that we copy _a, not a (for example, if slot == 0).

So I think this is fine.

If needed, I can dig more for behaviour on older devices (<2019), let me know!

My point is, if it's possible to introduce this change but keep the same old behavior (across both U-Boot and AOSP), it's usually better to do that that way. If I'm not missing anything and that's a valid concern, maybe a separate patch can be developed on top of the merged patch series handling that. Anyways, I don't have enough time to work on this right now, so just pointing out what I noticed, if it's useful for anybody. I'll let the maintainers decide :)

Yes, thanks a lot for noticing and taking the time to report your valid concerns :)

Thanks!

...

...

...

ab_select_slot() still returns an integer which needs to be converted via the BOOT_SLOT_NAME() macro.

...

$ grep -sIrHn 'slot_suffix _' include/configs/ include/configs/ti_omap5_common.h:107: "setenv slot_suffix _${slot_name};" include/configs/meson64_android.h:65: "setenv slot_suffix _${current_slot}; " \

Same goes for these 2 examples, we use: The "bcb ab_select current_slot" command to store the slot into the "current_slot" environment variable. Looking at cmd/bcb.c we can see:

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

"""

So I think this is fine.

[snip]