Refactor the bootm command to reduce code duplication

At present the bootm code is mostly duplicated for the plain 'bootm'
command and its sub-command variant. This makes the code harder to
maintain and means that changes must be made to several places.

Introduce do_bootm_states() which performs selected portions of the bootm
work, so that both plain 'bootm' and 'bootm <sub_command>' can use the
same code.

Additional duplication exists in bootz, so tidy that up as well. This
is not intended to change behaviour, apart from minor fixes where the
previously-duplicated code missed some chunks of code.

Signed-off-by: Simon Glass <sjg@chromium.org>

common/cmd_bootm.c

@@ -105,7 +105,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
  *  - loaded (first part of) image to header load address,
  *  - disabled interrupts.
  *
- * @flag: Command flags (CMD_FLAG_...)
+ * @flag: Flags indicating what to do (BOOTM_STATE_...)
  * @argc: Number of arguments. Note that the arguments are shifted down
  *	 so that 0 is the first argument not processed by U-Boot, and
  *	 argc is adjusted accordingly. This avoids confusion as to how
@@ -208,15 +208,21 @@ static inline void boot_start_lmb(bootm_headers_t *images) { }
 
 static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	const void *os_hdr;
-	int ret;
-
 	memset((void *)&images, 0, sizeof(images));
 	images.verify = getenv_yesno("verify");
 
 	boot_start_lmb(&images);
 
 	bootstage_mark_name(BOOTSTAGE_ID_BOOTM_START, "bootm_start");
+	images.state = BOOTM_STATE_START;
+
+	return 0;
+}
+
+static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char * const argv[])
+{
+	const void *os_hdr;
 
 	/* get kernel image header, start address and length */
 	os_hdr = boot_get_kernel(cmdtp, flag, argc, argv,
@@ -279,6 +285,8 @@ static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 		images.ep = image_get_ep(&images.legacy_hdr_os_copy);
 #if defined(CONFIG_FIT)
 	} else if (images.fit_uname_os) {
+		int ret;
+
 		ret = fit_image_get_entry(images.fit_hdr_os,
 					  images.fit_noffset_os, &images.ep);
 		if (ret) {
@@ -296,6 +304,16 @@ static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 		images.ep += images.os.load;
 	}
 
+	images.os.start = (ulong)os_hdr;
+
+	return 0;
+}
+
+static int bootm_find_other(cmd_tbl_t *cmdtp, int flag, int argc,
+			    char * const argv[])
+{
+	int ret;
+
 	if (((images.os.type == IH_TYPE_KERNEL) ||
 	     (images.os.type == IH_TYPE_KERNEL_NOLOAD) ||
 	     (images.os.type == IH_TYPE_MULTI)) &&
@@ -321,9 +339,6 @@ static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 #endif
 	}
 
-	images.os.start = (ulong)os_hdr;
-	images.state = BOOTM_STATE_START;
-
 	return 0;
 }
 
@@ -455,7 +470,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	return 0;
 }
 
-static int bootm_start_standalone(ulong iflag, int argc, char * const argv[])
+static int bootm_start_standalone(int argc, char * const argv[])
 {
 	char  *s;
 	int   (*appl)(int, char * const []);
@@ -487,103 +502,208 @@ static cmd_tbl_t cmd_bootm_sub[] = {
 	U_BOOT_CMD_MKENT(go, 0, 1, (void *)BOOTM_STATE_OS_GO, "", ""),
 };
 
+static int boot_selected_os(int argc, char * const argv[], int state,
+		bootm_headers_t *images, boot_os_fn *boot_fn, ulong *iflag)
+{
+	if (images->os.type == IH_TYPE_STANDALONE) {
+		/* This may return when 'autostart' is 'no' */
+		bootm_start_standalone(argc, argv);
+		return 0;
+	}
+	/*
+	 * We have reached the point of no return: we are going to
+	 * overwrite all exception vector code, so we cannot easily
+	 * recover from any failures any more...
+	 */
+	*iflag = disable_interrupts();
+#ifdef CONFIG_NETCONSOLE
+	/* Stop the ethernet stack if NetConsole could have left it up */
+	eth_halt();
+#endif
+
+#if defined(CONFIG_CMD_USB)
+	/*
+	 * turn off USB to prevent the host controller from writing to the
+	 * SDRAM while Linux is booting. This could happen (at least for OHCI
+	 * controller), because the HCCA (Host Controller Communication Area)
+	 * lies within the SDRAM and the host controller writes continously to
+	 * this area (as busmaster!). The HccaFrameNumber is for example
+	 * updated every 1 ms within the HCCA structure in SDRAM! For more
+	 * details see the OpenHCI specification.
+	 */
+	usb_stop();
+#endif
+#ifdef CONFIG_SILENT_CONSOLE
+	if (images->os.os == IH_OS_LINUX)
+		fixup_silent_linux();
+#endif
+	arch_preboot_os();
+	boot_fn(state, argc, argv, images);
+	bootstage_error(BOOTSTAGE_ID_BOOT_OS_RETURNED);
+#ifdef DEBUG
+	puts("\n## Control returned to monitor - resetting...\n");
+#endif
+	return BOOTM_ERR_RESET;
+}
+
+/**
+ * Execute selected states of the bootm command.
+ *
+ * Note the arguments to this state must be the first argument, Any 'bootm'
+ * or sub-command arguments must have already been taken.
+ *
+ * Note that if states contains more than one flag it MUST contain
+ * BOOTM_STATE_START, since this handles and consumes the command line args.
+ *
+ * @param cmdtp		Pointer to bootm command table entry
+ * @param flag		Command flags (CMD_FLAG_...)
+ * @param argc		Number of subcommand arguments (0 = no arguments)
+ * @param argv		Arguments
+ * @param states	Mask containing states to run (BOOTM_STATE_...)
+ * @param images	Image header information
+ * @param boot_progress 1 to show boot progress, 0 to not do this
+ * @return 0 if ok, something else on error. Some errors will cause this
+ *	function to perform a reboot! If states contains BOOTM_STATE_OS_GO
+ *	then the intent is to boot an OS, so this function will not return
+ *	unless the image type is standalone.
+ */
+static int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc,
+		char * const argv[], int states, bootm_headers_t *images,
+		int boot_progress)
+{
+	boot_os_fn *boot_fn;
+	ulong iflag = 0;
+	int ret = 0;
+
+	images->state |= states;
+
+	/*
+	 * Work through the states and see how far we get. We stop on
+	 * any error.
+	 */
+	if (states & BOOTM_STATE_START)
+		ret = bootm_start(cmdtp, flag, argc, argv);
+
+	if (!ret && (states & BOOTM_STATE_FINDOS))
+		ret = bootm_find_os(cmdtp, flag, argc, argv);
+
+	if (!ret && (states & BOOTM_STATE_FINDOTHER)) {
+		ret = bootm_find_other(cmdtp, flag, argc, argv);
+		argc = 0;	/* consume the args */
+	}
+
+	/* Load the OS */
+	if (!ret && (states & BOOTM_STATE_LOADOS)) {
+		ulong load_end;
+
+		ret = bootm_load_os(images->os, &load_end, 0);
+		if (!ret) {
+			lmb_reserve(&images->lmb, images->os.load,
+				    (load_end - images->os.load));
+		}
+	}
+
+	/* Relocate the ramdisk */
+#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
+	if (!ret && (states & BOOTM_STATE_RAMDISK)) {
+		ulong rd_len = images->rd_end - images->rd_start;
+
+		ret = boot_ramdisk_high(&images->lmb, images->rd_start,
+			rd_len, &images->initrd_start, &images->initrd_end);
+		if (!ret) {
+			setenv_hex("initrd_start", images->initrd_start);
+			setenv_hex("initrd_end", images->initrd_end);
+		}
+	}
+#endif
+#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_LMB)
+	if (!ret && (states & BOOTM_STATE_FDT)) {
+		boot_fdt_add_mem_rsv_regions(&images->lmb, images->ft_addr);
+		ret = boot_relocate_fdt(&images->lmb, &images->ft_addr,
+					&images->ft_len);
+	}
+#endif
+
+	/* From now on, we need the OS boot function */
+	if (ret)
+		return ret;
+	boot_fn = boot_os[images->os.os];
+	if (boot_fn == NULL) {
+		if (iflag)
+			enable_interrupts();
+		printf("ERROR: booting os '%s' (%d) is not supported\n",
+		       genimg_get_os_name(images->os.os), images->os.os);
+		bootstage_error(BOOTSTAGE_ID_CHECK_BOOT_OS);
+		return 1;
+	}
+
+	/* Call various other states that are not generally used */
+	if (!ret && (states & BOOTM_STATE_OS_CMDLINE))
+		ret = boot_fn(BOOTM_STATE_OS_CMDLINE, argc, argv, images);
+	if (!ret && (states & BOOTM_STATE_OS_BD_T))
+		ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images);
+	if (!ret && (states & BOOTM_STATE_OS_PREP))
+		ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images);
+
+	/* Now run the OS! We hope this doesn't return */
+	if (!ret && (states & BOOTM_STATE_OS_GO))
+		ret = boot_selected_os(argc, argv, BOOTM_STATE_OS_GO,
+				images, boot_fn, &iflag);
+
+	/* Deal with any fallout */
+	if (ret < 0) {
+		if (ret == BOOTM_ERR_UNIMPLEMENTED) {
+			if (iflag)
+				enable_interrupts();
+			bootstage_error(BOOTSTAGE_ID_DECOMP_UNIMPL);
+			return 1;
+		} else if (ret == BOOTM_ERR_OVERLAP) {
+			if (images->legacy_hdr_valid) {
+				if (image_get_type(&images->legacy_hdr_os_copy)
+						== IH_TYPE_MULTI)
+					puts("WARNING: legacy format multi component image overwritten\n");
+			} else {
+				puts("ERROR: new format image overwritten - must RESET the board to recover\n");
+				bootstage_error(BOOTSTAGE_ID_OVERWRITTEN);
+				ret = BOOTM_ERR_RESET;
+			}
+		}
+		if (ret == BOOTM_ERR_RESET)
+			do_reset(cmdtp, flag, argc, argv);
+	}
+	if (iflag)
+		enable_interrupts();
+	if (ret)
+		puts("subcommand not supported\n");
+
+	return ret;
+}
+
 static int do_bootm_subcommand(cmd_tbl_t *cmdtp, int flag, int argc,
 			char * const argv[])
 {
 	int ret = 0;
 	long state;
 	cmd_tbl_t *c;
-	boot_os_fn *boot_fn;
 
 	c = find_cmd_tbl(argv[0], &cmd_bootm_sub[0], ARRAY_SIZE(cmd_bootm_sub));
 	argc--; argv++;
 
 	if (c) {
 		state = (long)c->cmd;
-
-		/* treat start special since it resets the state machine */
 		if (state == BOOTM_STATE_START)
-			return bootm_start(cmdtp, flag, argc, argv);
+			state |= BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER;
 	} else {
 		/* Unrecognized command */
 		return CMD_RET_USAGE;
 	}
 
-	if (images.state < BOOTM_STATE_START ||
-	    images.state >= state) {
+	if (state != BOOTM_STATE_START && images.state >= state) {
 		printf("Trying to execute a command out of order\n");
 		return CMD_RET_USAGE;
 	}
 
-	images.state |= state;
-	boot_fn = boot_os[images.os.os];
-
-	switch (state) {
-		ulong load_end;
-		case BOOTM_STATE_START:
-			/* should never occur */
-			break;
-		case BOOTM_STATE_LOADOS:
-			ret = bootm_load_os(images.os, &load_end, 0);
-			if (ret)
-				return ret;
-
-			lmb_reserve(&images.lmb, images.os.load,
-					(load_end - images.os.load));
-			break;
-#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
-		case BOOTM_STATE_RAMDISK:
-		{
-			ulong rd_len = images.rd_end - images.rd_start;
-
-			ret = boot_ramdisk_high(&images.lmb, images.rd_start,
-				rd_len, &images.initrd_start, &images.initrd_end);
-			if (ret)
-				return ret;
-
-			setenv_hex("initrd_start", images.initrd_start);
-			setenv_hex("initrd_end", images.initrd_end);
-		}
-			break;
-#endif
-#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_LMB)
-		case BOOTM_STATE_FDT:
-		{
-			boot_fdt_add_mem_rsv_regions(&images.lmb,
-						     images.ft_addr);
-			ret = boot_relocate_fdt(&images.lmb,
-				&images.ft_addr, &images.ft_len);
-			break;
-		}
-#endif
-		case BOOTM_STATE_OS_CMDLINE:
-			ret = boot_fn(BOOTM_STATE_OS_CMDLINE, argc, argv, &images);
-			if (ret)
-				printf("cmdline subcommand not supported\n");
-			break;
-		case BOOTM_STATE_OS_BD_T:
-			ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, &images);
-			if (ret)
-				printf("bdt subcommand not supported\n");
-			break;
-		case BOOTM_STATE_OS_PREP:
-			ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, &images);
-			if (ret)
-				printf("prep subcommand not supported\n");
-			break;
-		case BOOTM_STATE_OS_GO:
-			disable_interrupts();
-#ifdef CONFIG_NETCONSOLE
-			/*
-			 * Stop the ethernet stack if NetConsole could have
-			 * left it up
-			 */
-			eth_halt();
-#endif
-			arch_preboot_os();
-			boot_fn(BOOTM_STATE_OS_GO, argc, argv, &images);
-			break;
-	}
+	ret = do_bootm_states(cmdtp, flag, argc, argv, state, &images, 0);
 
 	return ret;
 }
@@ -594,10 +714,6 @@ static int do_bootm_subcommand(cmd_tbl_t *cmdtp, int flag, int argc,
 
 int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	ulong		iflag;
-	ulong		load_end = 0;
-	int		ret;
-	boot_os_fn	*boot_fn;
 #ifdef CONFIG_NEEDS_MANUAL_RELOC
 	static int relocated = 0;
 
@@ -635,101 +751,10 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			return do_bootm_subcommand(cmdtp, flag, argc, argv);
 	}
 
-	if (bootm_start(cmdtp, flag, argc, argv))
-		return 1;
-
-	/*
-	 * We have reached the point of no return: we are going to
-	 * overwrite all exception vector code, so we cannot easily
-	 * recover from any failures any more...
-	 */
-	iflag = disable_interrupts();
-
-#ifdef CONFIG_NETCONSOLE
-	/* Stop the ethernet stack if NetConsole could have left it up */
-	eth_halt();
-#endif
-
-#if defined(CONFIG_CMD_USB)
-	/*
-	 * turn off USB to prevent the host controller from writing to the
-	 * SDRAM while Linux is booting. This could happen (at least for OHCI
-	 * controller), because the HCCA (Host Controller Communication Area)
-	 * lies within the SDRAM and the host controller writes continously to
-	 * this area (as busmaster!). The HccaFrameNumber is for example
-	 * updated every 1 ms within the HCCA structure in SDRAM! For more
-	 * details see the OpenHCI specification.
-	 */
-	usb_stop();
-#endif
-
-	ret = bootm_load_os(images.os, &load_end, 1);
-
-	if (ret < 0) {
-		if (ret == BOOTM_ERR_RESET)
-			do_reset(cmdtp, flag, argc, argv);
-		if (ret == BOOTM_ERR_OVERLAP) {
-			if (images.legacy_hdr_valid) {
-				image_header_t *hdr;
-				hdr = &images.legacy_hdr_os_copy;
-				if (image_get_type(hdr) == IH_TYPE_MULTI)
-					puts("WARNING: legacy format multi "
-						"component image "
-						"overwritten\n");
-			} else {
-				puts("ERROR: new format image overwritten - "
-					"must RESET the board to recover\n");
-				bootstage_error(BOOTSTAGE_ID_OVERWRITTEN);
-				do_reset(cmdtp, flag, argc, argv);
-			}
-		}
-		if (ret == BOOTM_ERR_UNIMPLEMENTED) {
-			if (iflag)
-				enable_interrupts();
-			bootstage_error(BOOTSTAGE_ID_DECOMP_UNIMPL);
-			return 1;
-		}
-	}
-
-	lmb_reserve(&images.lmb, images.os.load, (load_end - images.os.load));
-
-	if (images.os.type == IH_TYPE_STANDALONE) {
-		if (iflag)
-			enable_interrupts();
-		/* This may return when 'autostart' is 'no' */
-		bootm_start_standalone(iflag, argc, argv);
-		return 0;
-	}
-
-	bootstage_mark(BOOTSTAGE_ID_CHECK_BOOT_OS);
-
-#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
-	if (images.os.os == IH_OS_LINUX)
-		fixup_silent_linux();
-#endif
-
-	boot_fn = boot_os[images.os.os];
-
-	if (boot_fn == NULL) {
-		if (iflag)
-			enable_interrupts();
-		printf("ERROR: booting os '%s' (%d) is not supported\n",
-			genimg_get_os_name(images.os.os), images.os.os);
-		bootstage_error(BOOTSTAGE_ID_CHECK_BOOT_OS);
-		return 1;
-	}
-
-	arch_preboot_os();
-
-	boot_fn(0, argc, argv, &images);
-
-	bootstage_error(BOOTSTAGE_ID_BOOT_OS_RETURNED);
-#ifdef DEBUG
-	puts("\n## Control returned to monitor - resetting...\n");
-#endif
-	do_reset(cmdtp, flag, argc, argv);
-
-	return 1;
+	return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
+		BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
+		BOOTM_STATE_LOADOS | BOOTM_STATE_OS_PREP |
+		BOOTM_STATE_OS_GO, &images, 1);
 }
 
 int bootm_maybe_autostart(cmd_tbl_t *cmdtp, const char *cmd)
@@ -1671,9 +1696,8 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc,
 	int ret;
 	void *zi_start, *zi_end;
 
-	memset(images, 0, sizeof(bootm_headers_t));
-
-	boot_start_lmb(images);
+	ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START,
+			      images, 1);
 
 	/* Setup Linux kernel zImage entry point */
 	if (argc < 2) {
@@ -1692,73 +1716,24 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	lmb_reserve(&images->lmb, images->ep, zi_end - zi_start);
 
-	/* Find ramdisk */
-	ret = boot_get_ramdisk(argc, argv, images, IH_INITRD_ARCH,
-			&images->rd_start, &images->rd_end);
-	if (ret) {
-		puts("Ramdisk image is corrupt or invalid\n");
-		return 1;
-	}
-
-#if defined(CONFIG_OF_LIBFDT)
-	/* find flattened device tree */
-	ret = boot_get_fdt(flag, argc, argv, IH_ARCH_DEFAULT, images,
-			   &images->ft_addr, &images->ft_len);
-	if (ret) {
-		puts("Could not find a valid device tree\n");
-		return 1;
-	}
-
-	set_working_fdt_addr(images->ft_addr);
-#endif
+	ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_FINDOTHER,
+			      images, 1);
 
-	return 0;
+	return ret;
 }
 
 int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	bootm_headers_t	images;
+	int ret;
 
 	if (bootz_start(cmdtp, flag, argc, argv, &images))
 		return 1;
 
-	/*
-	 * We have reached the point of no return: we are going to
-	 * overwrite all exception vector code, so we cannot easily
-	 * recover from any failures any more...
-	 */
-	disable_interrupts();
-
-#ifdef CONFIG_NETCONSOLE
-	/* Stop the ethernet stack if NetConsole could have left it up */
-	eth_halt();
-#endif
+	ret = do_bootm_states(cmdtp, flag, argc, argv,
+			      BOOTM_STATE_OS_GO, &images, 1);
 
-#if defined(CONFIG_CMD_USB)
-	/*
-	 * turn off USB to prevent the host controller from writing to the
-	 * SDRAM while Linux is booting. This could happen (at least for OHCI
-	 * controller), because the HCCA (Host Controller Communication Area)
-	 * lies within the SDRAM and the host controller writes continously to
-	 * this area (as busmaster!). The HccaFrameNumber is for example
-	 * updated every 1 ms within the HCCA structure in SDRAM! For more
-	 * details see the OpenHCI specification.
-	 */
-	usb_stop();
-#endif
-
-#if defined(CONFIG_SILENT_CONSOLE) && !defined(CONFIG_SILENT_U_BOOT_ONLY)
-	fixup_silent_linux();
-#endif
-	arch_preboot_os();
-
-	do_bootm_linux(0, argc, argv, &images);
-#ifdef DEBUG
-	puts("\n## Control returned to monitor - resetting...\n");
-#endif
-	do_reset(cmdtp, flag, argc, argv);
-
-	return 1;
+	return ret;
 }
 
 #ifdef CONFIG_SYS_LONGHELP

include/image.h

@@ -320,13 +320,15 @@ typedef struct bootm_headers {
 	int		verify;		/* getenv("verify")[0] != 'n' */
 
 #define	BOOTM_STATE_START	(0x00000001)
-#define	BOOTM_STATE_LOADOS	(0x00000002)
-#define	BOOTM_STATE_RAMDISK	(0x00000004)
-#define	BOOTM_STATE_FDT		(0x00000008)
-#define	BOOTM_STATE_OS_CMDLINE	(0x00000010)
-#define	BOOTM_STATE_OS_BD_T	(0x00000020)
-#define	BOOTM_STATE_OS_PREP	(0x00000040)
-#define	BOOTM_STATE_OS_GO	(0x00000080)
+#define	BOOTM_STATE_FINDOS	(0x00000002)
+#define	BOOTM_STATE_FINDOTHER	(0x00000004)
+#define	BOOTM_STATE_LOADOS	(0x00000008)
+#define	BOOTM_STATE_RAMDISK	(0x00000010)
+#define	BOOTM_STATE_FDT		(0x00000020)
+#define	BOOTM_STATE_OS_CMDLINE	(0x00000040)
+#define	BOOTM_STATE_OS_BD_T	(0x00000080)
+#define	BOOTM_STATE_OS_PREP	(0x00000100)
+#define	BOOTM_STATE_OS_GO	(0x00000200)
 	int		state;
 
 #ifdef CONFIG_LMB