Re: [U-Boot] [PATCH V3 1/8] arm: Add Prep subcommand support to bootm

Dear Simon,

Am 25.08.2011 10:33, schrieb Simon Schwarz:

Adds prep subcommand to bootm implementation of ARM. When bootm is called with the subcommand prep the function stops right after ATAGS creation and before announce_and_cleanup.

This is used in savebp command

Signed-off-by: Simon Schwarz simonschwarzcor@gmail.com

V2 changes: nothing

V3 changes: nothing

---

arch/arm/lib/bootm.c | 116 +++++++++++++++++++++++++++---------------------- common/cmd_bootm.c | 2 +- 2 files changed, 65 insertions(+), 53 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 802e833..d3152ae 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -1,4 +1,7 @@ -/* +/* Copyright (C) 2011

• • Corscience GmbH & Co. KG - Simon Schwarz schwarz@corscience.de
• • • Added prep subcommand support
• • (C) Copyright 2002
  • Sysgo Real-Time Solutions, GmbH <www.elinos.com>
  • Marius Groeger mgroeger@sysgo.de

@@ -55,7 +58,7 @@ static struct tag *params;

static ulong get_sp(void); #if defined(CONFIG_OF_LIBFDT) -static int bootm_linux_fdt(int machid, bootm_headers_t *images); +static int bootm_linux_fdt(int machid, bootm_headers_t *images, int flag); #endif

void arch_lmb_reserve(struct lmb *lmb) @@ -98,63 +101,67 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) bd_t *bd = gd->bd; char *s; int machid = bd->bi_arch_number;

• void (*kernel_entry)(int zero, int arch, uint params);

• void (*kernel_entry)(int zero, int arch, uint params) = NULL;

This should not be necessary, kernel_entry would be on bss which should be initialized to zero by start.S.

#ifdef CONFIG_CMDLINE_TAG char *commandline = getenv ("bootargs"); #endif

• if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))

• return 1;
  

• s = getenv ("machid");
• if (s) {

• machid = simple_strtoul (s, NULL, 16);
  

• printf ("Using machid 0x%x from environment\n", machid);
  

• }
• show_boot_progress (15);

• if ((flag != 0) && (!(flag & BOOTM_STATE_OS_GO ||

• flag & BOOTM_STATE_OS_PREP)))
  

switch'n'case would be much cleaner here. And seperating the functionality into functions would be nice too.

• return 1; /* subcommand not implemented */
  

• else if ((flag == 0) || flag & BOOTM_STATE_OS_PREP) {

• s = getenv("machid");
  

• if (s) {
  

• 	strict_strtoul(s, 16, (long unsigned int *) &machid);
  

• 	printf("Using machid 0x%x from environment\n", machid);
  

• }
  

• show_boot_progress(15);
  
  

#ifdef CONFIG_OF_LIBFDT

• if (images->ft_len)

• return bootm_linux_fdt(machid, images);
  

• if (images->ft_len)
  

• 	return bootm_linux_fdt(machid, images, flag);
  

#endif

• kernel_entry = (void (*)(int, int, uint))images->ep;

• kernel_entry = (void (*)(int, int, uint))images->ep;
  
  

• debug ("## Transferring control to Linux (at address %08lx) ...\n",

•       (ulong) kernel_entry);
  

• debug("## Transferring control to Linux (at address %08lx)" \
  

• 	"...\n", (ulong) kernel_entry);
  
  

#if defined (CONFIG_SETUP_MEMORY_TAGS) || \ defined (CONFIG_CMDLINE_TAG) || \ defined (CONFIG_INITRD_TAG) || \ defined (CONFIG_SERIAL_TAG) || \ defined (CONFIG_REVISION_TAG)

• setup_start_tag (bd);

• setup_start_tag(bd);
  

#ifdef CONFIG_SERIAL_TAG

• setup_serial_tag (&params);

• setup_serial_tag(&params);
  

#endif #ifdef CONFIG_REVISION_TAG

• setup_revision_tag (&params);

• setup_revision_tag(&params);
  

#endif #ifdef CONFIG_SETUP_MEMORY_TAGS

• setup_memory_tags (bd);

• setup_memory_tags(bd);
  

#endif #ifdef CONFIG_CMDLINE_TAG

• setup_commandline_tag (bd, commandline);

• setup_commandline_tag(bd, commandline);
  

#endif #ifdef CONFIG_INITRD_TAG

• if (images->rd_start && images->rd_end)

• setup_initrd_tag (bd, images->rd_start, images->rd_end);
  

• if (images->rd_start && images->rd_end)
  

• 	setup_initrd_tag(bd, images->rd_start, images->rd_end);
  

#endif

• setup_end_tag(bd);

• setup_end_tag(bd);
  

#endif

• if (flag & BOOTM_STATE_OS_PREP)
  

• 	return 0;
  

• }

• announce_and_cleanup();
• kernel_entry(0, machid, bd->bi_boot_params);
• /* does not return */

• if (flag == 0 || flag & BOOTM_STATE_OS_GO) {

flag == 0? Shouldn't that be flag == BOOTM_STATE_OS_GO?

• announce_and_cleanup();
  
  

• kernel_entry(0, machid, bd->bi_boot_params);
  

• /* does not return */
  

• } return 1;

}

@@ -174,10 +181,10 @@ static int fixup_memory_node(void *blob) return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS); }

-static int bootm_linux_fdt(int machid, bootm_headers_t *images) +static int bootm_linux_fdt(int machid, bootm_headers_t *images, int flag) { ulong rd_len;

• void (*kernel_entry)(int zero, int dt_machid, void *dtblob);

• void (*kernel_entry)(int zero, int dt_machid, void *dtblob) = NULL;

same as above.

ulong of_size = images->ft_len; char **of_flat_tree = &images->ft_addr; ulong *initrd_start = &images->initrd_start; @@ -185,34 +192,39 @@ static int bootm_linux_fdt(int machid, bootm_headers_t *images) struct lmb *lmb = &images->lmb; int ret;

• kernel_entry = (void (*)(int, int, void *))images->ep;
• boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);

• if ((flag == 0) || flag & BOOTM_STATE_OS_PREP) {

• kernel_entry = (void (*)(int, int, void *))images->ep;
  
  

same as above, a cleaner aproach is to gather information for fdt in a dedicated function and in another do the hand-over to linux kernel.

• rd_len = images->rd_end - images->rd_start;
• ret = boot_ramdisk_high(lmb, images->rd_start, rd_len,

• 		initrd_start, initrd_end);
  

• if (ret)

• return ret;
  

• boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
  
  

• ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
• if (ret)

• return ret;
  

• rd_len = images->rd_end - images->rd_start;
  

• ret = boot_ramdisk_high(lmb, images->rd_start, rd_len,
  

• 			initrd_start, initrd_end);
  

• if (ret)
  

• 	return ret;
  
  

• debug("## Transferring control to Linux (at address %08lx) ...\n",

•       (ulong) kernel_entry);
  

• ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
  

• if (ret)
  

• 	return ret;
  
  

• fdt_chosen(*of_flat_tree, 1);

• debug("## Transferring control to Linux (at address %08lx)" \
  

• 	"...\n", (ulong) kernel_entry);
  
  

• fixup_memory_node(*of_flat_tree);

• fdt_chosen(*of_flat_tree, 1);
  
  

• fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);

• fixup_memory_node(*of_flat_tree);
  
  

• announce_and_cleanup();

• fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
  
  

• kernel_entry(0, machid, *of_flat_tree);
• /* does not return */

• if (flag & BOOTM_STATE_OS_PREP)
  

• 	return 0;
  

• }

• if (flag == 0 || flag & BOOTM_STATE_OS_GO) {

• announce_and_cleanup();
  
  

• kernel_entry(0, machid, *of_flat_tree);
  

• /* does not return */
  

• } return 1;

} #endif diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 1966da4..c642299 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -156,7 +156,7 @@ static boot_os_fn *boot_os[] = { #endif };

-static bootm_headers_t images; /* pointers to os/initrd/fdt images */ +bootm_headers_t images; /* pointers to os/initrd/fdt images */

is this necessary?

/* Allow for arch specific config before we boot */ void __arch_preboot_os(void)

regards

Andreas Bießmann