From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Tue, 12 Feb 2013 14:29:44 -0600
Subject: [U-Boot] [PATCH v5 12/23] Add spl load feature
In-Reply-To: <1360336339-10703-13-git-send-email-sjg@chromium.org> (from
	sjg@chromium.org on Fri Feb  8 09:12:08 2013)
References: <1360336339-10703-1-git-send-email-sjg@chromium.org>
	<1360336339-10703-13-git-send-email-sjg@chromium.org>
Message-ID: <1360700984.24612.9@snotra>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 02/08/2013 09:12:08 AM, Simon Glass wrote:
> This adds secondary program loader support to the generic board.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  common/board_f.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/common/board_f.c b/common/board_f.c
> index aa10f4b..3a8036f 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -309,6 +309,7 @@ static int reserve_uboot(void)
>  	return 0;
>  }
> 
> +#ifndef CONFIG_SPL_BUILD
>  /* reserve memory for malloc() area */
>  static int reserve_malloc(void)
>  {
> @@ -328,6 +329,7 @@ static int reserve_board(void)
>  			sizeof(bd_t), gd->dest_addr_sp);
>  	return 0;
>  }
> +#endif
> 
>  static int setup_machine(void)
>  {
> @@ -365,6 +367,7 @@ static int reserve_fdt(void)
>  	return 0;
>  }
> 
> +#ifndef CONFIG_SPL_BUILD
>  static int reserve_stacks(void)
>  {
>  	/* setup stack pointer for exceptions */
> @@ -384,6 +387,17 @@ static int reserve_stacks(void)
> 
>  	return 0;
>  }
> +#endif
> +
> +#ifdef CONFIG_SPL_BUILD
> +static int reserve_stacks_spl(void)
> +{
> +	/* Why not -= ? */
> +	gd->dest_addr_sp += 128;	/* leave 32 words for  
> abort-stack */
> +	gd->irq_sp = gd->dest_addr_sp;
> +	return 0;
> +}
> +#endif

"abort-stack" doesn't sound very generic, and that "why not" question  
should probably be answered.

>  static int display_new_sp(void)
>  {
> @@ -524,12 +538,18 @@ static init_fnc_t init_sequence_f[] = {
>  	reserve_lcd,
>  #endif
>  	reserve_uboot,
> +#ifndef CONFIG_SPL_BUILD
>  	reserve_malloc,
>  	reserve_board,
> +#endif
>  	setup_machine,
>  	reserve_global_data,
>  	reserve_fdt,
> +#ifdef CONFIG_SPL_BUILD
> +	reserve_stacks_spl,
> +#else
>  	reserve_stacks,
> +#endif

Why not just put the ifdef inside reserve_stacks()?  Or at least give  
them the same name?

-Scott