[PATCH] fastboot: remove #ifdef CONFIG when it is possible
Patrick DELAUNAY
patrick.delaunay at foss.st.com
Tue Jan 3 13:39:35 CET 2023
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 at 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 at 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 at seco.com>
regards
Patrick
More information about the U-Boot
mailing list