[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[Kumar Gala]

From: york <yorksun@freescale.com>

If enabled in config file and hwconfig, the memory test is performed
after DDR initialization when U-boot stills runs in flash and cache.
Whole memory is testable. However, only the low 2GB space is mapped
for DDR. The testing is conducted in the 2GB window and uses TLBs to
map the higher physical address into the 2GB window if the total
memory is more than 2GB. After the testing, DDR is remapped with up
to 2GB memory from the lowest address.

Memory testing has different patterns which may be improved later.

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

A worker function __setup_ddr_tlbs() is introduced to implemente more
control on physical address mapping.

Signed-off-by: York Sun <yorksun@freescale.com>
---
 arch/powerpc/cpu/mpc85xx/Makefile  |    2 +
 arch/powerpc/cpu/mpc85xx/memtest.c |  369 ++++++++++++++++++++++++++++++++++++
 arch/powerpc/cpu/mpc85xx/tlb.c     |   16 +-
 doc/README.fsl-ddr                 |   39 ++++
 4 files changed, 420 insertions(+), 6 deletions(-)
 create mode 100644 arch/powerpc/cpu/mpc85xx/memtest.c

diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
index 4ee0e9a..c0fc9ba 100644
--- a/arch/powerpc/cpu/mpc85xx/Makefile
+++ b/arch/powerpc/cpu/mpc85xx/Makefile
@@ -60,6 +60,8 @@ COBJS-$(CONFIG_P2010)	+= ddr-gen3.o
 COBJS-$(CONFIG_P2020)	+= ddr-gen3.o
 COBJS-$(CONFIG_PPC_P4080)	+= ddr-gen3.o
 
+COBJS-${CONFIG_SYS_DRAM_TEST}	+= memtest.o
+
 COBJS-$(CONFIG_CPM2)	+= ether_fcc.o
 COBJS-$(CONFIG_OF_LIBFDT) += fdt.o
 COBJS-$(CONFIG_MP)	+= mp.o
diff --git a/arch/powerpc/cpu/mpc85xx/memtest.c b/arch/powerpc/cpu/mpc85xx/memtest.c
new file mode 100644
index 0000000..7ad3326
--- /dev/null
+++ b/arch/powerpc/cpu/mpc85xx/memtest.c
@@ -0,0 +1,369 @@
+/*
+ * Copyright (C) 2010 Freescale Semiconductor, Inc.
+ * York Sun <yorksun@freescale.com>
+ *
+ * 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 be 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
+ */
+
+#include <common.h>
+#include <hwconfig.h>
+#include <asm/processor.h>
+#include <asm/mmu.h>
+#include <asm/fsl_ddr_sdram.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* 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);
+
+#define PATTERN(pattern, address) (pattern == 0x12345678 ? address : pattern)
+
+#define TOTAL_PROGRESS_DOTS 45
+#define TOTAL_PROGRESS_NUMBERS 9
+#define PROGRESS_DOTS_PER_NUMBER (TOTAL_PROGRESS_DOTS/TOTAL_PROGRESS_NUMBERS)
+#define TEST_SHOW_PROGRESS(scale, dots, digit, dots_sub) \
+{ \
+	dots -= (dots_sub); \
+	if ((scale > 0) && (dots <= 0)) { \
+		if ((digit % PROGRESS_DOTS_PER_NUMBER) == 0) \
+			printf("%d", digit / PROGRESS_DOTS_PER_NUMBER); \
+		else \
+			putc('.'); \
+		digit--; \
+		dots += (scale); \
+	} \
+}
+
+#if defined(CONFIG_SYS_DRAM_TEST)
+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];
+
+	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+		fsl_ddr_get_spd(spd[i], i);
+	}
+	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++);
+	puts("\n");
+	for (k = 0; k < sizeof(generic_spd_eeprom_t); k++) {
+		m = 0;
+		printf("%3d (0x%02x)  ", k, k);
+		for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+			for (j = 0; j < CONFIG_DIMM_SLOTS_PER_CTLR; j++) {
+				p_8 = (u8 *) &spd[i][j];
+				if (p_8[k]) {
+					printf("0x%02x  ", p_8[k]);
+					m++;
+				} else
+					puts("      ");
+			}
+		}
+		if (m)
+			puts("\n");
+		else
+			puts("\r");
+	}
+
+	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+		switch (i) {
+		case 0:
+			ddr[i] = (void *)CONFIG_SYS_MPC85xx_DDR_ADDR;
+			break;
+#ifdef CONFIG_SYS_MPC85xx_DDR2_ADDR
+		case 1:
+			ddr[i] = (void *)CONFIG_SYS_MPC85xx_DDR2_ADDR;
+			break;
+#endif
+		default:
+			printf("%s unexpected controller number = %u\n",
+				__FUNCTION__, i);
+			return;
+		}
+	}
+	printf("DDR registers dump for all controllers "
+		"(zero vaule is omitted)...\n");
+	puts("Offset (hex)   ");
+	for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++)
+		printf("     Base + 0x%04x", (u32)ddr[i] & 0xFFFF);
+	puts("\n");
+	for (k = 0; k < sizeof(ccsr_ddr_t)/4; k++) {
+		m = 0;
+		printf("%6d (0x%04x)", k * 4, k * 4);
+		for (i = 0; i < CONFIG_NUM_DDR_CONTROLLERS; i++) {
+			p_32 = (u32 *) ddr[i];
+			if (p_32[k]) {
+				printf("        0x%08x", p_32[k]);
+				m++;
+			} else
+				puts("                  ");
+		}
+		if (m)
+			puts("\n");
+		else
+			puts("\r");
+	}
+	puts("\n");
+}
+
+#define MODULOMASK 0x3F
+#define MODULOX 16
+int pattern_test(u32 start_addr, u32 size, u32 pattern, u64 offset)
+{
+	const int MaxError = 10;
+	volatile u32 *p;
+	u32 ptr, buffer, end_addr = start_addr + size;
+	int error = 0;
+	int digit, dots;
+	int scale;
+
+	puts("Pattern: Moving Inversions ");
+	if (pattern == 0x12345678)
+		puts("Address March\n");
+	else
+		printf("0x%08x\n", pattern);
+
+	scale = (int)(((size >> 20) + TOTAL_PROGRESS_DOTS - 1) /
+		TOTAL_PROGRESS_DOTS);
+	digit = TOTAL_PROGRESS_DOTS;
+	dots  = 0;
+	for (p = (volatile u32 *)(start_addr & 0xfffffffc);
+		p < (volatile u32 *)(end_addr & 0xfffffffc); p++) {
+		*p = PATTERN(pattern, (u32)p);
+		if (((u32)p % (1 << 20)) == 0)
+			TEST_SHOW_PROGRESS(scale, dots, digit, 1);
+	}
+	puts("Filled with pattern.\n");
+	digit = TOTAL_PROGRESS_DOTS;
+	dots  = 0;
+	for (ptr = (start_addr & 0xfffffffc);
+		ptr < (end_addr & 0xfffffffc); ptr += sizeof(u32)) {
+		p = (volatile u32 *) ptr;
+		buffer = *p;
+		if (buffer != PATTERN(pattern, ptr)) {
+			error++;
+			printf("\nAddress 0x%08llx, Write 0x%08x, Read 0x%08x",
+			offset + ptr, PATTERN(pattern, ptr), buffer);
+			if (error > MaxError)
+				break;
+		}
+		*p = ~PATTERN(pattern, ptr);
+		if ((ptr % (1 << 20)) == 0)
+			TEST_SHOW_PROGRESS(scale, dots, digit, 1);
+	}
+	if (error == 0)
+		puts("Verified and written with complement.\n");
+	else {
+		puts("\nFailed!\n\n");
+		return error;
+	}
+	digit = TOTAL_PROGRESS_DOTS;
+	dots  = 0;
+	for (ptr = ((end_addr - 1) & 0xfffffffc);
+		ptr > (start_addr & 0xfffffffc); ptr -= sizeof(u32)) {
+		p = (volatile u32 *) ptr;
+		buffer = *p;
+		if (buffer != ~PATTERN(pattern, ptr)) {
+			error++;
+			printf("\nAddress 0x%08llx, Write 0x%08x, Read 0x%08x",
+			offset + ptr, ~PATTERN(pattern, ptr), buffer);
+			if (error > MaxError)
+				break;
+		}
+		*p = PATTERN(pattern, ptr);
+		if ((ptr % (1 << 20)) == 0)
+			TEST_SHOW_PROGRESS(scale, dots, digit, 1);
+	}
+	if (error == 0)
+		puts("Verified OK.\n\n");
+	else {
+		puts("\nFailed!\n\n");
+		return error;
+	}
+
+	printf("Pattern: Modulo-%d ", MODULOX);
+	if (pattern == 0x12345678)
+		puts("Address March\n");
+	else
+		printf(" 0x%08x\n", pattern);
+	digit = TOTAL_PROGRESS_DOTS;
+	dots  = 0;
+	for (ptr = (start_addr & 0xfffffffc);
+		ptr < (end_addr & 0xfffffffc); ptr += sizeof(u32)) {
+		p = (volatile u32 *) ptr;
+		if (ptr & MODULOMASK)
+			*p = PATTERN(pattern, ptr);
+		else
+			*p = ~PATTERN(pattern, ptr);
+		if (ptr % (1 << 20) == 0)
+			TEST_SHOW_PROGRESS(scale, dots, digit, 1);
+	}
+	puts("Filled with pattern.\n");
+	scale = (int)(((size / MODULOX / sizeof(u32)) + TOTAL_PROGRESS_DOTS - 1) /
+		TOTAL_PROGRESS_DOTS);
+	digit = TOTAL_PROGRESS_DOTS;
+	dots  = 0;
+	for (ptr = (start_addr & 0xfffffffc);
+		ptr < (end_addr & 0xfffffffc); ptr += sizeof(u32)) {
+		if (ptr & MODULOMASK)
+			break;
+	}
+	for (; ptr < (end_addr & 0xfffffffc); ptr += MODULOX * sizeof(u32)) {
+		p = (volatile u32 *) ptr;
+		buffer = *p;
+		if (buffer != PATTERN(pattern, ptr)) {
+			error++;
+			printf("\nAddress 0x%08llx, Write 0x%08x, Read 0x%08x",
+				ptr + offset, PATTERN(pattern, ptr), buffer);
+			if (error > MaxError)
+				break;
+		}
+		TEST_SHOW_PROGRESS(scale, dots, digit, 1);
+	}
+	if (error == 0)
+		puts("OK.\n\n");
+	else
+		puts("\nFailed!\n\n");
+	return error;
+}
+
+/*
+ * This funciton is called before U-boot relocates itself to RAM
+ * The L1 cache should be locked and L2 cache is not enabled yet
+ */
+int __testdram(phys_addr_t p_addr, u32 size, int simple_test)
+{
+
+	u32 vstart = CONFIG_SYS_DDR_SDRAM_BASE;
+	u32 vend = vstart + size;
+	int failed = 0;
+	unsigned long epn;
+	u32 tsize, valid, ptr;
+	phys_addr_t rpn = 0;
+	int ddr_esel;
+
+	puts("DDR memory testing...\n");
+	size = min(size, CONFIG_MAX_MEM_MAPPED);
+	ptr = vstart;
+	while (ptr < vend) {
+		ddr_esel = find_tlb_idx((void *)ptr, 1);
+		if (ddr_esel != -1) {
+			read_tlbcam_entry(ddr_esel, &valid, &tsize, &epn, &rpn);
+			disable_tlb(ddr_esel);
+		}
+		ptr += TSIZE_TO_BYTES(tsize);
+	}
+	/* Setup new tlb to cover the physical address */
+	__setup_ddr_tlbs(p_addr, size>>20);
+
+	ptr = vstart;
+	ddr_esel = find_tlb_idx((void *)ptr, 1);
+	if (ddr_esel != -1) {
+		read_tlbcam_entry(ddr_esel, &valid, &tsize, &epn, &rpn);
+	} else {
+		printf("TLB error in function %s\n", __FUNCTION__);
+		return -1;
+	}
+
+	printf("Testing 0x%08llx - 0x%08llx\n",
+		(u64)vstart + rpn, (u64)vstart + rpn + size - 1);
+
+	failed = pattern_test(vstart, size, 0, rpn);
+	if (!simple_test) {
+		if (failed == 0)
+			failed = pattern_test(vstart, size, 0xaaaaaaaa, rpn);
+		if (failed == 0)
+			pattern_test(vstart, size, 0x55555555, rpn);
+		if (failed == 0)
+			failed = pattern_test(vstart, size, 0xffffffff, rpn);
+		if (failed == 0)
+			failed = pattern_test(vstart, size, 0x12345678, rpn);
+	}
+	if (failed != 0)
+		dump_spd_ddr_reg();
+
+	/* disable the TLBs for this testing */
+	ptr = vstart;
+	while (ptr < vend) {
+		ddr_esel = find_tlb_idx((void *)ptr, 1);
+		if (ddr_esel != -1) {
+			read_tlbcam_entry(ddr_esel, &valid, &tsize, &epn, &rpn);
+			disable_tlb(ddr_esel);
+		}
+		ptr += TSIZE_TO_BYTES(tsize);
+	}
+
+	return failed;
+}
+
+int testdram(void)
+{
+	int simple_test, failed = 0;
+	phys_addr_t test_cap, p_addr = CONFIG_SYS_DDR_SDRAM_BASE;
+	phys_size_t p_size;
+	if (hwconfig("memtest")) {
+		if (hwconfig_arg_cmp("memtest", "null") || \
+			(!hwconfig_arg_cmp("memtest", "true") && \
+			!hwconfig_arg_cmp("memtest", "simple"))) {
+			puts("memtest is not enabled in hwconfig, skipped.\n");
+			return 0;
+		}
+		simple_test = hwconfig_arg_cmp("memtest", "simple");
+		p_size = min(gd->ram_size, CONFIG_MAX_MEM_MAPPED);
+#if !defined(CONFIG_PHYS_64BIT) || \
+    !defined(CONFIG_SYS_INIT_RAM_ADDR_PHYS) || \
+	(CONFIG_SYS_INIT_RAM_ADDR_PHYS < 0x100000000ull)
+		if (gd->ram_size > CONFIG_MAX_MEM_MAPPED) {
+			puts("Cannot test more than ");
+			print_size(CONFIG_MAX_MEM_MAPPED,
+				" without proper 36BIT support.\n");
+		}
+		test_cap = p_size;
+#else
+		test_cap = gd->ram_size;
+#endif
+		while (p_addr < test_cap - 1) {
+			if (__testdram(p_addr, (u32)p_size, simple_test) != 0)
+				failed++;
+			p_addr += p_size;
+			p_size = min(test_cap - p_addr, CONFIG_MAX_MEM_MAPPED);
+		}
+		if (failed)
+			printf("\nWarning: Memory test failed."
+				" Software may not run as expected.\n");
+		puts("Remap DDR ");
+		setup_ddr_tlbs(gd->ram_size>>20);
+		puts("\n");
+	} else
+		puts("memtest is not set in hwconfig, skipped.\n");
+	return 0;
+}
+#endif
diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c
index f2833a5..019fab7 100644
--- a/arch/powerpc/cpu/mpc85xx/tlb.c
+++ b/arch/powerpc/cpu/mpc85xx/tlb.c
@@ -245,7 +245,8 @@ void init_addr_map(void)
 }
 #endif
 
-unsigned int setup_ddr_tlbs(unsigned int memsize_in_meg)
+unsigned int
+__setup_ddr_tlbs(phys_addr_t p_addr, unsigned int memsize_in_meg)
 {
 	int i;
 	unsigned int tlb_size;
@@ -275,21 +276,24 @@ unsigned int setup_ddr_tlbs(unsigned int memsize_in_meg)
 
 		tlb_size = (camsize - 10) / 2;
 
-		set_tlb(1, ram_tlb_address, ram_tlb_address,
+		set_tlb(1, ram_tlb_address, p_addr,
 			MAS3_SX|MAS3_SW|MAS3_SR, 0,
 			0, ram_tlb_index, tlb_size, 1);
 
 		size -= 1ULL << camsize;
 		memsize -= 1ULL << camsize;
 		ram_tlb_address += 1UL << camsize;
+		p_addr += 1UL << camsize;
 	}
 
 	if (memsize)
 		print_size(memsize, " left unmapped\n");
-
-	/*
-	 * Confirm that the requested amount of memory was mapped.
-	 */
 	return memsize_in_meg;
 }
+
+unsigned int setup_ddr_tlbs(unsigned int memsize_in_meg)
+{
+	return
+		__setup_ddr_tlbs(CONFIG_SYS_DDR_SDRAM_BASE, memsize_in_meg);
+}
 #endif /* !CONFIG_NAND_SPL */
diff --git a/doc/README.fsl-ddr b/doc/README.fsl-ddr
index e108a0d..f5deb4f 100644
--- a/doc/README.fsl-ddr
+++ b/doc/README.fsl-ddr
@@ -78,6 +78,45 @@ If the DDR controller supports address hashing, it can be enabled by hwconfig.
 Syntax is:
 hwconfig=fsl_ddr:addr_hash=true
 
+
+Memory testing options for mpc85xx
+==================================
+1. Memory test can be done one U-boot prompt comes up using mtest, or
+2. Memory test can be done with a built-in function, activated at compile time
+   and by hwconfig.
+   In order to enable the built-in memory test, CONFIG_SYS_DRAM_TEST needs to be
+   defined in board configuraiton header file. Hwconfig is also required. To test
+   memory bigger than 2GB, 36BIT support is needed. Memory is tested within a 2GB
+   window. TLBs are used to map the virtual 2GB window to physical address so that
+   all physical memory can be tested.
+
+   Syntax of hwconfig
+   hwconfig=memtest:true      or
+   hwconfig=memtest:simple
+
+   The "simple" test uses one pattern and "true" uses more patterns. The testing
+   patterns include moving inversions and modulo-16.
+
+Moving inversions
+=================
+Moving inversions test takes a pattern (eg. 0x00000000) and fills the memory with
+the pattern for the first round. The software then takes the second round to check
+each address against the pattern, if matched the complement data is written back
+and software moves on to next address, all the way to the end. In the third round,
+software checks the complement of pattern of each address.
+
+Modulo-16
+=========
+Modulo-16 is a test for selected address. In the first round, software fills the
+memory with a pattern and the complement of the pattern every 16-sizeof(int). In
+the second round, software checks the complement of the pattern@selected address.
+
+Patterns
+========
+Built-in patterns are 0x00000000, 0xAAAAAAAA, 0x55555555, 0xFFFFFFFF and Address
+Marching (i.e. the address of the cell).
+
+
 Combination of hwconfig
 =======================
 Hwconfig can be combined with multiple parameters, for example, on a supported
-- 
1.6.0.6

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[Wolfgang Denk]

Dear Kumar Gala,

In message <1279120502-6289-8-git-send-email-galak@kernel.crashing.org> you wrote:
> From: york <yorksun@freescale.com>
> 
> If enabled in config file and hwconfig, the memory test is performed
> after DDR initialization when U-boot stills runs in flash and cache.
> Whole memory is testable. However, only the low 2GB space is mapped
> for DDR. The testing is conducted in the 2GB window and uses TLBs to
> map the higher physical address into the 2GB window if the total
> memory is more than 2GB. After the testing, DDR is remapped with up
> to 2GB memory from the lowest address.
> 
> Memory testing has different patterns which may be improved later.
> 
> If memory test fails, DDR DIMM SPD and DDR controller registers are
> dumped. All zero values are omitted for better viewing.
> 
> A worker function __setup_ddr_tlbs() is introduced to implemente more
> control on physical address mapping.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> ---
>  arch/powerpc/cpu/mpc85xx/Makefile  |    2 +
>  arch/powerpc/cpu/mpc85xx/memtest.c |  369 ++++++++++++++++++++++++++++++++++++

NAK.

Please do not reinvent the wheel and add yet another meory test. Use
one of the existing memory tests we already have -
post/drivers/memory.c comes to mind.

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
"A dirty mind is a joy forever."                       - Randy Kunkee

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[Timur Tabi]

On Wed, Jul 14, 2010 at 3:16 PM, Wolfgang Denk <wd@denx.de> wrote:

> NAK.
>
> Please do not reinvent the wheel and add yet another meory test. Use
> one of the existing memory tests we already have -
> post/drivers/memory.c comes to mind.

That code is not capable of testing more than 2GB of RAM.  It assumes
that memory has already been initialized, but on PowerPC parts, we
only ever map 2GB of DDR.  We need a memory test that covers all of
DDR, but that requries PowerPC-specific code to create sliding TLB
windows during the memory test.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[Wolfgang Denk]

Dear Timur Tabi,

In message <AANLkTim6yjF6ORogtqE0vqiVqZqxu-EkkUhDTmJMgPqu@mail.gmail.com> you wrote:
> 
> > NAK.
> >
> > Please do not reinvent the wheel and add yet another meory test. Use
> > one of the existing memory tests we already have -
> > post/drivers/memory.c comes to mind.
> 
> That code is not capable of testing more than 2GB of RAM.  It assumes
> that memory has already been initialized, but on PowerPC parts, we
> only ever map 2GB of DDR.  We need a memory test that covers all of
> DDR, but that requries PowerPC-specific code to create sliding TLB
> windows during the memory test.

Then please extend the existing code and add the missing features.

We already have too many different implementations of a memory test in
U-Boot, and I will not accept adding yet another one.

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
Programmer's Lament: (Shakespeare, Macbeth, Act I, Scene vii)
        "That we but teach bloody instructions,
        which, being taught, return to plague the inventor..."

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[York Sun]

Wolfgang,

On Thu, 2010-07-15 at 11:07 +0200, Wolfgang Denk wrote: 
> Dear Timur Tabi,
> 
> In message <AANLkTim6yjF6ORogtqE0vqiVqZqxu-EkkUhDTmJMgPqu@mail.gmail.com> you wrote:
> > 
> > > NAK.
> > >
> > > Please do not reinvent the wheel and add yet another meory test. Use
> > > one of the existing memory tests we already have -
> > > post/drivers/memory.c comes to mind.
> > 
> > That code is not capable of testing more than 2GB of RAM.  It assumes
> > that memory has already been initialized, but on PowerPC parts, we
> > only ever map 2GB of DDR.  We need a memory test that covers all of
> > DDR, but that requries PowerPC-specific code to create sliding TLB
> > windows during the memory test.
> 
> Then please extend the existing code and add the missing features.
> 
> We already have too many different implementations of a memory test in
> U-Boot, and I will not accept adding yet another one.
> 
I can reuse your testing code but have to move the desired code out of
memory.c file to avoid the need for CONFIG_POST and
CONFIG_SYS_POST_MEMORY. I also add a progress indicator. My testing
target is 2GB at a time, up to physically memory size which is easily
over 8GB. Without progress indicator, it feels hung when it is actually
running.

Please take a look at the patch below.

Regards,

York

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[Wolfgang Denk]

Dear York Sun,

In message <1280351179.8571.63.camel@oslab-l1> you wrote:
> 
> > We already have too many different implementations of a memory test in
> > U-Boot, and I will not accept adding yet another one.
> > 
> I can reuse your testing code but have to move the desired code out of
> memory.c file to avoid the need for CONFIG_POST and
> CONFIG_SYS_POST_MEMORY. I also add a progress indicator. My testing

NAK, and NAK.

Please integrate your code into the existing POST framework instead,
as a number of other boards already did.

A progress indicator may be a nice little toy, but how does it
integrate into the POST framework?

> target is 2GB at a time, up to physically memory size which is easily
> over 8GB. Without progress indicator, it feels hung when it is actually
> running.

Yes, memory testing takes time. In the context of a power-on self
test (and this is what you are doing, right?) we should take care to
fit it into the existing framework, though.

> Please take a look at the patch below.

Instead of integrating your needs into an existing framework you
invent yet another one. I don't want to have this, sorry.

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
How can you tell when sour cream goes bad?

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[sun york-R58495]

Wolfgang,

As Timur pointed out, the post framework doesn't work for us. After U-boot relocate itself to RAM, we have only 2GB memory to test. The best place is before relocation. Many other boards do that. Following your idea of reusing code, I can only reuse the test pattern generator. I am open to suggestions.

York Sun
----- Original Message -----
From:"Wolfgang Denk" <wd@denx.de>
To:"York Sun" <yorksun@freescale.com>
Cc:"Timur Tabi" <timur.tabi@gmail.com>, "Kumar Gala" <galak@kernel.crashing.org>, "u-boot at lists.denx.de" <u-boot@lists.denx.de>
Sent:7/28/2010 4:50 PM
Subject:Re: [U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.


Dear York Sun,

In message <1280351179.8571.63.camel@oslab-l1> you wrote:
> 
> > We already have too many different implementations of a memory test in
> > U-Boot, and I will not accept adding yet another one.
> > 
> I can reuse your testing code but have to move the desired code out of
> memory.c file to avoid the need for CONFIG_POST and
> CONFIG_SYS_POST_MEMORY. I also add a progress indicator. My testing

NAK, and NAK.

Please integrate your code into the existing POST framework instead,
as a number of other boards already did.

A progress indicator may be a nice little toy, but how does it
integrate into the POST framework?

> target is 2GB at a time, up to physically memory size which is easily
> over 8GB. Without progress indicator, it feels hung when it is actually
> running.

Yes, memory testing takes time. In the context of a power-on self
test (and this is what you are doing, right?) we should take care to
fit it into the existing framework, though.

> Please take a look at the patch below.

Instead of integrating your needs into an existing framework you
invent yet another one. I don't want to have this, sorry.

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
How can you tell when sour cream goes bad?

[U-Boot] [PATCH 8/8] powerpc/85xx: Add memory test feature for mpc85xx.

[Wolfgang Denk]

Dear sun york-R58495,

In message <516c01cb2eac$1d67cdf1$0c23400a@fsl.freescale.net> you wrote:
>
> As Timur pointed out, the post framework doesn't work for us. After

And I explained that if it doesn't work as is please try to adapt /
extend it to match your needs instead of reinventing the wheel.

> U-boot relocate itself to RAM, we have only 2GB memory to test. The best
> place is before relocation. Many other boards do that. Following your

I don't understand what you are saying. You sound as if you assumed
that the POST code would run after relocation only?  This is NOT the
case. THere are tests that run before relocation (incluing the RAM
test), and others, that run after relocation.

> idea of reusing code, I can only reuse the test pattern generator. I am
> open to suggestions.

I suggest to check the code again, or to explain in a little more
detail why you think you can neither use nor extend the existing
framework.

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
"Remember, Information is not knowledge;  Knowledge  is  not  Wisdom;
Wisdom is not truth; Truth is not beauty; Beauty is not love; Love is
not music; Music is the best."                          - Frank Zappa

[U-Boot] help on POST

[York Sun]

Wolfgang,

Please help me to understand the post_word_write() and post_word_load().
I see some implementations which don't use NVRAM. Is it OK to use gd?
Does the post_word_load() require the data survive reset? How is the
data validated? I don't see any code to validate the value. 

Regards,

York

[U-Boot] help on POST

[Wolfgang Denk]

Dear York Sun,

In message <1280524829.25910.58.camel@oslab-l1> you wrote:
> 
> Please help me to understand the post_word_write() and post_word_load().
> I see some implementations which don't use NVRAM. Is it OK to use gd?

No.

> Does the post_word_load() require the data survive reset? How is the

Yes.

> data validated? I don't see any code to validate the value. 

It checks against BOOTMODE_MAGIC.

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
Faith may be defined briefly as an illogical belief in the  occurence
of the improbable.                                    - H. L. Mencken

[U-Boot] help on POST

[York Sun]

Dear Wolfgang,

Thanks for the quick respond.

On Fri, 2010-07-30 at 23:29 +0200, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <1280524829.25910.58.camel@oslab-l1> you wrote:
> > 
> > Please help me to understand the post_word_write() and post_word_load().
> > I see some implementations which don't use NVRAM. Is it OK to use gd?
> 
> No.


> > Does the post_word_load() require the data survive reset? How is the
> 
> Yes.
> 
> > data validated? I don't see any code to validate the value. 
> 
> It checks against BOOTMODE_MAGIC.

What's if I don't want to use flash and don't have NVRAM? Can I use gd
but let it fail the validation in case of data corruption?

York

[U-Boot] help on POST

[Wolfgang Denk]

Dear York Sun,

In message <1280525648.25910.61.camel@oslab-l1> you wrote:
> 
> > > data validated? I don't see any code to validate the value. 
> > 
> > It checks against BOOTMODE_MAGIC.
> 
> What's if I don't want to use flash and don't have NVRAM? Can I use gd
> but let it fail the validation in case of data corruption?

I don't understand the problem.  All we need is some register or
memory location that is guaranteed to keep it's value during a hard
reset of the processor. This can be NVRAM, or On-Chip-Memory, or some
register.  I am pretty sure you know this CPU much better than me, and
I am optimistic that you can find such a register.

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
365 Days of drinking Lo-Cal beer.                       = 1 Lite-year