On 27.05.20 15:56, Alex Kiernan wrote: > On Wed, May 27, 2020 at 12:14 PM Roman Kovalivskyi > <roman.kovalivs...@globallogic.com> wrote: >> From: Roman Stratiienko <r.stratiie...@gmail.com> >> >> Android 10 adds support for dynamic partitions and in order to support >> them userspace fastboot must be used[1]. New tool fastbootd is >> included into recovery image. >> >> Userspace fastboot works from recovery and is launched if: >> 1) - Dynamic partitioning is enabled >> 2) - Boot control block has 'boot-fastboot' value into command field >> The bootloader is expected to load and boot into the recovery image >> upon seeing boot-fastboot in the BCB command. Recovery then parses the >> BCB message and switches to fastbootd mode[2]. >> >> Please note that boot script is expected to handle 'boot-fastboot' >> command in BCB and load into recovery mode. >> >> Bootloader must support 'reboot fastboot' command which should reboot >> device into userspace fastboot to accomodate those changes[3]. >> >> [1] - https://source.android.com/devices/bootloader/fastbootd >> [2] - >> https://source.android.com/devices/bootloader/fastbootd#unified_fastboot_and_recovery >> [3] - >> https://source.android.com/devices/bootloader/fastbootd#modifications_to_the_bootloader >> >> Signed-off-by: Roman Kovalivskyi <roman.kovalivs...@globallogic.com> >> Signed-off-by: Roman Stratiienko <r.stratiie...@gmail.com> >> Change-Id: I9d2bdc9a6f6f31ea98572fe155e1cc8341e9af76 >> --- >> drivers/fastboot/fb_command.c | 42 +++++++++++++++++++++++++++++++++ >> drivers/fastboot/fb_common.c | 31 ++++++++++++++++++++++++ >> drivers/usb/gadget/f_fastboot.c | 2 ++ >> include/fastboot.h | 9 +++++++ >> 4 files changed, 84 insertions(+) >> >> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c >> index 49f6a61c3745..3616133b880e 100644 >> --- a/drivers/fastboot/fb_command.c >> +++ b/drivers/fastboot/fb_command.c >> @@ -37,6 +37,8 @@ static void flash(char *, char *); >> static void erase(char *, char *); >> #endif >> static void reboot_bootloader(char *, char *); >> +static void reboot_fastbootd(char *, char *); >> +static void reboot_recovery(char *, char *); >> #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT) >> static void oem_format(char *, char *); >> #endif >> @@ -79,6 +81,14 @@ static const struct { >> .command = "reboot-bootloader", >> .dispatch = reboot_bootloader >> }, >> + [FASTBOOT_COMMAND_REBOOT_FASTBOOTD] = { >> + .command = "reboot-fastboot", >> + .dispatch = reboot_fastbootd >> + }, >> + [FASTBOOT_COMMAND_REBOOT_RECOVERY] = { >> + .command = "reboot-recovery", >> + .dispatch = reboot_recovery >> + }, >> [FASTBOOT_COMMAND_SET_ACTIVE] = { >> .command = "set_active", >> .dispatch = okay >> @@ -307,12 +317,44 @@ static void erase(char *cmd_parameter, char *response) >> */ >> static void reboot_bootloader(char *cmd_parameter, char *response) >> { >> +#if CONFIG_IS_ENABLED(CMD_BCB) >> + if (fastboot_set_flag("bootonce-bootloader")) >> +#else >> if (fastboot_set_reboot_flag()) >> +#endif >> fastboot_fail("Cannot set reboot flag", response); >> else >> fastboot_okay(NULL, response); >> } >> >> +/** >> + * reboot_fastbootd() - Sets reboot fastboot flag. >> + * >> + * @cmd_parameter: Pointer to command parameter >> + * @response: Pointer to fastboot response buffer >> + */ >> +static void reboot_fastbootd(char *cmd_parameter, char *response) >> +{ >> + if (fastboot_set_flag("boot-fastboot")) >> + fastboot_fail("Cannot set fastboot flag", response); >> + else >> + fastboot_okay(NULL, response); >> +} >> + >> +/** >> + * reboot_recovery() - Sets reboot recovery flag. >> + * >> + * @cmd_parameter: Pointer to command parameter >> + * @response: Pointer to fastboot response buffer >> + */ >> +static void reboot_recovery(char *cmd_parameter, char *response) >> +{ >> + if (fastboot_set_flag("boot-recovery")) >> + fastboot_fail("Cannot set recovery flag", response); >> + else >> + fastboot_okay(NULL, response); >> +} >> + >> #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT) >> /** >> * oem_format() - Execute the OEM format command >> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c >> index c3735a44af74..b6401640ad06 100644 >> --- a/drivers/fastboot/fb_common.c >> +++ b/drivers/fastboot/fb_common.c >> @@ -93,6 +93,37 @@ int __weak fastboot_set_reboot_flag(void) >> return -ENOSYS; >> } >> >> +/** >> + * fastboot_set_flag() - Set flag to indicate reboot-recovery >> + * >> + * Set flag which indicates that we should reboot into the recovery >> + * following the reboot that fastboot executes after this function. >> + */ >> +int fastboot_set_flag(const char *command) >> +{ >> +#if CONFIG_IS_ENABLED(CMD_BCB) >> + char cmd[32]; >> + >> + snprintf(cmd, sizeof(cmd), "bcb load %d misc", >> + CONFIG_FASTBOOT_FLASH_MMC_DEV); >> + >> + if (run_command(cmd, 0)) >> + return -ENODEV; >> + >> + snprintf(cmd, sizeof(cmd), "bcb set command %s", command); >> + >> + if (run_command(cmd, 0)) >> + return -ENOEXEC; >> + >> + if (run_command("bcb store", 0)) >> + return -EIO; >> + >> + return 0; > I've not been keeping up with where fastboot has been going, but this > feels ugly in common code.
Could you please clarify what do you mean by ugly? Usage of bcb command or something else? >> +#else >> + return -ENOSYS; >> +#endif >> +} >> + >> /** >> * fastboot_get_progress_callback() - Return progress callback >> * > Should fastboot_set_flag/fastboot_set_reboot_flag be consolidated into > a single interface? Maybe, but as for now fastboot_set_reboot_flag is implemented in a way that expects it to be overridden into board-specific file, so I'm not quite sure how it should be consolidated into single interface without breaking anything. > >> diff --git a/drivers/usb/gadget/f_fastboot.c >> b/drivers/usb/gadget/f_fastboot.c >> index 384c0f6f6e27..30f7a52087fc 100644 >> --- a/drivers/usb/gadget/f_fastboot.c >> +++ b/drivers/usb/gadget/f_fastboot.c >> @@ -455,6 +455,8 @@ static void rx_handler_command(struct usb_ep *ep, struct >> usb_request *req) >> >> case FASTBOOT_COMMAND_REBOOT: >> case FASTBOOT_COMMAND_REBOOT_BOOTLOADER: >> + case FASTBOOT_COMMAND_REBOOT_FASTBOOTD: >> + case FASTBOOT_COMMAND_REBOOT_RECOVERY: >> fastboot_func->in_req->complete = compl_do_reset; >> break; >> } > I expect you need a similar change in net/fastboot.c:fastboot_send (a > piece of ugliness I failed to abstract away when redoing this). > >> diff --git a/include/fastboot.h b/include/fastboot.h >> index 1933b1d98e3b..8fd2a9e49d0f 100644 >> --- a/include/fastboot.h >> +++ b/include/fastboot.h >> @@ -32,6 +32,8 @@ enum { >> FASTBOOT_COMMAND_CONTINUE, >> FASTBOOT_COMMAND_REBOOT, >> FASTBOOT_COMMAND_REBOOT_BOOTLOADER, >> + FASTBOOT_COMMAND_REBOOT_FASTBOOTD, >> + FASTBOOT_COMMAND_REBOOT_RECOVERY, >> FASTBOOT_COMMAND_SET_ACTIVE, >> #if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT) >> FASTBOOT_COMMAND_OEM_FORMAT, >> @@ -79,6 +81,13 @@ void fastboot_okay(const char *reason, char *response); >> */ >> int fastboot_set_reboot_flag(void); >> >> +/** >> + * fastboot_set_flag() - Set flag to indicate reboot-fastboot >> + * >> + * Set flag which indicates that system should reboot into specified mode. >> + */ >> +int fastboot_set_flag(const char *command); >> + >> /** >> * fastboot_set_progress_callback() - set progress callback >> * >> -- >> 2.17.1 >> >