From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Mon, 19 May 2008 08:10:12 +0200 Subject: [U-Boot-Users] [PATCH V2] PPC4xx: Unify ECC Initialization Routines In-Reply-To: <1211008580-24532-1-git-send-email-gerickson@nuovations.com> References: <> <1211008580-24532-1-git-send-email-gerickson@nuovations.com> Message-ID: <200805190810.13041.sr@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > 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 > #include > > +#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 > + * > + * Copyright (c) 2005-2007 DENX Software Engineering, GmbH > + * Stefan Roese > + * > + * Copyright (c) 2002 Artesyn Technology > + * Jun Gu > + * > + * Copyright (c) 2001 Wave 7 Optics > + * Bill Hunter 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 > +#include > +#include > +#include > +#include > + > +#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 > + * > + * Copyright (c) 2007 DENX Software Engineering, GmbH > + * Stefan Roese > + * > + * 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 > #include > #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 =====================================================================