Hello Mattijs,
On Sat, Oct 12, 2024 at 10:49:08AM +0200, Mattijs Korpershoek wrote:
Hi Dmitry,
On ven., oct. 11, 2024 at 21:00, Dmitry Rokosov ddrokosov@salutedevices.com wrote:
On Fri, Oct 11, 2024 at 04:20:39PM +0200, Mattijs Korpershoek wrote:
On ven., oct. 11, 2024 at 15:30, "Mattijs Korpershoek via groups.io" mkorpershoek=baylibre.com@groups.io wrote:
Hi Dmitry,
Thank you for the patch.
On mar., oct. 08, 2024 at 23:18, 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 | 63 +++++++++++++++++++++++++++++ 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, 85 insertions(+), 104 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 7aefda93d017f07d616f0f6d191129914fbeb484..668ccec9ae6df47192b1af668e3fdbeb1dfa15ea 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 dd33266cec70a2b134b7244acae1b7f098b921e8..11e8d363dc9b137723a86a240412d82dd0dbccc5 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1067,6 +1067,7 @@ config CMD_ADC config CMD_BCB bool "bcb" depends on PARTITIONS
- depends on ANDROID_AB
When building with khadas-vim3_android_defconfig, we can see that CMD_BCB is no longer part of that build:
$ grep CMD_BCB .config
<empty>
However, if we look at include/configs/meson64_android.h, we can see that the "bcb" command is not only used for checking the _slot suffix.
It's also used for checking the bootloader reason. For example, in BOOTENV_DEV_FASTBOOT, we call:
"if bcb test command = bootonce-bootloader; then " \
Since CMD_BCB is no longer part of the .config (due to this dependency), the boot script now shows errors:
""" U-Boot 2024.10-00796-g969325278805 (Oct 11 2024 - 14:46:00 +0200) khadas-vim3
Model: Khadas VIM3 SoC: Amlogic Meson G12B (A311D) Revision 29:b (10:2) DRAM: 2 GiB (effective 3.8 GiB) Core: 411 devices, 36 uclasses, devicetree: separate MMC: mmc@ffe03000: 0, mmc@ffe05000: 1, mmc@ffe07000: 2 Loading Environment from MMC... fs uses incompatible features: 00020000, ignoring Reading from MMC(2)... *** Warning - bad CRC, using default environment
In: usbkbd,serial Out: vidconsole,serial Err: vidconsole,serial Net: eth0: ethernet@ff3f0000
Hit any key to stop autoboot: 0 Verify GPT: success! Unknown command 'bcb' - try 'help' Warning: BCB is corrupted or does not exist dev: pinctrl@14 dev: pinctrl@40 gpio: pin 88 (gpio 88) value is 1 Unknown command 'bcb' - try 'help' Warning: BCB is corrupted or does not exist Loading Android boot partition... switch to partitions #0, OK mmc2(part 0) is current device """
I know we should not be using a boot script, nor non A/B configs but it's a bummer that this series breaks an upstream defconfig (khadas-vim3_android_defconfig)
My recommendation:
Make BCB_CMD_AB_SELECT implementation dependant on ANDROID_AB. This way, users can use CMD_BCB with and without ANDROID_AB being enabled.
We could do: When ANDROID_AB=y, implement bcb ab_select subcommand When ANDROID_AB=n, command is not accessible.
I'll send you a diff shortly for this.
Here is an illustration on how that would work:
diff --git a/cmd/Kconfig b/cmd/Kconfig index 861c31e26408..e1a4a97b042d 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1055,7 +1055,6 @@ 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:
diff --git a/cmd/bcb.c b/cmd/bcb.c index 4fd32186ae65..4fe634f14cc5 100644 --- a/cmd/bcb.c +++ b/cmd/bcb.c @@ -438,6 +438,9 @@ static int do_bcb_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, char slot[2]; bool dec_tries = true;
- if (!CONFIG_IS_ENABLED(AB_SELECT))
return CMD_RET_SUCCESS;- for (int i = 4; i < argc; i++) { if (!strcmp(argv[i], "--no-dec")) dec_tries = false;
@@ -474,6 +477,9 @@ static int do_bcb_ab_dump(struct cmd_tbl *cmdtp, int flag, int argc, struct blk_desc *dev_desc; struct disk_partition part_info;
- if (!CONFIG_IS_ENABLED(AB_SELECT))
return CMD_RET_SUCCESS;- if (part_get_info_by_dev_and_name_or_num(argv[1], argv[2], &dev_desc, &part_info, false) < 0) {
We also need to include an #ifdef directive for the ab_select_slot() function usage; otherwise, the code will not compile successfully.
Are you sure? Per my understanding, it's possible that the compiler optimizes this out because CONFIG_IS_ENABLED(AB_SELECT) is known as build time.
When I tried this diff with khadas-vim3_android_defconfig I did not see any build errors. I will try again early next week.
As I recall, the IS_ENABLED() mechanism serves as a runtime checker to determine whether specific CONFIG_* options are enabled. Consequently, all code paths under this mechanism are always compiled. I attempted to disable CONFIG_ANDROID_AB for the sandbox_defconfig, but it resulted in the expected linker error.
/tmp/ccAvYrKL.ltrans25.ltrans.o: In function `do_bcb_ab_select': <artificial>:(.text+0x6d5d): undefined reference to `ab_select_slot' collect2: error: ld returned 1 exit status Makefile:1813: recipe for target 'u-boot' failed make: *** [u-boot] Error 1
I have already prepared a new version using #ifdef directives. I will send it shortly.
[...]