[PATCH v3 0/6] add new memtest command

[PATCH v3 0/6] add new memtest command

[Alexander Aring]

The new memtest command supports to enable or disable
caching during testing(only arm architecture) and
skipping barebox regions for avoid a barebox killing,
if you want to write on barebox allocated regions.

If you run memtest without a argument, memtest will
run on all registered memory banks.

The new memtest command has these arguments:
        -s #ADDRESS [for start address]
        -e #ADDRESS [for end address]
        -i #ITER [for numbers of iterations]
        -c [don't disable caching]
        -b [do bus testing only]

v2:
        - change stdout of memtest to make it more beautiful

v3:
	- fix testing of address bus <-- this never worked before
	- put mem_test function into common/memory_test.c
	- fix some comment issues
	- align quietly start and end address
	- fix some newline issues

Alexander Aring (6):
  common: fix codestyle in ALIGN macros
  common: add ALIGN_DOWN macro
  memory: add function address_in_sdram_regions
  memtest: remove memtest command
  common: add mem_test routine
  commands: add new memtest command

 commands/Kconfig      |  17 +-
 commands/Makefile     |   2 +-
 commands/memtest.c    | 573 +++++++++++++++++++++++++-------------------------
 common/Kconfig        |   7 +
 common/Makefile       |   1 +
 common/memory.c       |  16 ++
 common/memory_test.c  | 399 +++++++++++++++++++++++++++++++++++
 include/common.h      |   5 +-
 include/memory.h      |   6 +
 include/memory_test.h |  13 ++
 10 files changed, 747 insertions(+), 292 deletions(-)
 create mode 100644 common/memory_test.c
 create mode 100644 include/memory_test.h

-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/6] common: fix codestyle in ALIGN macros

[Alexander Aring]

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/common.h b/include/common.h
index e559b94..1c7d599 100644
--- a/include/common.h
+++ b/include/common.h
@@ -182,8 +182,8 @@ int run_shell(void);
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
 #define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
 
-#define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
-#define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
+#define ALIGN(x, a)		__ALIGN_MASK(x, (typeof(x))(a) - 1)
+#define __ALIGN_MASK(x, mask)	(((x) + (mask)) & ~(mask))
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
 #define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
 
-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] common: add ALIGN_DOWN macro

[Alexander Aring]

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 include/common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/common.h b/include/common.h
index 1c7d599..73d51c1 100644
--- a/include/common.h
+++ b/include/common.h
@@ -184,6 +184,7 @@ int run_shell(void);
 
 #define ALIGN(x, a)		__ALIGN_MASK(x, (typeof(x))(a) - 1)
 #define __ALIGN_MASK(x, mask)	(((x) + (mask)) & ~(mask))
+#define ALIGN_DOWN(x, a)	((x) & ~((typeof(x))(a) - 1))
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
 #define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
 
-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] memory: add function address_in_sdram_regions

[Alexander Aring]

Add function address_in_sdram_regions to check if a address
is in any sdram region.

Also added new macro 'ADDRESS_IN_REGIONS' to check if address
is in a special boundary with start and end address.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 common/memory.c  | 16 ++++++++++++++++
 include/memory.h |  6 ++++++
 2 files changed, 22 insertions(+)

diff --git a/common/memory.c b/common/memory.c
index 81641f0..d2e9164 100644
--- a/common/memory.c
+++ b/common/memory.c
@@ -162,6 +162,22 @@ int release_sdram_region(struct resource *res)
 	return release_region(res);
 }
 
+/*
+ * Check if specific address are in any allocated region.
+ */
+int address_in_sdram_regions(resource_size_t address)
+{
+	struct memory_bank *bank = NULL;
+	struct resource *r = NULL;
+
+	for_each_memory_bank(bank)
+		list_for_each_entry(r, &bank->res->children, sibling)
+			if (ADDRESS_IN_REGIONS(address, r->start, r->end))
+				return 1;
+
+	return 0;
+}
+
 #ifdef CONFIG_OFTREE
 
 /*
diff --git a/include/memory.h b/include/memory.h
index 165d2dc..e12a28d 100644
--- a/include/memory.h
+++ b/include/memory.h
@@ -27,4 +27,10 @@ struct resource *request_sdram_region(const char *name, resource_size_t start,
 		resource_size_t size);
 int release_sdram_region(struct resource *res);
 
+#define ADDRESS_IN_REGIONS(address, region_start, region_end) \
+	((address >= region_start) && \
+	 (address <= region_end))
+
+int address_in_sdram_regions(resource_size_t address);
+
 #endif
-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] memtest: remove memtest command

[Alexander Aring]

Remove memtest command.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 commands/Kconfig   |   9 --
 commands/Makefile  |   1 -
 commands/memtest.c | 351 -----------------------------------------------------
 3 files changed, 361 deletions(-)
 delete mode 100644 commands/memtest.c

diff --git a/commands/Kconfig b/commands/Kconfig
index d3c338c..7cc759c 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -516,15 +516,6 @@ config CMD_NANDTEST
 	select PARTITION_NEED_MTD
 	prompt "nandtest"
 
-config CMD_MTEST
-	tristate
-	prompt "mtest"
-
-config CMD_MTEST_ALTERNATIVE
-	bool
-	depends on CMD_MTEST
-	prompt "alternative mtest implementation"
-
 endmenu
 
 menu "video command"
diff --git a/commands/Makefile b/commands/Makefile
index 0ae6b95..393ba51 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -7,7 +7,6 @@ obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
 obj-$(CONFIG_CMD_LOADS)		+= loads.o
 obj-$(CONFIG_CMD_ECHO)		+= echo.o
 obj-$(CONFIG_CMD_MEMORY)	+= mem.o
-obj-$(CONFIG_CMD_MTEST)		+= memtest.o
 obj-$(CONFIG_CMD_EDIT)		+= edit.o
 obj-$(CONFIG_CMD_EXEC)		+= exec.o
 obj-$(CONFIG_CMD_SLEEP)		+= sleep.o
diff --git a/commands/memtest.c b/commands/memtest.c
deleted file mode 100644
index 2d64d00..0000000
--- a/commands/memtest.c
+++ /dev/null
@@ -1,351 +0,0 @@
-/*
- * mtest - Perform a memory test
- *
- * (C) Copyright 2000
- * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
- *
- * 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.
- *
- */
-
-#include <common.h>
-#include <command.h>
-#include <types.h>
-
-/*
- * Perform a memory test. A more complete alternative test can be
- * configured using CONFIG_CMD_MTEST_ALTERNATIVE. The complete test
- * loops until interrupted by ctrl-c or by a failure of one of the
- * sub-tests.
- */
-#ifdef CONFIG_CMD_MTEST_ALTERNATIVE
-static int mem_test(ulong _start, ulong _end, ulong pattern_unused)
-{
-	vu_long *start = (vu_long *)_start;
-	vu_long *end   = (vu_long *)_end;
-	vu_long *addr;
-	ulong	val;
-	ulong	readback;
-	vu_long	addr_mask;
-	vu_long	offset;
-	vu_long	test_offset;
-	vu_long	pattern;
-	vu_long	temp;
-	vu_long	anti_pattern;
-	vu_long	num_words;
-#ifdef CFG_MEMTEST_SCRATCH
-	vu_long *dummy = (vu_long*)CFG_MEMTEST_SCRATCH;
-#else
-	vu_long *dummy = start;
-#endif
-	int	j;
-	int iterations = 1;
-
-	static const ulong bitpattern[] = {
-		0x00000001,	/* single bit */
-		0x00000003,	/* two adjacent bits */
-		0x00000007,	/* three adjacent bits */
-		0x0000000F,	/* four adjacent bits */
-		0x00000005,	/* two non-adjacent bits */
-		0x00000015,	/* three non-adjacent bits */
-		0x00000055,	/* four non-adjacent bits */
-		0xaaaaaaaa,	/* alternating 1/0 */
-	};
-
-	/* XXX: enforce alignment of start and end? */
-	for (;;) {
-		if (ctrlc()) {
-			putchar ('\n');
-			return 1;
-		}
-
-		printf("Iteration: %6d\r", iterations);
-		iterations++;
-
-		/*
-		 * Data line test: write a pattern to the first
-		 * location, write the 1's complement to a 'parking'
-		 * address (changes the state of the data bus so a
-		 * floating bus doen't give a false OK), and then
-		 * read the value back. Note that we read it back
-		 * into a variable because the next time we read it,
-		 * it might be right (been there, tough to explain to
-		 * the quality guys why it prints a failure when the
-		 * "is" and "should be" are obviously the same in the
-		 * error message).
-		 *
-		 * Rather than exhaustively testing, we test some
-		 * patterns by shifting '1' bits through a field of
-		 * '0's and '0' bits through a field of '1's (i.e.
-		 * pattern and ~pattern).
-		 */
-		addr = start;
-		/* XXX */
-		if (addr == dummy) ++addr;
-		for (j = 0; j < sizeof(bitpattern)/sizeof(bitpattern[0]); j++) {
-		    val = bitpattern[j];
-		    for(; val != 0; val <<= 1) {
-			*addr  = val;
-			*dummy  = ~val; /* clear the test data off of the bus */
-			readback = *addr;
-			if(readback != val) {
-			     printf ("FAILURE (data line): "
-				"expected 0x%08lx, actual 0x%08lx at address 0x%p\n",
-					  val, readback, addr);
-			}
-			*addr  = ~val;
-			*dummy  = val;
-			readback = *addr;
-			if(readback != ~val) {
-			    printf ("FAILURE (data line): "
-				"Is 0x%08lx, should be 0x%08lx at address 0x%p\n",
-					readback, ~val, addr);
-			}
-		    }
-		}
-
-		/*
-		 * Based on code whose Original Author and Copyright
-		 * information follows: Copyright (c) 1998 by Michael
-		 * Barr. This software is placed into the public
-		 * domain and may be used for any purpose. However,
-		 * this notice must not be changed or removed and no
-		 * warranty is either expressed or implied by its
-		 * publication or distribution.
-		 */
-
-		/*
-		 * Address line test
-		 *
-		 * Description: Test the address bus wiring in a
-		 *              memory region by performing a walking
-		 *              1's test on the relevant bits of the
-		 *              address and checking for aliasing.
-		 *              This test will find single-bit
-		 *              address failures such as stuck -high,
-		 *              stuck-low, and shorted pins. The base
-		 *              address and size of the region are
-		 *              selected by the caller.
-		 *
-		 * Notes:	For best results, the selected base
-		 *              address should have enough LSB 0's to
-		 *              guarantee single address bit changes.
-		 *              For example, to test a 64-Kbyte
-		 *              region, select a base address on a
-		 *              64-Kbyte boundary. Also, select the
-		 *              region size as a power-of-two if at
-		 *              all possible.
-		 *
-		 * Returns:     0 if the test succeeds, 1 if the test fails.
-		 *
-		 * ## NOTE ##	Be sure to specify start and end
-		 *              addresses such that addr_mask has
-		 *              lots of bits set. For example an
-		 *              address range of 01000000 02000000 is
-		 *              bad while a range of 01000000
-		 *              01ffffff is perfect.
-		 */
-		addr_mask = ((ulong)end - (ulong)start)/sizeof(vu_long);
-		pattern = (vu_long) 0xaaaaaaaa;
-		anti_pattern = (vu_long) 0x55555555;
-
-		debug("%s:%d: addr mask = 0x%.8lx\n",
-			__FUNCTION__, __LINE__,
-			addr_mask);
-		/*
-		 * Write the default pattern at each of the
-		 * power-of-two offsets.
-		 */
-		for (offset = 1; (offset & addr_mask) != 0; offset <<= 1)
-			start[offset] = pattern;
-
-		/*
-		 * Check for address bits stuck high.
-		 */
-		test_offset = 0;
-		start[test_offset] = anti_pattern;
-
-		for (offset = 1; (offset & addr_mask) != 0; offset <<= 1) {
-		    temp = start[offset];
-		    if (temp != pattern) {
-			printf ("\nFAILURE: Address bit stuck high @ 0x%.8lx:"
-				" expected 0x%.8lx, actual 0x%.8lx\n",
-				(ulong)&start[offset], pattern, temp);
-			return 1;
-		    }
-		}
-		start[test_offset] = pattern;
-
-		/*
-		 * Check for addr bits stuck low or shorted.
-		 */
-		for (test_offset = 1; (test_offset & addr_mask) != 0; test_offset <<= 1) {
-		    start[test_offset] = anti_pattern;
-
-		    for (offset = 1; (offset & addr_mask) != 0; offset <<= 1) {
-			temp = start[offset];
-			if ((temp != pattern) && (offset != test_offset)) {
-			    printf ("\nFAILURE: Address bit stuck low or shorted @"
-				" 0x%.8lx: expected 0x%.8lx, actual 0x%.8lx\n",
-				(ulong)&start[offset], pattern, temp);
-			    return 1;
-			}
-		    }
-		    start[test_offset] = pattern;
-		}
-
-		/*
-		 * Description: Test the integrity of a physical
-		 *		memory device by performing an
-		 *		increment/decrement test over the
-		 *		entire region. In the process every
-		 *		storage bit in the device is tested
-		 *		as a zero and a one. The base address
-		 *		and the size of the region are
-		 *		selected by the caller.
-		 *
-		 * Returns:     0 if the test succeeds, 1 if the test fails.
-		 */
-		num_words = ((ulong)end - (ulong)start)/sizeof(vu_long) + 1;
-
-		/*
-		 * Fill memory with a known pattern.
-		 */
-		for (pattern = 1, offset = 0; offset < num_words; pattern++, offset++) {
-			start[offset] = pattern;
-		}
-
-		/*
-		 * Check each location and invert it for the second pass.
-		 */
-		for (pattern = 1, offset = 0; offset < num_words; pattern++, offset++) {
-		    temp = start[offset];
-		    if (temp != pattern) {
-			printf ("\nFAILURE (read/write) @ 0x%.8lx:"
-				" expected 0x%.8lx, actual 0x%.8lx)\n",
-				(ulong)&start[offset], pattern, temp);
-			return 1;
-		    }
-
-		    anti_pattern = ~pattern;
-		    start[offset] = anti_pattern;
-		}
-
-		/*
-		 * Check each location for the inverted pattern and zero it.
-		 */
-		for (pattern = 1, offset = 0; offset < num_words; pattern++, offset++) {
-		    anti_pattern = ~pattern;
-		    temp = start[offset];
-		    if (temp != anti_pattern) {
-			printf ("\nFAILURE (read/write): @ 0x%.8lx:"
-				" expected 0x%.8lx, actual 0x%.8lx)\n",
-				(ulong)&start[offset], anti_pattern, temp);
-			return 1;
-		    }
-		    start[offset] = 0;
-		}
-	}
-
-}
-#else
-static int mem_test(ulong _start, ulong _end, ulong pattern)
-{
-	vu_long	*addr;
-	vu_long *start = (vu_long *)_start;
-	vu_long *end   = (vu_long *)_end;
-	ulong	val;
-	ulong	readback;
-	ulong	incr;
-	int rcode;
-
-	incr = 1;
-	for (;;) {
-		if (ctrlc()) {
-			putchar('\n');
-			return 1;
-		}
-
-		printf ("\rPattern 0x%08lX  Writing..."
-			"%12s"
-			"\b\b\b\b\b\b\b\b\b\b",
-			pattern, "");
-
-		for (addr=start,val=pattern; addr<end; addr++) {
-			*addr = val;
-			val  += incr;
-		}
-
-		puts ("Reading...");
-
-		for (addr=start,val=pattern; addr<end; addr++) {
-			readback = *addr;
-			if (readback != val) {
-				printf ("\nMem error @ 0x%08X: "
-					"found 0x%08lX, expected 0x%08lX\n",
-					(uint)addr, readback, val);
-				rcode = 1;
-			}
-			val += incr;
-		}
-
-		/*
-		 * Flip the pattern each time to make lots of zeros and
-		 * then, the next time, lots of ones.  We decrement
-		 * the "negative" patterns and increment the "positive"
-		 * patterns to preserve this feature.
-		 */
-		if(pattern & 0x80000000) {
-			pattern = -pattern;	/* complement & increment */
-		}
-		else {
-			pattern = ~pattern;
-		}
-		incr = -incr;
-	}
-	return rcode;
-}
-#endif
-
-static int do_mem_mtest(int argc, char *argv[])
-{
-	ulong start, end, pattern = 0;
-
-	if (argc < 3)
-		return COMMAND_ERROR_USAGE;
-
-	start = simple_strtoul(argv[1], NULL, 0);
-	end = simple_strtoul(argv[2], NULL, 0);
-
-	if (argc > 3)
-		pattern = simple_strtoul(argv[3], NULL, 0);
-
-	printf ("Testing 0x%08x ... 0x%08x:\n", (uint)start, (uint)end);
-	
-	return mem_test(start, end, pattern);
-}
-
-static const __maybe_unused char cmd_mtest_help[] =
-"Usage: <start> <end> "
-#ifdef CONFIG_CMD_MTEST_ALTERNATIVE
-"[pattern]"
-#endif
-"\nsimple RAM read/write test\n";
-
-BAREBOX_CMD_START(mtest)
-	.cmd		= do_mem_mtest,
-	.usage		= "simple RAM test",
-	BAREBOX_CMD_HELP(cmd_mtest_help)
-BAREBOX_CMD_END
-
-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] common: add mem_test routine

[Alexander Aring]

Useful to detect timing problems if someone porting a new
device to barebox.

This test includes a data bus test, address bus test and
integrity check of memory.

Allocated barebox regions between start and end will skip
automatically.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 common/Kconfig        |   7 +
 common/Makefile       |   1 +
 common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/memory_test.h |  13 ++
 4 files changed, 420 insertions(+)
 create mode 100644 common/memory_test.c
 create mode 100644 include/memory_test.h

diff --git a/common/Kconfig b/common/Kconfig
index 3f6c11e..c6988df 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -100,6 +100,13 @@ config MEMINFO
 	bool "display memory info"
 	default y
 
+config MEMTEST
+	bool "Offers routines for memory test"
+	help
+	  Offers memtest routines in common/memory_test.c
+	  This is helpful for porting devices to detect
+	  memory timing problems.
+
 config ENVIRONMENT_VARIABLES
 	bool "environment variables support"
 
diff --git a/common/Makefile b/common/Makefile
index 7206eed..684953c 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
 obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
 obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
 obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
+obj-$(CONFIG_MEMTEST) += memory_test.o
 obj-y += clock.o
 obj-$(CONFIG_BANNER) += version.o
 obj-$(CONFIG_MEMINFO) += meminfo.o
diff --git a/common/memory_test.c b/common/memory_test.c
new file mode 100644
index 0000000..80b4ff4
--- /dev/null
+++ b/common/memory_test.c
@@ -0,0 +1,399 @@
+/*
+ * memory_test.c
+ *
+ * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
+ *
+ * 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 version 2
+ * as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ */
+
+#include <memory_test.h>
+
+static const vu_long bitpattern[] = {
+	0x00000001,	/* single bit */
+	0x00000003,	/* two adjacent bits */
+	0x00000007,	/* three adjacent bits */
+	0x0000000F,	/* four adjacent bits */
+	0x00000005,	/* two non-adjacent bits */
+	0x00000015,	/* three non-adjacent bits */
+	0x00000055,	/* four non-adjacent bits */
+	0xAAAAAAAA,	/* alternating 1/0 */
+};
+
+/*
+ * Perform a memory test. The complete test
+ * loops until interrupted by ctrl-c.
+ *
+ * Highly recommended to test with disabled and
+ * enabled cache.
+ *
+ * start: start address
+ * end: end address
+ * bus_only: skip integrity check
+ */
+int mem_test(vu_long _start, vu_long _end,
+		int bus_only)
+{
+	vu_long *start;
+	vu_long *dummy;
+
+	vu_long val;
+	vu_long readback;
+	vu_long offset;
+	vu_long offset2;
+	vu_long pattern;
+	vu_long temp;
+	vu_long anti_pattern;
+	vu_long num_words;
+
+	int i;
+	int ret;
+
+	if (!IS_ALIGNED(_start, sizeof(vu_long)))
+		_start = ALIGN(_start, sizeof(vu_long));
+	/*
+	 * check if end is a multiple of vu_long.
+	 * need to add 1 because ALIGNED works with
+	 * inclusive byte at end address.
+	 *
+	 * Also check on _end == 0. Otherwise we get a
+	 * underflow.
+	 */
+	if (!IS_ALIGNED(_end + 1, sizeof(vu_long)) && _end)
+		_end = ALIGN_DOWN(_end, sizeof(vu_long)) - 1;
+
+	/*
+	 * TODO
+	 * recheck after align. That's not a quite
+	 * solution now because we already done this
+	 * in memtest command.
+	 */
+	if (_end <= _start)
+		return -1;
+
+	start = (vu_long *)_start;
+	/*
+	 * Point the dummy to start[1]
+	 */
+	dummy = start+1;
+	num_words = (_end - _start + 1)/sizeof(vu_long);
+
+	/*
+	 * Checking if start and dummy address are in one
+	 * of barebox regions. Otherwise next data line testing
+	 * will maybe fail.
+	 */
+	ret = address_in_sdram_regions((vu_long)start);
+	ret |= address_in_sdram_regions((vu_long)dummy);
+	if (ret) {
+		printf("WARNING (data line): "
+				"address 0x%08lx is in sdram regions.\n"
+				"Try another start address to fix this issue.\n",
+				(vu_long)start);
+		return -1;
+	}
+
+	printf("Starting data line test.\n");
+
+	/*
+	 * Data line test: write a pattern to the first
+	 * location, write the 1's complement to a 'parking'
+	 * address (changes the state of the data bus so a
+	 * floating bus doen't give a false OK), and then
+	 * read the value back. Note that we read it back
+	 * into a variable because the next time we read it,
+	 * it might be right (been there, tough to explain to
+	 * the quality guys why it prints a failure when the
+	 * "is" and "should be" are obviously the same in the
+	 * error message).
+	 *
+	 * Rather than exhaustively testing, we test some
+	 * patterns by shifting '1' bits through a field of
+	 * '0's and '0' bits through a field of '1's (i.e.
+	 * pattern and ~pattern).
+	 */
+	for (i = 0; i < sizeof(bitpattern)/
+			sizeof(bitpattern[0]); i++) {
+		val = bitpattern[i];
+
+		for (; val != 0; val <<= 1) {
+			*start = val;
+			/* clear the test data off of the bus */
+			*dummy = ~val;
+			readback = *start;
+
+			if (readback != val) {
+				printf("FAILURE (data line): "
+					"expected 0x%08lx, actual 0x%08lx at address 0x%08lx.\n",
+					val, readback, (vu_long)start);
+				return -1;
+			}
+
+			*start = ~val;
+			*dummy = val;
+			readback = *start;
+			if (readback != ~val) {
+				printf("FAILURE (data line): "
+					"Is 0x%08lx, should be 0x%08lx at address 0x%08lx.\n",
+					readback,
+					~val, (vu_long)start);
+				return -1;
+			}
+		}
+	}
+
+
+	/*
+	 * Based on code whose Original Author and Copyright
+	 * information follows: Copyright (c) 1998 by Michael
+	 * Barr. This software is placed into the public
+	 * domain and may be used for any purpose. However,
+	 * this notice must not be changed or removed and no
+	 * warranty is either expressed or implied by its
+	 * publication or distribution.
+	 */
+
+	/*
+	 * Address line test
+	 *
+	 * Description: Test the address bus wiring in a
+	 *              memory region by performing a walking
+	 *              1's test on the relevant bits of the
+	 *              address and checking for aliasing.
+	 *              This test will find single-bit
+	 *              address failures such as stuck -high,
+	 *              stuck-low, and shorted pins. The base
+	 *              address and size of the region are
+	 *              selected by the caller.
+	 *
+	 * Notes:	For best results, the selected base
+	 *              address should have enough LSB 0's to
+	 *              guarantee single address bit changes.
+	 *              For example, to test a 64-Kbyte
+	 *              region, select a base address on a
+	 *              64-Kbyte boundary. Also, select the
+	 *              region size as a power-of-two if at
+	 *              all possible.
+	 *
+	 * ## NOTE ##	Be sure to specify start and end
+	 *              addresses such that num_words has
+	 *              lots of bits set. For example an
+	 *              address range of 01000000 02000000 is
+	 *              bad while a range of 01000000
+	 *              01ffffff is perfect.
+	 */
+
+	pattern = 0xAAAAAAAA;
+	anti_pattern = 0x55555555;
+
+	/*
+	 * Write the default pattern at each of the
+	 * power-of-two offsets.
+	 */
+	for (offset = 1; offset <= num_words; offset <<= 1) {
+		ret = address_in_sdram_regions((vu_long)&start[offset]);
+		if (ret) {
+			printf("WARNING (stuck high): "
+					"address 0x%08lx is in barebox regions.\n",
+					(vu_long)&start[offset]);
+			continue;
+		}
+
+		start[offset] = pattern;
+	}
+
+	printf("Check for address bits stuck high.\n");
+
+	/*
+	 * Check for address bits stuck high.
+	 */
+	for (offset = 1; offset <= num_words; offset <<= 1) {
+		ret = address_in_sdram_regions((vu_long)&start[offset]);
+		if (ret)
+			continue;
+
+		temp = start[offset];
+		if (temp != pattern) {
+			printf("FAILURE: Address bit "
+					"stuck high @ 0x%08lx:"
+					" expected 0x%08lx, actual 0x%08lx.\n",
+					(vu_long)&start[offset],
+					pattern, temp);
+			return -1;
+		}
+	}
+
+	printf("Check for address bits stuck "
+			"low or shorted.\n");
+
+	/*
+	 * Check for address bits stuck low or shorted.
+	 */
+	for (offset2 = 1; offset2 <= num_words; offset2 <<= 1) {
+		ret = address_in_sdram_regions(
+				(vu_long)&start[offset2]);
+		if (ret) {
+			printf("WARNING (low high): "
+					"address 0x%08lx is in barebox regions.\n",
+					(vu_long)&start[offset2]);
+			continue;
+		}
+
+		start[offset2] = anti_pattern;
+
+		for (offset = 1; offset <= num_words; offset <<= 1) {
+			ret = address_in_sdram_regions(
+					(vu_long)&start[offset]);
+			if (ret)
+				continue;
+
+			temp = start[offset];
+
+			/*
+			 * That's some complicated for loop with
+			 * condition offset != test_offset inside. I
+			 * think this is necessary to put some another
+			 * address on the bus.
+			 *
+			 * TODO
+			 * check if loop is necessary.
+			 */
+			if ((temp != pattern) &&
+					(offset != offset2)) {
+				printf("FAILURE: Address bit stuck"
+						" low or shorted @"
+						" 0x%08lx: expected 0x%08lx, actual 0x%08lx.\n",
+						(vu_long)&start[offset],
+						pattern, temp);
+				return -1;
+			}
+		}
+		start[offset2] = pattern;
+	}
+
+	/*
+	 * We tested only the bus if != 0
+	 * leaving here
+	 */
+	if (bus_only)
+		return 0;
+
+	printf("Starting integrity check of physicaly ram.\n"
+			"Filling ram with patterns...\n");
+
+	/*
+	 * Description: Test the integrity of a physical
+	 *		memory device by performing an
+	 *		increment/decrement test over the
+	 *		entire region. In the process every
+	 *		storage bit in the device is tested
+	 *		as a zero and a one. The base address
+	 *		and the size of the region are
+	 *		selected by the caller.
+	 */
+
+	/*
+	 * Fill memory with a known pattern.
+	 */
+	init_progression_bar(num_words);
+	for (offset = 0; offset < num_words; offset++) {
+		if (!(offset & 0xfff)) {
+			if (ctrlc())
+				return -EINTR;
+			show_progress(offset);
+		}
+
+		ret = address_in_sdram_regions((vu_long)&start[offset]);
+		if (ret)
+			continue;
+
+		start[offset] = offset + 1;
+	}
+
+	show_progress(offset);
+
+	printf("\nCompare written patterns...\n");
+
+	/*
+	 * Check each location and invert it for the second pass.
+	 */
+	init_progression_bar(num_words - 1);
+	for (offset = 0; offset < num_words; offset++) {
+		if (!(offset & 0xfff)) {
+			if (ctrlc())
+				return -EINTR;
+			show_progress(offset);
+		}
+
+		ret = address_in_sdram_regions((vu_long)&start[offset]);
+		if (ret)
+			continue;
+
+		temp = start[offset];
+		if (temp != (offset + 1)) {
+			printf("\nFAILURE (read/write) @ 0x%08lx:"
+					" expected 0x%08lx, actual 0x%08lx.\n",
+					(vu_long)&start[offset],
+					(offset + 1), temp);
+			return -1;
+		}
+
+		anti_pattern = ~(offset + 1);
+		start[offset] = anti_pattern;
+	}
+
+	show_progress(offset);
+
+	printf("\nFilling ram with inverted pattern and compare it...\n");
+
+	/*
+	 * Check each location for the inverted pattern and zero it.
+	 */
+	init_progression_bar(num_words - 1);
+	for (offset = 0; offset < num_words; offset++) {
+		if (!(offset & 0xfff)) {
+			if (ctrlc())
+				return -EINTR;
+			show_progress(offset);
+		}
+
+		ret = address_in_sdram_regions((vu_long)&start[offset]);
+		/*
+		 * Step over barebox mem usage
+		 */
+		if (ret)
+			continue;
+
+		anti_pattern = ~(offset + 1);
+		temp = start[offset];
+
+		if (temp != anti_pattern) {
+			printf("\nFAILURE (read/write): @ 0x%08lx:"
+					" expected 0x%08lx, actual 0x%08lx.\n",
+					(vu_long)&start[offset],
+					anti_pattern, temp);
+			return -1;
+		}
+
+		start[offset] = 0;
+	}
+
+	show_progress(offset);
+
+	/*
+	 * end of progressbar
+	 */
+	printf("\n");
+
+	return 0;
+}
diff --git a/include/memory_test.h b/include/memory_test.h
new file mode 100644
index 0000000..6959dc6
--- /dev/null
+++ b/include/memory_test.h
@@ -0,0 +1,13 @@
+
+#ifndef __MEMORY_TEST_H
+#define __MEMORY_TEST_H
+
+#include <progress.h>
+#include <common.h>
+#include <memory.h>
+#include <types.h>
+
+int mem_test(vu_long _start, vu_long _end,
+		int bus_only);
+
+#endif
-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] commands: add new memtest command

[Alexander Aring]

Add new memtest command which can enable or disable caching
on non allocted barebox regions(test area).

This command simply parse and check parameters then call
the mem_test routine.

If no address parameters are given then mem_test will call
for each memory bank.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 commands/Kconfig   |  10 ++
 commands/Makefile  |   1 +
 commands/memtest.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 373 insertions(+)
 create mode 100644 commands/memtest.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 7cc759c..d158c3f 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -516,6 +516,16 @@ config CMD_NANDTEST
 	select PARTITION_NEED_MTD
 	prompt "nandtest"
 
+config CMD_MEMTEST
+    tristate
+    select MEMTEST
+    prompt "memtest"
+	help
+	  This command enables a memtest to test installed memory.
+	  During this test allocated iomem regions will be skipped.
+	  If tested architecture has MMU with PTE flags support,
+	  caching can be set enabled or disabled.
+
 endmenu
 
 menu "video command"
diff --git a/commands/Makefile b/commands/Makefile
index 393ba51..b39b489 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
 obj-$(CONFIG_CMD_LOADS)		+= loads.o
 obj-$(CONFIG_CMD_ECHO)		+= echo.o
 obj-$(CONFIG_CMD_MEMORY)	+= mem.o
+obj-$(CONFIG_CMD_MEMTEST)	+= memtest.o
 obj-$(CONFIG_CMD_EDIT)		+= edit.o
 obj-$(CONFIG_CMD_EXEC)		+= exec.o
 obj-$(CONFIG_CMD_SLEEP)		+= sleep.o
diff --git a/commands/memtest.c b/commands/memtest.c
new file mode 100644
index 0000000..22e8006
--- /dev/null
+++ b/commands/memtest.c
@@ -0,0 +1,362 @@
+/*
+ * memtest - Perform a memory test
+ *
+ * (C) Copyright 2013
+ * Alexander Aring <aar@pengutronix.de>, Pengutronix
+ *
+ * (C) Copyright 2000
+ * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * 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 <command.h>
+#include <getopt.h>
+#include <asm/mmu.h>
+
+#include <memory_test.h>
+
+/*
+ * In CONFIG_MMU we have a special c flag.
+ */
+#ifdef CONFIG_MMU
+static char optstr[] = "s:e:i:cb";
+
+/*
+ * PTE flags variables to set cached and
+ * uncached regions.
+ */
+static uint32_t pte_flags_cached;
+static uint32_t pte_flags_uncached;
+#else
+static char optstr[] = "s:e:i:b";
+#endif
+
+#ifdef CONFIG_MMU
+static void print_region(vu_long start, vu_long size, uint32_t flags)
+{
+	if (!size)
+		return;
+
+	printf("\t0x%08lx - "
+			"0x%08lx (size 0x%08lx)\n",
+			start, start + size - 1, size);
+}
+
+static void do_remap_range(struct memory_bank *bank, uint32_t flags)
+{
+	struct resource *r = NULL;
+	struct resource *r_prev = NULL;
+
+	vu_long size;
+	vu_long start;
+	vu_long end;
+
+	if (flags == pte_flags_uncached)
+		printf("Set non caching regions:\n");
+	else if (flags == pte_flags_cached)
+		printf("Set caching regions:\n");
+	else
+		BUG();
+
+	/*
+	 * We assume that the regions are sorted in this list
+	 */
+	list_for_each_entry(r, &bank->res->children, sibling) {
+		/*
+		 * Do on head element for bank boundary
+		 */
+		if (r->sibling.prev == &bank->res->children) {
+			/*
+			 * remember last used element
+			 */
+			r_prev = r;
+
+			start = PAGE_ALIGN(bank->start);
+			end = PAGE_ALIGN_DOWN(r->start) - 1;
+			if (start >= end)
+				continue;
+			size = end - start + 1;
+
+			print_region(start, size, flags);
+			remap_range((void *)start, size, flags);
+
+			continue;
+		}
+		/*
+		 * Between used regions
+		 */
+		start = PAGE_ALIGN(r_prev->end);
+		end = PAGE_ALIGN_DOWN(r->start) - 1;
+		if (start < end) {
+			size = end - start + 1;
+			print_region(start, size, flags);
+			remap_range((void *)start, size, flags);
+		}
+
+		r_prev = r;
+		/*
+		 * Do on head element for bank boundary
+		 */
+		if (list_is_last(&r->sibling, &bank->res->children)) {
+			start = PAGE_ALIGN(r->end);
+			end = PAGE_ALIGN_DOWN(bank->start + bank->size) - 1;
+			if (start >= end)
+				continue;
+			size = end - start + 1;
+
+			print_region(start, size, flags);
+			remap_range((void *)start, size, flags);
+		}
+	}
+}
+#endif
+
+static int do_mem_memtest(int argc, char *argv[])
+{
+	/*
+	 * Set start address to 0xffffffff which
+	 * can't be.
+	 */
+	vu_long start = 0xffffffff;
+	vu_long end = 0;
+
+	uint i;
+	uint max_i = 1;
+
+#ifdef CONFIG_MMU
+	int cache = 0;
+#endif
+	int bus_only = 0;
+	int err = 0;
+	int cnt = 0;
+	int opt;
+
+	struct memory_bank *bank = NULL;
+	struct resource *r = NULL;
+
+	while ((opt = getopt(argc, argv, optstr)) > 0) {
+		switch (opt) {
+		case 's':
+			start = simple_strtoul(optarg, NULL, 0);
+			break;
+		case 'e':
+			end = simple_strtoul(optarg, NULL, 0);
+			break;
+		case 'i':
+			max_i = simple_strtoul(optarg, NULL, 0);
+			break;
+#ifdef CONFIG_MMU
+		case 'c':
+			cache = 1;
+			break;
+#endif
+		case 'b':
+			bus_only = 1;
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (optind > argc)
+		return COMMAND_ERROR_USAGE;
+
+	/*
+	 * Error if no end address
+	 */
+	if (start != 0xffffffff && !end) {
+		printf("Please add an end address.\n");
+		return 1;
+	}
+
+	/*
+	 * Error if no start address
+	 */
+	if (end && start == 0xffffffff) {
+		printf("Please add a start address.\n");
+		return 1;
+	}
+
+	/*
+	 * Check parameters
+	 */
+	if (start != 0xffffffff && end) {
+		if (end <= start) {
+			printf("End address less than or"
+					" equal start address.\n");
+			return 1;
+		}
+
+		/*
+		 * Check if given start and end address are in any banks
+		 */
+		for_each_memory_bank(bank) {
+			if (ADDRESS_IN_REGIONS(start, bank->start,
+						bank->start + bank->size))
+				cnt++;
+
+			if (ADDRESS_IN_REGIONS(end, bank->start,
+						bank->start + bank->size))
+				cnt++;
+		}
+
+		if (cnt != 2) {
+			printf("Start or end addresses are"
+					" not in any ram bank.\n");
+			return 1;
+		}
+	}
+
+#ifdef CONFIG_MMU
+	/*
+	 * Get pte flags. Which are configured at
+	 * runtime at booting.
+	 */
+	pte_flags_cached = mmu_get_pte_cached_flags();
+	pte_flags_uncached = mmu_get_pte_uncached_flags();
+#endif
+
+	printf("Skipping regions:\n");
+	for_each_memory_bank(bank) {
+		list_for_each_entry(r, &bank->res->children, sibling)
+			printf("\t0x%08x - "
+					"0x%08x (size 0x%08x) %s\n",
+					r->start, r->end,
+					r->end - r->start + 1, r->name);
+#ifdef CONFIG_MMU
+		/*
+		 * Disable or enable caching
+		 */
+		if (cache)
+			do_remap_range(bank, pte_flags_cached);
+		else
+			do_remap_range(bank, pte_flags_uncached);
+#endif
+	}
+
+	/*
+	 * Do test if we set a start or end address
+	 */
+	if (start != 0xffffffff && end) {
+		printf("Testing address range:\n\t0x%08lx - 0x%08lx"
+				" (size 0x%08lx)\n",
+				start, end, end - start + 1);
+
+		for (i = 1; (i <= max_i) || !max_i; i++) {
+			printf("Iteration: %u\n", i);
+
+			/*
+			 * Do the Memtest
+			 */
+			err = mem_test(start, end, bus_only);
+			if (err == -EINTR) {
+				printf("Test interrupted.\n");
+				goto err;
+			}
+
+			if (err < 0) {
+				printf("Test failed.\n");
+				goto err;
+			}
+			printf("Tested %u iteration(s) without errors.\n", i);
+		}
+#ifdef CONFIG_MMU
+		/*
+		 * Renable caching
+		 */
+		if (!cache)
+			for_each_memory_bank(bank)
+				do_remap_range(bank, pte_flags_cached);
+#endif
+		printf("Memtest done.\n");
+
+		return 0;
+	}
+
+	/*
+	 * If we set no start or end address
+	 * we do the test on all ram banks
+	 */
+	for (i = 1; (i <= max_i) || !max_i; i++) {
+		for_each_memory_bank(bank) {
+			start = bank->start;
+			end = bank->start + bank->size - 1;
+
+			printf("Iteration: %u\n", i);
+
+			printf("Testing address range:\n\t0x%08lx - "
+					"0x%08lx (size 0x%08lx) on bank /dev/%s\n",
+					start, end, bank->size,
+					bank->res->name);
+
+			err = mem_test(start, end, bus_only);
+			if (err == -EINTR) {
+				printf("Test interrupted.\n");
+				goto err;
+			}
+
+			if (err < 0) {
+				printf("Test on bank /dev/%s failed.\n",
+						bank->res->name);
+				goto err;
+			}
+			printf("Tested %u iteration(s) without errors.\n", i);
+		}
+	}
+#ifdef CONFIG_MMU
+	/*
+	 * Renable caching
+	 */
+	if (!cache)
+		for_each_memory_bank(bank)
+			do_remap_range(bank, pte_flags_cached);
+#endif
+	printf("Memtest done.\n");
+
+	return 0;
+
+err:
+#ifdef CONFIG_MMU
+	/*
+	 * Enable caching
+	 */
+	for_each_memory_bank(bank)
+		do_remap_range(bank, pte_flags_cached);
+#endif
+
+	return 1;
+}
+
+static const __maybe_unused char cmd_memtest_help[] =
+"Usage: memtest [OPTION]...\n"
+"memtest related commands\n"
+"	-s	<start>		start address to begin memtest.\n"
+"	-e	<end>		end address to stop memtest.\n"
+"	-i	<iterations>	iterations [default=1, endless=0].\n"
+#ifdef CONFIG_MMU
+"	-c			run test with enable cache.\n"
+#endif
+"	-b			only test bus datalines.";
+
+BAREBOX_CMD_START(memtest)
+	.cmd		= do_mem_memtest,
+	.usage		= "Memory Test",
+	BAREBOX_CMD_HELP(cmd_memtest_help)
+BAREBOX_CMD_END
-- 
1.8.1.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] common: add mem_test routine

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 14267 bytes --]

On 02/07/2013 11:44 AM, Alexander Aring wrote:
> Useful to detect timing problems if someone porting a new
> device to barebox.
> 
> This test includes a data bus test, address bus test and
> integrity check of memory.
> 
> Allocated barebox regions between start and end will skip
> automatically.

Some nitpicking inline.

Is there a nice alternative to usage of the vu_long type?
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  common/Kconfig        |   7 +
>  common/Makefile       |   1 +
>  common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/memory_test.h |  13 ++
>  4 files changed, 420 insertions(+)
>  create mode 100644 common/memory_test.c
>  create mode 100644 include/memory_test.h
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 3f6c11e..c6988df 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -100,6 +100,13 @@ config MEMINFO
>  	bool "display memory info"
>  	default y
>  
> +config MEMTEST
> +	bool "Offers routines for memory test"
> +	help
> +	  Offers memtest routines in common/memory_test.c
> +	  This is helpful for porting devices to detect
> +	  memory timing problems.
> +
>  config ENVIRONMENT_VARIABLES
>  	bool "environment variables support"
>  
> diff --git a/common/Makefile b/common/Makefile
> index 7206eed..684953c 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
>  obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
>  obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
>  obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
> +obj-$(CONFIG_MEMTEST) += memory_test.o
>  obj-y += clock.o
>  obj-$(CONFIG_BANNER) += version.o
>  obj-$(CONFIG_MEMINFO) += meminfo.o
> diff --git a/common/memory_test.c b/common/memory_test.c
> new file mode 100644
> index 0000000..80b4ff4
> --- /dev/null
> +++ b/common/memory_test.c
> @@ -0,0 +1,399 @@
> +/*
> + * memory_test.c
> + *
> + * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
> + *
> + * 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 version 2
> + * as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + */
> +
> +#include <memory_test.h>
> +
> +static const vu_long bitpattern[] = {
> +	0x00000001,	/* single bit */
> +	0x00000003,	/* two adjacent bits */
> +	0x00000007,	/* three adjacent bits */
> +	0x0000000F,	/* four adjacent bits */
> +	0x00000005,	/* two non-adjacent bits */
> +	0x00000015,	/* three non-adjacent bits */
> +	0x00000055,	/* four non-adjacent bits */
> +	0xAAAAAAAA,	/* alternating 1/0 */
> +};
> +
> +/*
> + * Perform a memory test. The complete test
> + * loops until interrupted by ctrl-c.
> + *
> + * Highly recommended to test with disabled and
> + * enabled cache.
> + *
> + * start: start address
> + * end: end address
> + * bus_only: skip integrity check
> + */
> +int mem_test(vu_long _start, vu_long _end,
> +		int bus_only)
> +{
> +	vu_long *start;
> +	vu_long *dummy;
> +
> +	vu_long val;
> +	vu_long readback;
> +	vu_long offset;
> +	vu_long offset2;
> +	vu_long pattern;
> +	vu_long temp;
> +	vu_long anti_pattern;
> +	vu_long num_words;
> +
> +	int i;
> +	int ret;
> +
> +	if (!IS_ALIGNED(_start, sizeof(vu_long)))
> +		_start = ALIGN(_start, sizeof(vu_long));
> +	/*
> +	 * check if end is a multiple of vu_long.
> +	 * need to add 1 because ALIGNED works with
> +	 * inclusive byte at end address.
> +	 *
> +	 * Also check on _end == 0. Otherwise we get a
> +	 * underflow.
> +	 */
> +	if (!IS_ALIGNED(_end + 1, sizeof(vu_long)) && _end)
> +		_end = ALIGN_DOWN(_end, sizeof(vu_long)) - 1;
> +
> +	/*
> +	 * TODO
> +	 * recheck after align. That's not a quite
> +	 * solution now because we already done this
> +	 * in memtest command.
> +	 */
> +	if (_end <= _start)
> +		return -1;
> +
> +	start = (vu_long *)_start;
> +	/*
> +	 * Point the dummy to start[1]
> +	 */
> +	dummy = start+1;
> +	num_words = (_end - _start + 1)/sizeof(vu_long);
> +
> +	/*
> +	 * Checking if start and dummy address are in one
> +	 * of barebox regions. Otherwise next data line testing
> +	 * will maybe fail.
> +	 */
> +	ret = address_in_sdram_regions((vu_long)start);
> +	ret |= address_in_sdram_regions((vu_long)dummy);
> +	if (ret) {
> +		printf("WARNING (data line): "
> +				"address 0x%08lx is in sdram regions.\n"
> +				"Try another start address to fix this issue.\n",
> +				(vu_long)start);
> +		return -1;
> +	}
> +
> +	printf("Starting data line test.\n");
> +
> +	/*
> +	 * Data line test: write a pattern to the first
> +	 * location, write the 1's complement to a 'parking'
> +	 * address (changes the state of the data bus so a
> +	 * floating bus doen't give a false OK), and then
> +	 * read the value back. Note that we read it back
> +	 * into a variable because the next time we read it,
> +	 * it might be right (been there, tough to explain to
> +	 * the quality guys why it prints a failure when the
> +	 * "is" and "should be" are obviously the same in the
> +	 * error message).
> +	 *
> +	 * Rather than exhaustively testing, we test some
> +	 * patterns by shifting '1' bits through a field of
> +	 * '0's and '0' bits through a field of '1's (i.e.
> +	 * pattern and ~pattern).
> +	 */
> +	for (i = 0; i < sizeof(bitpattern)/
> +			sizeof(bitpattern[0]); i++) {

ARRAY_SIZE(bitpattern)

> +		val = bitpattern[i];
> +
> +		for (; val != 0; val <<= 1) {
> +			*start = val;
> +			/* clear the test data off of the bus */
> +			*dummy = ~val;
> +			readback = *start;
> +
> +			if (readback != val) {
> +				printf("FAILURE (data line): "
> +					"expected 0x%08lx, actual 0x%08lx at address 0x%08lx.\n",
> +					val, readback, (vu_long)start);
> +				return -1;
> +			}
> +
> +			*start = ~val;
> +			*dummy = val;
> +			readback = *start;
> +			if (readback != ~val) {
> +				printf("FAILURE (data line): "
> +					"Is 0x%08lx, should be 0x%08lx at address 0x%08lx.\n",
> +					readback,
> +					~val, (vu_long)start);
> +				return -1;
> +			}
> +		}
> +	}
> +
> +
> +	/*
> +	 * Based on code whose Original Author and Copyright
> +	 * information follows: Copyright (c) 1998 by Michael
> +	 * Barr. This software is placed into the public
> +	 * domain and may be used for any purpose. However,
> +	 * this notice must not be changed or removed and no
> +	 * warranty is either expressed or implied by its
> +	 * publication or distribution.
> +	 */
> +
> +	/*
> +	 * Address line test
> +	 *
> +	 * Description: Test the address bus wiring in a
> +	 *              memory region by performing a walking
> +	 *              1's test on the relevant bits of the
> +	 *              address and checking for aliasing.
> +	 *              This test will find single-bit
> +	 *              address failures such as stuck -high,
> +	 *              stuck-low, and shorted pins. The base
> +	 *              address and size of the region are
> +	 *              selected by the caller.
> +	 *
> +	 * Notes:	For best results, the selected base
> +	 *              address should have enough LSB 0's to
> +	 *              guarantee single address bit changes.
> +	 *              For example, to test a 64-Kbyte
> +	 *              region, select a base address on a
> +	 *              64-Kbyte boundary. Also, select the
> +	 *              region size as a power-of-two if at
> +	 *              all possible.
> +	 *
> +	 * ## NOTE ##	Be sure to specify start and end
> +	 *              addresses such that num_words has
> +	 *              lots of bits set. For example an
> +	 *              address range of 01000000 02000000 is
> +	 *              bad while a range of 01000000
> +	 *              01ffffff is perfect.
> +	 */
> +
> +	pattern = 0xAAAAAAAA;
> +	anti_pattern = 0x55555555;
> +
> +	/*
> +	 * Write the default pattern at each of the
> +	 * power-of-two offsets.
> +	 */
> +	for (offset = 1; offset <= num_words; offset <<= 1) {
> +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> +		if (ret) {
> +			printf("WARNING (stuck high): "
> +					"address 0x%08lx is in barebox regions.\n",
> +					(vu_long)&start[offset]);
> +			continue;
> +		}
> +
> +		start[offset] = pattern;
> +	}
> +
> +	printf("Check for address bits stuck high.\n");
> +
> +	/*
> +	 * Check for address bits stuck high.
> +	 */
> +	for (offset = 1; offset <= num_words; offset <<= 1) {
> +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> +		if (ret)
> +			continue;
> +
> +		temp = start[offset];
> +		if (temp != pattern) {
> +			printf("FAILURE: Address bit "
> +					"stuck high @ 0x%08lx:"
> +					" expected 0x%08lx, actual 0x%08lx.\n",
> +					(vu_long)&start[offset],
> +					pattern, temp);
> +			return -1;
> +		}
> +	}
> +
> +	printf("Check for address bits stuck "
> +			"low or shorted.\n");
> +
> +	/*
> +	 * Check for address bits stuck low or shorted.
> +	 */
> +	for (offset2 = 1; offset2 <= num_words; offset2 <<= 1) {
> +		ret = address_in_sdram_regions(
> +				(vu_long)&start[offset2]);
> +		if (ret) {
> +			printf("WARNING (low high): "
> +					"address 0x%08lx is in barebox regions.\n",
> +					(vu_long)&start[offset2]);
> +			continue;
> +		}
> +
> +		start[offset2] = anti_pattern;
> +
> +		for (offset = 1; offset <= num_words; offset <<= 1) {
> +			ret = address_in_sdram_regions(
> +					(vu_long)&start[offset]);
> +			if (ret)
> +				continue;
> +
> +			temp = start[offset];
> +
> +			/*
> +			 * That's some complicated for loop with
> +			 * condition offset != test_offset inside. I
> +			 * think this is necessary to put some another
> +			 * address on the bus.
> +			 *
> +			 * TODO
> +			 * check if loop is necessary.
> +			 */
> +			if ((temp != pattern) &&
> +					(offset != offset2)) {
> +				printf("FAILURE: Address bit stuck"
> +						" low or shorted @"
> +						" 0x%08lx: expected 0x%08lx, actual 0x%08lx.\n",
> +						(vu_long)&start[offset],
> +						pattern, temp);
> +				return -1;
> +			}
> +		}
> +		start[offset2] = pattern;
> +	}
> +
> +	/*
> +	 * We tested only the bus if != 0
> +	 * leaving here
> +	 */
> +	if (bus_only)
> +		return 0;
> +
> +	printf("Starting integrity check of physicaly ram.\n"
> +			"Filling ram with patterns...\n");
> +
> +	/*
> +	 * Description: Test the integrity of a physical
> +	 *		memory device by performing an
> +	 *		increment/decrement test over the
> +	 *		entire region. In the process every
> +	 *		storage bit in the device is tested
> +	 *		as a zero and a one. The base address
> +	 *		and the size of the region are
> +	 *		selected by the caller.
> +	 */
> +
> +	/*
> +	 * Fill memory with a known pattern.
> +	 */
> +	init_progression_bar(num_words);
> +	for (offset = 0; offset < num_words; offset++) {
> +		if (!(offset & 0xfff)) {
> +			if (ctrlc())
> +				return -EINTR;
> +			show_progress(offset);
> +		}
> +
> +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> +		if (ret)
> +			continue;
> +
> +		start[offset] = offset + 1;
> +	}
> +
> +	show_progress(offset);
> +
> +	printf("\nCompare written patterns...\n");
> +
> +	/*
> +	 * Check each location and invert it for the second pass.
> +	 */
> +	init_progression_bar(num_words - 1);
> +	for (offset = 0; offset < num_words; offset++) {
> +		if (!(offset & 0xfff)) {
> +			if (ctrlc())
> +				return -EINTR;
> +			show_progress(offset);
> +		}
> +
> +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> +		if (ret)
> +			continue;
> +
> +		temp = start[offset];
> +		if (temp != (offset + 1)) {
> +			printf("\nFAILURE (read/write) @ 0x%08lx:"
> +					" expected 0x%08lx, actual 0x%08lx.\n",
> +					(vu_long)&start[offset],
> +					(offset + 1), temp);
> +			return -1;
> +		}
> +
> +		anti_pattern = ~(offset + 1);
> +		start[offset] = anti_pattern;
> +	}
> +
> +	show_progress(offset);
> +
> +	printf("\nFilling ram with inverted pattern and compare it...\n");
> +
> +	/*
> +	 * Check each location for the inverted pattern and zero it.
> +	 */
> +	init_progression_bar(num_words - 1);
> +	for (offset = 0; offset < num_words; offset++) {
> +		if (!(offset & 0xfff)) {
> +			if (ctrlc())
> +				return -EINTR;
> +			show_progress(offset);
> +		}
> +
> +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> +		/*
> +		 * Step over barebox mem usage
> +		 */
> +		if (ret)
> +			continue;
> +
> +		anti_pattern = ~(offset + 1);
> +		temp = start[offset];
> +
> +		if (temp != anti_pattern) {
> +			printf("\nFAILURE (read/write): @ 0x%08lx:"
> +					" expected 0x%08lx, actual 0x%08lx.\n",
> +					(vu_long)&start[offset],
> +					anti_pattern, temp);
> +			return -1;

what about returning an errno?

> +		}
> +
> +		start[offset] = 0;
> +	}
> +
> +	show_progress(offset);
> +
> +	/*
> +	 * end of progressbar
> +	 */
> +	printf("\n");
> +
> +	return 0;
> +}
> diff --git a/include/memory_test.h b/include/memory_test.h
> new file mode 100644
> index 0000000..6959dc6
> --- /dev/null
> +++ b/include/memory_test.h
> @@ -0,0 +1,13 @@
> +
> +#ifndef __MEMORY_TEST_H
> +#define __MEMORY_TEST_H
> +
> +#include <progress.h>
> +#include <common.h>
> +#include <memory.h>
> +#include <types.h>
> +
> +int mem_test(vu_long _start, vu_long _end,
> +		int bus_only);
> +
> +#endif
> 

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] commands: add new memtest command

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 11397 bytes --]

On 02/07/2013 11:45 AM, Alexander Aring wrote:
> Add new memtest command which can enable or disable caching
> on non allocted barebox regions(test area).
> 
> This command simply parse and check parameters then call
> the mem_test routine.
> 
> If no address parameters are given then mem_test will call
> for each memory bank.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

A howto-get-rid-of-ifdef nitpick inline

> ---
>  commands/Kconfig   |  10 ++
>  commands/Makefile  |   1 +
>  commands/memtest.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 373 insertions(+)
>  create mode 100644 commands/memtest.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 7cc759c..d158c3f 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -516,6 +516,16 @@ config CMD_NANDTEST
>  	select PARTITION_NEED_MTD
>  	prompt "nandtest"
>  
> +config CMD_MEMTEST
> +    tristate
> +    select MEMTEST
> +    prompt "memtest"
> +	help
> +	  This command enables a memtest to test installed memory.
> +	  During this test allocated iomem regions will be skipped.
> +	  If tested architecture has MMU with PTE flags support,
> +	  caching can be set enabled or disabled.
> +
>  endmenu
>  
>  menu "video command"
> diff --git a/commands/Makefile b/commands/Makefile
> index 393ba51..b39b489 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
>  obj-$(CONFIG_CMD_LOADS)		+= loads.o
>  obj-$(CONFIG_CMD_ECHO)		+= echo.o
>  obj-$(CONFIG_CMD_MEMORY)	+= mem.o
> +obj-$(CONFIG_CMD_MEMTEST)	+= memtest.o
>  obj-$(CONFIG_CMD_EDIT)		+= edit.o
>  obj-$(CONFIG_CMD_EXEC)		+= exec.o
>  obj-$(CONFIG_CMD_SLEEP)		+= sleep.o
> diff --git a/commands/memtest.c b/commands/memtest.c
> new file mode 100644
> index 0000000..22e8006
> --- /dev/null
> +++ b/commands/memtest.c
> @@ -0,0 +1,362 @@
> +/*
> + * memtest - Perform a memory test
> + *
> + * (C) Copyright 2013
> + * Alexander Aring <aar@pengutronix.de>, Pengutronix
> + *
> + * (C) Copyright 2000
> + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> + *
> + * 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 <command.h>
> +#include <getopt.h>
> +#include <asm/mmu.h>
> +
> +#include <memory_test.h>
> +
> +/*
> + * In CONFIG_MMU we have a special c flag.
> + */
> +#ifdef CONFIG_MMU
> +static char optstr[] = "s:e:i:cb";

const?

> +
> +/*
> + * PTE flags variables to set cached and
> + * uncached regions.
> + */
> +static uint32_t pte_flags_cached;
> +static uint32_t pte_flags_uncached;
> +#else
> +static char optstr[] = "s:e:i:b";

const?

> +#endif
> +
> +#ifdef CONFIG_MMU

Can
> +static void print_region(vu_long start, vu_long size, uint32_t flags)
> +{
> +	if (!size)
> +		return;
> +
> +	printf("\t0x%08lx - "
> +			"0x%08lx (size 0x%08lx)\n",
> +			start, start + size - 1, size);
> +}
> +
> +static void do_remap_range(struct memory_bank *bank, uint32_t flags)
> +{
> +	struct resource *r = NULL;
> +	struct resource *r_prev = NULL;
> +
> +	vu_long size;
> +	vu_long start;
> +	vu_long end;
> +
> +	if (flags == pte_flags_uncached)
> +		printf("Set non caching regions:\n");
> +	else if (flags == pte_flags_cached)
> +		printf("Set caching regions:\n");
> +	else
> +		BUG();
> +
> +	/*
> +	 * We assume that the regions are sorted in this list
> +	 */
> +	list_for_each_entry(r, &bank->res->children, sibling) {
> +		/*
> +		 * Do on head element for bank boundary
> +		 */
> +		if (r->sibling.prev == &bank->res->children) {
> +			/*
> +			 * remember last used element
> +			 */
> +			r_prev = r;
> +
> +			start = PAGE_ALIGN(bank->start);
> +			end = PAGE_ALIGN_DOWN(r->start) - 1;
> +			if (start >= end)
> +				continue;
> +			size = end - start + 1;
> +
> +			print_region(start, size, flags);
> +			remap_range((void *)start, size, flags);
> +
> +			continue;
> +		}
> +		/*
> +		 * Between used regions
> +		 */
> +		start = PAGE_ALIGN(r_prev->end);
> +		end = PAGE_ALIGN_DOWN(r->start) - 1;
> +		if (start < end) {
> +			size = end - start + 1;
> +			print_region(start, size, flags);
> +			remap_range((void *)start, size, flags);
> +		}
> +
> +		r_prev = r;
> +		/*
> +		 * Do on head element for bank boundary
> +		 */
> +		if (list_is_last(&r->sibling, &bank->res->children)) {
> +			start = PAGE_ALIGN(r->end);
> +			end = PAGE_ALIGN_DOWN(bank->start + bank->size) - 1;
> +			if (start >= end)
> +				continue;
> +			size = end - start + 1;
> +
> +			print_region(start, size, flags);
> +			remap_range((void *)start, size, flags);
> +		}
> +	}
> +}
> +#endif
> +
> +static int do_mem_memtest(int argc, char *argv[])
> +{
> +	/*
> +	 * Set start address to 0xffffffff which
> +	 * can't be.
> +	 */
> +	vu_long start = 0xffffffff;
> +	vu_long end = 0;
> +
> +	uint i;
> +	uint max_i = 1;
> +
> +#ifdef CONFIG_MMU
> +	int cache = 0;
> +#endif
> +	int bus_only = 0;
> +	int err = 0;
> +	int cnt = 0;
> +	int opt;
> +
> +	struct memory_bank *bank = NULL;
> +	struct resource *r = NULL;
> +
> +	while ((opt = getopt(argc, argv, optstr)) > 0) {
> +		switch (opt) {
> +		case 's':
> +			start = simple_strtoul(optarg, NULL, 0);
> +			break;
> +		case 'e':
> +			end = simple_strtoul(optarg, NULL, 0);
> +			break;
> +		case 'i':
> +			max_i = simple_strtoul(optarg, NULL, 0);
> +			break;
> +#ifdef CONFIG_MMU
> +		case 'c':
> +			cache = 1;
> +			break;
> +#endif
> +		case 'b':
> +			bus_only = 1;
> +			break;
> +		default:
> +			return COMMAND_ERROR_USAGE;
> +		}
> +	}
> +
> +	if (optind > argc)
> +		return COMMAND_ERROR_USAGE;
> +
> +	/*
> +	 * Error if no end address
> +	 */
> +	if (start != 0xffffffff && !end) {
> +		printf("Please add an end address.\n");
> +		return 1;
> +	}
> +
> +	/*
> +	 * Error if no start address
> +	 */
> +	if (end && start == 0xffffffff) {
> +		printf("Please add a start address.\n");
> +		return 1;
> +	}
> +
> +	/*
> +	 * Check parameters
> +	 */
> +	if (start != 0xffffffff && end) {
> +		if (end <= start) {
> +			printf("End address less than or"
> +					" equal start address.\n");
> +			return 1;
> +		}
> +
> +		/*
> +		 * Check if given start and end address are in any banks
> +		 */
> +		for_each_memory_bank(bank) {
> +			if (ADDRESS_IN_REGIONS(start, bank->start,
> +						bank->start + bank->size))
> +				cnt++;
> +
> +			if (ADDRESS_IN_REGIONS(end, bank->start,
> +						bank->start + bank->size))
> +				cnt++;
> +		}
> +
> +		if (cnt != 2) {
> +			printf("Start or end addresses are"
> +					" not in any ram bank.\n");
> +			return 1;
> +		}
> +	}
> +
> +#ifdef CONFIG_MMU
> +	/*
> +	 * Get pte flags. Which are configured at
> +	 * runtime at booting.
> +	 */
> +	pte_flags_cached = mmu_get_pte_cached_flags();
> +	pte_flags_uncached = mmu_get_pte_uncached_flags();
> +#endif
> +
> +	printf("Skipping regions:\n");
> +	for_each_memory_bank(bank) {
> +		list_for_each_entry(r, &bank->res->children, sibling)
> +			printf("\t0x%08x - "
> +					"0x%08x (size 0x%08x) %s\n",
> +					r->start, r->end,
> +					r->end - r->start + 1, r->name);
> +#ifdef CONFIG_MMU

Use if (IS_ENABLED(CONFIG_MMU) and you can get rid of most ifdefs

Marc


> +		/*
> +		 * Disable or enable caching
> +		 */
> +		if (cache)
> +			do_remap_range(bank, pte_flags_cached);
> +		else
> +			do_remap_range(bank, pte_flags_uncached);
> +#endif
> +	}
> +
> +	/*
> +	 * Do test if we set a start or end address
> +	 */
> +	if (start != 0xffffffff && end) {
> +		printf("Testing address range:\n\t0x%08lx - 0x%08lx"
> +				" (size 0x%08lx)\n",
> +				start, end, end - start + 1);
> +
> +		for (i = 1; (i <= max_i) || !max_i; i++) {
> +			printf("Iteration: %u\n", i);
> +
> +			/*
> +			 * Do the Memtest
> +			 */
> +			err = mem_test(start, end, bus_only);
> +			if (err == -EINTR) {
> +				printf("Test interrupted.\n");
> +				goto err;
> +			}
> +
> +			if (err < 0) {
> +				printf("Test failed.\n");
> +				goto err;
> +			}
> +			printf("Tested %u iteration(s) without errors.\n", i);
> +		}
> +#ifdef CONFIG_MMU
> +		/*
> +		 * Renable caching
> +		 */
> +		if (!cache)
> +			for_each_memory_bank(bank)
> +				do_remap_range(bank, pte_flags_cached);
> +#endif
> +		printf("Memtest done.\n");
> +
> +		return 0;
> +	}
> +
> +	/*
> +	 * If we set no start or end address
> +	 * we do the test on all ram banks
> +	 */
> +	for (i = 1; (i <= max_i) || !max_i; i++) {
> +		for_each_memory_bank(bank) {
> +			start = bank->start;
> +			end = bank->start + bank->size - 1;
> +
> +			printf("Iteration: %u\n", i);
> +
> +			printf("Testing address range:\n\t0x%08lx - "
> +					"0x%08lx (size 0x%08lx) on bank /dev/%s\n",
> +					start, end, bank->size,
> +					bank->res->name);
> +
> +			err = mem_test(start, end, bus_only);
> +			if (err == -EINTR) {
> +				printf("Test interrupted.\n");
> +				goto err;
> +			}
> +
> +			if (err < 0) {
> +				printf("Test on bank /dev/%s failed.\n",
> +						bank->res->name);
> +				goto err;
> +			}
> +			printf("Tested %u iteration(s) without errors.\n", i);
> +		}
> +	}
> +#ifdef CONFIG_MMU
> +	/*
> +	 * Renable caching
> +	 */
> +	if (!cache)
> +		for_each_memory_bank(bank)
> +			do_remap_range(bank, pte_flags_cached);
> +#endif
> +	printf("Memtest done.\n");
> +
> +	return 0;
> +
> +err:
> +#ifdef CONFIG_MMU
> +	/*
> +	 * Enable caching
> +	 */
> +	for_each_memory_bank(bank)
> +		do_remap_range(bank, pte_flags_cached);
> +#endif
> +
> +	return 1;
> +}
> +
> +static const __maybe_unused char cmd_memtest_help[] =
> +"Usage: memtest [OPTION]...\n"
> +"memtest related commands\n"
> +"	-s	<start>		start address to begin memtest.\n"
> +"	-e	<end>		end address to stop memtest.\n"
> +"	-i	<iterations>	iterations [default=1, endless=0].\n"
> +#ifdef CONFIG_MMU
> +"	-c			run test with enable cache.\n"
> +#endif
> +"	-b			only test bus datalines.";
> +
> +BAREBOX_CMD_START(memtest)
> +	.cmd		= do_mem_memtest,
> +	.usage		= "Memory Test",
> +	BAREBOX_CMD_HELP(cmd_memtest_help)
> +BAREBOX_CMD_END
> 


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] common: add mem_test routine

[Sascha Hauer]

On Thu, Feb 07, 2013 at 11:44:59AM +0100, Alexander Aring wrote:
> Useful to detect timing problems if someone porting a new
> device to barebox.
> 
> This test includes a data bus test, address bus test and
> integrity check of memory.
> 
> Allocated barebox regions between start and end will skip
> automatically.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  common/Kconfig        |   7 +
>  common/Makefile       |   1 +
>  common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/memory_test.h |  13 ++
>  4 files changed, 420 insertions(+)
>  create mode 100644 common/memory_test.c
>  create mode 100644 include/memory_test.h
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 3f6c11e..c6988df 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -100,6 +100,13 @@ config MEMINFO
>  	bool "display memory info"
>  	default y
>  
> +config MEMTEST
> +	bool "Offers routines for memory test"
> +	help
> +	  Offers memtest routines in common/memory_test.c
> +	  This is helpful for porting devices to detect
> +	  memory timing problems.
> +
>  config ENVIRONMENT_VARIABLES
>  	bool "environment variables support"
>  
> diff --git a/common/Makefile b/common/Makefile
> index 7206eed..684953c 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
>  obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
>  obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
>  obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
> +obj-$(CONFIG_MEMTEST) += memory_test.o
>  obj-y += clock.o
>  obj-$(CONFIG_BANNER) += version.o
>  obj-$(CONFIG_MEMINFO) += meminfo.o
> diff --git a/common/memory_test.c b/common/memory_test.c
> new file mode 100644
> index 0000000..80b4ff4
> --- /dev/null
> +++ b/common/memory_test.c
> @@ -0,0 +1,399 @@
> +/*
> + * memory_test.c
> + *
> + * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
> + *
> + * 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 version 2
> + * as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + */
> +
> +#include <memory_test.h>
> +
> +static const vu_long bitpattern[] = {
> +	0x00000001,	/* single bit */
> +	0x00000003,	/* two adjacent bits */
> +	0x00000007,	/* three adjacent bits */
> +	0x0000000F,	/* four adjacent bits */
> +	0x00000005,	/* two non-adjacent bits */
> +	0x00000015,	/* three non-adjacent bits */
> +	0x00000055,	/* four non-adjacent bits */
> +	0xAAAAAAAA,	/* alternating 1/0 */
> +};
> +
> +/*
> + * Perform a memory test. The complete test
> + * loops until interrupted by ctrl-c.
> + *
> + * Highly recommended to test with disabled and
> + * enabled cache.
> + *
> + * start: start address
> + * end: end address
> + * bus_only: skip integrity check
> + */
> +int mem_test(vu_long _start, vu_long _end,
> +		int bus_only)
> +{

I thought we agreed on using request_sdram_region here and drop the
address_in_sdram_regions in this function.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] common: add mem_test routine

[Alexander Aring]

Hi,

On Thu, Feb 07, 2013 at 11:52:47AM +0100, Marc Kleine-Budde wrote:
> On 02/07/2013 11:44 AM, Alexander Aring wrote:
> > Useful to detect timing problems if someone porting a new
> > device to barebox.
> > 
> > This test includes a data bus test, address bus test and
> > integrity check of memory.
> > 
> > Allocated barebox regions between start and end will skip
> > automatically.
> 
> Some nitpicking inline.
> 
> Is there a nice alternative to usage of the vu_long type?

I can use a volatile version of resource_size_t.

> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > ---
> >  common/Kconfig        |   7 +
> >  common/Makefile       |   1 +
> >  common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/memory_test.h |  13 ++
> >  4 files changed, 420 insertions(+)
> >  create mode 100644 common/memory_test.c
> >  create mode 100644 include/memory_test.h
> > 
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 3f6c11e..c6988df 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -100,6 +100,13 @@ config MEMINFO
> >  	bool "display memory info"
> >  	default y
> >  
> > +config MEMTEST
> > +	bool "Offers routines for memory test"
> > +	help
> > +	  Offers memtest routines in common/memory_test.c
> > +	  This is helpful for porting devices to detect
> > +	  memory timing problems.
> > +
> >  config ENVIRONMENT_VARIABLES
> >  	bool "environment variables support"
> >  
> > diff --git a/common/Makefile b/common/Makefile
> > index 7206eed..684953c 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
> >  obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
> >  obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
> >  obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
> > +obj-$(CONFIG_MEMTEST) += memory_test.o
> >  obj-y += clock.o
> >  obj-$(CONFIG_BANNER) += version.o
> >  obj-$(CONFIG_MEMINFO) += meminfo.o
> > diff --git a/common/memory_test.c b/common/memory_test.c
> > new file mode 100644
> > index 0000000..80b4ff4
> > --- /dev/null
> > +++ b/common/memory_test.c
> > @@ -0,0 +1,399 @@
> > +/*
> > + * memory_test.c
> > + *
> > + * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
> > + *
> > + * 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 version 2
> > + * as published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#include <memory_test.h>
> > +
> > +static const vu_long bitpattern[] = {
> > +	0x00000001,	/* single bit */
> > +	0x00000003,	/* two adjacent bits */
> > +	0x00000007,	/* three adjacent bits */
> > +	0x0000000F,	/* four adjacent bits */
> > +	0x00000005,	/* two non-adjacent bits */
> > +	0x00000015,	/* three non-adjacent bits */
> > +	0x00000055,	/* four non-adjacent bits */
> > +	0xAAAAAAAA,	/* alternating 1/0 */
> > +};
> > +
> > +/*
> > + * Perform a memory test. The complete test
> > + * loops until interrupted by ctrl-c.
> > + *
> > + * Highly recommended to test with disabled and
> > + * enabled cache.
> > + *
> > + * start: start address
> > + * end: end address
> > + * bus_only: skip integrity check
> > + */
> > +int mem_test(vu_long _start, vu_long _end,
> > +		int bus_only)
> > +{
> > +	vu_long *start;
> > +	vu_long *dummy;
> > +
> > +	vu_long val;
> > +	vu_long readback;
> > +	vu_long offset;
> > +	vu_long offset2;
> > +	vu_long pattern;
> > +	vu_long temp;
> > +	vu_long anti_pattern;
> > +	vu_long num_words;
> > +
> > +	int i;
> > +	int ret;
> > +
> > +	if (!IS_ALIGNED(_start, sizeof(vu_long)))
> > +		_start = ALIGN(_start, sizeof(vu_long));
> > +	/*
> > +	 * check if end is a multiple of vu_long.
> > +	 * need to add 1 because ALIGNED works with
> > +	 * inclusive byte at end address.
> > +	 *
> > +	 * Also check on _end == 0. Otherwise we get a
> > +	 * underflow.
> > +	 */
> > +	if (!IS_ALIGNED(_end + 1, sizeof(vu_long)) && _end)
> > +		_end = ALIGN_DOWN(_end, sizeof(vu_long)) - 1;
> > +
> > +	/*
> > +	 * TODO
> > +	 * recheck after align. That's not a quite
> > +	 * solution now because we already done this
> > +	 * in memtest command.
> > +	 */
> > +	if (_end <= _start)
> > +		return -1;
> > +
> > +	start = (vu_long *)_start;
> > +	/*
> > +	 * Point the dummy to start[1]
> > +	 */
> > +	dummy = start+1;
> > +	num_words = (_end - _start + 1)/sizeof(vu_long);
> > +
> > +	/*
> > +	 * Checking if start and dummy address are in one
> > +	 * of barebox regions. Otherwise next data line testing
> > +	 * will maybe fail.
> > +	 */
> > +	ret = address_in_sdram_regions((vu_long)start);
> > +	ret |= address_in_sdram_regions((vu_long)dummy);
> > +	if (ret) {
> > +		printf("WARNING (data line): "
> > +				"address 0x%08lx is in sdram regions.\n"
> > +				"Try another start address to fix this issue.\n",
> > +				(vu_long)start);
> > +		return -1;
> > +	}
> > +
> > +	printf("Starting data line test.\n");
> > +
> > +	/*
> > +	 * Data line test: write a pattern to the first
> > +	 * location, write the 1's complement to a 'parking'
> > +	 * address (changes the state of the data bus so a
> > +	 * floating bus doen't give a false OK), and then
> > +	 * read the value back. Note that we read it back
> > +	 * into a variable because the next time we read it,
> > +	 * it might be right (been there, tough to explain to
> > +	 * the quality guys why it prints a failure when the
> > +	 * "is" and "should be" are obviously the same in the
> > +	 * error message).
> > +	 *
> > +	 * Rather than exhaustively testing, we test some
> > +	 * patterns by shifting '1' bits through a field of
> > +	 * '0's and '0' bits through a field of '1's (i.e.
> > +	 * pattern and ~pattern).
> > +	 */
> > +	for (i = 0; i < sizeof(bitpattern)/
> > +			sizeof(bitpattern[0]); i++) {
> 
> ARRAY_SIZE(bitpattern)
> 

Ok.

> > +		val = bitpattern[i];
> > +
> > +		for (; val != 0; val <<= 1) {
> > +			*start = val;
> > +			/* clear the test data off of the bus */
> > +			*dummy = ~val;
> > +			readback = *start;
> > +
> > +			if (readback != val) {
> > +				printf("FAILURE (data line): "
> > +					"expected 0x%08lx, actual 0x%08lx at address 0x%08lx.\n",
> > +					val, readback, (vu_long)start);
> > +				return -1;
> > +			}
> > +
> > +			*start = ~val;
> > +			*dummy = val;
> > +			readback = *start;
> > +			if (readback != ~val) {
> > +				printf("FAILURE (data line): "
> > +					"Is 0x%08lx, should be 0x%08lx at address 0x%08lx.\n",
> > +					readback,
> > +					~val, (vu_long)start);
> > +				return -1;
> > +			}
> > +		}
> > +	}
> > +
> > +
> > +	/*
> > +	 * Based on code whose Original Author and Copyright
> > +	 * information follows: Copyright (c) 1998 by Michael
> > +	 * Barr. This software is placed into the public
> > +	 * domain and may be used for any purpose. However,
> > +	 * this notice must not be changed or removed and no
> > +	 * warranty is either expressed or implied by its
> > +	 * publication or distribution.
> > +	 */
> > +
> > +	/*
> > +	 * Address line test
> > +	 *
> > +	 * Description: Test the address bus wiring in a
> > +	 *              memory region by performing a walking
> > +	 *              1's test on the relevant bits of the
> > +	 *              address and checking for aliasing.
> > +	 *              This test will find single-bit
> > +	 *              address failures such as stuck -high,
> > +	 *              stuck-low, and shorted pins. The base
> > +	 *              address and size of the region are
> > +	 *              selected by the caller.
> > +	 *
> > +	 * Notes:	For best results, the selected base
> > +	 *              address should have enough LSB 0's to
> > +	 *              guarantee single address bit changes.
> > +	 *              For example, to test a 64-Kbyte
> > +	 *              region, select a base address on a
> > +	 *              64-Kbyte boundary. Also, select the
> > +	 *              region size as a power-of-two if at
> > +	 *              all possible.
> > +	 *
> > +	 * ## NOTE ##	Be sure to specify start and end
> > +	 *              addresses such that num_words has
> > +	 *              lots of bits set. For example an
> > +	 *              address range of 01000000 02000000 is
> > +	 *              bad while a range of 01000000
> > +	 *              01ffffff is perfect.
> > +	 */
> > +
> > +	pattern = 0xAAAAAAAA;
> > +	anti_pattern = 0x55555555;
> > +
> > +	/*
> > +	 * Write the default pattern at each of the
> > +	 * power-of-two offsets.
> > +	 */
> > +	for (offset = 1; offset <= num_words; offset <<= 1) {
> > +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> > +		if (ret) {
> > +			printf("WARNING (stuck high): "
> > +					"address 0x%08lx is in barebox regions.\n",
> > +					(vu_long)&start[offset]);
> > +			continue;
> > +		}
> > +
> > +		start[offset] = pattern;
> > +	}
> > +
> > +	printf("Check for address bits stuck high.\n");
> > +
> > +	/*
> > +	 * Check for address bits stuck high.
> > +	 */
> > +	for (offset = 1; offset <= num_words; offset <<= 1) {
> > +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> > +		if (ret)
> > +			continue;
> > +
> > +		temp = start[offset];
> > +		if (temp != pattern) {
> > +			printf("FAILURE: Address bit "
> > +					"stuck high @ 0x%08lx:"
> > +					" expected 0x%08lx, actual 0x%08lx.\n",
> > +					(vu_long)&start[offset],
> > +					pattern, temp);
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	printf("Check for address bits stuck "
> > +			"low or shorted.\n");
> > +
> > +	/*
> > +	 * Check for address bits stuck low or shorted.
> > +	 */
> > +	for (offset2 = 1; offset2 <= num_words; offset2 <<= 1) {
> > +		ret = address_in_sdram_regions(
> > +				(vu_long)&start[offset2]);
> > +		if (ret) {
> > +			printf("WARNING (low high): "
> > +					"address 0x%08lx is in barebox regions.\n",
> > +					(vu_long)&start[offset2]);
> > +			continue;
> > +		}
> > +
> > +		start[offset2] = anti_pattern;
> > +
> > +		for (offset = 1; offset <= num_words; offset <<= 1) {
> > +			ret = address_in_sdram_regions(
> > +					(vu_long)&start[offset]);
> > +			if (ret)
> > +				continue;
> > +
> > +			temp = start[offset];
> > +
> > +			/*
> > +			 * That's some complicated for loop with
> > +			 * condition offset != test_offset inside. I
> > +			 * think this is necessary to put some another
> > +			 * address on the bus.
> > +			 *
> > +			 * TODO
> > +			 * check if loop is necessary.
> > +			 */
> > +			if ((temp != pattern) &&
> > +					(offset != offset2)) {
> > +				printf("FAILURE: Address bit stuck"
> > +						" low or shorted @"
> > +						" 0x%08lx: expected 0x%08lx, actual 0x%08lx.\n",
> > +						(vu_long)&start[offset],
> > +						pattern, temp);
> > +				return -1;
> > +			}
> > +		}
> > +		start[offset2] = pattern;
> > +	}
> > +
> > +	/*
> > +	 * We tested only the bus if != 0
> > +	 * leaving here
> > +	 */
> > +	if (bus_only)
> > +		return 0;
> > +
> > +	printf("Starting integrity check of physicaly ram.\n"
> > +			"Filling ram with patterns...\n");
> > +
> > +	/*
> > +	 * Description: Test the integrity of a physical
> > +	 *		memory device by performing an
> > +	 *		increment/decrement test over the
> > +	 *		entire region. In the process every
> > +	 *		storage bit in the device is tested
> > +	 *		as a zero and a one. The base address
> > +	 *		and the size of the region are
> > +	 *		selected by the caller.
> > +	 */
> > +
> > +	/*
> > +	 * Fill memory with a known pattern.
> > +	 */
> > +	init_progression_bar(num_words);
> > +	for (offset = 0; offset < num_words; offset++) {
> > +		if (!(offset & 0xfff)) {
> > +			if (ctrlc())
> > +				return -EINTR;
> > +			show_progress(offset);
> > +		}
> > +
> > +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> > +		if (ret)
> > +			continue;
> > +
> > +		start[offset] = offset + 1;
> > +	}
> > +
> > +	show_progress(offset);
> > +
> > +	printf("\nCompare written patterns...\n");
> > +
> > +	/*
> > +	 * Check each location and invert it for the second pass.
> > +	 */
> > +	init_progression_bar(num_words - 1);
> > +	for (offset = 0; offset < num_words; offset++) {
> > +		if (!(offset & 0xfff)) {
> > +			if (ctrlc())
> > +				return -EINTR;
> > +			show_progress(offset);
> > +		}
> > +
> > +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> > +		if (ret)
> > +			continue;
> > +
> > +		temp = start[offset];
> > +		if (temp != (offset + 1)) {
> > +			printf("\nFAILURE (read/write) @ 0x%08lx:"
> > +					" expected 0x%08lx, actual 0x%08lx.\n",
> > +					(vu_long)&start[offset],
> > +					(offset + 1), temp);
> > +			return -1;
> > +		}
> > +
> > +		anti_pattern = ~(offset + 1);
> > +		start[offset] = anti_pattern;
> > +	}
> > +
> > +	show_progress(offset);
> > +
> > +	printf("\nFilling ram with inverted pattern and compare it...\n");
> > +
> > +	/*
> > +	 * Check each location for the inverted pattern and zero it.
> > +	 */
> > +	init_progression_bar(num_words - 1);
> > +	for (offset = 0; offset < num_words; offset++) {
> > +		if (!(offset & 0xfff)) {
> > +			if (ctrlc())
> > +				return -EINTR;
> > +			show_progress(offset);
> > +		}
> > +
> > +		ret = address_in_sdram_regions((vu_long)&start[offset]);
> > +		/*
> > +		 * Step over barebox mem usage
> > +		 */
> > +		if (ret)
> > +			continue;
> > +
> > +		anti_pattern = ~(offset + 1);
> > +		temp = start[offset];
> > +
> > +		if (temp != anti_pattern) {
> > +			printf("\nFAILURE (read/write): @ 0x%08lx:"
> > +					" expected 0x%08lx, actual 0x%08lx.\n",
> > +					(vu_long)&start[offset],
> > +					anti_pattern, temp);
> > +			return -1;
> 
> what about returning an errno?
> 

Ok. I will change that.

Regards
Alex

> > +		}
> > +
> > +		start[offset] = 0;
> > +	}
> > +
> > +	show_progress(offset);
> > +
> > +	/*
> > +	 * end of progressbar
> > +	 */
> > +	printf("\n");
> > +
> > +	return 0;
> > +}
> > diff --git a/include/memory_test.h b/include/memory_test.h
> > new file mode 100644
> > index 0000000..6959dc6
> > --- /dev/null
> > +++ b/include/memory_test.h
> > @@ -0,0 +1,13 @@
> > +
> > +#ifndef __MEMORY_TEST_H
> > +#define __MEMORY_TEST_H
> > +
> > +#include <progress.h>
> > +#include <common.h>
> > +#include <memory.h>
> > +#include <types.h>
> > +
> > +int mem_test(vu_long _start, vu_long _end,
> > +		int bus_only);
> > +
> > +#endif
> > 
> 
> Marc
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] commands: add new memtest command

[Alexander Aring]

On Thu, Feb 07, 2013 at 11:56:07AM +0100, Marc Kleine-Budde wrote:
> On 02/07/2013 11:45 AM, Alexander Aring wrote:
> > Add new memtest command which can enable or disable caching
> > on non allocted barebox regions(test area).
> > 
> > This command simply parse and check parameters then call
> > the mem_test routine.
> > 
> > If no address parameters are given then mem_test will call
> > for each memory bank.
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> 
> A howto-get-rid-of-ifdef nitpick inline
> 
> > ---
> >  commands/Kconfig   |  10 ++
> >  commands/Makefile  |   1 +
> >  commands/memtest.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 373 insertions(+)
> >  create mode 100644 commands/memtest.c
> > 
> > diff --git a/commands/Kconfig b/commands/Kconfig
> > index 7cc759c..d158c3f 100644
> > --- a/commands/Kconfig
> > +++ b/commands/Kconfig
> > @@ -516,6 +516,16 @@ config CMD_NANDTEST
> >  	select PARTITION_NEED_MTD
> >  	prompt "nandtest"
> >  
> > +config CMD_MEMTEST
> > +    tristate
> > +    select MEMTEST
> > +    prompt "memtest"
> > +	help
> > +	  This command enables a memtest to test installed memory.
> > +	  During this test allocated iomem regions will be skipped.
> > +	  If tested architecture has MMU with PTE flags support,
> > +	  caching can be set enabled or disabled.
> > +
> >  endmenu
> >  
> >  menu "video command"
> > diff --git a/commands/Makefile b/commands/Makefile
> > index 393ba51..b39b489 100644
> > --- a/commands/Makefile
> > +++ b/commands/Makefile
> > @@ -7,6 +7,7 @@ obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
> >  obj-$(CONFIG_CMD_LOADS)		+= loads.o
> >  obj-$(CONFIG_CMD_ECHO)		+= echo.o
> >  obj-$(CONFIG_CMD_MEMORY)	+= mem.o
> > +obj-$(CONFIG_CMD_MEMTEST)	+= memtest.o
> >  obj-$(CONFIG_CMD_EDIT)		+= edit.o
> >  obj-$(CONFIG_CMD_EXEC)		+= exec.o
> >  obj-$(CONFIG_CMD_SLEEP)		+= sleep.o
> > diff --git a/commands/memtest.c b/commands/memtest.c
> > new file mode 100644
> > index 0000000..22e8006
> > --- /dev/null
> > +++ b/commands/memtest.c
> > @@ -0,0 +1,362 @@
> > +/*
> > + * memtest - Perform a memory test
> > + *
> > + * (C) Copyright 2013
> > + * Alexander Aring <aar@pengutronix.de>, Pengutronix
> > + *
> > + * (C) Copyright 2000
> > + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> > + *
> > + * 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 <command.h>
> > +#include <getopt.h>
> > +#include <asm/mmu.h>
> > +
> > +#include <memory_test.h>
> > +
> > +/*
> > + * In CONFIG_MMU we have a special c flag.
> > + */
> > +#ifdef CONFIG_MMU
> > +static char optstr[] = "s:e:i:cb";
> 
> const?
>

This will print a compiler warning because the getopt implementation
doesn't accept a const char* because getopt will change this string.

But I can cast it later.

Regards
Alex

> > +
> > +/*
> > + * PTE flags variables to set cached and
> > + * uncached regions.
> > + */
> > +static uint32_t pte_flags_cached;
> > +static uint32_t pte_flags_uncached;
> > +#else
> > +static char optstr[] = "s:e:i:b";
> 
> const?
> 
> > +#endif
> > +
> > +#ifdef CONFIG_MMU
> 
> Can
> > +static void print_region(vu_long start, vu_long size, uint32_t flags)
> > +{
> > +	if (!size)
> > +		return;
> > +
> > +	printf("\t0x%08lx - "
> > +			"0x%08lx (size 0x%08lx)\n",
> > +			start, start + size - 1, size);
> > +}
> > +
> > +static void do_remap_range(struct memory_bank *bank, uint32_t flags)
> > +{
> > +	struct resource *r = NULL;
> > +	struct resource *r_prev = NULL;
> > +
> > +	vu_long size;
> > +	vu_long start;
> > +	vu_long end;
> > +
> > +	if (flags == pte_flags_uncached)
> > +		printf("Set non caching regions:\n");
> > +	else if (flags == pte_flags_cached)
> > +		printf("Set caching regions:\n");
> > +	else
> > +		BUG();
> > +
> > +	/*
> > +	 * We assume that the regions are sorted in this list
> > +	 */
> > +	list_for_each_entry(r, &bank->res->children, sibling) {
> > +		/*
> > +		 * Do on head element for bank boundary
> > +		 */
> > +		if (r->sibling.prev == &bank->res->children) {
> > +			/*
> > +			 * remember last used element
> > +			 */
> > +			r_prev = r;
> > +
> > +			start = PAGE_ALIGN(bank->start);
> > +			end = PAGE_ALIGN_DOWN(r->start) - 1;
> > +			if (start >= end)
> > +				continue;
> > +			size = end - start + 1;
> > +
> > +			print_region(start, size, flags);
> > +			remap_range((void *)start, size, flags);
> > +
> > +			continue;
> > +		}
> > +		/*
> > +		 * Between used regions
> > +		 */
> > +		start = PAGE_ALIGN(r_prev->end);
> > +		end = PAGE_ALIGN_DOWN(r->start) - 1;
> > +		if (start < end) {
> > +			size = end - start + 1;
> > +			print_region(start, size, flags);
> > +			remap_range((void *)start, size, flags);
> > +		}
> > +
> > +		r_prev = r;
> > +		/*
> > +		 * Do on head element for bank boundary
> > +		 */
> > +		if (list_is_last(&r->sibling, &bank->res->children)) {
> > +			start = PAGE_ALIGN(r->end);
> > +			end = PAGE_ALIGN_DOWN(bank->start + bank->size) - 1;
> > +			if (start >= end)
> > +				continue;
> > +			size = end - start + 1;
> > +
> > +			print_region(start, size, flags);
> > +			remap_range((void *)start, size, flags);
> > +		}
> > +	}
> > +}
> > +#endif
> > +
> > +static int do_mem_memtest(int argc, char *argv[])
> > +{
> > +	/*
> > +	 * Set start address to 0xffffffff which
> > +	 * can't be.
> > +	 */
> > +	vu_long start = 0xffffffff;
> > +	vu_long end = 0;
> > +
> > +	uint i;
> > +	uint max_i = 1;
> > +
> > +#ifdef CONFIG_MMU
> > +	int cache = 0;
> > +#endif
> > +	int bus_only = 0;
> > +	int err = 0;
> > +	int cnt = 0;
> > +	int opt;
> > +
> > +	struct memory_bank *bank = NULL;
> > +	struct resource *r = NULL;
> > +
> > +	while ((opt = getopt(argc, argv, optstr)) > 0) {
> > +		switch (opt) {
> > +		case 's':
> > +			start = simple_strtoul(optarg, NULL, 0);
> > +			break;
> > +		case 'e':
> > +			end = simple_strtoul(optarg, NULL, 0);
> > +			break;
> > +		case 'i':
> > +			max_i = simple_strtoul(optarg, NULL, 0);
> > +			break;
> > +#ifdef CONFIG_MMU
> > +		case 'c':
> > +			cache = 1;
> > +			break;
> > +#endif
> > +		case 'b':
> > +			bus_only = 1;
> > +			break;
> > +		default:
> > +			return COMMAND_ERROR_USAGE;
> > +		}
> > +	}
> > +
> > +	if (optind > argc)
> > +		return COMMAND_ERROR_USAGE;
> > +
> > +	/*
> > +	 * Error if no end address
> > +	 */
> > +	if (start != 0xffffffff && !end) {
> > +		printf("Please add an end address.\n");
> > +		return 1;
> > +	}
> > +
> > +	/*
> > +	 * Error if no start address
> > +	 */
> > +	if (end && start == 0xffffffff) {
> > +		printf("Please add a start address.\n");
> > +		return 1;
> > +	}
> > +
> > +	/*
> > +	 * Check parameters
> > +	 */
> > +	if (start != 0xffffffff && end) {
> > +		if (end <= start) {
> > +			printf("End address less than or"
> > +					" equal start address.\n");
> > +			return 1;
> > +		}
> > +
> > +		/*
> > +		 * Check if given start and end address are in any banks
> > +		 */
> > +		for_each_memory_bank(bank) {
> > +			if (ADDRESS_IN_REGIONS(start, bank->start,
> > +						bank->start + bank->size))
> > +				cnt++;
> > +
> > +			if (ADDRESS_IN_REGIONS(end, bank->start,
> > +						bank->start + bank->size))
> > +				cnt++;
> > +		}
> > +
> > +		if (cnt != 2) {
> > +			printf("Start or end addresses are"
> > +					" not in any ram bank.\n");
> > +			return 1;
> > +		}
> > +	}
> > +
> > +#ifdef CONFIG_MMU
> > +	/*
> > +	 * Get pte flags. Which are configured at
> > +	 * runtime at booting.
> > +	 */
> > +	pte_flags_cached = mmu_get_pte_cached_flags();
> > +	pte_flags_uncached = mmu_get_pte_uncached_flags();
> > +#endif
> > +
> > +	printf("Skipping regions:\n");
> > +	for_each_memory_bank(bank) {
> > +		list_for_each_entry(r, &bank->res->children, sibling)
> > +			printf("\t0x%08x - "
> > +					"0x%08x (size 0x%08x) %s\n",
> > +					r->start, r->end,
> > +					r->end - r->start + 1, r->name);
> > +#ifdef CONFIG_MMU
> 
> Use if (IS_ENABLED(CONFIG_MMU) and you can get rid of most ifdefs
> 
> Marc
> 
> 
> > +		/*
> > +		 * Disable or enable caching
> > +		 */
> > +		if (cache)
> > +			do_remap_range(bank, pte_flags_cached);
> > +		else
> > +			do_remap_range(bank, pte_flags_uncached);
> > +#endif
> > +	}
> > +
> > +	/*
> > +	 * Do test if we set a start or end address
> > +	 */
> > +	if (start != 0xffffffff && end) {
> > +		printf("Testing address range:\n\t0x%08lx - 0x%08lx"
> > +				" (size 0x%08lx)\n",
> > +				start, end, end - start + 1);
> > +
> > +		for (i = 1; (i <= max_i) || !max_i; i++) {
> > +			printf("Iteration: %u\n", i);
> > +
> > +			/*
> > +			 * Do the Memtest
> > +			 */
> > +			err = mem_test(start, end, bus_only);
> > +			if (err == -EINTR) {
> > +				printf("Test interrupted.\n");
> > +				goto err;
> > +			}
> > +
> > +			if (err < 0) {
> > +				printf("Test failed.\n");
> > +				goto err;
> > +			}
> > +			printf("Tested %u iteration(s) without errors.\n", i);
> > +		}
> > +#ifdef CONFIG_MMU
> > +		/*
> > +		 * Renable caching
> > +		 */
> > +		if (!cache)
> > +			for_each_memory_bank(bank)
> > +				do_remap_range(bank, pte_flags_cached);
> > +#endif
> > +		printf("Memtest done.\n");
> > +
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * If we set no start or end address
> > +	 * we do the test on all ram banks
> > +	 */
> > +	for (i = 1; (i <= max_i) || !max_i; i++) {
> > +		for_each_memory_bank(bank) {
> > +			start = bank->start;
> > +			end = bank->start + bank->size - 1;
> > +
> > +			printf("Iteration: %u\n", i);
> > +
> > +			printf("Testing address range:\n\t0x%08lx - "
> > +					"0x%08lx (size 0x%08lx) on bank /dev/%s\n",
> > +					start, end, bank->size,
> > +					bank->res->name);
> > +
> > +			err = mem_test(start, end, bus_only);
> > +			if (err == -EINTR) {
> > +				printf("Test interrupted.\n");
> > +				goto err;
> > +			}
> > +
> > +			if (err < 0) {
> > +				printf("Test on bank /dev/%s failed.\n",
> > +						bank->res->name);
> > +				goto err;
> > +			}
> > +			printf("Tested %u iteration(s) without errors.\n", i);
> > +		}
> > +	}
> > +#ifdef CONFIG_MMU
> > +	/*
> > +	 * Renable caching
> > +	 */
> > +	if (!cache)
> > +		for_each_memory_bank(bank)
> > +			do_remap_range(bank, pte_flags_cached);
> > +#endif
> > +	printf("Memtest done.\n");
> > +
> > +	return 0;
> > +
> > +err:
> > +#ifdef CONFIG_MMU
> > +	/*
> > +	 * Enable caching
> > +	 */
> > +	for_each_memory_bank(bank)
> > +		do_remap_range(bank, pte_flags_cached);
> > +#endif
> > +
> > +	return 1;
> > +}
> > +
> > +static const __maybe_unused char cmd_memtest_help[] =
> > +"Usage: memtest [OPTION]...\n"
> > +"memtest related commands\n"
> > +"	-s	<start>		start address to begin memtest.\n"
> > +"	-e	<end>		end address to stop memtest.\n"
> > +"	-i	<iterations>	iterations [default=1, endless=0].\n"
> > +#ifdef CONFIG_MMU
> > +"	-c			run test with enable cache.\n"
> > +#endif
> > +"	-b			only test bus datalines.";
> > +
> > +BAREBOX_CMD_START(memtest)
> > +	.cmd		= do_mem_memtest,
> > +	.usage		= "Memory Test",
> > +	BAREBOX_CMD_HELP(cmd_memtest_help)
> > +BAREBOX_CMD_END
> > 
> 
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] common: add mem_test routine

[Alexander Aring]

Hi Sascha,

On Thu, Feb 07, 2013 at 12:00:48PM +0100, Sascha Hauer wrote:
> On Thu, Feb 07, 2013 at 11:44:59AM +0100, Alexander Aring wrote:
> > Useful to detect timing problems if someone porting a new
> > device to barebox.
> > 
> > This test includes a data bus test, address bus test and
> > integrity check of memory.
> > 
> > Allocated barebox regions between start and end will skip
> > automatically.
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > ---
> >  common/Kconfig        |   7 +
> >  common/Makefile       |   1 +
> >  common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/memory_test.h |  13 ++
> >  4 files changed, 420 insertions(+)
> >  create mode 100644 common/memory_test.c
> >  create mode 100644 include/memory_test.h
> > 
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 3f6c11e..c6988df 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -100,6 +100,13 @@ config MEMINFO
> >  	bool "display memory info"
> >  	default y
> >  
> > +config MEMTEST
> > +	bool "Offers routines for memory test"
> > +	help
> > +	  Offers memtest routines in common/memory_test.c
> > +	  This is helpful for porting devices to detect
> > +	  memory timing problems.
> > +
> >  config ENVIRONMENT_VARIABLES
> >  	bool "environment variables support"
> >  
> > diff --git a/common/Makefile b/common/Makefile
> > index 7206eed..684953c 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
> >  obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
> >  obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
> >  obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
> > +obj-$(CONFIG_MEMTEST) += memory_test.o
> >  obj-y += clock.o
> >  obj-$(CONFIG_BANNER) += version.o
> >  obj-$(CONFIG_MEMINFO) += meminfo.o
> > diff --git a/common/memory_test.c b/common/memory_test.c
> > new file mode 100644
> > index 0000000..80b4ff4
> > --- /dev/null
> > +++ b/common/memory_test.c
> > @@ -0,0 +1,399 @@
> > +/*
> > + * memory_test.c
> > + *
> > + * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
> > + *
> > + * 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 version 2
> > + * as published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#include <memory_test.h>
> > +
> > +static const vu_long bitpattern[] = {
> > +	0x00000001,	/* single bit */
> > +	0x00000003,	/* two adjacent bits */
> > +	0x00000007,	/* three adjacent bits */
> > +	0x0000000F,	/* four adjacent bits */
> > +	0x00000005,	/* two non-adjacent bits */
> > +	0x00000015,	/* three non-adjacent bits */
> > +	0x00000055,	/* four non-adjacent bits */
> > +	0xAAAAAAAA,	/* alternating 1/0 */
> > +};
> > +
> > +/*
> > + * Perform a memory test. The complete test
> > + * loops until interrupted by ctrl-c.
> > + *
> > + * Highly recommended to test with disabled and
> > + * enabled cache.
> > + *
> > + * start: start address
> > + * end: end address
> > + * bus_only: skip integrity check
> > + */
> > +int mem_test(vu_long _start, vu_long _end,
> > +		int bus_only)
> > +{
> 
> I thought we agreed on using request_sdram_region here and drop the
> address_in_sdram_regions in this function.
> 

I don't know what you meant :-(
Sorry.

You want that I am checking if a region is already requested.
In that case request_sdram_region will return NULL. Then I know it isn't
a barebox region.
That's ok until integrity check.
This will check 4-bytewise if this is a reqyested region and if not it
will create them.
I can do this, but this will result a long list of requested regions.

A nice solution is to handle it in the function which enable and disable
caching. There I can request a big area of free memory.
I need to request free regions there and call mem_test function for that.

I will try that out.

Regards
Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] common: add mem_test routine

[Sascha Hauer]

On Thu, Feb 07, 2013 at 12:40:32PM +0100, Alexander Aring wrote:
> Hi Sascha,
> 
> On Thu, Feb 07, 2013 at 12:00:48PM +0100, Sascha Hauer wrote:
> > On Thu, Feb 07, 2013 at 11:44:59AM +0100, Alexander Aring wrote:
> > > Useful to detect timing problems if someone porting a new
> > > device to barebox.
> > > 
> > > This test includes a data bus test, address bus test and
> > > integrity check of memory.
> > > 
> > > Allocated barebox regions between start and end will skip
> > > automatically.
> > > 
> > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > > ---
> > >  common/Kconfig        |   7 +
> > >  common/Makefile       |   1 +
> > >  common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/memory_test.h |  13 ++
> > >  4 files changed, 420 insertions(+)
> > >  create mode 100644 common/memory_test.c
> > >  create mode 100644 include/memory_test.h
> > > 
> > > diff --git a/common/Kconfig b/common/Kconfig
> > > index 3f6c11e..c6988df 100644
> > > --- a/common/Kconfig
> > > +++ b/common/Kconfig
> > > @@ -100,6 +100,13 @@ config MEMINFO
> > >  	bool "display memory info"
> > >  	default y
> > >  
> > > +config MEMTEST
> > > +	bool "Offers routines for memory test"
> > > +	help
> > > +	  Offers memtest routines in common/memory_test.c
> > > +	  This is helpful for porting devices to detect
> > > +	  memory timing problems.
> > > +
> > >  config ENVIRONMENT_VARIABLES
> > >  	bool "environment variables support"
> > >  
> > > diff --git a/common/Makefile b/common/Makefile
> > > index 7206eed..684953c 100644
> > > --- a/common/Makefile
> > > +++ b/common/Makefile
> > > @@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
> > >  obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
> > >  obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
> > >  obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
> > > +obj-$(CONFIG_MEMTEST) += memory_test.o
> > >  obj-y += clock.o
> > >  obj-$(CONFIG_BANNER) += version.o
> > >  obj-$(CONFIG_MEMINFO) += meminfo.o
> > > diff --git a/common/memory_test.c b/common/memory_test.c
> > > new file mode 100644
> > > index 0000000..80b4ff4
> > > --- /dev/null
> > > +++ b/common/memory_test.c
> > > @@ -0,0 +1,399 @@
> > > +/*
> > > + * memory_test.c
> > > + *
> > > + * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
> > > + *
> > > + * 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 version 2
> > > + * as published by the Free Software Foundation.
> > > + *
> > > + * 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.
> > > + *
> > > + */
> > > +
> > > +#include <memory_test.h>
> > > +
> > > +static const vu_long bitpattern[] = {
> > > +	0x00000001,	/* single bit */
> > > +	0x00000003,	/* two adjacent bits */
> > > +	0x00000007,	/* three adjacent bits */
> > > +	0x0000000F,	/* four adjacent bits */
> > > +	0x00000005,	/* two non-adjacent bits */
> > > +	0x00000015,	/* three non-adjacent bits */
> > > +	0x00000055,	/* four non-adjacent bits */
> > > +	0xAAAAAAAA,	/* alternating 1/0 */
> > > +};
> > > +
> > > +/*
> > > + * Perform a memory test. The complete test
> > > + * loops until interrupted by ctrl-c.
> > > + *
> > > + * Highly recommended to test with disabled and
> > > + * enabled cache.
> > > + *
> > > + * start: start address
> > > + * end: end address
> > > + * bus_only: skip integrity check
> > > + */
> > > +int mem_test(vu_long _start, vu_long _end,
> > > +		int bus_only)
> > > +{
> > 
> > I thought we agreed on using request_sdram_region here and drop the
> > address_in_sdram_regions in this function.
> > 
> 
> I don't know what you meant :-(
> Sorry.
> 
> You want that I am checking if a region is already requested.

I didn't want that you check if a region is already requested, I want
you to request the region that you are testing. That's what
request_sdram_region is good for. Just use it.

> In that case request_sdram_region will return NULL. Then I know it isn't
> a barebox region.
> That's ok until integrity check.
> This will check 4-bytewise if this is a reqyested region and if not it
> will create them.
> I can do this, but this will result a long list of requested regions.

Drop the address_in_sdram_regions completely from this function.
Instead, do a request_sdram_region in the memtest command.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] commands: add new memtest command

[Sascha Hauer]

On Thu, Feb 07, 2013 at 12:20:06PM +0100, Alexander Aring wrote:
> On Thu, Feb 07, 2013 at 11:56:07AM +0100, Marc Kleine-Budde wrote:
> > On 02/07/2013 11:45 AM, Alexander Aring wrote:
> > > Add new memtest command which can enable or disable caching
> > > on non allocted barebox regions(test area).
> > > 
> > > This command simply parse and check parameters then call
> > > the mem_test routine.
> > > 
> > > If no address parameters are given then mem_test will call
> > > for each memory bank.
> > > 
> > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > 
> > A howto-get-rid-of-ifdef nitpick inline
> > 
> > > ---
> > >  commands/Kconfig   |  10 ++
> > >  commands/Makefile  |   1 +
> > >  commands/memtest.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 373 insertions(+)
> > >  create mode 100644 commands/memtest.c
> > > 
> > > diff --git a/commands/Kconfig b/commands/Kconfig
> > > index 7cc759c..d158c3f 100644
> > > --- a/commands/Kconfig
> > > +++ b/commands/Kconfig
> > > @@ -516,6 +516,16 @@ config CMD_NANDTEST
> > >  	select PARTITION_NEED_MTD
> > >  	prompt "nandtest"
> > >  
> > > +config CMD_MEMTEST
> > > +    tristate
> > > +    select MEMTEST
> > > +    prompt "memtest"
> > > +	help
> > > +	  This command enables a memtest to test installed memory.
> > > +	  During this test allocated iomem regions will be skipped.
> > > +	  If tested architecture has MMU with PTE flags support,
> > > +	  caching can be set enabled or disabled.
> > > +
> > >  endmenu
> > >  
> > >  menu "video command"
> > > diff --git a/commands/Makefile b/commands/Makefile
> > > index 393ba51..b39b489 100644
> > > --- a/commands/Makefile
> > > +++ b/commands/Makefile
> > > @@ -7,6 +7,7 @@ obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
> > >  obj-$(CONFIG_CMD_LOADS)		+= loads.o
> > >  obj-$(CONFIG_CMD_ECHO)		+= echo.o
> > >  obj-$(CONFIG_CMD_MEMORY)	+= mem.o
> > > +obj-$(CONFIG_CMD_MEMTEST)	+= memtest.o
> > >  obj-$(CONFIG_CMD_EDIT)		+= edit.o
> > >  obj-$(CONFIG_CMD_EXEC)		+= exec.o
> > >  obj-$(CONFIG_CMD_SLEEP)		+= sleep.o
> > > diff --git a/commands/memtest.c b/commands/memtest.c
> > > new file mode 100644
> > > index 0000000..22e8006
> > > --- /dev/null
> > > +++ b/commands/memtest.c
> > > @@ -0,0 +1,362 @@
> > > +/*
> > > + * memtest - Perform a memory test
> > > + *
> > > + * (C) Copyright 2013
> > > + * Alexander Aring <aar@pengutronix.de>, Pengutronix
> > > + *
> > > + * (C) Copyright 2000
> > > + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> > > + *
> > > + * 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 <command.h>
> > > +#include <getopt.h>
> > > +#include <asm/mmu.h>
> > > +
> > > +#include <memory_test.h>
> > > +
> > > +/*
> > > + * In CONFIG_MMU we have a special c flag.
> > > + */
> > > +#ifdef CONFIG_MMU
> > > +static char optstr[] = "s:e:i:cb";
> > 
> > const?
> >
> 
> This will print a compiler warning because the getopt implementation
> doesn't accept a const char* because getopt will change this string.

getopt does not modify this string. The prototype of getopt could be changed to
take a const char *.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] common: add mem_test routine

[Alexander Aring]

Hi Sascha,

On Thu, Feb 07, 2013 at 12:54:46PM +0100, Sascha Hauer wrote:
> On Thu, Feb 07, 2013 at 12:40:32PM +0100, Alexander Aring wrote:
> > Hi Sascha,
> > 
> > On Thu, Feb 07, 2013 at 12:00:48PM +0100, Sascha Hauer wrote:
> > > On Thu, Feb 07, 2013 at 11:44:59AM +0100, Alexander Aring wrote:
> > > > Useful to detect timing problems if someone porting a new
> > > > device to barebox.
> > > > 
> > > > This test includes a data bus test, address bus test and
> > > > integrity check of memory.
> > > > 
> > > > Allocated barebox regions between start and end will skip
> > > > automatically.
> > > > 
> > > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > > > ---
> > > >  common/Kconfig        |   7 +
> > > >  common/Makefile       |   1 +
> > > >  common/memory_test.c  | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  include/memory_test.h |  13 ++
> > > >  4 files changed, 420 insertions(+)
> > > >  create mode 100644 common/memory_test.c
> > > >  create mode 100644 include/memory_test.h
> > > > 
> > > > diff --git a/common/Kconfig b/common/Kconfig
> > > > index 3f6c11e..c6988df 100644
> > > > --- a/common/Kconfig
> > > > +++ b/common/Kconfig
> > > > @@ -100,6 +100,13 @@ config MEMINFO
> > > >  	bool "display memory info"
> > > >  	default y
> > > >  
> > > > +config MEMTEST
> > > > +	bool "Offers routines for memory test"
> > > > +	help
> > > > +	  Offers memtest routines in common/memory_test.c
> > > > +	  This is helpful for porting devices to detect
> > > > +	  memory timing problems.
> > > > +
> > > >  config ENVIRONMENT_VARIABLES
> > > >  	bool "environment variables support"
> > > >  
> > > > diff --git a/common/Makefile b/common/Makefile
> > > > index 7206eed..684953c 100644
> > > > --- a/common/Makefile
> > > > +++ b/common/Makefile
> > > > @@ -17,6 +17,7 @@ obj-$(CONFIG_MALLOC_DLMALLOC) += dlmalloc.o
> > > >  obj-$(CONFIG_MALLOC_TLSF) += tlsf_malloc.o
> > > >  obj-$(CONFIG_MALLOC_TLSF) += tlsf.o
> > > >  obj-$(CONFIG_MALLOC_DUMMY) += dummy_malloc.o
> > > > +obj-$(CONFIG_MEMTEST) += memory_test.o
> > > >  obj-y += clock.o
> > > >  obj-$(CONFIG_BANNER) += version.o
> > > >  obj-$(CONFIG_MEMINFO) += meminfo.o
> > > > diff --git a/common/memory_test.c b/common/memory_test.c
> > > > new file mode 100644
> > > > index 0000000..80b4ff4
> > > > --- /dev/null
> > > > +++ b/common/memory_test.c
> > > > @@ -0,0 +1,399 @@
> > > > +/*
> > > > + * memory_test.c
> > > > + *
> > > > + * Copyright (c) 2013 Alexander Aring <aar@pengutronix.de>, Pengutronix
> > > > + *
> > > > + * 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 version 2
> > > > + * as published by the Free Software Foundation.
> > > > + *
> > > > + * 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.
> > > > + *
> > > > + */
> > > > +
> > > > +#include <memory_test.h>
> > > > +
> > > > +static const vu_long bitpattern[] = {
> > > > +	0x00000001,	/* single bit */
> > > > +	0x00000003,	/* two adjacent bits */
> > > > +	0x00000007,	/* three adjacent bits */
> > > > +	0x0000000F,	/* four adjacent bits */
> > > > +	0x00000005,	/* two non-adjacent bits */
> > > > +	0x00000015,	/* three non-adjacent bits */
> > > > +	0x00000055,	/* four non-adjacent bits */
> > > > +	0xAAAAAAAA,	/* alternating 1/0 */
> > > > +};
> > > > +
> > > > +/*
> > > > + * Perform a memory test. The complete test
> > > > + * loops until interrupted by ctrl-c.
> > > > + *
> > > > + * Highly recommended to test with disabled and
> > > > + * enabled cache.
> > > > + *
> > > > + * start: start address
> > > > + * end: end address
> > > > + * bus_only: skip integrity check
> > > > + */
> > > > +int mem_test(vu_long _start, vu_long _end,
> > > > +		int bus_only)
> > > > +{
> > > 
> > > I thought we agreed on using request_sdram_region here and drop the
> > > address_in_sdram_regions in this function.
> > > 
> > 
> > I don't know what you meant :-(
> > Sorry.
> > 
> > You want that I am checking if a region is already requested.
> 
> I didn't want that you check if a region is already requested, I want
> you to request the region that you are testing. That's what
> request_sdram_region is good for. Just use it.
> 
> > In that case request_sdram_region will return NULL. Then I know it isn't
> > a barebox region.
> > That's ok until integrity check.
> > This will check 4-bytewise if this is a reqyested region and if not it
> > will create them.
> > I can do this, but this will result a long list of requested regions.
> 
> Drop the address_in_sdram_regions completely from this function.
> Instead, do a request_sdram_region in the memtest command.
>

I will try to do that. Apologize for the inconvenience. It takes long
time to write a new memtest command... but good software will take a
long time.

Regards
Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] commands: add new memtest command

[Alexander Aring]

Hi,

On Thu, Feb 07, 2013 at 01:01:41PM +0100, Sascha Hauer wrote:
> On Thu, Feb 07, 2013 at 12:20:06PM +0100, Alexander Aring wrote:
> > On Thu, Feb 07, 2013 at 11:56:07AM +0100, Marc Kleine-Budde wrote:
> > > On 02/07/2013 11:45 AM, Alexander Aring wrote:
> > > > Add new memtest command which can enable or disable caching
> > > > on non allocted barebox regions(test area).
> > > > 
> > > > This command simply parse and check parameters then call
> > > > the mem_test routine.
> > > > 
> > > > If no address parameters are given then mem_test will call
> > > > for each memory bank.
> > > > 
> > > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > > 
> > > A howto-get-rid-of-ifdef nitpick inline
> > > 
> > > > ---
> > > >  commands/Kconfig   |  10 ++
> > > >  commands/Makefile  |   1 +
> > > >  commands/memtest.c | 362 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 373 insertions(+)
> > > >  create mode 100644 commands/memtest.c
> > > > 
> > > > diff --git a/commands/Kconfig b/commands/Kconfig
> > > > index 7cc759c..d158c3f 100644
> > > > --- a/commands/Kconfig
> > > > +++ b/commands/Kconfig
> > > > @@ -516,6 +516,16 @@ config CMD_NANDTEST
> > > >  	select PARTITION_NEED_MTD
> > > >  	prompt "nandtest"
> > > >  
> > > > +config CMD_MEMTEST
> > > > +    tristate
> > > > +    select MEMTEST
> > > > +    prompt "memtest"
> > > > +	help
> > > > +	  This command enables a memtest to test installed memory.
> > > > +	  During this test allocated iomem regions will be skipped.
> > > > +	  If tested architecture has MMU with PTE flags support,
> > > > +	  caching can be set enabled or disabled.
> > > > +
> > > >  endmenu
> > > >  
> > > >  menu "video command"
> > > > diff --git a/commands/Makefile b/commands/Makefile
> > > > index 393ba51..b39b489 100644
> > > > --- a/commands/Makefile
> > > > +++ b/commands/Makefile
> > > > @@ -7,6 +7,7 @@ obj-$(CONFIG_CMD_LOADY)		+= loadxy.o
> > > >  obj-$(CONFIG_CMD_LOADS)		+= loads.o
> > > >  obj-$(CONFIG_CMD_ECHO)		+= echo.o
> > > >  obj-$(CONFIG_CMD_MEMORY)	+= mem.o
> > > > +obj-$(CONFIG_CMD_MEMTEST)	+= memtest.o
> > > >  obj-$(CONFIG_CMD_EDIT)		+= edit.o
> > > >  obj-$(CONFIG_CMD_EXEC)		+= exec.o
> > > >  obj-$(CONFIG_CMD_SLEEP)		+= sleep.o
> > > > diff --git a/commands/memtest.c b/commands/memtest.c
> > > > new file mode 100644
> > > > index 0000000..22e8006
> > > > --- /dev/null
> > > > +++ b/commands/memtest.c
> > > > @@ -0,0 +1,362 @@
> > > > +/*
> > > > + * memtest - Perform a memory test
> > > > + *
> > > > + * (C) Copyright 2013
> > > > + * Alexander Aring <aar@pengutronix.de>, Pengutronix
> > > > + *
> > > > + * (C) Copyright 2000
> > > > + * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> > > > + *
> > > > + * 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 <command.h>
> > > > +#include <getopt.h>
> > > > +#include <asm/mmu.h>
> > > > +
> > > > +#include <memory_test.h>
> > > > +
> > > > +/*
> > > > + * In CONFIG_MMU we have a special c flag.
> > > > + */
> > > > +#ifdef CONFIG_MMU
> > > > +static char optstr[] = "s:e:i:cb";
> > > 
> > > const?
> > >
> > 
> > This will print a compiler warning because the getopt implementation
> > doesn't accept a const char* because getopt will change this string.
> 
> getopt does not modify this string. The prototype of getopt could be changed to
> take a const char *.
>

Ok I will fix this.

Regards
Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox