All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] Add support for booting into userspace fastboot
@ 2020-05-27  8:50 Roman Kovalivskyi
  2020-05-27  8:50 ` [PATCH 1/1] fastboot: add support for 'reboot fastboot' command Roman Kovalivskyi
  0 siblings, 1 reply; 5+ messages in thread
From: Roman Kovalivskyi @ 2020-05-27  8:50 UTC (permalink / raw)
  To: u-boot

Android 10 adds support for dynamic partitions and in order to support
them userspace fastboot must be used[1]. New tool called 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[2] 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.

Please note that boot script is expected to handle 'boot-fastboot' command
in BCB and load into recovery mode.

Bootloader must support[3] 'reboot fastboot' command which should reboot
device into userspace fastboot to accomodate those changes.

Another command that bootloader must support[3] is 'reboot recovery'. This 
command should simply reboot device into recovery mode.

[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

Roman Stratiienko (1):
  fastboot: add support for 'reboot fastboot' command

 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(+)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] fastboot: add support for 'reboot fastboot' command
  2020-05-27  8:50 [PATCH 0/1] Add support for booting into userspace fastboot Roman Kovalivskyi
@ 2020-05-27  8:50 ` Roman Kovalivskyi
  2020-05-27 12:56   ` Alex Kiernan
  0 siblings, 1 reply; 5+ messages in thread
From: Roman Kovalivskyi @ 2020-05-27  8:50 UTC (permalink / raw)
  To: u-boot

From: Roman Stratiienko <r.stratiienko@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@globallogic.com>
Signed-off-by: Roman Stratiienko <r.stratiienko@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;
+#else
+	return -ENOSYS;
+#endif
+}
+
 /**
  * fastboot_get_progress_callback() - Return progress callback
  *
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;
 		}
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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 1/1] fastboot: add support for 'reboot fastboot' command
  2020-05-27  8:50 ` [PATCH 1/1] fastboot: add support for 'reboot fastboot' command Roman Kovalivskyi
@ 2020-05-27 12:56   ` Alex Kiernan
  2020-06-01 12:55     ` Roman Kovalivskyi
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Kiernan @ 2020-05-27 12:56 UTC (permalink / raw)
  To: u-boot

On Wed, May 27, 2020 at 12:14 PM Roman Kovalivskyi
<roman.kovalivskyi@globallogic.com> wrote:
>
> From: Roman Stratiienko <r.stratiienko@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@globallogic.com>
> Signed-off-by: Roman Stratiienko <r.stratiienko@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.

> +#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?

> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] fastboot: add support for 'reboot fastboot' command
  2020-05-27 12:56   ` Alex Kiernan
@ 2020-06-01 12:55     ` Roman Kovalivskyi
  2020-06-01 16:23       ` Alex Kiernan
  0 siblings, 1 reply; 5+ messages in thread
From: Roman Kovalivskyi @ 2020-06-01 12:55 UTC (permalink / raw)
  To: u-boot

On 27.05.20 15:56, Alex Kiernan wrote:

> On Wed, May 27, 2020 at 12:14 PM Roman Kovalivskyi
> <roman.kovalivskyi@globallogic.com> wrote:
>> From: Roman Stratiienko <r.stratiienko@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@globallogic.com>
>> Signed-off-by: Roman Stratiienko <r.stratiienko@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
>>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] fastboot: add support for 'reboot fastboot' command
  2020-06-01 12:55     ` Roman Kovalivskyi
@ 2020-06-01 16:23       ` Alex Kiernan
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Kiernan @ 2020-06-01 16:23 UTC (permalink / raw)
  To: u-boot

On Mon, Jun 1, 2020 at 1:55 PM Roman Kovalivskyi
<roman.kovalivskyi@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@globallogic.com> wrote:
> >> From: Roman Stratiienko <r.stratiienko@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@globallogic.com>
> >> Signed-off-by: Roman Stratiienko <r.stratiienko@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-01 16:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27  8:50 [PATCH 0/1] Add support for booting into userspace fastboot Roman Kovalivskyi
2020-05-27  8:50 ` [PATCH 1/1] fastboot: add support for 'reboot fastboot' command Roman Kovalivskyi
2020-05-27 12:56   ` Alex Kiernan
2020-06-01 12:55     ` Roman Kovalivskyi
2020-06-01 16:23       ` Alex Kiernan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.