diff mbox

[U-Boot] arm: Add Prep subcommand support to bootm

Message ID 1314634093-8494-1-git-send-email-simonschwarzcor@gmail.com
State Superseded
Headers show

Commit Message

Simon Schwarz Aug. 29, 2011, 4:08 p.m. UTC
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

changes after slicing this from patch
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106422:
DEL Prototype declaration - not necessary if reordered
DEL Most #ifdefs - relying on -ffunction-sections and --gc-sections to handle unused
CHG reorganized bootm. powerpc implementation was the model
ADD get_board_serial fake implementation - this is needed if setup_serial_tag
	is compiled but the board doesn't support it - tradeoff for removing
	#ifdefs
---
 arch/arm/lib/bootm.c |  330 +++++++++++++++++++++++++------------------------
 1 files changed, 168 insertions(+), 162 deletions(-)

Comments

Andreas Bießmann Sept. 5, 2011, 10:58 a.m. UTC | #1
Dear Simon,

Am Mo 29 Aug 2011 18:08:13 CEST, Simon Schwarz schrieb:
> 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

savebp? I thought this command is now 'spl' with some subcommands.

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

this four times '-' will not be recognized as comment section by git am

>
> V2 changes:
> nothing
>
> V3 changes:
> nothing
>
> changes after slicing this from patch
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106422:
> DEL Prototype declaration - not necessary if reordered
> DEL Most #ifdefs - relying on -ffunction-sections and --gc-sections to handle unused
> CHG reorganized bootm. powerpc implementation was the model
> ADD get_board_serial fake implementation - this is needed if setup_serial_tag
> 	is compiled but the board doesn't support it - tradeoff for removing
> 	#ifdefs
> ---
>  arch/arm/lib/bootm.c |  330 +++++++++++++++++++++++++------------------------
>  1 files changed, 168 insertions(+), 162 deletions(-)
>
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 802e833..5f112e2 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -1,4 +1,8 @@
> -/*
> +/* Copyright (C) 2011
> + * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@corscience.de>
> + *  - Added prep subcommand support
> + *  - Reorganized source - modeled after powerpc version
> + *
>   * (C) Copyright 2002
>   * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>   * Marius Groeger <mgroeger@sysgo.de>
> @@ -32,31 +36,16 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -#if defined (CONFIG_SETUP_MEMORY_TAGS) || \
> -    defined (CONFIG_CMDLINE_TAG) || \
> -    defined (CONFIG_INITRD_TAG) || \
> -    defined (CONFIG_SERIAL_TAG) || \
> -    defined (CONFIG_REVISION_TAG)
> -static void setup_start_tag (bd_t *bd);
> -
> -# ifdef CONFIG_SETUP_MEMORY_TAGS
> -static void setup_memory_tags (bd_t *bd);
> -# endif
> -static void setup_commandline_tag (bd_t *bd, char *commandline);
> -
> -# ifdef CONFIG_INITRD_TAG
> -static void setup_initrd_tag (bd_t *bd, ulong initrd_start,
> -			      ulong initrd_end);
> -# endif
> -static void setup_end_tag (bd_t *bd);
> -
> +static void (*kernel_entry)(int zero, int arch, uint params);

NAK (see later on)

>  static struct tag *params;
> -#endif /* CONFIG_SETUP_MEMORY_TAGS || CONFIG_CMDLINE_TAG || CONFIG_INITRD_TAG */
>  
> -static ulong get_sp(void);
> -#if defined(CONFIG_OF_LIBFDT)
> -static int bootm_linux_fdt(int machid, bootm_headers_t *images);
> -#endif
> +static ulong get_sp(void)
> +{
> +	ulong ret;
> +
> +	asm("mov %0, sp" : "=r"(ret) : );
> +	return ret;
> +}
>  
>  void arch_lmb_reserve(struct lmb *lmb)
>  {
> @@ -80,85 +69,6 @@ void arch_lmb_reserve(struct lmb *lmb)
>  		    gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size - sp);
>  }
>  
> -static void announce_and_cleanup(void)
> -{
> -	printf("\nStarting kernel ...\n\n");
> -
> -#ifdef CONFIG_USB_DEVICE
> -	{
> -		extern void udc_disconnect(void);
> -		udc_disconnect();
> -	}
> -#endif
> -	cleanup_before_linux();
> -}

I can not see why git decided to remove that here and add it later on ..
did you change anything in announce_and_cleanup()?

> -
> -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);
> -
> -#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);
> -
> -#ifdef CONFIG_OF_LIBFDT
> -	if (images->ft_len)
> -		return bootm_linux_fdt(machid, images);
> -#endif
> -
> -	kernel_entry = (void (*)(int, int, uint))images->ep;
> -
> -	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);
> -#ifdef CONFIG_SERIAL_TAG
> -	setup_serial_tag (&params);
> -#endif
> -#ifdef CONFIG_REVISION_TAG
> -	setup_revision_tag (&params);
> -#endif
> -#ifdef CONFIG_SETUP_MEMORY_TAGS
> -	setup_memory_tags (bd);
> -#endif
> -#ifdef CONFIG_CMDLINE_TAG
> -	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);
> -#endif
> -	setup_end_tag(bd);
> -#endif
> -
> -	announce_and_cleanup();
> -
> -	kernel_entry(0, machid, bd->bi_boot_params);
> -	/* does not return */
> -
> -	return 1;
> -}
> -
> -#if defined(CONFIG_OF_LIBFDT)
>  static int fixup_memory_node(void *blob)
>  {
>  	bd_t	*bd = gd->bd;
> @@ -174,54 +84,19 @@ 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 void announce_and_cleanup(void)
>  {
> -	ulong rd_len;
> -	void (*kernel_entry)(int zero, int dt_machid, void *dtblob);
> -	ulong of_size = images->ft_len;
> -	char **of_flat_tree = &images->ft_addr;
> -	ulong *initrd_start = &images->initrd_start;
> -	ulong *initrd_end = &images->initrd_end;
> -	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);
> -
> -	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;
> -
> -	ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
> -	if (ret)
> -		return ret;
> -
> -	debug("## Transferring control to Linux (at address %08lx) ...\n",
> -	       (ulong) kernel_entry);
> -
> -	fdt_chosen(*of_flat_tree, 1);
> -
> -	fixup_memory_node(*of_flat_tree);
> -
> -	fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
> -
> -	announce_and_cleanup();
> -
> -	kernel_entry(0, machid, *of_flat_tree);
> -	/* does not return */
> +	printf("\nStarting kernel ...\n\n");
>  
> -	return 1;
> -}
> +#ifdef CONFIG_USB_DEVICE
> +	{
> +		extern void udc_disconnect(void);
> +		udc_disconnect();
> +	}
>  #endif
> +	cleanup_before_linux();
> +}
>  
> -#if defined (CONFIG_SETUP_MEMORY_TAGS) || \
> -    defined (CONFIG_CMDLINE_TAG) || \
> -    defined (CONFIG_INITRD_TAG) || \
> -    defined (CONFIG_SERIAL_TAG) || \
> -    defined (CONFIG_REVISION_TAG)
>  static void setup_start_tag (bd_t *bd)
>  {
>  	params = (struct tag *) bd->bi_boot_params;
> @@ -237,7 +112,6 @@ static void setup_start_tag (bd_t *bd)
>  }
>  
>  
> -#ifdef CONFIG_SETUP_MEMORY_TAGS
>  static void setup_memory_tags (bd_t *bd)
>  {
>  	int i;
> @@ -252,8 +126,6 @@ static void setup_memory_tags (bd_t *bd)
>  		params = tag_next (params);
>  	}
>  }
> -#endif /* CONFIG_SETUP_MEMORY_TAGS */
> -
>  
>  static void setup_commandline_tag (bd_t *bd, char *commandline)
>  {
> @@ -280,8 +152,6 @@ static void setup_commandline_tag (bd_t *bd, char *commandline)
>  	params = tag_next (params);
>  }
>  
> -
> -#ifdef CONFIG_INITRD_TAG
>  static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end)
>  {
>  	/* an ATAG_INITRD node tells the kernel where the compressed
> @@ -295,9 +165,18 @@ static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end)
>  
>  	params = tag_next (params);
>  }
> -#endif /* CONFIG_INITRD_TAG */
>  
> -#ifdef CONFIG_SERIAL_TAG
> +/* This is a workaround - if boards don't implement
> + * get_board_serial */
> +__attribute__((weak))
> +void get_board_serial(struct tag_serialnr *serialnr)
> +{
> +	printf("This board does not implement get_board_serial() \
> +		but calls it serialnr is filled with junk!\n");

WARNING: Avoid line continuations in quoted strings
#282: FILE: arch/arm/lib/bootm.c:174:
+	printf("This board does not implement get_board_serial() \

> +	serialnr->high = 0xABCDF;
> +	serialnr->low = 0xABCDF;
> +}

This was not mentioned in commit message, please split this into another
patch (if really required). I vote for 'remove that snipped completely'
cause we should see at compiletime that this funtion is mising.

BTW: you could remove the forward declaration for 'void
get_board_serial(struct tag_serialnr *serialnr)' from setup_serial_tag()
if you implement it some lines above.

> +
>  void setup_serial_tag (struct tag **tmp)

------------------------^
Remove whitespace between function name and parameter list (fix globally
when you edit that file).

>  {
>  	struct tag *params = *tmp;
> @@ -312,9 +191,7 @@ void setup_serial_tag (struct tag **tmp)
>  	params = tag_next (params);
>  	*tmp = params;
>  }
> -#endif
>  
> -#ifdef CONFIG_REVISION_TAG
>  void setup_revision_tag(struct tag **in_params)
>  {
>  	u32 rev = 0;
> @@ -326,19 +203,148 @@ void setup_revision_tag(struct tag **in_params)
>  	params->u.revision.rev = rev;
>  	params = tag_next (params);
>  }
> -#endif  /* CONFIG_REVISION_TAG */
>  
>  static void setup_end_tag (bd_t *bd)
>  {
>  	params->hdr.tag = ATAG_NONE;
>  	params->hdr.size = 0;
>  }
> -#endif /* CONFIG_SETUP_MEMORY_TAGS || CONFIG_CMDLINE_TAG || CONFIG_INITRD_TAG */
>  
> -static ulong get_sp(void)
> +static void create_atags(bootm_headers_t *images)
>  {
> -	ulong ret;
> +	bd_t	*bd = gd->bd;

no tab here ^

> +	char *commandline = getenv("bootargs");
>  
> -	asm("mov %0, sp" : "=r"(ret) : );
> -	return ret;
> +	setup_start_tag(bd);
> +#ifdef CONFIG_SERIAL_TAG
> +	setup_serial_tag(&params);
> +#endif
> +#ifdef CONFIG_CMDLINE_TAG
> +	setup_commandline_tag(gd->bd, commandline);
> +#endif
> +#ifdef CONFIG_REVISION_TAG
> +	setup_revision_tag(&params);
> +#endif
> +#ifdef CONFIG_SETUP_MEMORY_TAGS
> +	setup_memory_tags(bd);
> +#endif
> +#ifdef CONFIG_INITRD_TAG
> +	if (images->rd_start && images->rd_end)
> +		setup_initrd_tag(bd, images->rd_start, images->rd_end);
> +#endif
> +	setup_end_tag(bd);
> +}
> +
> +static int create_fdt(bootm_headers_t *images)
> +{
> +	ulong of_size = images->ft_len;
> +	char **of_flat_tree = &images->ft_addr;
> +	ulong *initrd_start = &images->initrd_start;
> +	ulong *initrd_end = &images->initrd_end;
> +	struct lmb *lmb = &images->lmb;
> +	ulong rd_len;
> +	int ret;
> +
> +	debug("using: FDT\n");
> +
> +	boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
> +
> +	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;
> +
> +	ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
> +	if (ret)
> +		return ret;
> +
> +	fdt_chosen(*of_flat_tree, 1);
> +	fixup_memory_node(*of_flat_tree);
> +	fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
> +
> +	return 0;
> +}
> +
> +/* Subcommand: CMDLINE */
> +static int boot_cmdline_linux(bootm_headers_t *images)
> +{
> +	return -1; /* not implemented */
> +}
> +
> +/* Subcommand: BDT */
> +static int boot_bd_t_linux(bootm_headers_t *images)
> +{
> +	return -1; /* not implemented */

Shouldn't that return 0 for 'no error' even if not implemented?

> +}
> +
> +/* Subcommand: PREP */
> +static void boot_prep_linux(bootm_headers_t *images)
> +{
> +#ifdef CONFIG_OF_LIBFDT
> +	if (images->ft_len) {
> +		debug("using: FDT\n");
> +		if (create_fdt(images)) {
> +			printf("FDT creation failed! hanging...");
> +			hang();
> +		}
> +	} else
> +#endif
> +	{
> +#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
> +	defined(CONFIG_CMDLINE_TAG) || \
> +	defined(CONFIG_INITRD_TAG) || \
> +	defined(CONFIG_SERIAL_TAG) || \
> +	defined(CONFIG_REVISION_TAG)
> +		debug("using: ATAGS\n");
> +		create_atags(images);

I can not see any reason why we should keep create_atags() as another
function here, I think it is cleaner to move the content of
create_atags() to boot_prep_linux() and remove this large list of
requirements for create_atags().

> +#else
> +	printf("FDT and ATAGS failed - hanging\n");

Wrong text here, only FDT did fail, ATAGS where not defined at build time.

> +	hang();
> +#endif
> +	}
> +
> +	kernel_entry = (void (*)(int, int, uint))images->ep;

NAK, setting (and using) kernel_entry is part of GO subcommand.

> +}
> +
> +/* Subcommand: GO */
> +static void boot_jump_linux(bootm_headers_t *images)
> +{
> +	int	machid = gd->bd->bi_arch_number;
> +	char	*s;

No tab here ^

Declare kernel_entry function pointer here.

> +	s = getenv("machid");
> +	if (s) {
> +		strict_strtoul(s, 16, (long unsigned int *) &machid);

How about strict_strtoul() returning something wrong?

> +		debug("Using machid 0x%x from environment\n", machid);

Yoiu should use printf() here as it was bfore so one could see that fact
when booting.

> +	}
> +

Set kernel_entry function pointer here.

> +	debug("Using machid 0x%x from bd\n", machid);

This statement is wrong ... you need to set this in an else statement.
Or reword the content. How about:

debug("Using machid 0x%x\n", machid); ?

> +	debug("## Transferring control to Linux (at address %08lx)" \
> +		"...\n", (ulong) kernel_entry);

Use kernel_entry function pointer here ...

> +	show_boot_progress(15);
> +	announce_and_cleanup();
> +	kernel_entry(0, machid, gd->bd->bi_boot_params);

and here.

> +}
> +
> +/* Main Entry point for arm bootm implementation
> + *
> + * Modeled after the powerpc implementation
> + * DIFFERENCE: Instead of calling prep and go at the end
> + * they are called if subommand is equal 0.

s/subommand/subcommand/

> + */
> +int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> +{
> +	if (flag & BOOTM_STATE_OS_CMDLINE)
> +		boot_cmdline_linux(images);
> +
> +	if (flag & BOOTM_STATE_OS_BD_T)
> +		boot_bd_t_linux(images);

NAK, remove these two functions. Since the ARM linux boot requirements
are different to powerpc we do not need these two states of bootm at all.

The powerpc entry_32.S (in linux) show they need commandline pointer
apart from 'residual board info' pointer. The arm implementation in
head.S (also linux source) says:

---8<---
 * This is normally called from the decompressor code.  The requirements
 * are: MMU = off, D-cache = off, I-cache = dont care, r0 = 0,
 * r1 = machine nr, r2 = atags or dtb pointer.
--->8---

For arm we do not need to prepare the cmdline apart from 'bd_t', we just
need to setup the ATAGS (ord FDT) which contains all information we
need. This could all be done in the prep state.

> +
> +	if (flag & BOOTM_STATE_OS_PREP || flag == 0)
> +		boot_prep_linux(images);
> +
> +	if (flag & BOOTM_STATE_OS_GO || flag == 0)
> +		boot_jump_linux(images);
> +
> +	return 0;
>  }

NAK, the ppc implementation does here

if (flag & FLAG) {
  do_flag_specific()
  return
}
...
do whatever to do without any flag set

which seems much cleaner to me.

I personally dislike the 'if specific flag set or no flag set then do
...' logic here.

best regards

Andreas Bießmann
Simon Schwarz Sept. 5, 2011, 2:06 p.m. UTC | #2
Dear Andreas,

On 09/05/2011 12:58 PM, Andreas Bießmann wrote:
> Dear Simon,
>
> Am Mo 29 Aug 2011 18:08:13 CEST, Simon Schwarz schrieb:
>> 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
>
> savebp? I thought this command is now 'spl' with some subcommands.
fixed.
>
>>
>> Signed-off-by: Simon Schwarz<simonschwarzcor@gmail.com>
>> ----
>
> this four times '-' will not be recognized as comment section by git am
>
fixed
>>
>> V2 changes:
>> nothing
>>
>> V3 changes:
>> nothing
>>
>> changes after slicing this from patch
>> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/106422:
>> DEL Prototype declaration - not necessary if reordered
>> DEL Most #ifdefs - relying on -ffunction-sections and --gc-sections to handle unused
>> CHG reorganized bootm. powerpc implementation was the model
>> ADD get_board_serial fake implementation - this is needed if setup_serial_tag
>> 	is compiled but the board doesn't support it - tradeoff for removing
>> 	#ifdefs
>> ---
>>   arch/arm/lib/bootm.c |  330 +++++++++++++++++++++++++------------------------
>>   1 files changed, 168 insertions(+), 162 deletions(-)
>>
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index 802e833..5f112e2 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -1,4 +1,8 @@
>> -/*
>> +/* Copyright (C) 2011
>> + * Corscience GmbH&  Co. KG - Simon Schwarz<schwarz@corscience.de>
>> + *  - Added prep subcommand support
>> + *  - Reorganized source - modeled after powerpc version
>> + *
>>    * (C) Copyright 2002
>>    * Sysgo Real-Time Solutions, GmbH<www.elinos.com>
>>    * Marius Groeger<mgroeger@sysgo.de>
>> @@ -32,31 +36,16 @@
>>
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>> -#if defined (CONFIG_SETUP_MEMORY_TAGS) || \
>> -    defined (CONFIG_CMDLINE_TAG) || \
>> -    defined (CONFIG_INITRD_TAG) || \
>> -    defined (CONFIG_SERIAL_TAG) || \
>> -    defined (CONFIG_REVISION_TAG)
>> -static void setup_start_tag (bd_t *bd);
>> -
>> -# ifdef CONFIG_SETUP_MEMORY_TAGS
>> -static void setup_memory_tags (bd_t *bd);
>> -# endif
>> -static void setup_commandline_tag (bd_t *bd, char *commandline);
>> -
>> -# ifdef CONFIG_INITRD_TAG
>> -static void setup_initrd_tag (bd_t *bd, ulong initrd_start,
>> -			      ulong initrd_end);
>> -# endif
>> -static void setup_end_tag (bd_t *bd);
>> -
>> +static void (*kernel_entry)(int zero, int arch, uint params);
>
> NAK (see later on)
done.
>
>>   static struct tag *params;
>> -#endif /* CONFIG_SETUP_MEMORY_TAGS || CONFIG_CMDLINE_TAG || CONFIG_INITRD_TAG */
>>
>> -static ulong get_sp(void);
>> -#if defined(CONFIG_OF_LIBFDT)
>> -static int bootm_linux_fdt(int machid, bootm_headers_t *images);
>> -#endif
>> +static ulong get_sp(void)
>> +{
>> +	ulong ret;
>> +
>> +	asm("mov %0, sp" : "=r"(ret) : );
>> +	return ret;
>> +}
>>
>>   void arch_lmb_reserve(struct lmb *lmb)
>>   {
>> @@ -80,85 +69,6 @@ void arch_lmb_reserve(struct lmb *lmb)
>>   		    gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size - sp);
>>   }
>>
>> -static void announce_and_cleanup(void)
>> -{
>> -	printf("\nStarting kernel ...\n\n");
>> -
>> -#ifdef CONFIG_USB_DEVICE
>> -	{
>> -		extern void udc_disconnect(void);
>> -		udc_disconnect();
>> -	}
>> -#endif
>> -	cleanup_before_linux();
>> -}
>
> I can not see why git decided to remove that here and add it later on ..
> did you change anything in announce_and_cleanup()?

Nope. I added something before and there were major changes in the file 
(moved large codejunks around)- not the best environment for a pretty 
diff i guess...
>
>> -
>> -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);
>> -
>> -#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);
>> -
>> -#ifdef CONFIG_OF_LIBFDT
>> -	if (images->ft_len)
>> -		return bootm_linux_fdt(machid, images);
>> -#endif
>> -
>> -	kernel_entry = (void (*)(int, int, uint))images->ep;
>> -
>> -	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);
>> -#ifdef CONFIG_SERIAL_TAG
>> -	setup_serial_tag (&params);
>> -#endif
>> -#ifdef CONFIG_REVISION_TAG
>> -	setup_revision_tag (&params);
>> -#endif
>> -#ifdef CONFIG_SETUP_MEMORY_TAGS
>> -	setup_memory_tags (bd);
>> -#endif
>> -#ifdef CONFIG_CMDLINE_TAG
>> -	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);
>> -#endif
>> -	setup_end_tag(bd);
>> -#endif
>> -
>> -	announce_and_cleanup();
>> -
>> -	kernel_entry(0, machid, bd->bi_boot_params);
>> -	/* does not return */
>> -
>> -	return 1;
>> -}
>> -
>> -#if defined(CONFIG_OF_LIBFDT)
>>   static int fixup_memory_node(void *blob)
>>   {
>>   	bd_t	*bd = gd->bd;
>> @@ -174,54 +84,19 @@ 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 void announce_and_cleanup(void)
>>   {
>> -	ulong rd_len;
>> -	void (*kernel_entry)(int zero, int dt_machid, void *dtblob);
>> -	ulong of_size = images->ft_len;
>> -	char **of_flat_tree =&images->ft_addr;
>> -	ulong *initrd_start =&images->initrd_start;
>> -	ulong *initrd_end =&images->initrd_end;
>> -	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);
>> -
>> -	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;
>> -
>> -	ret = boot_relocate_fdt(lmb, of_flat_tree,&of_size);
>> -	if (ret)
>> -		return ret;
>> -
>> -	debug("## Transferring control to Linux (at address %08lx) ...\n",
>> -	       (ulong) kernel_entry);
>> -
>> -	fdt_chosen(*of_flat_tree, 1);
>> -
>> -	fixup_memory_node(*of_flat_tree);
>> -
>> -	fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
>> -
>> -	announce_and_cleanup();
>> -
>> -	kernel_entry(0, machid, *of_flat_tree);
>> -	/* does not return */
>> +	printf("\nStarting kernel ...\n\n");
>>
>> -	return 1;
>> -}
>> +#ifdef CONFIG_USB_DEVICE
>> +	{
>> +		extern void udc_disconnect(void);
>> +		udc_disconnect();
>> +	}
>>   #endif
>> +	cleanup_before_linux();
>> +}
>>
>> -#if defined (CONFIG_SETUP_MEMORY_TAGS) || \
>> -    defined (CONFIG_CMDLINE_TAG) || \
>> -    defined (CONFIG_INITRD_TAG) || \
>> -    defined (CONFIG_SERIAL_TAG) || \
>> -    defined (CONFIG_REVISION_TAG)
>>   static void setup_start_tag (bd_t *bd)
>>   {
>>   	params = (struct tag *) bd->bi_boot_params;
>> @@ -237,7 +112,6 @@ static void setup_start_tag (bd_t *bd)
>>   }
>>
>>
>> -#ifdef CONFIG_SETUP_MEMORY_TAGS
>>   static void setup_memory_tags (bd_t *bd)
>>   {
>>   	int i;
>> @@ -252,8 +126,6 @@ static void setup_memory_tags (bd_t *bd)
>>   		params = tag_next (params);
>>   	}
>>   }
>> -#endif /* CONFIG_SETUP_MEMORY_TAGS */
>> -
>>
>>   static void setup_commandline_tag (bd_t *bd, char *commandline)
>>   {
>> @@ -280,8 +152,6 @@ static void setup_commandline_tag (bd_t *bd, char *commandline)
>>   	params = tag_next (params);
>>   }
>>
>> -
>> -#ifdef CONFIG_INITRD_TAG
>>   static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end)
>>   {
>>   	/* an ATAG_INITRD node tells the kernel where the compressed
>> @@ -295,9 +165,18 @@ static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end)
>>
>>   	params = tag_next (params);
>>   }
>> -#endif /* CONFIG_INITRD_TAG */
>>
>> -#ifdef CONFIG_SERIAL_TAG
>> +/* This is a workaround - if boards don't implement
>> + * get_board_serial */
>> +__attribute__((weak))
>> +void get_board_serial(struct tag_serialnr *serialnr)
>> +{
>> +	printf("This board does not implement get_board_serial() \
>> +		but calls it serialnr is filled with junk!\n");
>
> WARNING: Avoid line continuations in quoted strings
> #282: FILE: arch/arm/lib/bootm.c:174:
> +	printf("This board does not implement get_board_serial() \
>
Hmmm. This warning is not emitted with my checkpatch.pl (V 0.31)

>> +	serialnr->high = 0xABCDF;
>> +	serialnr->low = 0xABCDF;
>> +}
>
> This was not mentioned in commit message, please split this into another
> patch (if really required). I vote for 'remove that snipped completely'
> cause we should see at compiletime that this funtion is mising.
>
> BTW: you could remove the forward declaration for 'void
> get_board_serial(struct tag_serialnr *serialnr)' from setup_serial_tag()
> if you implement it some lines above.
>

I readded the #ifdef CONFIG_SERIAL_TAG. This avoids the above
problems.
>> +
>>   void setup_serial_tag (struct tag **tmp)
>
> ------------------------^
> Remove whitespace between function name and parameter list (fix globally
> when you edit that file).
done.
>
>>   {
>>   	struct tag *params = *tmp;
>> @@ -312,9 +191,7 @@ void setup_serial_tag (struct tag **tmp)
>>   	params = tag_next (params);
>>   	*tmp = params;
>>   }
>> -#endif
>>
>> -#ifdef CONFIG_REVISION_TAG
>>   void setup_revision_tag(struct tag **in_params)
>>   {
>>   	u32 rev = 0;
>> @@ -326,19 +203,148 @@ void setup_revision_tag(struct tag **in_params)
>>   	params->u.revision.rev = rev;
>>   	params = tag_next (params);
>>   }
>> -#endif  /* CONFIG_REVISION_TAG */
>>
>>   static void setup_end_tag (bd_t *bd)
>>   {
>>   	params->hdr.tag = ATAG_NONE;
>>   	params->hdr.size = 0;
>>   }
>> -#endif /* CONFIG_SETUP_MEMORY_TAGS || CONFIG_CMDLINE_TAG || CONFIG_INITRD_TAG */
>>
>> -static ulong get_sp(void)
>> +static void create_atags(bootm_headers_t *images)
>>   {
>> -	ulong ret;
>> +	bd_t	*bd = gd->bd;
>
> no tab here ^
>
done.
>> +	char *commandline = getenv("bootargs");
>>
>> -	asm("mov %0, sp" : "=r"(ret) : );
>> -	return ret;
>> +	setup_start_tag(bd);
>> +#ifdef CONFIG_SERIAL_TAG
>> +	setup_serial_tag(&params);
>> +#endif
>> +#ifdef CONFIG_CMDLINE_TAG
>> +	setup_commandline_tag(gd->bd, commandline);
>> +#endif
>> +#ifdef CONFIG_REVISION_TAG
>> +	setup_revision_tag(&params);
>> +#endif
>> +#ifdef CONFIG_SETUP_MEMORY_TAGS
>> +	setup_memory_tags(bd);
>> +#endif
>> +#ifdef CONFIG_INITRD_TAG
>> +	if (images->rd_start&&  images->rd_end)
>> +		setup_initrd_tag(bd, images->rd_start, images->rd_end);
>> +#endif
>> +	setup_end_tag(bd);
>> +}
>> +
>> +static int create_fdt(bootm_headers_t *images)
>> +{
>> +	ulong of_size = images->ft_len;
>> +	char **of_flat_tree =&images->ft_addr;
>> +	ulong *initrd_start =&images->initrd_start;
>> +	ulong *initrd_end =&images->initrd_end;
>> +	struct lmb *lmb =&images->lmb;
>> +	ulong rd_len;
>> +	int ret;
>> +
>> +	debug("using: FDT\n");
>> +
>> +	boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
>> +
>> +	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;
>> +
>> +	ret = boot_relocate_fdt(lmb, of_flat_tree,&of_size);
>> +	if (ret)
>> +		return ret;
>> +
>> +	fdt_chosen(*of_flat_tree, 1);
>> +	fixup_memory_node(*of_flat_tree);
>> +	fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +/* Subcommand: CMDLINE */
>> +static int boot_cmdline_linux(bootm_headers_t *images)
>> +{
>> +	return -1; /* not implemented */
>> +}
>> +
>> +/* Subcommand: BDT */
>> +static int boot_bd_t_linux(bootm_headers_t *images)
>> +{
>> +	return -1; /* not implemented */
>
> Shouldn't that return 0 for 'no error' even if not implemented?
>

common/bootm.c calls subcommands like this:
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;

So I assume a value different from zero is interpreted as not 
implemented. But as I just saw these returns were never returned to 
common/bootm.c ...

Anyway, removed theses functions.

>> +}
>> +
>> +/* Subcommand: PREP */
>> +static void boot_prep_linux(bootm_headers_t *images)
>> +{
>> +#ifdef CONFIG_OF_LIBFDT
>> +	if (images->ft_len) {
>> +		debug("using: FDT\n");
>> +		if (create_fdt(images)) {
>> +			printf("FDT creation failed! hanging...");
>> +			hang();
>> +		}
>> +	} else
>> +#endif
>> +	{
>> +#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
>> +	defined(CONFIG_CMDLINE_TAG) || \
>> +	defined(CONFIG_INITRD_TAG) || \
>> +	defined(CONFIG_SERIAL_TAG) || \
>> +	defined(CONFIG_REVISION_TAG)
>> +		debug("using: ATAGS\n");
>> +		create_atags(images);
>
> I can not see any reason why we should keep create_atags() as another
> function here, I think it is cleaner to move the content of
> create_atags() to boot_prep_linux() and remove this large list of
> requirements for create_atags().

My reasons to do it that way:
- It is similar to create_fdt - duno if it is just me but I see this as 
more consistent
- The requirements list will be there anyway setup_start_tag and 
setup_end_tag have to be protected. The only way to make it smaller
would be a macro doing the same.

I have no strong feeling about how to do that but IMHO it is better 
readable now - if there are more votes for moving this to 
boot_prep_linux i will do it.

>
>> +#else
>> +	printf("FDT and ATAGS failed - hanging\n");
>
> Wrong text here, only FDT did fail, ATAGS where not defined at build time.
>
Actually it is that both aren't defined because the FDT has it's own 
hang in case of failure. So I will change to "FDT and ATAGS support not 
compiled in - hanging".

>> +	hang();
>> +#endif
>> +	}
>> +
>> +	kernel_entry = (void (*)(int, int, uint))images->ep;
>
> NAK, setting (and using) kernel_entry is part of GO subcommand.
>

changed.

>> +}
>> +
>> +/* Subcommand: GO */
>> +static void boot_jump_linux(bootm_headers_t *images)
>> +{
>> +	int	machid = gd->bd->bi_arch_number;
>> +	char	*s;
>
> No tab here ^
changed.
>
> Declare kernel_entry function pointer here.
done.
>
>> +	s = getenv("machid");
>> +	if (s) {
>> +		strict_strtoul(s, 16, (long unsigned int *)&machid);
>
> How about strict_strtoul() returning something wrong?

>
>> +		debug("Using machid 0x%x from environment\n", machid);
>
> Yoiu should use printf() here as it was bfore so one could see that fact
> when booting.
Hm. I considered it too noisy since the the machid is normally not 
displayed on boot. Will change anyway...

>
>> +	}
>> +
>
> Set kernel_entry function pointer here.
>
>> +	debug("Using machid 0x%x from bd\n", machid);
>
> This statement is wrong ... you need to set this in an else statement.
> Or reword the content. How about:
>
> debug("Using machid 0x%x\n", machid); ?
>
Ha, this was a leftover from debugging - I deleted it.

>> +	debug("## Transferring control to Linux (at address %08lx)" \
>> +		"...\n", (ulong) kernel_entry);
>
> Use kernel_entry function pointer here ...
>
>> +	show_boot_progress(15);1
>> +	announce_and_cleanup();
>> +	kernel_entry(0, machid, gd->bd->bi_boot_params);
>
> and here.


>
>> +}
>> +
>> +/* Main Entry point for arm bootm implementation
>> + *
>> + * Modeled after the powerpc implementation
>> + * DIFFERENCE: Instead of calling prep and go at the end
>> + * they are called if subommand is equal 0.
>
> s/subommand/subcommand/
done
>
>> + */
>> +int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>> +{
>> +	if (flag&  BOOTM_STATE_OS_CMDLINE)
>> +		boot_cmdline_linux(images);
>> +
>> +	if (flag&  BOOTM_STATE_OS_BD_T)
>> +		boot_bd_t_linux(images);
>
> NAK, remove these two functions. Since the ARM linux boot requirements
> are different to powerpc we do not need these two states of bootm at all.
>
> The powerpc entry_32.S (in linux) show they need commandline pointer
> apart from 'residual board info' pointer. The arm implementation in
> head.S (also linux source) says:
>
> ---8<---
>   * This is normally called from the decompressor code.  The requirements
>   * are: MMU = off, D-cache = off, I-cache = dont care, r0 = 0,
>   * r1 = machine nr, r2 = atags or dtb pointer.
> --->8---
>
> For arm we do not need to prepare the cmdline apart from 'bd_t', we just
> need to setup the ATAGS (ord FDT) which contains all information we
> need. This could all be done in the prep state.
>

changed.

But they are shown in bootm help message regardles of the architecture. 
Shouldn't we add #ifdefs in the help message then? (or, and I really 
hate to bring this up, change the way the helpmessage is created if the
function is arch dependent)

>> +
>> +	if (flag&  BOOTM_STATE_OS_PREP || flag == 0)
>> +		boot_prep_linux(images);
>> +
>> +	if (flag&  BOOTM_STATE_OS_GO || flag == 0)
>> +		boot_jump_linux(images);
>> +
>> +	return 0;
>>   }
>
> NAK, the ppc implementation does here
>
> if (flag&  FLAG) {
>    do_flag_specific()
>    return
> }
> ...
> do whatever to do without any flag set
>
> which seems much cleaner to me.
>
> I personally dislike the 'if specific flag set or no flag set then do
> ...' logic here.
>
I already changed this. Just waited for more comments to send in a new 
version.
> best regards
>
> Andreas Bießmann

Regards & thx
Simon
Andreas Bießmann Sept. 5, 2011, 3:25 p.m. UTC | #3
Dear Simon,

Am 05.09.2011 16:06, schrieb Simon Schwarz:
> Dear Andreas,
> 
> On 09/05/2011 12:58 PM, Andreas Bießmann wrote:
>> Dear Simon,
>>
>> Am Mo 29 Aug 2011 18:08:13 CEST, Simon Schwarz schrieb:

<snip>

>>
>>> +}
>>> +
>>> +/* Main Entry point for arm bootm implementation
>>> + *
>>> + * Modeled after the powerpc implementation
>>> + * DIFFERENCE: Instead of calling prep and go at the end
>>> + * they are called if subommand is equal 0.
>>
>> s/subommand/subcommand/
> done
>>
>>> + */
>>> +int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t
>>> *images)
>>> +{
>>> +    if (flag&  BOOTM_STATE_OS_CMDLINE)
>>> +        boot_cmdline_linux(images);
>>> +
>>> +    if (flag&  BOOTM_STATE_OS_BD_T)
>>> +        boot_bd_t_linux(images);
>>
>> NAK, remove these two functions. Since the ARM linux boot requirements
>> are different to powerpc we do not need these two states of bootm at all.
>>
>> The powerpc entry_32.S (in linux) show they need commandline pointer
>> apart from 'residual board info' pointer. The arm implementation in
>> head.S (also linux source) says:
>>
>> ---8<---
>>   * This is normally called from the decompressor code.  The requirements
>>   * are: MMU = off, D-cache = off, I-cache = dont care, r0 = 0,
>>   * r1 = machine nr, r2 = atags or dtb pointer.
>> --->8---
>>
>> For arm we do not need to prepare the cmdline apart from 'bd_t', we just
>> need to setup the ATAGS (ord FDT) which contains all information we
>> need. This could all be done in the prep state.
>>
> 
> changed.
> 
> But they are shown in bootm help message regardles of the architecture.
> Shouldn't we add #ifdefs in the help message then? (or, and I really
> hate to bring this up, change the way the helpmessage is created if the
> function is arch dependent)

No, I don't think we should change the bootm command. We could either
return 'not implemented' or 'everything is ok' here for these two flags.
But we do not need extra empty functions for that.

How about:

/* OS_CMDLINE is not needed by ARM cause of ... the bootargs (==cmdline)
is set in ATAGS/FDT ... */
if (flag&  BOOTM_STATE_OS_CMDLINE)
	return 0; /* pretend everything is ok */

/* OS_BD_T is not needed by ARM casue of ... */
if (flag&  BOOTM_STATE_OS_BD_T)
	return 0; /* pretend everything is ok */

...

best regards

Andreas Bießmann
diff mbox

Patch

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 802e833..5f112e2 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -1,4 +1,8 @@ 
-/*
+/* Copyright (C) 2011
+ * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@corscience.de>
+ *  - Added prep subcommand support
+ *  - Reorganized source - modeled after powerpc version
+ *
  * (C) Copyright 2002
  * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
  * Marius Groeger <mgroeger@sysgo.de>
@@ -32,31 +36,16 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if defined (CONFIG_SETUP_MEMORY_TAGS) || \
-    defined (CONFIG_CMDLINE_TAG) || \
-    defined (CONFIG_INITRD_TAG) || \
-    defined (CONFIG_SERIAL_TAG) || \
-    defined (CONFIG_REVISION_TAG)
-static void setup_start_tag (bd_t *bd);
-
-# ifdef CONFIG_SETUP_MEMORY_TAGS
-static void setup_memory_tags (bd_t *bd);
-# endif
-static void setup_commandline_tag (bd_t *bd, char *commandline);
-
-# ifdef CONFIG_INITRD_TAG
-static void setup_initrd_tag (bd_t *bd, ulong initrd_start,
-			      ulong initrd_end);
-# endif
-static void setup_end_tag (bd_t *bd);
-
+static void (*kernel_entry)(int zero, int arch, uint params);
 static struct tag *params;
-#endif /* CONFIG_SETUP_MEMORY_TAGS || CONFIG_CMDLINE_TAG || CONFIG_INITRD_TAG */
 
-static ulong get_sp(void);
-#if defined(CONFIG_OF_LIBFDT)
-static int bootm_linux_fdt(int machid, bootm_headers_t *images);
-#endif
+static ulong get_sp(void)
+{
+	ulong ret;
+
+	asm("mov %0, sp" : "=r"(ret) : );
+	return ret;
+}
 
 void arch_lmb_reserve(struct lmb *lmb)
 {
@@ -80,85 +69,6 @@  void arch_lmb_reserve(struct lmb *lmb)
 		    gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size - sp);
 }
 
-static void announce_and_cleanup(void)
-{
-	printf("\nStarting kernel ...\n\n");
-
-#ifdef CONFIG_USB_DEVICE
-	{
-		extern void udc_disconnect(void);
-		udc_disconnect();
-	}
-#endif
-	cleanup_before_linux();
-}
-
-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);
-
-#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);
-
-#ifdef CONFIG_OF_LIBFDT
-	if (images->ft_len)
-		return bootm_linux_fdt(machid, images);
-#endif
-
-	kernel_entry = (void (*)(int, int, uint))images->ep;
-
-	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);
-#ifdef CONFIG_SERIAL_TAG
-	setup_serial_tag (&params);
-#endif
-#ifdef CONFIG_REVISION_TAG
-	setup_revision_tag (&params);
-#endif
-#ifdef CONFIG_SETUP_MEMORY_TAGS
-	setup_memory_tags (bd);
-#endif
-#ifdef CONFIG_CMDLINE_TAG
-	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);
-#endif
-	setup_end_tag(bd);
-#endif
-
-	announce_and_cleanup();
-
-	kernel_entry(0, machid, bd->bi_boot_params);
-	/* does not return */
-
-	return 1;
-}
-
-#if defined(CONFIG_OF_LIBFDT)
 static int fixup_memory_node(void *blob)
 {
 	bd_t	*bd = gd->bd;
@@ -174,54 +84,19 @@  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 void announce_and_cleanup(void)
 {
-	ulong rd_len;
-	void (*kernel_entry)(int zero, int dt_machid, void *dtblob);
-	ulong of_size = images->ft_len;
-	char **of_flat_tree = &images->ft_addr;
-	ulong *initrd_start = &images->initrd_start;
-	ulong *initrd_end = &images->initrd_end;
-	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);
-
-	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;
-
-	ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
-	if (ret)
-		return ret;
-
-	debug("## Transferring control to Linux (at address %08lx) ...\n",
-	       (ulong) kernel_entry);
-
-	fdt_chosen(*of_flat_tree, 1);
-
-	fixup_memory_node(*of_flat_tree);
-
-	fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
-
-	announce_and_cleanup();
-
-	kernel_entry(0, machid, *of_flat_tree);
-	/* does not return */
+	printf("\nStarting kernel ...\n\n");
 
-	return 1;
-}
+#ifdef CONFIG_USB_DEVICE
+	{
+		extern void udc_disconnect(void);
+		udc_disconnect();
+	}
 #endif
+	cleanup_before_linux();
+}
 
-#if defined (CONFIG_SETUP_MEMORY_TAGS) || \
-    defined (CONFIG_CMDLINE_TAG) || \
-    defined (CONFIG_INITRD_TAG) || \
-    defined (CONFIG_SERIAL_TAG) || \
-    defined (CONFIG_REVISION_TAG)
 static void setup_start_tag (bd_t *bd)
 {
 	params = (struct tag *) bd->bi_boot_params;
@@ -237,7 +112,6 @@  static void setup_start_tag (bd_t *bd)
 }
 
 
-#ifdef CONFIG_SETUP_MEMORY_TAGS
 static void setup_memory_tags (bd_t *bd)
 {
 	int i;
@@ -252,8 +126,6 @@  static void setup_memory_tags (bd_t *bd)
 		params = tag_next (params);
 	}
 }
-#endif /* CONFIG_SETUP_MEMORY_TAGS */
-
 
 static void setup_commandline_tag (bd_t *bd, char *commandline)
 {
@@ -280,8 +152,6 @@  static void setup_commandline_tag (bd_t *bd, char *commandline)
 	params = tag_next (params);
 }
 
-
-#ifdef CONFIG_INITRD_TAG
 static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end)
 {
 	/* an ATAG_INITRD node tells the kernel where the compressed
@@ -295,9 +165,18 @@  static void setup_initrd_tag (bd_t *bd, ulong initrd_start, ulong initrd_end)
 
 	params = tag_next (params);
 }
-#endif /* CONFIG_INITRD_TAG */
 
-#ifdef CONFIG_SERIAL_TAG
+/* This is a workaround - if boards don't implement
+ * get_board_serial */
+__attribute__((weak))
+void get_board_serial(struct tag_serialnr *serialnr)
+{
+	printf("This board does not implement get_board_serial() \
+		but calls it serialnr is filled with junk!\n");
+	serialnr->high = 0xABCDF;
+	serialnr->low = 0xABCDF;
+}
+
 void setup_serial_tag (struct tag **tmp)
 {
 	struct tag *params = *tmp;
@@ -312,9 +191,7 @@  void setup_serial_tag (struct tag **tmp)
 	params = tag_next (params);
 	*tmp = params;
 }
-#endif
 
-#ifdef CONFIG_REVISION_TAG
 void setup_revision_tag(struct tag **in_params)
 {
 	u32 rev = 0;
@@ -326,19 +203,148 @@  void setup_revision_tag(struct tag **in_params)
 	params->u.revision.rev = rev;
 	params = tag_next (params);
 }
-#endif  /* CONFIG_REVISION_TAG */
 
 static void setup_end_tag (bd_t *bd)
 {
 	params->hdr.tag = ATAG_NONE;
 	params->hdr.size = 0;
 }
-#endif /* CONFIG_SETUP_MEMORY_TAGS || CONFIG_CMDLINE_TAG || CONFIG_INITRD_TAG */
 
-static ulong get_sp(void)
+static void create_atags(bootm_headers_t *images)
 {
-	ulong ret;
+	bd_t	*bd = gd->bd;
+	char *commandline = getenv("bootargs");
 
-	asm("mov %0, sp" : "=r"(ret) : );
-	return ret;
+	setup_start_tag(bd);
+#ifdef CONFIG_SERIAL_TAG
+	setup_serial_tag(&params);
+#endif
+#ifdef CONFIG_CMDLINE_TAG
+	setup_commandline_tag(gd->bd, commandline);
+#endif
+#ifdef CONFIG_REVISION_TAG
+	setup_revision_tag(&params);
+#endif
+#ifdef CONFIG_SETUP_MEMORY_TAGS
+	setup_memory_tags(bd);
+#endif
+#ifdef CONFIG_INITRD_TAG
+	if (images->rd_start && images->rd_end)
+		setup_initrd_tag(bd, images->rd_start, images->rd_end);
+#endif
+	setup_end_tag(bd);
+}
+
+static int create_fdt(bootm_headers_t *images)
+{
+	ulong of_size = images->ft_len;
+	char **of_flat_tree = &images->ft_addr;
+	ulong *initrd_start = &images->initrd_start;
+	ulong *initrd_end = &images->initrd_end;
+	struct lmb *lmb = &images->lmb;
+	ulong rd_len;
+	int ret;
+
+	debug("using: FDT\n");
+
+	boot_fdt_add_mem_rsv_regions(lmb, *of_flat_tree);
+
+	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;
+
+	ret = boot_relocate_fdt(lmb, of_flat_tree, &of_size);
+	if (ret)
+		return ret;
+
+	fdt_chosen(*of_flat_tree, 1);
+	fixup_memory_node(*of_flat_tree);
+	fdt_initrd(*of_flat_tree, *initrd_start, *initrd_end, 1);
+
+	return 0;
+}
+
+/* Subcommand: CMDLINE */
+static int boot_cmdline_linux(bootm_headers_t *images)
+{
+	return -1; /* not implemented */
+}
+
+/* Subcommand: BDT */
+static int boot_bd_t_linux(bootm_headers_t *images)
+{
+	return -1; /* not implemented */
+}
+
+/* Subcommand: PREP */
+static void boot_prep_linux(bootm_headers_t *images)
+{
+#ifdef CONFIG_OF_LIBFDT
+	if (images->ft_len) {
+		debug("using: FDT\n");
+		if (create_fdt(images)) {
+			printf("FDT creation failed! hanging...");
+			hang();
+		}
+	} else
+#endif
+	{
+#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
+	defined(CONFIG_CMDLINE_TAG) || \
+	defined(CONFIG_INITRD_TAG) || \
+	defined(CONFIG_SERIAL_TAG) || \
+	defined(CONFIG_REVISION_TAG)
+		debug("using: ATAGS\n");
+		create_atags(images);
+#else
+	printf("FDT and ATAGS failed - hanging\n");
+	hang();
+#endif
+	}
+
+	kernel_entry = (void (*)(int, int, uint))images->ep;
+}
+
+/* Subcommand: GO */
+static void boot_jump_linux(bootm_headers_t *images)
+{
+	int	machid = gd->bd->bi_arch_number;
+	char	*s;
+	s = getenv("machid");
+	if (s) {
+		strict_strtoul(s, 16, (long unsigned int *) &machid);
+		debug("Using machid 0x%x from environment\n", machid);
+	}
+
+	debug("Using machid 0x%x from bd\n", machid);
+	debug("## Transferring control to Linux (at address %08lx)" \
+		"...\n", (ulong) kernel_entry);
+	show_boot_progress(15);
+	announce_and_cleanup();
+	kernel_entry(0, machid, gd->bd->bi_boot_params);
+}
+
+/* Main Entry point for arm bootm implementation
+ *
+ * Modeled after the powerpc implementation
+ * DIFFERENCE: Instead of calling prep and go at the end
+ * they are called if subommand is equal 0.
+ */
+int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
+{
+	if (flag & BOOTM_STATE_OS_CMDLINE)
+		boot_cmdline_linux(images);
+
+	if (flag & BOOTM_STATE_OS_BD_T)
+		boot_bd_t_linux(images);
+
+	if (flag & BOOTM_STATE_OS_PREP || flag == 0)
+		boot_prep_linux(images);
+
+	if (flag & BOOTM_STATE_OS_GO || flag == 0)
+		boot_jump_linux(images);
+
+	return 0;
 }