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

[Stefan Roese <sr@denx.de>]

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
=====================================================================