[U-Boot] [PATCH 3/7] Add memory test feature for mpc85xx POST.

[Wolfgang Denk]

Dear York Sun,

In message <1282944356-4020-3-git-send-email-yorksun at freescale.com> you wrote:
> If enabled in config file and hwconfig, the memory test is performed

POST memory testing is not controlled through hwconfig, but through
it's own set of environment variable settings.  Please don't mix
these.

NAK for the current implementation.


> after DDR initialization when U-boot stills runs in flash and cache.
> Whole memory is testable. It is mapped 2GB at a time using a sliding
> TLB window. After the testing, DDR is remapped with up to 2GB memory
> from the lowest address as normal.
> 
> Memory testing has different patterns which may be improved later.

What does that mean?


> If memory test fails, DDR DIMM SPD and DDR controller registers are
> dumped. All zero values are omitted for better viewing.

Do yoxpu consider this a good idea? I don't.

> A worker function __setup_ddr_tlbs() is introduced to implement more
> control on physical address mapping.
> 
> Signed-off-by: York Sun <yorksun at freescale.com>
> ---
>  arch/powerpc/cpu/mpc85xx/cpu.c |  195 ++++++++++++++++++++++++++++++++++++++++
>  arch/powerpc/cpu/mpc85xx/tlb.c |   16 ++--
>  doc/README.fsl-ddr             |   21 ++++-
>  3 files changed, 225 insertions(+), 7 deletions(-)

Should the code not live in the post/ directory?


> diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c
> index 00696f8..634a0cd 100644
> --- a/arch/powerpc/cpu/mpc85xx/cpu.c
> +++ b/arch/powerpc/cpu/mpc85xx/cpu.c
> @@ -34,6 +34,9 @@
>  #include <asm/io.h>
>  #include <asm/mmu.h>
>  #include <asm/fsl_law.h>
> +#include <post.h>
> +#include <asm/processor.h>
> +#include <asm/fsl_ddr_sdram.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -282,3 +285,195 @@ void mpc85xx_reginfo(void)
>  	print_laws();
>  	print_lbc_regs();
>  }
> +
> +#if CONFIG_POST & CONFIG_SYS_POST_MEMORY
> +
> +/* Board-specific functions defined in each board's ddr.c */
> +void fsl_ddr_get_spd(generic_spd_eeprom_t *ctrl_dimms_spd,
> +	unsigned int ctrl_num);
> +void read_tlbcam_entry(int idx, u32 *valid, u32 *tsize, unsigned long *epn,
> +		       phys_addr_t *rpn);
> +unsigned int __setup_ddr_tlbs(phys_addr_t p_addr, unsigned int memsize_in_meg);
> +
> +static void dump_spd_ddr_reg(void)
> +{
> +	int i, j, k, m;
> +	u8 *p_8;
> +	u32 *p_32;
> +	ccsr_ddr_t *ddr[CONFIG_NUM_DDR_CONTROLLERS];
> +	generic_spd_eeprom_t
> +	   spd[CONFIG_NUM_DDR_CONTROLLERS][CONFIG_DIMM_SLOTS_PER_CTLR];

Indentation by TABs only, please.

Blank line between declarations and code.

> +	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++)
> +		fsl_ddr_get_spd(spd[i], i);

Blank line here, too.

> +	puts("SPD data of all dimms (zero vaule is omitted)...\n");
> +	puts("Byte (hex)  ");
> +	k = 1;
> +	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++)
> +		for (j = 0; j < CONFIG_DIMM_SLOTS_PER_CTLR; j++)
> +			printf("Dimm%d ", k++);

Need braces for multiline statements.

> +   In order to enable the POST memory test, CONFIG_POST needs to be
> +   defined in board configuraiton header file. By default, POST memory test performs
> +   a fast test. A slow test can be enabled by changing the flag, or hwconfig can
> +   be used to active slow test. To test memory bigger than 2GB, 36BIT support is

NAK. We provide exactly one way to control the POST system.  hwconfig
has nothing to do with that, so please don't pull that in here.


> -hwconfig=fsl_ddr:addr_hash=true,ctlr_intlv=cacheline,bank_intlv=cs0_cs1_cs2_cs3
> +hwconfig=fsl_ddr:addr_hash=true,ctlr_intlv=cacheline,bank_intlv=cs0_cs1_cs2_cs3,;memtest=slow

NAK.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Software suppliers are trying to make their  software  packages  more
``user-friendly''.  .  .  .  Their best approach, so far, has been to
take all the old brochures, and stamp the words, ``user-friendly'' on
the cover.                                               - Bill Gates