[PATCH 1/1] fastboot: add support for 'reboot fastboot' command
Alex Kiernan
alex.kiernan at gmail.com
Mon Jun 1 18:23:20 CEST 2020
On Mon, Jun 1, 2020 at 1:55 PM Roman Kovalivskyi
<roman.kovalivskyi at globallogic.com> wrote:
>
> On 27.05.20 15:56, Alex Kiernan wrote:
>
> > On Wed, May 27, 2020 at 12:14 PM Roman Kovalivskyi
> > <roman.kovalivskyi at globallogic.com> wrote:
> >> From: Roman Stratiienko <r.stratiienko at 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.kovalivskyi at globallogic.com>
> >> Signed-off-by: Roman Stratiienko <r.stratiienko at 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?
>
Yes, that - hardcoding of the bcb flow rather than (say) delegating to
something picked from an environment variable. But it may be that in
the current fastboot world, you have to have a bcb and the only
meaningful flow is precisely this one.
> >> +#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.
>
Refactor all of them? Either the code that consumes these is in the
mainline tree, or whoever is the keeper of it out of tree has to
accommodate upstream changes.
> >
> >> 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
> >>
> >
--
Alex Kiernan
More information about the U-Boot
mailing list