Re: [U-Boot-Users] Antw: Re: [PATCH] Add first Netstal board HCU4

Hi Niklaus,

On Wednesday 14 February 2007 18:10, Niklaus Giger wrote:

Here it is:

diff --git a/cpu/ppc4xx/spd_sdram.c b/cpu/ppc4xx/spd_sdram.c

Could you please generate a new patch against the current git repository? I reorganized the SPD files. "40x_spd_sdram.c" is now the file you want.

Please see some comments already below.

old mode 100644 new mode 100755 index c24456b..ca922d4 --- a/cpu/ppc4xx/spd_sdram.c +++ b/cpu/ppc4xx/spd_sdram.c @@ -42,6 +42,7 @@ #include <common.h> #include <asm/processor.h> #include <i2c.h> #include <ppc4xx.h> +#include <asm/io.h>

#ifdef CONFIG_SPD_EEPROM

@@ -98,6 +99,8 @@ #else # define SPD_ERR(x) do { printf(x); return(0); } while (0) #endif

+void program_ecc (void *bank_base_addr, unsigned long num_bytes, int bank); +

Line wrapped. Please make sure next time, that your mail client doesn't wrap the lines.

#define sdram_HZ_to_ns(hertz) (1000000000/(hertz))

/* function prototypes */ @@ -304,6 +307,10 @@ #endif sdram0_ecccfg = 0xf << SDRAM0_ECCCFG_SHIFT; ecc_on = 1; } else { +#if defined(CONFIG_ECC) && defined(DEBUG)

• printf("%s: no ECC as spd 11: %d   6: %d 14: %d\n", __FUNCTION__,
  

•        read_spd(11), read_spd(6), read_spd(14));
  

+#endif

Better would be to use the debug() macro here:

#if defined(CONFIG_ECC) debug("%s: no ECC as spd 11: %d 6: %d 14: %d\n", __FUNCTION__, read_spd(11), read_spd(6), read_spd(14)); #endif

sdram0_ecccfg = 0;
ecc_on = 0;

} @@ -424,7 +431,8 @@ #endif * program all the registers. * -------------------------------------------------------------------*/

-#define mtsdram0(reg, data) mtdcr(memcfga,reg);mtdcr(memcfgd,data) +#define mtsdram0(reg, data) mtdcr(memcfga,reg);mtdcr(memcfgd,data) +#define mfsdram0(reg, data) { mtdcr(memcfga,reg);data = mfdcr(memcfgd); } /* disable memcontroller so updates work */ mtsdram0( mem_mcopt1, 0 );

@@ -447,10 +455,10 @@ #endif /* SDRAM have a power on delay, 500 micro should do */ udelay(500); sdram0_cfg = SDRAM0_CFG_DCE | SDRAM0_CFG_BRPF(1) | SDRAM0_CFG_ECCDD | SDRAM0_CFG_EMDULR;

• if (ecc_on)

• sdram0_cfg |= SDRAM0_CFG_MEMCHK;
  

  mtsdram0(mem_mcopt1, sdram0_cfg);

+#ifdef CONFIG_ECC

• if (ecc_on) program_ecc(0, total_size, 0);

+#endif

New line after the if () statement please.

return (total_size); }

@@ -696,8 +704,6 @@ void program_tr0 (unsigned long* dimm_po

void program_tr1 (void);

-void program_ecc (unsigned long num_bytes);

unsigned long program_bxcr(unsigned long* dimm_populated, unsigned char* iic0_dimm_addr, @@ -1785,47 +1791,76 @@ #endif return(bank_base_addr); }

-void program_ecc (unsigned long num_bytes) +#endif /* CONFIG_440 */

+#define SDRAM_ECCCFG_CE0 0x00800000 /* ECC Correction Enable for Bank 0 */ +#define SDRAM_ECCCFG_CE1 0x00400000 /* ECC Correction Enable for Bank 1 */ +#define SDRAM_ECCCFG_CE2 0x00200000 /* ECC Correction Enable for Bank 2 */ +#define SDRAM_ECCCFG_CE3 0x00100000 /* ECC Correction Enable for Bank 3 */ + +#define SDRAM_ECCESR_ERROR_MASK 0xFFF0F000 /* All possible ECC errors */ +#define ECC_TEST_VALUE 0xaffeaffe

+/*

• • Prepare for ECC operation
• • Step 1: Enable ECC generation but not checks
• • Step 2: Fill all memory
• • Step 3: Enable ECC generation and checks
• • Only programmed for and tested on a PPC405GPr board using only bank 0

and 32 bit wide !!!

• */

+void program_ecc (void *bank_base_addr, unsigned long num_bytes, int bank) {

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

• if(bank != 0) {

• printf("\n%s: only bank 0 supported",  __FUNCTION__);
  

• }

No parentheses needed here. And a space after the "if".

/* * get Memory Controller Options 0 data */

• mfsdram(mem_cfg0, cfg0);

• mfsdram0(mem_mcopt1, cfg0);
• cfg0 &= ~SDRAM0_CFG_EMDULR & ~SDRAM0_CFG_MEMCHK;

+#ifdef DEBUG

• printf("%s: length 0x%x bytes bank_base_addr %p\n", __FUNCTION__,

num_bytes, bank_base_addr);

• printf("%s: cfg0 disable checking -> 0x%08x\n", __FUNCTION__, cfg0);

+#endif

debug()

/* * reset the bank_base address */

• bank_base_addr = CFG_SDRAM_BASE;

• mtsdram0(mem_ecccf, 0); /* disable correction */
• mtsdram0(mem_eccerr, SDRAM_ECCESR_ERROR_MASK); /* Clear all errors */
• mtsdram0(mem_mcopt1, cfg0);

• if ((cfg0 & SDRAM_CFG0_MCHK_MASK) != SDRAM_CFG0_MCHK_NON) {

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

• 	SDRAM_CFG0_MCHK_GEN);
  

• address_increment = 4;
• current_address = (unsigned long)(bank_base_addr);
• end_address = (unsigned long)(bank_base_addr) + num_bytes;

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

• 	address_increment = 4;
  

• } else {
  

• 	address_increment = 8;
  

• }
  

• while (current_address < end_address) {

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

• current_address += address_increment;
  

• }

• current_address = (unsigned long)(bank_base_addr);
  

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

• mtsdram0(mem_eccerr, SDRAM_ECCESR_ERROR_MASK); /* Clear all errors */

• while (current_address < end_address) {
  

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

• 	current_address += address_increment;
  

• }
  

+#ifdef DEBUG

• printf("%s: cfg0 enable checking\n", __FUNCTION__);

+#endif

• mtsdram0(mem_ecccf, SDRAM_ECCCFG_CE0); /* enable correction */
• printf("ECC ");

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

• 	SDRAM_CFG0_MCHK_CHK);
  

+#ifdef DEBUG

• { /* A small sanity check */

• unsigned long *check;
  

• check= (unsigned long *)bank_base_addr;
  

• *check=ECC_TEST_VALUE;
  

• if(*check != ECC_TEST_VALUE)
  

• 	printf("%s: checking at %p is 0x%x failed\n",  __FUNCTION__, check,
  

*check);

Coding style.

I will review the next version more thoroughly. And faster, I promise.

Sorry again for the delay.

Best regards, Stefan

===================================================================== DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk Office: Kirchenstr. 5, D-82194 Groebenzell, Germany =====================================================================