[PATCH] fastboot: remove #ifdef CONFIG when it is possible

[PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Patrick Delaunay]

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_SUPPORT, (run_ucmd), (NULL))
 	},
 	[FASTBOOT_COMMAND_ACMD] = {
 		.command = "ACmd",
-		.dispatch = run_acmd,
+		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_acmd), (NULL))
 	},
-#endif
 };
 
 /**
@@ -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;
 }
 
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 /**
  * flash() - write the downloaded image to the indicated partition.
  *
@@ -309,16 +289,15 @@ void fastboot_data_complete(char *response)
  * Writes the previously downloaded image to the partition indicated by
  * cmd_parameter. Writes to response.
  */
-static void flash(char *cmd_parameter, char *response)
+static void __maybe_unused flash(char *cmd_parameter, char *response)
 {
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
-	fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
-				 response);
-#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
-	fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
-				  response);
-#endif
+	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
+		fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
+					 image_size, response);
+
+	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
+		fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr,
+					  image_size, response);
 }
 
 /**
@@ -330,25 +309,22 @@ static void flash(char *cmd_parameter, char *response)
  * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
  * to response.
  */
-static void erase(char *cmd_parameter, char *response)
+static void __maybe_unused erase(char *cmd_parameter, char *response)
 {
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
-	fastboot_mmc_erase(cmd_parameter, response);
-#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
-	fastboot_nand_erase(cmd_parameter, response);
-#endif
+	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
+		fastboot_mmc_erase(cmd_parameter, response);
+
+	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
+		fastboot_nand_erase(cmd_parameter, response);
 }
-#endif
 
-#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
 /**
  * run_ucmd() - Execute the UCmd command
  *
  * @cmd_parameter: Pointer to command parameter
  * @response: Pointer to fastboot response buffer
  */
-static void run_ucmd(char *cmd_parameter, char *response)
+static void __maybe_unused run_ucmd(char *cmd_parameter, char *response)
 {
 	if (!cmd_parameter) {
 		pr_err("missing slot suffix\n");
@@ -375,7 +351,7 @@ void fastboot_acmd_complete(void)
  * @cmd_parameter: Pointer to command parameter
  * @response: Pointer to fastboot response buffer
  */
-static void run_acmd(char *cmd_parameter, char *response)
+static void __maybe_unused run_acmd(char *cmd_parameter, char *response)
 {
 	if (!cmd_parameter) {
 		pr_err("missing slot suffix\n");
@@ -392,7 +368,6 @@ static void run_acmd(char *cmd_parameter, char *response)
 	strcpy(g_a_cmd_buff, cmd_parameter);
 	fastboot_okay(NULL, response);
 }
-#endif
 
 /**
  * reboot_bootloader() - Sets reboot bootloader flag.
@@ -436,40 +411,40 @@ static void reboot_recovery(char *cmd_parameter, char *response)
 		fastboot_okay(NULL, response);
 }
 
-#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT)
 /**
  * oem_format() - Execute the OEM format command
  *
  * @cmd_parameter: Pointer to command parameter
  * @response: Pointer to fastboot response buffer
  */
-static void oem_format(char *cmd_parameter, char *response)
+static void __maybe_unused oem_format(char *cmd_parameter, char *response)
 {
 	char cmdbuf[32];
+	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
+					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
 
 	if (!env_get("partitions")) {
 		fastboot_fail("partitions not set", response);
 	} else {
-		sprintf(cmdbuf, "gpt write mmc %x $partitions",
-			CONFIG_FASTBOOT_FLASH_MMC_DEV);
+		sprintf(cmdbuf, "gpt write mmc %x $partitions", mmc_dev);
 		if (run_command(cmdbuf, 0))
 			fastboot_fail("", response);
 		else
 			fastboot_okay(NULL, response);
 	}
 }
-#endif
 
-#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF)
 /**
  * oem_partconf() - Execute the OEM partconf command
  *
  * @cmd_parameter: Pointer to command parameter
  * @response: Pointer to fastboot response buffer
  */
-static void oem_partconf(char *cmd_parameter, char *response)
+static void __maybe_unused oem_partconf(char *cmd_parameter, char *response)
 {
 	char cmdbuf[32];
+	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
+					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
 
 	if (!cmd_parameter) {
 		fastboot_fail("Expected command parameter", response);
@@ -477,26 +452,25 @@ static void oem_partconf(char *cmd_parameter, char *response)
 	}
 
 	/* execute 'mmc partconfg' command with cmd_parameter arguments*/
-	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0",
-		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
+	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0", mmc_dev, cmd_parameter);
 	printf("Execute: %s\n", cmdbuf);
 	if (run_command(cmdbuf, 0))
 		fastboot_fail("Cannot set oem partconf", response);
 	else
 		fastboot_okay(NULL, response);
 }
-#endif
 
-#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS)
 /**
  * oem_bootbus() - Execute the OEM bootbus command
  *
  * @cmd_parameter: Pointer to command parameter
  * @response: Pointer to fastboot response buffer
  */
-static void oem_bootbus(char *cmd_parameter, char *response)
+static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response)
 {
 	char cmdbuf[32];
+	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
+					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
 
 	if (!cmd_parameter) {
 		fastboot_fail("Expected command parameter", response);
@@ -504,12 +478,10 @@ static void oem_bootbus(char *cmd_parameter, char *response)
 	}
 
 	/* execute 'mmc bootbus' command with cmd_parameter arguments*/
-	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s",
-		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
+	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s", mmc_dev, cmd_parameter);
 	printf("Execute: %s\n", cmdbuf);
 	if (run_command(cmdbuf, 0))
 		fastboot_fail("Cannot set oem bootbus", response);
 	else
 		fastboot_okay(NULL, response);
 }
-#endif
diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
index ef399d0c4abb..7563650d07db 100644
--- a/drivers/fastboot/fb_common.c
+++ b/drivers/fastboot/fb_common.c
@@ -91,20 +91,21 @@ void fastboot_okay(const char *reason, char *response)
  */
 int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
 {
-#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
 	static const char * const boot_cmds[] = {
 		[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
 		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
 		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
 	};
+	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
+					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
+
+	if (!CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
+		return -EINVAL;
 
 	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
 		return -EINVAL;
 
-	return bcb_write_reboot_reason(CONFIG_FASTBOOT_FLASH_MMC_DEV, "misc", boot_cmds[reason]);
-#else
-    return -EINVAL;
-#endif
+	return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]);
 }
 
 /**
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 018989dd1667..2fbd285db384 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -21,15 +21,9 @@ static void getvar_version_baseband(char *var_parameter, char *response);
 static void getvar_product(char *var_parameter, char *response);
 static void getvar_platform(char *var_parameter, char *response);
 static void getvar_current_slot(char *var_parameter, char *response);
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 static void getvar_has_slot(char *var_parameter, char *response);
-#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
 static void getvar_partition_type(char *part_name, char *response);
-#endif
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 static void getvar_partition_size(char *part_name, char *response);
-#endif
 static void getvar_is_userspace(char *var_parameter, char *response);
 
 static const struct {
@@ -84,7 +78,6 @@ static const struct {
 	}
 };
 
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
 /**
  * Get partition number and size for any storage type.
  *
@@ -102,28 +95,26 @@ static int getvar_get_part_info(const char *part_name, char *response,
 				size_t *size)
 {
 	int r;
-# if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
 	struct blk_desc *dev_desc;
-	struct disk_partition part_info;
-
-	r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info,
-				       response);
-	if (r >= 0 && size)
-		*size = part_info.size * part_info.blksz;
-# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
+	struct disk_partition disk_part;
 	struct part_info *part_info;
 
-	r = fastboot_nand_get_part_info(part_name, &part_info, response);
-	if (r >= 0 && size)
-		*size = part_info->size;
-# else
-	fastboot_fail("this storage is not supported in bootloader", response);
-	r = -ENODEV;
-# endif
+	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)) {
+		r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part,
+					       response);
+		if (r >= 0 && size)
+			*size = disk_part.size * disk_part.blksz;
+	} else if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)) {
+		r = fastboot_nand_get_part_info(part_name, &part_info, response);
+		if (r >= 0 && size)
+			*size = part_info->size;
+	} else {
+		fastboot_fail("this storage is not supported in bootloader", response);
+		r = -ENODEV;
+	}
 
 	return r;
 }
-#endif
 
 static void getvar_version(char *var_parameter, char *response)
 {
@@ -181,8 +172,7 @@ static void getvar_current_slot(char *var_parameter, char *response)
 	fastboot_okay("a", response);
 }
 
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
-static void getvar_has_slot(char *part_name, char *response)
+static void __maybe_unused getvar_has_slot(char *part_name, char *response)
 {
 	char part_name_wslot[PART_NAME_LEN];
 	size_t len;
@@ -213,10 +203,8 @@ static void getvar_has_slot(char *part_name, char *response)
 fail:
 	fastboot_fail("invalid partition name", response);
 }
-#endif
 
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
-static void getvar_partition_type(char *part_name, char *response)
+static void __maybe_unused getvar_partition_type(char *part_name, char *response)
 {
 	int r;
 	struct blk_desc *dev_desc;
@@ -232,10 +220,8 @@ static void getvar_partition_type(char *part_name, char *response)
 			fastboot_okay(fs_get_type_name(), response);
 	}
 }
-#endif
 
-#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
-static void getvar_partition_size(char *part_name, char *response)
+static void __maybe_unused getvar_partition_size(char *part_name, char *response)
 {
 	int r;
 	size_t size;
@@ -244,7 +230,6 @@ static void getvar_partition_size(char *part_name, char *response)
 	if (r >= 0)
 		fastboot_response("OKAY", response, "0x%016zx", size);
 }
-#endif
 
 static void getvar_is_userspace(char *var_parameter, char *response)
 {
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 07b1681c8a9a..c6e7f4240758 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -495,7 +495,6 @@ static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req)
 	do_exit_on_complete(ep, req);
 }
 
-#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
 static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	/* When usb dequeue complete will be called
@@ -505,7 +504,6 @@ static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
 	if (req->status == 0)
 		fastboot_acmd_complete();
 }
-#endif
 
 static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
 {
@@ -546,11 +544,10 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
 			fastboot_func->in_req->complete = compl_do_reset;
 			g_dnl_trigger_detach();
 			break;
-#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
 		case FASTBOOT_COMMAND_ACMD:
-			fastboot_func->in_req->complete = do_acmd_complete;
+			if (CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT))
+				fastboot_func->in_req->complete = do_acmd_complete;
 			break;
-#endif
 		}
 	}
 
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 */
-- 
2.25.1

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Mattijs Korpershoek]

On Thu, Dec 15, 2022 at 10:15, Patrick Delaunay <patrick.delaunay@foss.st.com> 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>

Hi Patrick,

Thank you for this, it's a nice improvement!

I will test this on vim3l tomorrow, but I've already reviewed it:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.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_SUPPORT, (run_ucmd), (NULL))
>  	},
>  	[FASTBOOT_COMMAND_ACMD] = {
>  		.command = "ACmd",
> -		.dispatch = run_acmd,
> +		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_acmd), (NULL))
>  	},
> -#endif
>  };
>  
>  /**
> @@ -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;
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  /**
>   * flash() - write the downloaded image to the indicated partition.
>   *
> @@ -309,16 +289,15 @@ void fastboot_data_complete(char *response)
>   * Writes the previously downloaded image to the partition indicated by
>   * cmd_parameter. Writes to response.
>   */
> -static void flash(char *cmd_parameter, char *response)
> +static void __maybe_unused flash(char *cmd_parameter, char *response)
>  {
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
> -	fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
> -				 response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> -	fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
> -				  response);
> -#endif
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> +		fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
> +					 image_size, response);
> +
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
> +		fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr,
> +					  image_size, response);
>  }
>  
>  /**
> @@ -330,25 +309,22 @@ static void flash(char *cmd_parameter, char *response)
>   * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
>   * to response.
>   */
> -static void erase(char *cmd_parameter, char *response)
> +static void __maybe_unused erase(char *cmd_parameter, char *response)
>  {
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
> -	fastboot_mmc_erase(cmd_parameter, response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> -	fastboot_nand_erase(cmd_parameter, response);
> -#endif
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> +		fastboot_mmc_erase(cmd_parameter, response);
> +
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
> +		fastboot_nand_erase(cmd_parameter, response);
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>  /**
>   * run_ucmd() - Execute the UCmd command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void run_ucmd(char *cmd_parameter, char *response)
> +static void __maybe_unused run_ucmd(char *cmd_parameter, char *response)
>  {
>  	if (!cmd_parameter) {
>  		pr_err("missing slot suffix\n");
> @@ -375,7 +351,7 @@ void fastboot_acmd_complete(void)
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void run_acmd(char *cmd_parameter, char *response)
> +static void __maybe_unused run_acmd(char *cmd_parameter, char *response)
>  {
>  	if (!cmd_parameter) {
>  		pr_err("missing slot suffix\n");
> @@ -392,7 +368,6 @@ static void run_acmd(char *cmd_parameter, char *response)
>  	strcpy(g_a_cmd_buff, cmd_parameter);
>  	fastboot_okay(NULL, response);
>  }
> -#endif
>  
>  /**
>   * reboot_bootloader() - Sets reboot bootloader flag.
> @@ -436,40 +411,40 @@ static void reboot_recovery(char *cmd_parameter, char *response)
>  		fastboot_okay(NULL, response);
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT)
>  /**
>   * oem_format() - Execute the OEM format command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void oem_format(char *cmd_parameter, char *response)
> +static void __maybe_unused oem_format(char *cmd_parameter, char *response)
>  {
>  	char cmdbuf[32];
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
>  	if (!env_get("partitions")) {
>  		fastboot_fail("partitions not set", response);
>  	} else {
> -		sprintf(cmdbuf, "gpt write mmc %x $partitions",
> -			CONFIG_FASTBOOT_FLASH_MMC_DEV);
> +		sprintf(cmdbuf, "gpt write mmc %x $partitions", mmc_dev);
>  		if (run_command(cmdbuf, 0))
>  			fastboot_fail("", response);
>  		else
>  			fastboot_okay(NULL, response);
>  	}
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF)
>  /**
>   * oem_partconf() - Execute the OEM partconf command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void oem_partconf(char *cmd_parameter, char *response)
> +static void __maybe_unused oem_partconf(char *cmd_parameter, char *response)
>  {
>  	char cmdbuf[32];
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
>  	if (!cmd_parameter) {
>  		fastboot_fail("Expected command parameter", response);
> @@ -477,26 +452,25 @@ static void oem_partconf(char *cmd_parameter, char *response)
>  	}
>  
>  	/* execute 'mmc partconfg' command with cmd_parameter arguments*/
> -	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0",
> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
> +	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0", mmc_dev, cmd_parameter);
>  	printf("Execute: %s\n", cmdbuf);
>  	if (run_command(cmdbuf, 0))
>  		fastboot_fail("Cannot set oem partconf", response);
>  	else
>  		fastboot_okay(NULL, response);
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS)
>  /**
>   * oem_bootbus() - Execute the OEM bootbus command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void oem_bootbus(char *cmd_parameter, char *response)
> +static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response)
>  {
>  	char cmdbuf[32];
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
>  	if (!cmd_parameter) {
>  		fastboot_fail("Expected command parameter", response);
> @@ -504,12 +478,10 @@ static void oem_bootbus(char *cmd_parameter, char *response)
>  	}
>  
>  	/* execute 'mmc bootbus' command with cmd_parameter arguments*/
> -	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s",
> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
> +	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s", mmc_dev, cmd_parameter);
>  	printf("Execute: %s\n", cmdbuf);
>  	if (run_command(cmdbuf, 0))
>  		fastboot_fail("Cannot set oem bootbus", response);
>  	else
>  		fastboot_okay(NULL, response);
>  }
> -#endif
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index ef399d0c4abb..7563650d07db 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -91,20 +91,21 @@ void fastboot_okay(const char *reason, char *response)
>   */
>  int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
>  {
> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
>  	static const char * const boot_cmds[] = {
>  		[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
>  		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
>  		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
>  	};
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
> +
> +	if (!CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> +		return -EINVAL;
>  
>  	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
>  		return -EINVAL;
>  
> -	return bcb_write_reboot_reason(CONFIG_FASTBOOT_FLASH_MMC_DEV, "misc", boot_cmds[reason]);
> -#else
> -    return -EINVAL;
> -#endif
> +	return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]);
>  }
>  
>  /**
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 018989dd1667..2fbd285db384 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -21,15 +21,9 @@ static void getvar_version_baseband(char *var_parameter, char *response);
>  static void getvar_product(char *var_parameter, char *response);
>  static void getvar_platform(char *var_parameter, char *response);
>  static void getvar_current_slot(char *var_parameter, char *response);
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  static void getvar_has_slot(char *var_parameter, char *response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>  static void getvar_partition_type(char *part_name, char *response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  static void getvar_partition_size(char *part_name, char *response);
> -#endif
>  static void getvar_is_userspace(char *var_parameter, char *response);
>  
>  static const struct {
> @@ -84,7 +78,6 @@ static const struct {
>  	}
>  };
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  /**
>   * Get partition number and size for any storage type.
>   *
> @@ -102,28 +95,26 @@ static int getvar_get_part_info(const char *part_name, char *response,
>  				size_t *size)
>  {
>  	int r;
> -# if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>  	struct blk_desc *dev_desc;
> -	struct disk_partition part_info;
> -
> -	r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info,
> -				       response);
> -	if (r >= 0 && size)
> -		*size = part_info.size * part_info.blksz;
> -# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> +	struct disk_partition disk_part;
>  	struct part_info *part_info;
>  
> -	r = fastboot_nand_get_part_info(part_name, &part_info, response);
> -	if (r >= 0 && size)
> -		*size = part_info->size;
> -# else
> -	fastboot_fail("this storage is not supported in bootloader", response);
> -	r = -ENODEV;
> -# endif
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)) {
> +		r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part,
> +					       response);
> +		if (r >= 0 && size)
> +			*size = disk_part.size * disk_part.blksz;
> +	} else if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)) {
> +		r = fastboot_nand_get_part_info(part_name, &part_info, response);
> +		if (r >= 0 && size)
> +			*size = part_info->size;
> +	} else {
> +		fastboot_fail("this storage is not supported in bootloader", response);
> +		r = -ENODEV;
> +	}
>  
>  	return r;
>  }
> -#endif
>  
>  static void getvar_version(char *var_parameter, char *response)
>  {
> @@ -181,8 +172,7 @@ static void getvar_current_slot(char *var_parameter, char *response)
>  	fastboot_okay("a", response);
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> -static void getvar_has_slot(char *part_name, char *response)
> +static void __maybe_unused getvar_has_slot(char *part_name, char *response)
>  {
>  	char part_name_wslot[PART_NAME_LEN];
>  	size_t len;
> @@ -213,10 +203,8 @@ static void getvar_has_slot(char *part_name, char *response)
>  fail:
>  	fastboot_fail("invalid partition name", response);
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
> -static void getvar_partition_type(char *part_name, char *response)
> +static void __maybe_unused getvar_partition_type(char *part_name, char *response)
>  {
>  	int r;
>  	struct blk_desc *dev_desc;
> @@ -232,10 +220,8 @@ static void getvar_partition_type(char *part_name, char *response)
>  			fastboot_okay(fs_get_type_name(), response);
>  	}
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> -static void getvar_partition_size(char *part_name, char *response)
> +static void __maybe_unused getvar_partition_size(char *part_name, char *response)
>  {
>  	int r;
>  	size_t size;
> @@ -244,7 +230,6 @@ static void getvar_partition_size(char *part_name, char *response)
>  	if (r >= 0)
>  		fastboot_response("OKAY", response, "0x%016zx", size);
>  }
> -#endif
>  
>  static void getvar_is_userspace(char *var_parameter, char *response)
>  {
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index 07b1681c8a9a..c6e7f4240758 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -495,7 +495,6 @@ static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req)
>  	do_exit_on_complete(ep, req);
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>  static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
>  {
>  	/* When usb dequeue complete will be called
> @@ -505,7 +504,6 @@ static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
>  	if (req->status == 0)
>  		fastboot_acmd_complete();
>  }
> -#endif
>  
>  static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
>  {
> @@ -546,11 +544,10 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
>  			fastboot_func->in_req->complete = compl_do_reset;
>  			g_dnl_trigger_detach();
>  			break;
> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>  		case FASTBOOT_COMMAND_ACMD:
> -			fastboot_func->in_req->complete = do_acmd_complete;
> +			if (CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT))
> +				fastboot_func->in_req->complete = do_acmd_complete;
>  			break;
> -#endif
>  		}
>  	}
>  
> 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 */
> -- 
> 2.25.1

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Marek Vasut]

On 12/15/22 10: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)

Unrelated to this, don't we need to define Kconfig entries for 'config 
SPL_UDP_FUNCTION_FASTBOOT' and 'config TPL_UDP_FUNCTION_FASTBOOT' and 
the other macros tested in fastboot, so it would be correctly 
configurable in SPL ?

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Sean Anderson]

On 12/15/22 09:30, Marek Vasut wrote:
> On 12/15/22 10: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)
> 
> Unrelated to this, don't we need to define Kconfig entries for 'config SPL_UDP_FUNCTION_FASTBOOT' and 'config TPL_UDP_FUNCTION_FASTBOOT' and the other macros tested in fastboot, so it would be correctly configurable in SPL ?

Is fastboot even supported in SPL? This should probably be IS_ENABLED.

--Sean

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Sean Anderson]

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_SUPPORT, (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?

>  };
>  
>  /**
> @@ -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;
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  /**
>   * flash() - write the downloaded image to the indicated partition.
>   *
> @@ -309,16 +289,15 @@ void fastboot_data_complete(char *response)
>   * Writes the previously downloaded image to the partition indicated by
>   * cmd_parameter. Writes to response.
>   */
> -static void flash(char *cmd_parameter, char *response)
> +static void __maybe_unused flash(char *cmd_parameter, char *response)
>  {
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
> -	fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
> -				 response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> -	fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
> -				  response);
> -#endif
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> +		fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
> +					 image_size, response);
> +
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
> +		fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr,
> +					  image_size, response);
>  }
>  
>  /**
> @@ -330,25 +309,22 @@ static void flash(char *cmd_parameter, char *response)
>   * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
>   * to response.
>   */
> -static void erase(char *cmd_parameter, char *response)
> +static void __maybe_unused erase(char *cmd_parameter, char *response)
>  {
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
> -	fastboot_mmc_erase(cmd_parameter, response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> -	fastboot_nand_erase(cmd_parameter, response);
> -#endif
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> +		fastboot_mmc_erase(cmd_parameter, response);
> +
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
> +		fastboot_nand_erase(cmd_parameter, response);
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>  /**
>   * run_ucmd() - Execute the UCmd command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void run_ucmd(char *cmd_parameter, char *response)
> +static void __maybe_unused run_ucmd(char *cmd_parameter, char *response)
>  {
>  	if (!cmd_parameter) {
>  		pr_err("missing slot suffix\n");
> @@ -375,7 +351,7 @@ void fastboot_acmd_complete(void)
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void run_acmd(char *cmd_parameter, char *response)
> +static void __maybe_unused run_acmd(char *cmd_parameter, char *response)
>  {
>  	if (!cmd_parameter) {
>  		pr_err("missing slot suffix\n");
> @@ -392,7 +368,6 @@ static void run_acmd(char *cmd_parameter, char *response)
>  	strcpy(g_a_cmd_buff, cmd_parameter);
>  	fastboot_okay(NULL, response);
>  }
> -#endif
>  
>  /**
>   * reboot_bootloader() - Sets reboot bootloader flag.
> @@ -436,40 +411,40 @@ static void reboot_recovery(char *cmd_parameter, char *response)
>  		fastboot_okay(NULL, response);
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT)
>  /**
>   * oem_format() - Execute the OEM format command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void oem_format(char *cmd_parameter, char *response)
> +static void __maybe_unused oem_format(char *cmd_parameter, char *response)
>  {
>  	char cmdbuf[32];
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
>  	if (!env_get("partitions")) {
>  		fastboot_fail("partitions not set", response);
>  	} else {
> -		sprintf(cmdbuf, "gpt write mmc %x $partitions",
> -			CONFIG_FASTBOOT_FLASH_MMC_DEV);
> +		sprintf(cmdbuf, "gpt write mmc %x $partitions", mmc_dev);
>  		if (run_command(cmdbuf, 0))
>  			fastboot_fail("", response);
>  		else
>  			fastboot_okay(NULL, response);
>  	}
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF)
>  /**
>   * oem_partconf() - Execute the OEM partconf command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void oem_partconf(char *cmd_parameter, char *response)
> +static void __maybe_unused oem_partconf(char *cmd_parameter, char *response)
>  {
>  	char cmdbuf[32];
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
>  	if (!cmd_parameter) {
>  		fastboot_fail("Expected command parameter", response);
> @@ -477,26 +452,25 @@ static void oem_partconf(char *cmd_parameter, char *response)
>  	}
>  
>  	/* execute 'mmc partconfg' command with cmd_parameter arguments*/
> -	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0",
> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
> +	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0", mmc_dev, cmd_parameter);
>  	printf("Execute: %s\n", cmdbuf);
>  	if (run_command(cmdbuf, 0))
>  		fastboot_fail("Cannot set oem partconf", response);
>  	else
>  		fastboot_okay(NULL, response);
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS)
>  /**
>   * oem_bootbus() - Execute the OEM bootbus command
>   *
>   * @cmd_parameter: Pointer to command parameter
>   * @response: Pointer to fastboot response buffer
>   */
> -static void oem_bootbus(char *cmd_parameter, char *response)
> +static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response)
>  {
>  	char cmdbuf[32];
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
>  	if (!cmd_parameter) {
>  		fastboot_fail("Expected command parameter", response);
> @@ -504,12 +478,10 @@ static void oem_bootbus(char *cmd_parameter, char *response)
>  	}
>  
>  	/* execute 'mmc bootbus' command with cmd_parameter arguments*/
> -	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s",
> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
> +	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s", mmc_dev, cmd_parameter);
>  	printf("Execute: %s\n", cmdbuf);
>  	if (run_command(cmdbuf, 0))
>  		fastboot_fail("Cannot set oem bootbus", response);
>  	else
>  		fastboot_okay(NULL, response);
>  }
> -#endif
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index ef399d0c4abb..7563650d07db 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -91,20 +91,21 @@ void fastboot_okay(const char *reason, char *response)
>   */
>  int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
>  {
> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
>  	static const char * const boot_cmds[] = {
>  		[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
>  		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
>  		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
>  	};
> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
> +
> +	if (!CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
> +		return -EINVAL;
>  
>  	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
>  		return -EINVAL;
>  
> -	return bcb_write_reboot_reason(CONFIG_FASTBOOT_FLASH_MMC_DEV, "misc", boot_cmds[reason]);
> -#else
> -    return -EINVAL;
> -#endif
> +	return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]);
>  }
>  
>  /**
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 018989dd1667..2fbd285db384 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -21,15 +21,9 @@ static void getvar_version_baseband(char *var_parameter, char *response);
>  static void getvar_product(char *var_parameter, char *response);
>  static void getvar_platform(char *var_parameter, char *response);
>  static void getvar_current_slot(char *var_parameter, char *response);
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  static void getvar_has_slot(char *var_parameter, char *response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>  static void getvar_partition_type(char *part_name, char *response);
> -#endif
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  static void getvar_partition_size(char *part_name, char *response);
> -#endif
>  static void getvar_is_userspace(char *var_parameter, char *response);
>  
>  static const struct {
> @@ -84,7 +78,6 @@ static const struct {
>  	}
>  };
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>  /**
>   * Get partition number and size for any storage type.
>   *
> @@ -102,28 +95,26 @@ static int getvar_get_part_info(const char *part_name, char *response,
>  				size_t *size)
>  {
>  	int r;
> -# if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>  	struct blk_desc *dev_desc;
> -	struct disk_partition part_info;
> -
> -	r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info,
> -				       response);
> -	if (r >= 0 && size)
> -		*size = part_info.size * part_info.blksz;
> -# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> +	struct disk_partition disk_part;
>  	struct part_info *part_info;
>  
> -	r = fastboot_nand_get_part_info(part_name, &part_info, response);
> -	if (r >= 0 && size)
> -		*size = part_info->size;
> -# else
> -	fastboot_fail("this storage is not supported in bootloader", response);
> -	r = -ENODEV;
> -# endif
> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)) {
> +		r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part,
> +					       response);
> +		if (r >= 0 && size)
> +			*size = disk_part.size * disk_part.blksz;
> +	} else if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)) {
> +		r = fastboot_nand_get_part_info(part_name, &part_info, response);
> +		if (r >= 0 && size)
> +			*size = part_info->size;
> +	} else {
> +		fastboot_fail("this storage is not supported in bootloader", response);
> +		r = -ENODEV;
> +	}
>  
>  	return r;
>  }
> -#endif
>  
>  static void getvar_version(char *var_parameter, char *response)
>  {
> @@ -181,8 +172,7 @@ static void getvar_current_slot(char *var_parameter, char *response)
>  	fastboot_okay("a", response);
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> -static void getvar_has_slot(char *part_name, char *response)
> +static void __maybe_unused getvar_has_slot(char *part_name, char *response)
>  {
>  	char part_name_wslot[PART_NAME_LEN];
>  	size_t len;
> @@ -213,10 +203,8 @@ static void getvar_has_slot(char *part_name, char *response)
>  fail:
>  	fastboot_fail("invalid partition name", response);
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
> -static void getvar_partition_type(char *part_name, char *response)
> +static void __maybe_unused getvar_partition_type(char *part_name, char *response)
>  {
>  	int r;
>  	struct blk_desc *dev_desc;
> @@ -232,10 +220,8 @@ static void getvar_partition_type(char *part_name, char *response)
>  			fastboot_okay(fs_get_type_name(), response);
>  	}
>  }
> -#endif
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> -static void getvar_partition_size(char *part_name, char *response)
> +static void __maybe_unused getvar_partition_size(char *part_name, char *response)
>  {
>  	int r;
>  	size_t size;
> @@ -244,7 +230,6 @@ static void getvar_partition_size(char *part_name, char *response)
>  	if (r >= 0)
>  		fastboot_response("OKAY", response, "0x%016zx", size);
>  }
> -#endif
>  
>  static void getvar_is_userspace(char *var_parameter, char *response)
>  {
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index 07b1681c8a9a..c6e7f4240758 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -495,7 +495,6 @@ static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req)
>  	do_exit_on_complete(ep, req);
>  }
>  
> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>  static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
>  {
>  	/* When usb dequeue complete will be called
> @@ -505,7 +504,6 @@ static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
>  	if (req->status == 0)
>  		fastboot_acmd_complete();
>  }
> -#endif
>  
>  static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
>  {
> @@ -546,11 +544,10 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
>  			fastboot_func->in_req->complete = compl_do_reset;
>  			g_dnl_trigger_detach();
>  			break;
> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>  		case FASTBOOT_COMMAND_ACMD:
> -			fastboot_func->in_req->complete = do_acmd_complete;
> +			if (CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT))
> +				fastboot_func->in_req->complete = do_acmd_complete;
>  			break;
> -#endif
>  		}
>  	}
>  
> 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>

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Mattijs Korpershoek]

On Thu, Dec 15, 2022 at 14:45, Mattijs Korpershoek <mkorpershoek@baylibre.com> wrote:

> On Thu, Dec 15, 2022 at 10:15, Patrick Delaunay <patrick.delaunay@foss.st.com> 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>
>
> Hi Patrick,
>
> Thank you for this, it's a nice improvement!
>
> I will test this on vim3l tomorrow, but I've already reviewed it:
>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Did a bootloader reflash to MMC following commands from [1]

Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3l


[1] https://source.android.com/docs/setup/create/devices#vim3-fastboot

>
>> ---
>>
>>  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_SUPPORT, (run_ucmd), (NULL))
>>  	},
>>  	[FASTBOOT_COMMAND_ACMD] = {
>>  		.command = "ACmd",
>> -		.dispatch = run_acmd,
>> +		.dispatch = CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT, (run_acmd), (NULL))
>>  	},
>> -#endif
>>  };
>>  
>>  /**
>> @@ -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;
>>  }
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>>  /**
>>   * flash() - write the downloaded image to the indicated partition.
>>   *
>> @@ -309,16 +289,15 @@ void fastboot_data_complete(char *response)
>>   * Writes the previously downloaded image to the partition indicated by
>>   * cmd_parameter. Writes to response.
>>   */
>> -static void flash(char *cmd_parameter, char *response)
>> +static void __maybe_unused flash(char *cmd_parameter, char *response)
>>  {
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>> -	fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
>> -				 response);
>> -#endif
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
>> -	fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr, image_size,
>> -				  response);
>> -#endif
>> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
>> +		fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
>> +					 image_size, response);
>> +
>> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
>> +		fastboot_nand_flash_write(cmd_parameter, fastboot_buf_addr,
>> +					  image_size, response);
>>  }
>>  
>>  /**
>> @@ -330,25 +309,22 @@ static void flash(char *cmd_parameter, char *response)
>>   * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
>>   * to response.
>>   */
>> -static void erase(char *cmd_parameter, char *response)
>> +static void __maybe_unused erase(char *cmd_parameter, char *response)
>>  {
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>> -	fastboot_mmc_erase(cmd_parameter, response);
>> -#endif
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
>> -	fastboot_nand_erase(cmd_parameter, response);
>> -#endif
>> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
>> +		fastboot_mmc_erase(cmd_parameter, response);
>> +
>> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND))
>> +		fastboot_nand_erase(cmd_parameter, response);
>>  }
>> -#endif
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>>  /**
>>   * run_ucmd() - Execute the UCmd command
>>   *
>>   * @cmd_parameter: Pointer to command parameter
>>   * @response: Pointer to fastboot response buffer
>>   */
>> -static void run_ucmd(char *cmd_parameter, char *response)
>> +static void __maybe_unused run_ucmd(char *cmd_parameter, char *response)
>>  {
>>  	if (!cmd_parameter) {
>>  		pr_err("missing slot suffix\n");
>> @@ -375,7 +351,7 @@ void fastboot_acmd_complete(void)
>>   * @cmd_parameter: Pointer to command parameter
>>   * @response: Pointer to fastboot response buffer
>>   */
>> -static void run_acmd(char *cmd_parameter, char *response)
>> +static void __maybe_unused run_acmd(char *cmd_parameter, char *response)
>>  {
>>  	if (!cmd_parameter) {
>>  		pr_err("missing slot suffix\n");
>> @@ -392,7 +368,6 @@ static void run_acmd(char *cmd_parameter, char *response)
>>  	strcpy(g_a_cmd_buff, cmd_parameter);
>>  	fastboot_okay(NULL, response);
>>  }
>> -#endif
>>  
>>  /**
>>   * reboot_bootloader() - Sets reboot bootloader flag.
>> @@ -436,40 +411,40 @@ static void reboot_recovery(char *cmd_parameter, char *response)
>>  		fastboot_okay(NULL, response);
>>  }
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT)
>>  /**
>>   * oem_format() - Execute the OEM format command
>>   *
>>   * @cmd_parameter: Pointer to command parameter
>>   * @response: Pointer to fastboot response buffer
>>   */
>> -static void oem_format(char *cmd_parameter, char *response)
>> +static void __maybe_unused oem_format(char *cmd_parameter, char *response)
>>  {
>>  	char cmdbuf[32];
>> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
>> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>>  
>>  	if (!env_get("partitions")) {
>>  		fastboot_fail("partitions not set", response);
>>  	} else {
>> -		sprintf(cmdbuf, "gpt write mmc %x $partitions",
>> -			CONFIG_FASTBOOT_FLASH_MMC_DEV);
>> +		sprintf(cmdbuf, "gpt write mmc %x $partitions", mmc_dev);
>>  		if (run_command(cmdbuf, 0))
>>  			fastboot_fail("", response);
>>  		else
>>  			fastboot_okay(NULL, response);
>>  	}
>>  }
>> -#endif
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_PARTCONF)
>>  /**
>>   * oem_partconf() - Execute the OEM partconf command
>>   *
>>   * @cmd_parameter: Pointer to command parameter
>>   * @response: Pointer to fastboot response buffer
>>   */
>> -static void oem_partconf(char *cmd_parameter, char *response)
>> +static void __maybe_unused oem_partconf(char *cmd_parameter, char *response)
>>  {
>>  	char cmdbuf[32];
>> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
>> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>>  
>>  	if (!cmd_parameter) {
>>  		fastboot_fail("Expected command parameter", response);
>> @@ -477,26 +452,25 @@ static void oem_partconf(char *cmd_parameter, char *response)
>>  	}
>>  
>>  	/* execute 'mmc partconfg' command with cmd_parameter arguments*/
>> -	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0",
>> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
>> +	snprintf(cmdbuf, sizeof(cmdbuf), "mmc partconf %x %s 0", mmc_dev, cmd_parameter);
>>  	printf("Execute: %s\n", cmdbuf);
>>  	if (run_command(cmdbuf, 0))
>>  		fastboot_fail("Cannot set oem partconf", response);
>>  	else
>>  		fastboot_okay(NULL, response);
>>  }
>> -#endif
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_BOOTBUS)
>>  /**
>>   * oem_bootbus() - Execute the OEM bootbus command
>>   *
>>   * @cmd_parameter: Pointer to command parameter
>>   * @response: Pointer to fastboot response buffer
>>   */
>> -static void oem_bootbus(char *cmd_parameter, char *response)
>> +static void __maybe_unused oem_bootbus(char *cmd_parameter, char *response)
>>  {
>>  	char cmdbuf[32];
>> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
>> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>>  
>>  	if (!cmd_parameter) {
>>  		fastboot_fail("Expected command parameter", response);
>> @@ -504,12 +478,10 @@ static void oem_bootbus(char *cmd_parameter, char *response)
>>  	}
>>  
>>  	/* execute 'mmc bootbus' command with cmd_parameter arguments*/
>> -	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s",
>> -		 CONFIG_FASTBOOT_FLASH_MMC_DEV, cmd_parameter);
>> +	snprintf(cmdbuf, sizeof(cmdbuf), "mmc bootbus %x %s", mmc_dev, cmd_parameter);
>>  	printf("Execute: %s\n", cmdbuf);
>>  	if (run_command(cmdbuf, 0))
>>  		fastboot_fail("Cannot set oem bootbus", response);
>>  	else
>>  		fastboot_okay(NULL, response);
>>  }
>> -#endif
>> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
>> index ef399d0c4abb..7563650d07db 100644
>> --- a/drivers/fastboot/fb_common.c
>> +++ b/drivers/fastboot/fb_common.c
>> @@ -91,20 +91,21 @@ void fastboot_okay(const char *reason, char *response)
>>   */
>>  int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
>>  {
>> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
>>  	static const char * const boot_cmds[] = {
>>  		[FASTBOOT_REBOOT_REASON_BOOTLOADER] = "bootonce-bootloader",
>>  		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
>>  		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
>>  	};
>> +	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
>> +					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>> +
>> +	if (!CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC))
>> +		return -EINVAL;
>>  
>>  	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
>>  		return -EINVAL;
>>  
>> -	return bcb_write_reboot_reason(CONFIG_FASTBOOT_FLASH_MMC_DEV, "misc", boot_cmds[reason]);
>> -#else
>> -    return -EINVAL;
>> -#endif
>> +	return bcb_write_reboot_reason(mmc_dev, "misc", boot_cmds[reason]);
>>  }
>>  
>>  /**
>> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
>> index 018989dd1667..2fbd285db384 100644
>> --- a/drivers/fastboot/fb_getvar.c
>> +++ b/drivers/fastboot/fb_getvar.c
>> @@ -21,15 +21,9 @@ static void getvar_version_baseband(char *var_parameter, char *response);
>>  static void getvar_product(char *var_parameter, char *response);
>>  static void getvar_platform(char *var_parameter, char *response);
>>  static void getvar_current_slot(char *var_parameter, char *response);
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>>  static void getvar_has_slot(char *var_parameter, char *response);
>> -#endif
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>>  static void getvar_partition_type(char *part_name, char *response);
>> -#endif
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>>  static void getvar_partition_size(char *part_name, char *response);
>> -#endif
>>  static void getvar_is_userspace(char *var_parameter, char *response);
>>  
>>  static const struct {
>> @@ -84,7 +78,6 @@ static const struct {
>>  	}
>>  };
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>>  /**
>>   * Get partition number and size for any storage type.
>>   *
>> @@ -102,28 +95,26 @@ static int getvar_get_part_info(const char *part_name, char *response,
>>  				size_t *size)
>>  {
>>  	int r;
>> -# if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>>  	struct blk_desc *dev_desc;
>> -	struct disk_partition part_info;
>> -
>> -	r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info,
>> -				       response);
>> -	if (r >= 0 && size)
>> -		*size = part_info.size * part_info.blksz;
>> -# elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
>> +	struct disk_partition disk_part;
>>  	struct part_info *part_info;
>>  
>> -	r = fastboot_nand_get_part_info(part_name, &part_info, response);
>> -	if (r >= 0 && size)
>> -		*size = part_info->size;
>> -# else
>> -	fastboot_fail("this storage is not supported in bootloader", response);
>> -	r = -ENODEV;
>> -# endif
>> +	if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)) {
>> +		r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part,
>> +					       response);
>> +		if (r >= 0 && size)
>> +			*size = disk_part.size * disk_part.blksz;
>> +	} else if (CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)) {
>> +		r = fastboot_nand_get_part_info(part_name, &part_info, response);
>> +		if (r >= 0 && size)
>> +			*size = part_info->size;
>> +	} else {
>> +		fastboot_fail("this storage is not supported in bootloader", response);
>> +		r = -ENODEV;
>> +	}
>>  
>>  	return r;
>>  }
>> -#endif
>>  
>>  static void getvar_version(char *var_parameter, char *response)
>>  {
>> @@ -181,8 +172,7 @@ static void getvar_current_slot(char *var_parameter, char *response)
>>  	fastboot_okay("a", response);
>>  }
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>> -static void getvar_has_slot(char *part_name, char *response)
>> +static void __maybe_unused getvar_has_slot(char *part_name, char *response)
>>  {
>>  	char part_name_wslot[PART_NAME_LEN];
>>  	size_t len;
>> @@ -213,10 +203,8 @@ static void getvar_has_slot(char *part_name, char *response)
>>  fail:
>>  	fastboot_fail("invalid partition name", response);
>>  }
>> -#endif
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC)
>> -static void getvar_partition_type(char *part_name, char *response)
>> +static void __maybe_unused getvar_partition_type(char *part_name, char *response)
>>  {
>>  	int r;
>>  	struct blk_desc *dev_desc;
>> @@ -232,10 +220,8 @@ static void getvar_partition_type(char *part_name, char *response)
>>  			fastboot_okay(fs_get_type_name(), response);
>>  	}
>>  }
>> -#endif
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
>> -static void getvar_partition_size(char *part_name, char *response)
>> +static void __maybe_unused getvar_partition_size(char *part_name, char *response)
>>  {
>>  	int r;
>>  	size_t size;
>> @@ -244,7 +230,6 @@ static void getvar_partition_size(char *part_name, char *response)
>>  	if (r >= 0)
>>  		fastboot_response("OKAY", response, "0x%016zx", size);
>>  }
>> -#endif
>>  
>>  static void getvar_is_userspace(char *var_parameter, char *response)
>>  {
>> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
>> index 07b1681c8a9a..c6e7f4240758 100644
>> --- a/drivers/usb/gadget/f_fastboot.c
>> +++ b/drivers/usb/gadget/f_fastboot.c
>> @@ -495,7 +495,6 @@ static void do_bootm_on_complete(struct usb_ep *ep, struct usb_request *req)
>>  	do_exit_on_complete(ep, req);
>>  }
>>  
>> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>>  static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
>>  {
>>  	/* When usb dequeue complete will be called
>> @@ -505,7 +504,6 @@ static void do_acmd_complete(struct usb_ep *ep, struct usb_request *req)
>>  	if (req->status == 0)
>>  		fastboot_acmd_complete();
>>  }
>> -#endif
>>  
>>  static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
>>  {
>> @@ -546,11 +544,10 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)
>>  			fastboot_func->in_req->complete = compl_do_reset;
>>  			g_dnl_trigger_detach();
>>  			break;
>> -#if CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT)
>>  		case FASTBOOT_COMMAND_ACMD:
>> -			fastboot_func->in_req->complete = do_acmd_complete;
>> +			if (CONFIG_IS_ENABLED(FASTBOOT_UUU_SUPPORT))
>> +				fastboot_func->in_req->complete = do_acmd_complete;
>>  			break;
>> -#endif
>>  		}
>>  	}
>>  
>> 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 */
>> -- 
>> 2.25.1

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Patrick DELAUNAY]

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

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

On Thu, Dec 15, 2022 at 10:15:50AM +0100, 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
>  }

This probably needs to become an if (CONFIG_IS_ENABLED(...)) { ... }
else { ... } in order to remain size-neutral.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Patrick DELAUNAY]

Hi Tom,

On 1/3/23 21:35, Tom Rini wrote:
> On Thu, Dec 15, 2022 at 10:15:50AM +0100, 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
>>   }
> This probably needs to become an if (CONFIG_IS_ENABLED(...)) { ... }
> else { ... } in order to remain size-neutral.


Are you sure ?


{
     if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
         ....
         return CMD_RET_FAILURE;
     }

     ....

     return CMD_RET_SUCCESS;
}


For me, it is exactly the same size after compiler/linker than :


{
     if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
         ....
         return CMD_RET_FAILURE;
     } else {
     ....
           return CMD_RET_SUCCESS;

     }

}


if UDP_FUNCTION_FASTBOOTis activated or not....

or I forget something during the Christmas break.


Patrick

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2797 bytes --]

On Thu, Jan 05, 2023 at 12:37:58PM +0100, Patrick DELAUNAY wrote:
> Hi Tom,
> 
> On 1/3/23 21:35, Tom Rini wrote:
> > On Thu, Dec 15, 2022 at 10:15:50AM +0100, 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
> > >   }
> > This probably needs to become an if (CONFIG_IS_ENABLED(...)) { ... }
> > else { ... } in order to remain size-neutral.
> 
> 
> Are you sure ?
> 
> 
> {
>     if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
>         ....
>         return CMD_RET_FAILURE;
>     }
> 
>     ....
> 
>     return CMD_RET_SUCCESS;
> }
> 
> 
> For me, it is exactly the same size after compiler/linker than :
> 
> 
> {
>     if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
>         ....
>         return CMD_RET_FAILURE;
>     } else {
>     ....
>           return CMD_RET_SUCCESS;
> 
>     }
> 
> }
> 
> 
> if UDP_FUNCTION_FASTBOOTis activated or not....
> 
> or I forget something during the Christmas break.

If you've size-tested and it's the same, OK. I'm just worried about
strings not being discarded since that's sometimes an unexpected side
effect / bug.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] fastboot: remove #ifdef CONFIG when it is possible

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 547 bytes --]

On Thu, Dec 15, 2022 at 10:15:50AM +0100, 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>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Reviewed-by: Sean Anderson <sean.anderson@seco.com>
> Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on vim3l

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]