Re: [U-Boot-Users] [PATCH V2] PPC4xx: Unify ECC Initialization Routines

On Saturday 17 May 2008, Grant Erickson wrote:

This patch continues laying the ground work for moving out-of-assembly and unifying the SDRAM initialization code for PowerPC 405EX[r]-based boards.

To do so, this deduces by one the number of nearly-identical DRAM ECC initialization implementations for PowerPC 4xx processors utilizing a DDR/DDR2 SDRAM controller by merging two of them into a common, shared implementation.

Good idea. Thanks. Please find some comments below.

The last revision of this patch failed to compile on 440ERX/GPX systems, so the appropriate preprocessor wrappers were added to address it.

Signed-off-by: Grant Erickson gerickson@nuovations.com

cpu/ppc4xx/44x_spd_ddr.c | 51 ++------------------ cpu/ppc4xx/Makefile | 1 + cpu/ppc4xx/ecc.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++ cpu/ppc4xx/ecc.h | 42 ++++++++++++++++ cpu/ppc4xx/sdram.c | 46 +----------------- 5 files changed, 170 insertions(+), 89 deletions(-) create mode 100644 cpu/ppc4xx/ecc.c create mode 100644 cpu/ppc4xx/ecc.h

diff --git a/cpu/ppc4xx/44x_spd_ddr.c b/cpu/ppc4xx/44x_spd_ddr.c index b9cf5cb..7f04ca6 100644 --- a/cpu/ppc4xx/44x_spd_ddr.c +++ b/cpu/ppc4xx/44x_spd_ddr.c @@ -53,6 +53,10 @@ #include <ppc4xx.h> #include <asm/mmu.h>

+#ifdef CONFIG_DDR_ECC +#include "ecc.h" +#endif

It should be save to include this header unconditionally. Please remove the #ifdef here.

#if defined(CONFIG_SPD_EEPROM) && \ (defined(CONFIG_440GP) || defined(CONFIG_440GX) || \ defined(CONFIG_440EP) || defined(CONFIG_440GR)) @@ -296,10 +300,6 @@ static void program_tr0(unsigned long *dimm_populated, unsigned long num_dimm_banks); static void program_tr1(void);

-#ifdef CONFIG_DDR_ECC -static void program_ecc(unsigned long num_bytes); -#endif

static unsigned long program_bxcr(unsigned long *dimm_populated, unsigned char *iic0_dimm_addr, unsigned long num_dimm_banks); @@ -418,7 +418,7 @@ long int spd_sdram(void) { /* * If ecc is enabled, initialize the parity bits. */

• program_ecc(total_size);

• ecc_init(CFG_SDRAM_BASE, total_size);

#endif

return total_size; @@ -1402,45 +1402,4 @@ static unsigned long program_bxcr(unsigned long *dimm_populated,

return(bank_base_addr); }

-#ifdef CONFIG_DDR_ECC -static void program_ecc(unsigned long num_bytes) -{

• unsigned long bank_base_addr;
• unsigned long current_address;
• unsigned long end_address;
• unsigned long address_increment;
• unsigned long cfg0;
• /*

• * get Memory Controller Options 0 data
  

• */
  

• mfsdram(mem_cfg0, cfg0);
• /*

• * reset the bank_base address
  

• */
  

• bank_base_addr = CFG_SDRAM_BASE;
• if ((cfg0 & SDRAM_CFG0_MCHK_MASK) != SDRAM_CFG0_MCHK_NON) {

• mtsdram(mem_cfg0, (cfg0 & ~SDRAM_CFG0_MCHK_MASK) | SDRAM_CFG0_MCHK_GEN);
  

• if ((cfg0 & SDRAM_CFG0_DMWD_MASK) == SDRAM_CFG0_DMWD_32)
  

• 	address_increment = 4;
  

• else
  

• 	address_increment = 8;
  

• current_address = (unsigned long)(bank_base_addr);
  

• end_address = (unsigned long)(bank_base_addr) + num_bytes;
  

• while (current_address < end_address) {
  

• 	*((unsigned long*)current_address) = 0x00000000;
  

• 	current_address += address_increment;
  

• }
  

• mtsdram(mem_cfg0, (cfg0 & ~SDRAM_CFG0_MCHK_MASK) |
  

• 	SDRAM_CFG0_MCHK_CHK);
  

• }

-} -#endif /* CONFIG_DDR_ECC */ #endif /* CONFIG_SPD_EEPROM */ diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile index 178c5c6..800bb41 100644 --- a/cpu/ppc4xx/Makefile +++ b/cpu/ppc4xx/Makefile @@ -45,6 +45,7 @@ COBJS += cpu.o COBJS += cpu_init.o COBJS += denali_data_eye.o COBJS += denali_spd_ddr2.o +COBJS += ecc.o COBJS += fdt.o COBJS += gpio.o COBJS += i2c.o diff --git a/cpu/ppc4xx/ecc.c b/cpu/ppc4xx/ecc.c new file mode 100644 index 0000000..dd45b50 --- /dev/null +++ b/cpu/ppc4xx/ecc.c @@ -0,0 +1,119 @@ +/*

• • Copyright (c) 2008 Nuovation System Designs, LLC
• • Grant Erickson gerickson@nuovations.com
• • Copyright (c) 2005-2007 DENX Software Engineering, GmbH
• • Stefan Roese sr@denx.de
• • Copyright (c) 2002 Artesyn Technology
• • Jun Gu jung@artesyncp.com
• • Copyright (c) 2001 Wave 7 Optics
• • Bill Hunter williamhunter@attbi.comx
• • See file CREDITS for list of people who contributed to this
• • project.
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will abe useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• • Description:
• • This file implements generic DRAM ECC initialization for
• • PowerPC processors using a SDRAM DDR/DDR2 controller,
• • including the 405EX(r), 440GP/GX/EP/GR, 440SP(E), and
• • 460EX/GT.
• */

+#include <common.h> +#include <ppc4xx.h> +#include <ppc_asm.tmpl> +#include <ppc_defs.h> +#include <asm/processor.h>

+#include "ecc.h"

+#if !defined(CONFIG_440EPX) && !defined(CONFIG_440GRX) +#if defined(CONFIG_DDR_ECC) || defined(CONFIG_SDRAM_ECC) +/*

• • void ecc_init()
• • Description:
• • This routine initializes a range of DRAM ECC memory with known
• • data and enables ECC checking.
• • TO DO:
• • Improve performance by utilizing cache.

Yes. And please add something like this here:

* Make is more generally usable for other 4xx PPC variants (like 440EPx...).

• • Input(s):
• • start - A pointer to the start of memory covered by ECC requiring

• •      initialization.
    

• • size - The size, in bytes, of the memory covered by ECC requiring

• •      initialization.
    

• • Output(s):
• • start - A pointer to the start of memory covered by ECC with

• •      CFG_ECC_PATTERN written to all locations and ECC data
    

• •      primed.
    

• • Returns:
• • N/A
• */

+void ecc_init(unsigned long * const start, unsigned long size) +{

• const unsigned long pattern = CFG_ECC_PATTERN;
• unsigned * const end = (unsigned long * const)((long)start + size);
• unsigned long * current = start;
• unsigned long mcopt1;
• long increment;
• if (start >= end)

• return;
  

• mfsdram(SDRAM_MCOPT1, mcopt1);
• /* Enable ECC generation without checking or reporting */
• mtsdram(SDRAM_MCOPT1, ((mcopt1 & ~SDRAM_MCOPT1_MCHK_MASK) |

• 	       SDRAM_MCOPT1_MCHK_GEN));
  

• increment = sizeof(u32);

+#if defined(CONFIG_440)

• /*

• * Look at the geometry of SDRAM (data width) to determine whether we
  

• * can skip words when writing.
  

• */
  

• if ((mcopt1 & SDRAM_MCOPT1_DMWD_MASK) != SDRAM_MCOPT1_DMWD_32)

• increment = sizeof(u64);
  

+#endif /* defined(CONFIG_440) */

• while (current < end) {

• *current = pattern;
  

•  current = (unsigned long *)((long)current + increment);
  

• }
• /* Wait until the writes are finished. */
• sync();
• eieio();

Please drop the eieio(). It's not needed since we have the sync().

• /* Enable ECC generation with checking and no reporting */
• mtsdram(SDRAM_MCOPT1, ((mcopt1 & ~SDRAM_MCOPT1_MCHK_MASK) |

• 	       SDRAM_MCOPT1_MCHK_CHK));
  

+} +#endif /* defined(CONFIG_DDR_ECC) || defined(CONFIG_SDRAM_ECC) */ +#endif /* !defined(CONFIG_440EPX) && !defined(CONFIG_440GRX) */ diff --git a/cpu/ppc4xx/ecc.h b/cpu/ppc4xx/ecc.h new file mode 100644 index 0000000..da1c4fd --- /dev/null +++ b/cpu/ppc4xx/ecc.h @@ -0,0 +1,42 @@ +/*

• • Copyright (c) 2008 Nuovation System Designs, LLC
• • Grant Erickson gerickson@nuovations.com
• • Copyright (c) 2007 DENX Software Engineering, GmbH
• • Stefan Roese sr@denx.de
• • See file CREDITS for list of people who contributed to this
• • project.
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License as
• • published by the Free Software Foundation; either version 2 of
• • the License, or (at your option) any later version.
• • This program is distributed in the hope that it will abe useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• • You should have received a copy of the GNU General Public License
• • along with this program; if not, write to the Free Software
• • Foundation, Inc., 59 Temple Place, Suite 330, Boston,
• • MA 02111-1307 USA
• • Description:
• • This file implements ECC initialization for PowerPC processors
• • using the SDRAM DDR2 controller, including the 405EX(r),
• • 440SP(E), 460EX and 460GT.
• */

+#ifndef _ECC_H_ +#define _ECC_H_

+#if !defined(CFG_ECC_PATTERN) +#define CFG_ECC_PATTERN 0x00000000 +#endif /* !defined(CFG_ECC_PATTERN) */

+extern void ecc_init(unsigned long * const start, unsigned long size);

+#endif /* _ECC_H_ */ diff --git a/cpu/ppc4xx/sdram.c b/cpu/ppc4xx/sdram.c index 2724d91..bd0a827 100644 --- a/cpu/ppc4xx/sdram.c +++ b/cpu/ppc4xx/sdram.c @@ -31,6 +31,9 @@ #include <ppc4xx.h> #include <asm/processor.h> #include "sdram.h" +#ifdef CONFIG_SDRAM_ECC +#include "ecc.h" +#endif

Again, please make it unconditional.

Thanks a lot.

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@denx.de =====================================================================