Re: [PATCH v2 2/2] fastboot: add default fastboot_set_reboot_flag implementation

Hi Roman,

чт, 4 июн. 2020 г. в 10:56, Roman Kovalivskyi roman.kovalivskyi@globallogic.com:

Default implementation of fastboot_set_reboot_flag function that depends on "bcb" commands could be used in general case if there are no need to make any platform-specific implementation, otherwise it could be disabled via Kconfig option FASTBOOT_USE_BCB_SET_REBOOT_FLAG.

Please note that FASTBOOT_USE_BCB_SET_REBOOT_FLAG is mutually exclusive with some platforms which already have their own implementation of this function.

Signed-off-by: Roman Kovalivskyi roman.kovalivskyi@globallogic.com

drivers/fastboot/Kconfig | 9 +++++++++ drivers/fastboot/fb_common.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)

diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig index d4436dfc9173..bcb43bc5d556 100644 --- a/drivers/fastboot/Kconfig +++ b/drivers/fastboot/Kconfig @@ -165,6 +165,15 @@ config FASTBOOT_CMD_OEM_FORMAT relies on the env variable partitions to contain the list of partitions as required by the gpt command.

+config FASTBOOT_USE_BCB_SET_REBOOT_FLAG

•   bool "Enable default fastboot_set_reboot_flag implementation"
  

•   depends on CMD_BCB && !ARCH_MESON && !ARCH_ROCKCHIP && !TARGET_KC1 && \
  

•           !TARGET_SNIPER && !TARGET_AM57XX_EVM && !TARGET_DRA7XX_EVM
  

•   default 1
  

•   help
  

•           Add default implementation of fastboot_set_reboot_flag that uses
  

•           "bcb" commands.
  

endif # FASTBOOT

endmenu diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c index 012a6288c187..77fe22e88f3d 100644 --- a/drivers/fastboot/fb_common.c +++ b/drivers/fastboot/fb_common.c @@ -88,6 +88,41 @@ void fastboot_okay(const char *reason, char *response)

• which sets whatever flag your board specific Android bootloader flow
• requires in order to re-enter the bootloader.

*/

Consider creating an array : const char *bcb_boot_reason_commands = {"bootonce-bootloader", "boot-fastboot", "boot-recovery");

+#if CONFIG_IS_ENABLED(FASTBOOT_USE_BCB_SET_REBOOT_FLAG) +int fastboot_set_reboot_flag(int reason) +{

•   char cmd[32];
  

Check the reason range is in between 0 to FASTBOOT_REBOOT_REASON_COUNT-1.

•   snprintf(cmd, sizeof(cmd), "bcb load %d misc",
  

•            CONFIG_FASTBOOT_FLASH_MMC_DEV);
  

•   if (run_command(cmd, 0))
  

•           return -ENODEV;
  

•   switch (reason) {
  

•   case FASTBOOT_REBOOT_BOOTLOADER:
  

•           snprintf(cmd, sizeof(cmd),
  

•                    "bcb set command bootonce-bootloader");
  

•           break;
  

•   case FASTBOOT_REBOOT_FASTBOOTD:
  

•           snprintf(cmd, sizeof(cmd), "bcb set command boot-fastboot");
  

•           break;
  

•   case FASTBOOT_REBOOT_RECOVERY:
  

•           snprintf(cmd, sizeof(cmd), "bcb set command boot-recovery");
  

•           break;
  

•   default:
  

•           return -EINVAL;
  

•   }
  

Instead of doing switch-case, consider using: snprintf(cmd, sizeof(cmd), "bcb set command %s", bcb_boot_reason_commands[reason]);

•   if (run_command(cmd, 0))
  

•           return -ENOEXEC;
  

•   if (run_command("bcb store", 0))
  

•           return -EIO;
  

•   return 0;
  

+} +#else int __weak fastboot_set_reboot_flag(int reason) { if (reason != FASTBOOT_REBOOT_BOOTLOADER) @@ -95,6 +130,7 @@ int __weak fastboot_set_reboot_flag(int reason)

    return -ENOSYS;

} +#endif

/**

• fastboot_get_progress_callback() - Return progress callback

-- 2.17.1

Why not move this logic into a separate file?

Regards, Roman Stratiienko