Hi,
On 12/15/22 16:40, Sean Anderson wrote:
On 12/15/22 04:15, Patrick Delaunay wrote:
Much of the fastboot code predates the introduction of Kconfig and has quite a few #ifdefs in it which is unnecessary now that we can use IS_ENABLED() et al.
Signed-off-by: Patrick Delaunay patrick.delaunay@foss.st.com
cmd/fastboot.c | 35 +++++------ drivers/fastboot/fb_command.c | 104 ++++++++++++-------------------- drivers/fastboot/fb_common.c | 11 ++-- drivers/fastboot/fb_getvar.c | 49 ++++++--------- drivers/usb/gadget/f_fastboot.c | 7 +-- include/fastboot.h | 13 ---- net/fastboot.c | 8 +-- 7 files changed, 82 insertions(+), 145 deletions(-)
diff --git a/cmd/fastboot.c b/cmd/fastboot.c index b498e4b22bb3..b94dbd548843 100644 --- a/cmd/fastboot.c +++ b/cmd/fastboot.c @@ -19,8 +19,14 @@ static int do_fastboot_udp(int argc, char *const argv[], uintptr_t buf_addr, size_t buf_size) { -#if CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT) - int err = net_loop(FASTBOOT); + int err;
+ if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) { + pr_err("Fastboot UDP not enabled\n"); + return CMD_RET_FAILURE; + }
+ err = net_loop(FASTBOOT); if (err < 0) { printf("fastboot udp error: %d\n", err); @@ -28,21 +34,21 @@ static int do_fastboot_udp(int argc, char *const argv[], } return CMD_RET_SUCCESS; -#else - pr_err("Fastboot UDP not enabled\n"); - return CMD_RET_FAILURE; -#endif } static int do_fastboot_usb(int argc, char *const argv[], uintptr_t buf_addr, size_t buf_size) { -#if CONFIG_IS_ENABLED(USB_FUNCTION_FASTBOOT) int controller_index; char *usb_controller; char *endp; int ret; + if (!CONFIG_IS_ENABLED(USB_FUNCTION_FASTBOOT)) { + pr_err("Fastboot USB not enabled\n"); + return CMD_RET_FAILURE; + }
if (argc < 2) return CMD_RET_USAGE; @@ -88,10 +94,6 @@ exit: g_dnl_clear_detach(); return ret; -#else - pr_err("Fastboot USB not enabled\n"); - return CMD_RET_FAILURE; -#endif } static int do_fastboot(struct cmd_tbl *cmdtp, int flag, int argc, @@ -148,17 +150,12 @@ NXTARG: return do_fastboot_usb(argc, argv, buf_addr, buf_size); } -#ifdef CONFIG_SYS_LONGHELP -static char fastboot_help_text[] = +U_BOOT_CMD( + fastboot, CONFIG_SYS_MAXARGS, 1, do_fastboot, + "run as a fastboot usb or udp device", "[-l addr] [-s size] usb <controller> | udp\n" "\taddr - address of buffer used during data transfers (" __stringify(CONFIG_FASTBOOT_BUF_ADDR) ")\n" "\tsize - size of buffer used during data transfers (" __stringify(CONFIG_FASTBOOT_BUF_SIZE) ")" - ; -#endif
-U_BOOT_CMD( - fastboot, CONFIG_SYS_MAXARGS, 1, do_fastboot, - "run as a fastboot usb or udp device", fastboot_help_text ); diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c index bdfdf262c8a3..f0fd605854da 100644 --- a/drivers/fastboot/fb_command.c +++ b/drivers/fastboot/fb_command.c @@ -31,27 +31,16 @@ static u32 fastboot_bytes_expected; static void okay(char *, char *); static void getvar(char *, char *); static void download(char *, char *); -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH) 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 -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF) static void oem_partconf(char *, char *); -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS) static void oem_bootbus(char *, char *); -#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT) static void run_ucmd(char *, char *); static void run_acmd(char *, char *); -#endif static const struct { const char *command; @@ -65,16 +54,14 @@ static const struct { .command = "download", .dispatch = download }, -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH) [FASTBOOT_COMMAND_FLASH] = { .command = "flash", - .dispatch = flash + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_FLASH, (flash), (NULL)) }, [FASTBOOT_COMMAND_ERASE] = { .command = "erase", - .dispatch = erase + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_FLASH, (erase), (NULL)) }, -#endif [FASTBOOT_COMMAND_BOOT] = { .command = "boot", .dispatch = okay @@ -103,34 +90,26 @@ static const struct { .command = "set_active", .dispatch = okay }, -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT) [FASTBOOT_COMMAND_OEM_FORMAT] = { .command = "oem format", - .dispatch = oem_format, + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT, (oem_format), (NULL)) }, -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF) [FASTBOOT_COMMAND_OEM_PARTCONF] = { .command = "oem partconf", - .dispatch = oem_partconf, + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF, (oem_partconf), (NULL)) }, -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS) [FASTBOOT_COMMAND_OEM_BOOTBUS] = { .command = "oem bootbus", - .dispatch = oem_bootbus, + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS, (oem_bootbus), (NULL)) }, -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT) [FASTBOOT_COMMAND_UCMD] = { .command = "UCmd", - .dispatch = run_ucmd, + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPOR_______________________________________________ Uboot-stm32 mailing list Uboot-stm32@st-md-mailman.stormreply.com https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32 T, (run_ucmd), (NULL)) }, [FASTBOOT_COMMAND_ACMD] = { .command = "ACmd", - .dispatch = run_acmd, + .dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_acmd), (NULL)) }, -#endif
Does this affect binary size?
Yes the size of U-Boot binary with FastBoot option is increasing with this patch.
because "commands[FASTBOOT_COMMAND_COUNT]"
have always the max size for known commands in U-Boot,
even for not supported commands when .dispatch ops is NULL,
and it is detected dynamically in fastboot_handle_command()
with the added trace "command %s not supported."
I don't found a better solution because in include/fastboot.h
I remove the ifdef for FASTBOOT_COMMAND_COUNT definition
Today it is not blocking, the CI build are ok,
I hope it is not a blocking problem.
}; /** @@ -156,7 +135,9 @@ int fastboot_handle_command(char *cmd_string, char *response) response); return i; } else { - break; + pr_err("command %s not supported.\n", cmd_string); + fastboot_fail("Unsupported command", response); + return -1; } } } @@ -299,7 +280,6 @@ void fastboot_data_complete(char *response) fastboot_bytes_received = 0; }
....
diff --git a/include/fastboot.h b/include/fastboot.h index 57daaf129821..d062a3469ef9 100644 --- a/include/fastboot.h +++ b/include/fastboot.h @@ -24,10 +24,8 @@ enum { FASTBOOT_COMMAND_GETVAR = 0, FASTBOOT_COMMAND_DOWNLOAD, -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH) FASTBOOT_COMMAND_FLASH, FASTBOOT_COMMAND_ERASE, -#endif FASTBOOT_COMMAND_BOOT, FASTBOOT_COMMAND_CONTINUE, FASTBOOT_COMMAND_REBOOT, @@ -35,20 +33,11 @@ enum { FASTBOOT_COMMAND_REBOOT_FASTBOOTD, FASTBOOT_COMMAND_REBOOT_RECOVERY, FASTBOOT_COMMAND_SET_ACTIVE, -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT) FASTBOOT_COMMAND_OEM_FORMAT, -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF) FASTBOOT_COMMAND_OEM_PARTCONF, -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS) FASTBOOT_COMMAND_OEM_BOOTBUS, -#endif -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT) FASTBOOT_COMMAND_ACMD, FASTBOOT_COMMAND_UCMD, -#endif
FASTBOOT_COMMAND_COUNT }; @@ -173,7 +162,5 @@ void fastboot_data_download(const void *fastboot_data, */ void fastboot_data_complete(char *response); -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT) void fastboot_acmd_complete(void); -#endif #endif /* _FASTBOOT_H_ */ diff --git a/net/fastboot.c b/net/fastboot.c index 139233b86c61..96bdf5486fa6 100644 --- a/net/fastboot.c +++ b/net/fastboot.c @@ -42,7 +42,6 @@ static int fastboot_our_port; static void boot_downloaded_image(void); -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH) /** * fastboot_udp_send_info() - Send an INFO packet during long commands. * @@ -104,7 +103,6 @@ static void fastboot_timed_send_info(const char *msg) fastboot_udp_send_info(msg); } } -#endif /** * fastboot_send() - Sends a packet in response to received fastboot packet @@ -309,9 +307,9 @@ void fastboot_start_server(void) fastboot_our_port = CONFIG_UDP_FUNCTION_FASTBOOT_PORT; -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH) - fastboot_set_progress_callback(fastboot_timed_send_info); -#endif + if (CONFIG_IS_ENABLED(FASTBOOT_FLASH))
- fastboot_set_progress_callback(fastboot_timed_send_info);
net_set_udp_handler(fastboot_handler); /* zero out server ether in case the server ip has changed */
Reviewed-by: Sean Anderson sean.anderson@seco.com
regards
Patrick