Hi Alex, On 2 April 2017 at 02:49, Alex Deymo <de...@google.com> wrote: > An Android Bootloader must comply with certain boot modes and change > the kernel command line accordingly. This patch introduces the Android > boot mode concept which determines whether the device should boot to > one of the following: > * recovery: which should boot to the recovery image, > * bootloader: which should boot to the "bootloader" (fastboot) and > * normal: which should boot to the system image. > > The boot mode is determined in part by the Boot Control Block (BCB) > which is stored at the beginning of the "misc" partition. The BCB > is defined in the "bootloader_message.h" file in AOSP, now copied > here as android_bootloader_message.h with minor modifications. > > This patch implements the basic boot flow that loads and boots an > Android kernel image assuming an A/B device which implies that it uses > boot as recovery (BOARD_USES_RECOVERY_AS_BOOT in the BoardConfig.mk). > This means that the recovery image shares the same kernel with the > normal boot system image, but stores the recovery image as a ramdisk > which is not used in normal mode. > > Among the limitations, this patch doesn't implement the A/B slot > selection, it only boots from the provided slot. > > Test: Booted a rpi3 with this flow.
Could we have a sandbox test for this? Set test_vboot.py for an example. > Signed-off-by: Alex Deymo <de...@google.com> > --- > README | 19 +- > common/Kconfig | 19 ++ > common/Makefile | 1 + > common/android_bootloader.c | 350 > +++++++++++++++++++++++++++++++++++ > include/android_bootloader.h | 48 +++++ > include/android_bootloader_message.h | 174 +++++++++++++++++ > 6 files changed, 607 insertions(+), 4 deletions(-) > create mode 100644 common/android_bootloader.c > create mode 100644 include/android_bootloader.h > create mode 100644 include/android_bootloader_messa.ge.h > This looks pretty clean so most of my comments are style-related > diff --git a/README b/README > index aa907ced8a..384cc6aabb 100644 > --- a/README > +++ b/README > @@ -1483,6 +1483,21 @@ The following options need to be configured: > entering dfuMANIFEST state. Host waits this timeout, before > sending again an USB request to the device. > > +- Android Bootloader support: > + CONFIG_ANDROID_BOOTLOADER This name seems slightly off to me. Isn't this adding Android boot support to and existing bootloader? Perhaps CONFIG_ANDROID_BOOT? Also I'm not quite sure what is different between this and CONFIG_ANDROID_BOOT_IMAGE. Does this new boot method have a name by itself? (assuming it is a boot method). > + This enables support for the Android bootloader flow. Android > + devices can boot in normal mode, recovery mode or bootloader > + mode. The normal mode is the most common boot mode, but > + recovery mode is often used to perform factory reset and OTA > + (over-the-air) updates in the legacy updater. Also it is > + possible for an Android system to request a reboot to the > + "bootloader", which often means reboot to fastboot but may > also > + include a UI with a menu. > + > + CONFIG_ANDROID_BOOT_IMAGE > + This enables support for booting images which use the Android > + image format header. I think it is enough to put this documentation in Kconfig. It might be useful for have a separate README for the boot flow description. > + > - USB Device Android Fastboot support: > CONFIG_USB_FUNCTION_FASTBOOT > This enables the USB part of the fastboot gadget > @@ -1494,10 +1509,6 @@ The following options need to be configured: > used on Android devices. > See doc/README.android-fastboot for more information. > > - CONFIG_ANDROID_BOOT_IMAGE > - This enables support for booting images which use the Android > - image format header. > - > CONFIG_FASTBOOT_BUF_ADDR > The fastboot protocol requires a large memory buffer for > downloads. Define this to the starting RAM address to use for > diff --git a/common/Kconfig b/common/Kconfig > index 8f73c8f757..47e2ffa3d6 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -393,6 +393,25 @@ config DISPLAY_BOARDINFO > when U-Boot starts up. The board function checkboard() is called > to do this. > > +config ANDROID_BOOTLOADER > + bool "Support for Android Bootloader boot flow" > + default n > + depends on ANDROID_BOOT_IMAGE > + help > + If enabled, adds support to boot an Android device following the > + Android Bootloader boot flow. This flow requires an Android > Bootloader > + to handle the Android Bootloader Message stored in the Boot Control > + Block (BCB), normally in the "misc" partition of an Android device. > + The BCB is used to determine the boot mode of the device (normal > mode, > + recovery mode or bootloader mode) and, if enabled, the slot to boot > + from in devices with multiple boot slots (A/B devices). > + > +config ANDROID_BOOT_IMAGE > + bool "Enable support for Android Boot Images" > + help > + This enables support for booting images which use the Android > + image format header. > + > menu "Start-up hooks" > > config ARCH_EARLY_INIT_R > diff --git a/common/Makefile b/common/Makefile > index 86225f1564..9de0a77063 100644 > --- a/common/Makefile > +++ b/common/Makefile > @@ -147,6 +147,7 @@ endif > obj-$(CONFIG_CMD_IDE) += ide.o > obj-y += image.o > obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o > +obj-$(CONFIG_ANDROID_BOOTLOADER) += android_bootloader.o > obj-$(CONFIG_$(SPL_)OF_LIBFDT) += image-fdt.o > obj-$(CONFIG_$(SPL_)FIT) += image-fit.o > obj-$(CONFIG_$(SPL_)FIT_SIGNATURE) += image-sig.o > diff --git a/common/android_bootloader.c b/common/android_bootloader.c > new file mode 100644 > index 0000000000..6656311ee3 > --- /dev/null > +++ b/common/android_bootloader.c > @@ -0,0 +1,350 @@ > +/* > + * Copyright (C) 2016 The Android Open Source Project > + * > + * SPDX-License-Identifier: BSD-2-Clause > + */ > + > +#include <android_bootloader.h> > +#include <android_bootloader_message.h> > + > +#include <cli.h> > +#include <common.h> > +#include <malloc.h> common.h always goes first. See http://www.denx.de/wiki/U-Boot/CodingStyle under 'Include file order" > + > +#define ANDROID_PARTITION_BOOT "boot" > +#define ANDROID_PARTITION_SYSTEM "system" > + > +#define ANDROID_ARG_SLOT_SUFFIX "androidboot.slot_suffix=" > +#define ANDROID_ARG_ROOT "root=" > + > +static int android_bootloader_message_load( > + struct blk_desc *dev_desc, Better to use struct udevice > + const disk_partition_t *part_info, > + struct android_bootloader_message *message) The android_bootloader prefix is very long. Is there a sensible abbreviation that could be used? > +{ > + ulong message_blocks = sizeof(struct android_bootloader_message) / > + part_info->blksz; > + if (message_blocks > part_info->size) { > + printf("misc partition too small.\n"); > + return -1; Please return real error numbers, like -ENOSPC, etc. > + } > + > + if (blk_dread(dev_desc, part_info->start, message_blocks, message) != > + message_blocks) { > + printf("Could not read from misc partition\n"); > + return -1; > + } > + debug("ANDROID: Loaded BCB, %lu blocks.\n", message_blocks); blank line before last return in a function > + return 0; > +} > + > +static int android_bootloader_message_write( > + struct blk_desc *dev_desc, > + const disk_partition_t *part_info, > + struct android_bootloader_message *message) > +{ > + ulong message_blocks = sizeof(struct android_bootloader_message) / > + part_info->blksz; > + if (message_blocks > part_info->size) { > + printf("misc partition too small.\n"); > + return -1; > + } > + > + if (blk_dwrite(dev_desc, part_info->start, message_blocks, message) != > + message_blocks) { > + printf("Could not write to misc partition\n"); > + return -1; > + } > + debug("ANDROID: Wrote new BCB, %lu blocks.\n", message_blocks); > + return 0; > +} > + > +static enum android_boot_mode android_bootloader_load_and_clear_mode( > + struct blk_desc *dev_desc, > + const disk_partition_t *misc_part_info) > +{ > + struct android_bootloader_message bcb; > + > +#ifdef CONFIG_FASTBOOT > + char *bootloader_str; > + > + /* Check for message from bootloader stored in RAM from a previous > boot. > + */ /* * multi-line comment starts here * ... */ > + bootloader_str = (char *)CONFIG_FASTBOOT_BUF_ADDR; > + if (!strcmp("reboot-bootloader", bootloader_str)) { > + bootloader_str[0] = '\0'; > + return ANDROID_BOOT_MODE_BOOTLOADER; > + } > +#endif > + > + /* Check and update the BCB message if needed. */ > + if (android_bootloader_message_load(dev_desc, misc_part_info, &bcb) < > + 0) { > + printf("WARNING: Unable to load the BCB.\n"); > + return ANDROID_BOOT_MODE_NORMAL; > + } > + > + if (!strcmp("bootonce-bootloader", bcb.command)) { > + /* Erase the message in the BCB since this value should be > used > + * only once. > + */ > + memset(bcb.command, 0, sizeof(bcb.command)); > + android_bootloader_message_write(dev_desc, misc_part_info, > + &bcb); > + return ANDROID_BOOT_MODE_BOOTLOADER; > + } > + > + if (!strcmp("boot-recovery", bcb.command)) > + return ANDROID_BOOT_MODE_RECOVERY; > + > + return ANDROID_BOOT_MODE_NORMAL; > +} > + > +/** > + * Return the reboot reason string for the passed boot mode. > + * > + * @param mode The Android Boot mode. > + * @return a pointer to the reboot reason string for mode. > + */ > +static const char *android_boot_mode_str(enum android_boot_mode mode) > +{ > + switch (mode) { > + case ANDROID_BOOT_MODE_NORMAL: > + return "(none)"; > + case ANDROID_BOOT_MODE_RECOVERY: > + return "recovery"; > + case ANDROID_BOOT_MODE_BOOTLOADER: > + return "bootloader"; > + } > + return NULL; > +} > + This could use a function comment. /* * android_part_get_info_by_name_suffix() - short description * * @dev_desc: blah blah * ... * @return parition number found (> 0) or -ve error number */ > +static int android_part_get_info_by_name_suffix(struct blk_desc *dev_desc, > + const char *base_name, > + const char *slot_suffix, > + disk_partition_t *part_info) > +{ > + char *part_name; > + int part_num; > + size_t part_name_len; > + > + part_name_len = strlen(base_name) + 1; > + if (slot_suffix) > + part_name_len += strlen(slot_suffix); > + part_name = malloc(part_name_len); > + if (!part_name) > + return -1; > + strcpy(part_name, base_name); > + if (slot_suffix) > + strcat(part_name, slot_suffix); > + > + part_num = part_get_info_by_name(dev_desc, part_name, part_info); > + if (part_num < 0) { > + debug("ANDROID: Could not find partition \"%s\"\n", > part_name); > + part_num = -1; > + } > + > + free(part_name); > + return part_num; > +} > + > +static int android_bootloader_boot_bootloader(void) > +{ > + const char *fastboot_cmd = getenv("fastbootcmd"); > + > + if (fastboot_cmd) > + return run_command(fastboot_cmd, CMD_FLAG_ENV); > + return -1; > +} > + > +static int android_bootloader_boot_kernel(unsigned long kernel_address) > +{ > + char kernel_addr_str[12]; > + char *fdt_addr = getenv("fdt_addr"); > + char *bootm_args[] = { "bootm", kernel_addr_str, "-", fdt_addr, NULL > }; > + > + sprintf(kernel_addr_str, "0x%lx", kernel_address); > + > + printf("Booting kernel at %s with fdt at %s...\n\n\n", > + kernel_addr_str, fdt_addr); > + do_bootm(NULL, 0, 4, bootm_args); > + > + return -1; > +} > + > +static char *strjoin(const char **chunks, char separator) > +{ > + int len, joined_len = 0; > + char *ret, *current; > + const char **p; > + > + for (p = chunks; *p; p++) > + joined_len += strlen(*p) + 1; > + > + if (!joined_len) { > + ret = mallo=c(1); calloc() would avoid the need for the next two lines if you like. > + if (ret) > + ret[0] = '\0'; > + return ret; > + } > + > + ret = malloc(joined_len); > + current = ret; > + if (!ret) > + return ret; > + > + for (p = chunks; *p; p++) { > + len = strlen(*p); > + memcpy(current, *p, len); > + current += len; > + *current = separator; > + current++; > + } > + /* Replace the last separator by a \0. */ Avoid . at end of comment > + current[-1] = '\0'; > + return ret; > +} > + > +/** android_assemble_cmdline - Assemble the command line to pass to the > kernel Please add args > + * @return a newly allocated string > + */ > +static char *android_assemble_cmdline(const char *slot_suffix, > + const char *extra_args) > +{ > + const char *cmdline_chunks[16]; > + const char **current_chunk = cmdline_chunks; > + char *env_cmdline, *cmdline, *rootdev_input; > + char *allocated_suffix = NULL; > + char *allocated_rootdev = NULL; > + unsigned long rootdev_len; > + > + env_cmdline = getenv("bootargs"); > + if (env_cmdline) > + *(current_chunk++) = env_cmdline; > + > + /* The |slot_suffix| needs to be passed to the kernel to know what > + * slot to boot from. > + */ > + if (slot_suffix) { > + allocated_suffix = malloc(strlen(ANDROID_ARG_SLOT_SUFFIX) + > + strlen(slot_suffix)); > + strcpy(allocated_suffix, ANDROID_ARG_SLOT_SUFFIX); > + strcat(allocated_suffix, slot_suffix); > + *(current_chunk++) = allocated_suffix; > + } > + > + rootdev_input = getenv("android_rootdev"); > + if (rootdev_input) { > + rootdev_len = strlen(ANDROID_ARG_ROOT) + CONFIG_SYS_CBSIZE + > 1; > + allocated_rootdev = malloc(rootdev_len); > + strcpy(allocated_rootdev, ANDROID_ARG_ROOT); > + cli_simple_process_macros(rootdev_input, > + allocated_rootdev + > + strlen(ANDROID_ARG_ROOT)); > + /* Make sure that the string is null-terminated since the > + * previous could not copy to the end of the input string if > it > + * is too big. > + */ > + allocated_rootdev[rootdev_len - 1] = '\0'; > + *(current_chunk++) = allocated_rootdev; > + } > + > + if (extra_args) > + *(current_chunk++) = extra_args; > + > + *(current_chunk++) = NULL; > + cmdline = strjoin(cmdline_chunks, ' '); > + free(allocated_suffix); > + free(allocated_rootdev); > + return cmdline; > +} > + > +int android_bootloader_boot_flow(struct blk_desc *dev_desc, > + const disk_partition_t *misc_part_info, > + const char *slot, > + unsigned long kernel_address) > +{ > + enum android_boot_mode mode; > + disk_partition_t boot_part_info; > + disk_partition_t system_part_info; > + int boot_part_num, system_part_num; > + int ret; > + char *command_line; > + char slot_suffix[3]; > + const char *mode_cmdline = NULL; > + > + /* Determine the boot mode and clear its value for the next boot if > + * needed. > + */ > + mode = android_bootloader_load_and_clear_mode(dev_desc, > misc_part_info); > + printf("ANDROID: reboot reason: \"%s\"\n", > android_boot_mode_str(mode)); > + > + switch (mode) { > + case ANDROID_BOOT_MODE_NORMAL: > + /* In normal mode, we load the kernel from "boot" but append > + * "skip_initramfs" to the cmdline to make it ignore the > + * recovery initramfs in the boot partition. > + */ > + mode_cmdline = "skip_initramfs"; > + break; > + case ANDROID_BOOT_MODE_RECOVERY: > + /* In recovery mode we still boot the kernel from "boot" but > + * don't skip the initramfs so it boots to recovery. > + */ > + break; > + case ANDROID_BOOT_MODE_BOOTLOADER: > + /* Bootloader mode enters fastboot. If this operation fails we > + * simply return since we can't recover from this situation by > + * switching to another slot. > + */ > + return android_bootloader_boot_bootloader(); > + } > + > + slot_suffix[0] = '\0'; > + if (slot && slot[0]) { > + slot_suffix[0] = '_'; > + slot_suffix[1] = slot[0]; > + slot_suffix[2] = '\0'; > + } > + > + /* Load the kernel from the desired "boot" partition. */ > + boot_part_num = > + android_part_get_info_by_name_suffix(dev_desc, > + ANDROID_PARTITION_BOOT, > + slot_suffix, > &boot_part_info); > + if (boot_part_num < 0) > + return -1; > + debug("ANDROID: Loading kernel from \"%s\", partition %d.\n", > + boot_part_info.name, boot_part_num); > + > + system_part_num = > + android_part_get_info_by_name_suffix(dev_desc, > + ANDROID_PARTITION_SYSTEM, > + slot_suffix, > + &system_part_info); > + if (system_part_num < 0) > + return -1; > + debug("ANDROID: Using system image from \"%s\", partition %d.\n", > + system_part_info.name, system_part_num); > + > + ret = android_image_load(dev_desc, &boot_part_info, kernel_address, > + -1UL); > + if (ret < 0) > + return ret; > + > + /* Set Android root variables. */ > + setenv_ulong("android_root_devnum", dev_desc->devnum); > + setenv_ulong("android_root_partnum", system_part_num); > + setenv("android_slotsufix", slot_suffix); > + > + /* Assemble the command line */ > + command_line = android_assemble_cmdline(slot_suffix, mode_cmdline); > + setenv("bootargs", command_line); > + > + debug("ANDROID: bootargs: \"%s\"\n", command_line); > + > + android_bootloader_boot_kernel(kernel_address); > + > + /* TODO: If the kernel doesn't boot mark the selected slot as bad. */ > + return -1; > +} > diff --git a/include/android_bootloader.h b/include/android_bootloader.h > new file mode 100644 > index 0000000000..ddf6d76f64 > --- /dev/null > +++ b/include/android_bootloader.h > @@ -0,0 +1,48 @@ > +/* > + * Copyright (C) 2016 The Android Open Source Project > + * > + * SPDX-License-Identifier: BSD-2-Clause > + */ > + > +#ifndef __ANDROID_BOOTLOADER_H > +#define __ANDROID_BOOTLOADER_H > + > +#include <common.h> You should be able to drop this if you have common.h first in your includes in .c files. > + > +enum android_boot_mode { > + ANDROID_BOOT_MODE_NORMAL = 0, > + > + /* "recovery" mode is triggered by the "reboot recovery" command or > + * equivalent adb/fastboot command. It can also be triggered by > writing > + * "boot-recovery" in the BCB message. This mode should boot the > + * recovery kernel. > + */ > + ANDROID_BOOT_MODE_RECOVERY, > + > + /* "bootloader" mode is triggered by the "reboot bootloader" command > or > + * equivalent adb/fastboot command. It can also be triggered by > writing > + * "bootonce-bootloader" in the BCB message. This mode should boot > into > + * fastboot. > + */ > + ANDROID_BOOT_MODE_BOOTLOADER, > +}; > + > +/** android_bootloader_boot_flow - Execute the Android Bootloader Flow. > + * Performs the Android Bootloader boot flow, loading the appropriate Android > + * image (normal kernel, recovery kernel or "bootloader" mode) and booting > it. > + * The boot mode is determined by the contents of the Android Bootloader > + * Message. On success it doesn't return. > + * > + * @dev_desc: device where to load the kernel and system to boot > from. > + * @misc_part_info: the "misc" partition descriptor in 'dev_desc'. > + * @slot: the boot slot to boot from. > + * @kernel_address: address where to load the kernel if needed. > + * > + * @return a negative number in case of error, otherwise it doesn't return. > + */ > +int android_bootloader_boot_flow(struct blk_desc *dev_desc, Particularly for an exported function like this, struct udevice * is better. > + const disk_partition_t *misc_part_info, > + const char *slot, > + unsigned long kernel_address); > + > +#endif /* __ANDROID_BOOTLOADER_H */ > diff --git a/include/android_bootloader_message.h > b/include/android_bootloader_message.h > new file mode 100644 > index 0000000000..2c2142dc6f > --- /dev/null > +++ b/include/android_bootloader_message.h > @@ -0,0 +1,174 @@ > +/* > + * This is from the Android Project, > + * Repository: https://android.googlesource.com/platform/bootable/recovery/ > + * File: bootloader_message/include/bootloader_message/bootloader_message.h > + * Commit: 8b309f6970ab3b7c53cc529c51a2cb44e1c7a7e1 > + * > + * Copyright (C) 2008 The Android Open Source Project > + * > + * SPDX-License-Identifier: BSD-2-Clause > + */ > + > +#ifndef __ANDROID_BOOTLOADER_MESSAGE_H > +#define __ANDROID_BOOTLOADER_MESSAGE_H > + > +/* compiler.h defines the types that otherwise are included from stdint.h and > + * stddef.h > + */ > +#include <compiler.h> > + > +/* Spaces used by misc partition are as below: > + * 0 - 2K Bootloader Message > + * 2K - 16K Used by Vendor's bootloader (the 2K - 4K range may be > optionally used > + * as bootloader_message_ab struct) > + * 16K - 64K Used by uncrypt and recovery to store wipe_package for A/B > devices > + * Note that these offsets are admitted by bootloader,recovery and uncrypt, > so they > + * are not configurable without changing all of them. > + */ > +static const size_t ANDROID_BOOTLOADER_MESSAGE_OFFSET_IN_MISC = 0; > +static const size_t ANDROID_WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024; > + > +/* Bootloader Message (2-KiB) > + * > + * This structure describes the content of a block in flash > + * that is used for recovery and the bootloader to talk to > + * each other. > + * > + * The command field is updated by linux when it wants to > + * reboot into recovery or to update radio or bootloader firmware. > + * It is also updated by the bootloader when firmware update > + * is complete (to boot into recovery for any final cleanup) > + * > + * The status field is written by the bootloader after the > + * completion of an "update-radio" or "update-hboot" command. > + * > + * The recovery field is only written by linux and used > + * for the system to send a message to recovery or the > + * other way around. > + * > + * The stage field is written by packages which restart themselves > + * multiple times, so that the UI can reflect which invocation of the > + * package it is. If the value is of the format "#/#" (eg, "1/3"), > + * the UI will add a simple indicator of that status. > + * > + * We used to have slot_suffix field for A/B boot control metadata in > + * this struct, which gets unintentionally cleared by recovery or > + * uncrypt. Move it into struct bootloader_message_ab to avoid the > + * issue. > + */ > +struct android_bootloader_message { > + char command[32]; > + char status[32]; > + char recovery[768]; > + > + /* The 'recovery' field used to be 1024 bytes. It has only ever > + * been used to store the recovery command line, so 768 bytes > + * should be plenty. We carve off the last 256 bytes to store the > + * stage string (for multistage packages) and possible future > + * expansion. */ > + char stage[32]; > + > + /* The 'reserved' field used to be 224 bytes when it was initially > + * carved off from the 1024-byte recovery field. Bump it up to > + * 1184-byte so that the entire bootloader_message struct rounds up > + * to 2048-byte. */ > + char reserved[1184]; > +}; > + > +/** > + * We must be cautious when changing the bootloader_message struct size, > + * because A/B-specific fields may end up with different offsets. > + */ > +#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus) > +static_assert(sizeof(struct android_bootloader_message) == 2048, > + "struct bootloader_message size changes, which may break A/B > devices"); > +#endif The check_member() macros might be useful here to avoid this #if > + > +/** > + * The A/B-specific bootloader message structure (4-KiB). > + * > + * We separate A/B boot control metadata from the regular bootloader > + * message struct and keep it here. Everything that's A/B-specific > + * stays after struct bootloader_message, which should be managed by > + * the A/B-bootloader or boot control HAL. > + * > + * The slot_suffix field is used for A/B implementations where the > + * bootloader does not set the androidboot.ro.boot.slot_suffix kernel > + * commandline parameter. This is used by fs_mgr to mount /system and > + * other partitions with the slotselect flag set in fstab. A/B > + * implementations are free to use all 32 bytes and may store private > + * data past the first NUL-byte in this field. It is encouraged, but > + * not mandatory, to use 'struct bootloader_control' described below. > + */ > +struct android_bootloader_message_ab { > + struct android_bootloader_message message; > + char slot_suffix[32]; > + > + /* Round up the entire struct to 4096-byte. */ > + char reserved[2016]; > +}; > + > +/** > + * Be cautious about the struct size change, in case we put anything post > + * bootloader_message_ab struct (b/29159185). > + */ > +#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus) > +static_assert(sizeof(struct android_bootloader_message_ab) == 4096, > + "struct bootloader_message_ab size changes"); > +#endif > + > +#define ANDROID_BOOT_CTRL_MAGIC 0x42414342 /* Bootloader Control AB */ > +#define ANDROID_BOOT_CTRL_VERSION 1 > + > +struct android_slot_metadata { I think thee read better if you put the comments at the top. See for example struct udevice. > + /* Slot priority with 15 meaning highest priority, 1 lowest > + * priority and 0 the slot is unbootable. */ > + uint8_t priority : 4; > + /* Number of times left attempting to boot this slot. */ > + uint8_t tries_remaining : 3; > + /* 1 if this slot has booted successfully, 0 otherwise. */ > + uint8_t successful_boot : 1; > + /* 1 if this slot is corrupted from a dm-verity corruption, 0 */ > + /* otherwise. */ > + uint8_t verity_corrupted : 1; > + /* Reserved for further use. */ > + uint8_t reserved : 7; > +} __attribute__((packed)); > + > +/* Bootloader Control AB > + * > + * This struct can be used to manage A/B metadata. It is designed to > + * be put in the 'slot_suffix' field of the 'bootloader_message' > + * structure described above. It is encouraged to use the > + * 'bootloader_control' structure to store the A/B metadata, but not > + * mandatory. > + */ > +struct android_bootloader_control { > + /* NUL terminated active slot suffix. */ > + char slot_suffix[4]; > + /* Bootloader Control AB magic number (see BOOT_CTRL_MAGIC). */ > + uint32_t magic; > + /* Version of struct being used (see BOOT_CTRL_VERSION). */ > + uint8_t version; > + /* Number of slots being managed. */ > + uint8_t nb_slot : 3; > + /* Number of times left attempting to boot recovery. */ > + uint8_t recovery_tries_remaining : 3; > + /* Ensure 4-bytes alignment for slot_info field. */ > + uint8_t reserved0[2]; > + /* Per-slot information. Up to 4 slots. */ > + struct android_slot_metadata slot_info[4]; > + /* Reserved for further use. */ > + uint8_t reserved1[8]; > + /* CRC32 of all 28 bytes preceding this field (little endian > + * format). */ > + uint32_t crc32_le; > +} __attribute__((packed)); > + > +#if (__STDC_VERSION__ >= 201112L) || defined(__cplusplus) > +static_assert(sizeof(struct android_bootloader_control) == > + sizeof(((struct android_bootloader_message_ab > *)0)->slot_suffix), > + "struct bootloader_control has wrong size"); > +#endif > + > +#endif /* __ANDROID_BOOTLOADER_MESSAGE_H */ > -- > 2.12.2.564.g063fe858b8-goog > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot