[U-Boot-Users] [PATCH] PPC4xx: Add a non-EEPROM-driven SDRAM Initialization Function for the 405EX(r).

[Stefan Roese <sr@denx.de>]

On Saturday 17 May 2008, Grant Erickson wrote:
> This patch adds a non-EEPROM-driven SDRAM initialization function
> driven by compile-time CFG_SDRAM_* parameters usable by 405EX(r)-based
> boards that have discrete SDRAM chips rather than DIMMs.
>
> This patch continues laying the ground work for moving out-of-assembly
> and unifying the SDRAM initialization code for PowerPC 405EX[r]-based
> boards.
>
> Integration and unification of the EEPROM-driven version with this
> parameter driven version may be tackled on another, later pass.

Good, thanks.

Please find some comments below.

> Signed-off-by: Grant Erickson <gerickson@nuovations.com>
> ---
>  cpu/ppc4xx/44x_spd_ddr2.c |  126
> ++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 120
> insertions(+), 6 deletions(-)
>
> diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c
> index ec76b71..c88d098 100644
> --- a/cpu/ppc4xx/44x_spd_ddr2.c
> +++ b/cpu/ppc4xx/44x_spd_ddr2.c
> @@ -3,11 +3,14 @@
>   * This SPD SDRAM detection code supports AMCC PPC44x cpu's with a
>   * DDR2 controller (non Denali Core). Those currently are:
>   *
> - * 405:		405EX
> + * 405:		405EX(r)
>   * 440/460:	440SP/440SPe/460EX/460GT
>   *
> - * (C) Copyright 2007-2008
> - * Stefan Roese, DENX Software Engineering, sr at denx.de.
> + * Copyright (c) 2008 Nuovation System Designs, LLC
> + *   Grant Erickson <gerickson@nuovations.com>
> + *
> + * Copyright (c) 2007-2008 DENX Software Engineering, GmbH
> + *   Stefan Roese <sr@denx.de>
>   *
>   * COPYRIGHT   AMCC   CORPORATION 2004
>   *
> @@ -45,6 +48,10 @@
>  #include <asm/mmu.h>
>  #include <asm/cache.h>
>
> +#if defined(CONFIG_DDR_ECC)
> +#include "ecc.h"
> +#endif

It should be save to remove this #ifdef.

>  #if defined(CONFIG_SPD_EEPROM) &&				\
>  	(defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
>  	 defined(CONFIG_460EX) || defined(CONFIG_460GT))
> @@ -3064,9 +3071,116 @@ static void ppc440sp_sdram_register_dump(void)
>  	dcr_data = mfdcr(SDRAM_R3BAS);
>  	printf("        MQ3_B0BAS       = 0x%08X\n", dcr_data);
>  }
> -#else
> +#else /* !defined(DEBUG) */
>  static void ppc440sp_sdram_register_dump(void)
>  {
>  }
> -#endif
> -#endif /* CONFIG_SPD_EEPROM */
> +#endif /* defined(DEBUG) */
> +#elif defined(CONFIG_405EX)
> +/*------------------------------------------------------------------------
>----- + * Function:	initdram
> + * Description: Configures the PPC405EX(r) DDR1/DDR2 SDRAM memory
> + * 		banks. The configuration is performed using static, compile-
> + *		time parameters.
> +
> *--------------------------------------------------------------------------

Why is this block 405EX specific? This could be used on other 4xx variants 
using the same DDR2 controller, not?

And I'm wondering if this code really should go into this 
file "44x_spd_ddr2.c". Since now a 405 variant (405EX) can use this code too 
we should probably change the name to "4xx_spd_ddr2.c". And with this new 
fixed DDR2 init code it the SPD is not really fitting anymore. So the new 
name should probably be "4xx_ddr2.c".

Comments welcome.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================