Dear Andreas,
On 08/25/2011 11:40 AM, Andreas Bießmann wrote:
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 Schwarzsimonschwarzcor@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 Schwarzschwarz@corscience.de
- Added prep subcommand support
- (C) Copyright 2002
- Sysgo Real-Time Solutions, GmbH<www.elinos.com>
- Marius Groegermgroeger@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.
I added this because of compiler warnings that it could be used uninitialized.
#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.
Hehe. Somehow I did know that this topic will come up. I intended to change the code as little as possible.
So essentially this would mean to rewrite this to reflect the structure of the ppc version. Will do if there are no objections.
If there are no objections I also would like to separate this patch from this series. This has some advantages: - Support for the prep subcommand is essential for saving the boot parameters. (if prep is in saving can also be done manually) - I think that there won't be much discussion about the usefulness of implementing this - just some about the how.
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);
- 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);
- setup_serial_tag (¶ms);
setup_serial_tag(¶ms);
- setup_revision_tag (¶ms);
setup_revision_tag(¶ms);
- setup_memory_tags (bd);
setup_memory_tags(bd);
- setup_commandline_tag (bd, commandline);
setup_commandline_tag(bd, commandline);
- 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);
- setup_end_tag(bd);
setup_end_tag(bd);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?
flag = 0 means that no subcommand was issued -> execute everything.
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.
see 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.
If no objections - will be done in the rewrite.
- 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?
This is done to be able to get the fdt address in arch-dependent savebp cmd. Suggestions for a cleaner solution are welcome ;)
/* Allow for arch specific config before we boot */ void __arch_preboot_os(void)
regards
Andreas Bießmann
Regards, thx for reviewing Simon