Skip to contentSkip to conversation

PPC4xx: Unify ECC Initialization Routines

PATCH Superseded Grant Erickson 2 participants 3 messages May 17, 2008 – May 19, 2008

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 PPC4xx: Unify ECC Initialization Routines

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.

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

---
 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
cpu/ppc4xx/44x_spd_ddr.c +5 -46
index b9cf5cb..7f04ca6 100644
@@ -53,6 +53,10 @@
 #include <ppc4xx.h>
 #include <asm/mmu.h>
 
+#ifdef CONFIG_DDR_ECC
+#include "ecc.h"
+#endif
+
 #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 */
cpu/ppc4xx/Makefile +1 -0
index 178c5c6..800bb41 100644
@@ -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
cpu/ppc4xx/ecc.c new file +119 -0

119 lines not on this page · raw message

cpu/ppc4xx/ecc.h new file +42 -0

42 lines not on this page · raw message

cpu/ppc4xx/sdram.c +3 -43

58 lines not on this page · raw message

Signature · 1 line
1.5.4.3
#1 lore raw

PATCH v2 1/1 PPC4xx: Unify ECC Initialization Routines

Quoted 8 lines · Grant Erickson

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.

Quoted 21 lines

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

---
 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
cpu/ppc4xx/44x_spd_ddr.c +0 -0
index b9cf5cb..7f04ca6 100644

cpu/ppc4xx/44x_spd_ddr.c line 58

Quoted 6 lines
cpu/ppc4xx/44x_spd_ddr.c +3 -0
@@ -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.

Quoted 135 lines
cpu/ppc4xx/44x_spd_ddr.c +1 -46
 #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 */

cpu/ppc4xx/Makefile +1 -0
index 178c5c6..800bb41 100644
@@ -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
cpu/ppc4xx/ecc.c new file +47 -0
new file mode 100644
index 0000000..dd45b50
@@ -0,0 +1,119 @@
+/*
+ *    Copyright (c) 2008 Nuovation System Designs, LLC
+ *	Grant Erickson <[email protected]>
+ *
+ *    Copyright (c) 2005-2007 DENX Software Engineering, GmbH
+ *	Stefan Roese <[email protected]>
+ *
+ *    Copyright (c) 2002 Artesyn Technology
+ *	Jun Gu <[email protected]>
+ *
+ *    Copyright (c) 2001 Wave 7 Optics
+ *	Bill Hunter <[email protected]>x
+ *
+ *    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)

cpu/ppc4xx/ecc.c line 57

Quoted 10 lines
cpu/ppc4xx/ecc.c new file +10 -0
+#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...).

Quoted 44 lines
cpu/ppc4xx/ecc.c new file +44 -0
+ *
+ *  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) */

cpu/ppc4xx/ecc.c line 111

Quoted 10 lines
cpu/ppc4xx/ecc.c new file +10 -0
+
+	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().

Quoted 60 lines
cpu/ppc4xx/ecc.c new file +7 -0
+	/* 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) */
cpu/ppc4xx/ecc.h new file +42 -0
new file mode 100644
index 0000000..da1c4fd
@@ -0,0 +1,42 @@
+/*
+ *    Copyright (c) 2008 Nuovation System Designs, LLC
+ *	Grant Erickson <[email protected]>
+ *
+ *    Copyright (c) 2007 DENX Software Engineering, GmbH
+ *	Stefan Roese <[email protected]>
+ *
+ *    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_ */
cpu/ppc4xx/sdram.c +0 -0
index 2724d91..bd0a827 100644

cpu/ppc4xx/sdram.c line 36

Quoted 6 lines
cpu/ppc4xx/sdram.c +3 -0
@@ -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 at denx.de
=====================================================================

#2 lore raw

PATCH v3 1/1 PPC4xx: Unify ECC Initialization Routines

In reply to Grant Erickson

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 reduces 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.

This revision of the patch reflects feedback from Stefan Roese:
- Remove wrappers around ecc.h.
- Remove superfluous eieio().

---
 cpu/ppc4xx/44x_spd_ddr.c |   49 +-----------------
 cpu/ppc4xx/Makefile      |    1 +
 cpu/ppc4xx/ecc.c         |  121 +++++++++++++++++++++++++++++++++++++++++++
 cpu/ppc4xx/ecc.h         |   42 ++++++++++++++++
 cpu/ppc4xx/sdram.c       |   44 +----------------
 5 files changed, 172 insertions(+), 89 deletions(-)
 create mode 100644 cpu/ppc4xx/ecc.c
 create mode 100644 cpu/ppc4xx/ecc.h
cpu/ppc4xx/44x_spd_ddr.c +3 -46
index b9cf5cb..b7eeaf2 100644
@@ -53,6 +53,8 @@
 #include <ppc4xx.h>
 #include <asm/mmu.h>
 
+#include "ecc.h"
+
 #if defined(CONFIG_SPD_EEPROM) &&					\
 	(defined(CONFIG_440GP) || defined(CONFIG_440GX) ||		\
 	 defined(CONFIG_440EP) || defined(CONFIG_440GR))
@@ -296,10 +298,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 +416,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 +1400,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 */
cpu/ppc4xx/Makefile +1 -0
index 178c5c6..800bb41 100644
@@ -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
cpu/ppc4xx/ecc.c new file +121 -0

121 lines not on this page · raw message

cpu/ppc4xx/ecc.h new file +42 -0

42 lines not on this page · raw message

cpu/ppc4xx/sdram.c +1 -43

56 lines not on this page · raw message

Signature · 1 line
1.5.4.3