[PATCH 01/10] common/memtest.c: Fix incorrect array boundary check

[PATCH 01/10] common/memtest.c: Fix incorrect array boundary check

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/memtest.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/common/memtest.c b/common/memtest.c
index 25a97d8..3b0bcb7 100644
--- a/common/memtest.c
+++ b/common/memtest.c
@@ -91,8 +91,7 @@ int mem_test(resource_size_t _start,
 	 * '0's and '0' bits through a field of '1's (i.e.
 	 * pattern and ~pattern).
 	 */
-	for (i = 0; i < ARRAY_SIZE(bitpattern)/
-			sizeof(resource_size_t); i++) {
+	for (i = 0; i < ARRAY_SIZE(bitpattern); i++) {
 		val = bitpattern[i];

 		for (; val != 0; val <<= 1) {
--
2.1.4

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

[PATCH 02/10] common/memtest.c: Do not omit offset of 0 from tests

[Andrey Smirnov]

Ommiting offset 0 from address line tests allows certain corner cases
of faults to be undetected.

For the "stuck high" case, consider scenario in which all of the
tested address lines are stuck high. In original code first data
filling loop would execute writing data to a single cell multiple
times and second loop would just read data from that cell over and
over again. Adding a write to start[0] should prevent this since it
would cause the second loop to read incorrect data on its first
iteration.

For the "stuck low" case, having any of the tested bits of the address
shorted would effectively "remap" that memory cell to start[0] in this
case excluding start[0] during the verification phase would result in
a false positive result.

Note that both of the changes are present in Michael Barr's code here:
http://www.esacademy.com/en/library/technical-articles-and-documents/miscellaneous/software-based-memory-testing.html

and the code in barebox is based on that code.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/memtest.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/common/memtest.c b/common/memtest.c
index 3b0bcb7..9eda788 100644
--- a/common/memtest.c
+++ b/common/memtest.c
@@ -170,6 +170,15 @@ int mem_test(resource_size_t _start,
 	for (offset = 1; offset <= num_words; offset <<= 1)
 		start[offset] = pattern;

+	/*
+	 * Now write anti-pattern at offset 0. If during the previous
+	 * step one of the address lines got stuck high this
+	 * operation would result in a memory cell at power-of-two
+	 * offset being set to anti-pattern which hopefully would be
+	 * detected byt the loop that follows.
+	 */
+	start[0] = anti_pattern;
+
 	printf("Check for address bits stuck high.\n");

 	/*
@@ -187,6 +196,11 @@ int mem_test(resource_size_t _start,
 		}
 	}

+	/*
+	  Restore original value
+	 */
+	start[0] = pattern;
+
 	printf("Check for address bits stuck "
 			"low or shorted.\n");

@@ -196,7 +210,8 @@ int mem_test(resource_size_t _start,
 	for (offset2 = 1; offset2 <= num_words; offset2 <<= 1) {
 		start[offset2] = anti_pattern;

-		for (offset = 1; offset <= num_words; offset <<= 1) {
+		for (offset = 0; offset <= num_words;
+		     offset = (offset) ? offset << 1 : 1) {
 			temp = start[offset];

 			if ((temp != pattern) &&
--
2.1.4

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

[PATCH 03/10] common/memtest.c: Refactor mem_test() into three surbroutines

[Andrey Smirnov]

Original mem_test() was rather long an contained code to perform two
distinct operations. This patch moves that code into two separate
subroutines and converts mem_test into a high level interface that
calls the subroutines.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/memtest.c | 105 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 68 insertions(+), 37 deletions(-)

diff --git a/common/memtest.c b/common/memtest.c
index 9eda788..57e2ad9 100644
--- a/common/memtest.c
+++ b/common/memtest.c
@@ -28,35 +28,22 @@
 #include <errno.h>
 #include <memtest.h>

-static const resource_size_t 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.
- *
- * Prameters:
- * start: start address for memory test.
- * end: end address of memory test.
- * bus_only: skip integrity check and do only a address/data bus
- *	     testing.
- *
- * Return value can be -EINVAL for invalid parameter or -EINTR
- * if memory test was interrupted.
- */
-int mem_test(resource_size_t _start,
-	       resource_size_t _end, int bus_only)
+int mem_test_bus_integrity(resource_size_t _start,
+			   resource_size_t _end)
 {
-	volatile resource_size_t *start, *dummy, val, readback, offset,
-		offset2, pattern, temp, anti_pattern, num_words;
+	static const resource_size_t 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 */
+	};
+
+	volatile resource_size_t *start, *dummy, num_words, val, readback, offset,
+		offset2, pattern, temp, anti_pattern;
 	int i;

 	_start = ALIGN(_start, sizeof(resource_size_t));
@@ -66,7 +53,7 @@ int mem_test(resource_size_t _start,
 		return -EINVAL;

 	start = (resource_size_t *)_start;
-	/*
+		/*
 	 * Point the dummy to start[1]
 	 */
 	dummy = start + 1;
@@ -227,15 +214,25 @@ int mem_test(resource_size_t _start,
 		start[offset2] = pattern;
 	}

-	/*
-	 * We tested only the bus if != 0
-	 * leaving here
-	 */
-	if (bus_only)
-		return 0;
+	return 0;
+}
+
+int mem_test_dram(resource_size_t _start,
+		  resource_size_t _end)
+{
+	volatile resource_size_t *start, num_words, offset, temp, anti_pattern;
+
+	_start = ALIGN(_start, sizeof(resource_size_t));
+	_end = ALIGN_DOWN(_end, sizeof(resource_size_t)) - 1;
+
+	if (_end <= _start)
+		return -EINVAL;
+
+	start = (resource_size_t *)_start;
+	num_words = (_end - _start + 1)/sizeof(resource_size_t);

 	printf("Starting integrity check of physicaly ram.\n"
-			"Filling ram with patterns...\n");
+	       "Filling ram with patterns...\n");

 	/*
 	 * Description: Test the integrity of a physical
@@ -252,16 +249,17 @@ int mem_test(resource_size_t _start,
 	 * Fill memory with a known pattern.
 	 */
 	init_progression_bar(num_words);
+
 	for (offset = 0; offset < num_words; offset++) {
 		/*
 		 * Every 4K we update the progressbar.
 		 */
+
 		if (!(offset & (SZ_4K - 1))) {
 			if (ctrlc())
 				return -EINTR;
 			show_progress(offset);
 		}
-
 		start[offset] = offset + 1;
 	}
 	show_progress(offset);
@@ -326,3 +324,36 @@ int mem_test(resource_size_t _start,

 	return 0;
 }
+
+/*
+ * Perform a memory test. The complete test
+ * loops until interrupted by ctrl-c.
+ *
+ * Prameters:
+ * start: start address for memory test.
+ * end: end address of memory test.
+ * bus_only: skip integrity check and do only a address/data bus
+ *	     testing.
+ *
+ * Return value can be -EINVAL for invalid parameter or -EINTR
+ * if memory test was interrupted.
+ */
+int mem_test(resource_size_t _start,
+	       resource_size_t _end, int bus_only)
+{
+	int ret;
+
+	ret = mem_test_bus_integrity(_start, _end);
+
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * We tested only the bus if != 0
+	 * leaving here
+	 */
+	if (!bus_only)
+		ret = mem_test_dram(_start, _end);
+
+	return ret;
+}
--
2.1.4

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

[PATCH 04/10] common/memtest.c: Distil common error reporting code

[Andrey Smirnov]

Move all of the common code for error message output into a new
function mem_test_report_failure() and convert the rest of the code to
use it.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/memtest.c | 53 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/common/memtest.c b/common/memtest.c
index 57e2ad9..d8d1154 100644
--- a/common/memtest.c
+++ b/common/memtest.c
@@ -28,6 +28,17 @@
 #include <errno.h>
 #include <memtest.h>

+static void mem_test_report_failure(const char *failure_description,
+				    resource_size_t expected_value,
+				    resource_size_t actual_value,
+				    volatile resource_size_t *address)
+{
+	printf("FAILURE (%s): "
+	       "expected 0x%08x, actual 0x%08x at address 0x%08x.\n",
+	       failure_description, expected_value, actual_value,
+	       (resource_size_t)address);
+}
+
 int mem_test_bus_integrity(resource_size_t _start,
 			   resource_size_t _end)
 {
@@ -87,9 +98,8 @@ int mem_test_bus_integrity(resource_size_t _start,
 			*dummy = ~val;
 			readback = *start;
 			if (readback != val) {
-				printf("FAILURE (data line): "
-					"expected 0x%08x, actual 0x%08x at address 0x%08x.\n",
-					val, readback, (resource_size_t)start);
+				mem_test_report_failure("data line",
+							val, readback, start);
 				return -EIO;
 			}

@@ -97,10 +107,8 @@ int mem_test_bus_integrity(resource_size_t _start,
 			*dummy = val;
 			readback = *start;
 			if (readback != ~val) {
-				printf("FAILURE (data line): "
-					"Is 0x%08x, should be 0x%08x at address 0x%08x.\n",
-					readback,
-					~val, (resource_size_t)start);
+				mem_test_report_failure("data line",
+							~val, readback, start);
 				return -EIO;
 			}
 		}
@@ -174,11 +182,8 @@ int mem_test_bus_integrity(resource_size_t _start,
 	for (offset = 1; offset <= num_words; offset <<= 1) {
 		temp = start[offset];
 		if (temp != pattern) {
-			printf("FAILURE: Address bit "
-					"stuck high @ 0x%08x:"
-					" expected 0x%08x, actual 0x%08x.\n",
-					(resource_size_t)&start[offset],
-					pattern, temp);
+			mem_test_report_failure("address bit stuck high",
+						pattern, temp, &start[offset]);
 			return -EIO;
 		}
 	}
@@ -203,11 +208,9 @@ int mem_test_bus_integrity(resource_size_t _start,

 			if ((temp != pattern) &&
 					(offset != offset2)) {
-				printf("FAILURE: Address bit stuck"
-						" low or shorted @"
-						" 0x%08x: expected 0x%08x, actual 0x%08x.\n",
-						(resource_size_t)&start[offset],
-						pattern, temp);
+				mem_test_report_failure(
+					"address bit stuck low or shorted",
+					pattern, temp, &start[offset]);
 				return -EIO;
 			}
 		}
@@ -278,10 +281,10 @@ int mem_test_dram(resource_size_t _start,

 		temp = start[offset];
 		if (temp != (offset + 1)) {
-			printf("\nFAILURE (read/write) @ 0x%08x:"
-					" expected 0x%08x, actual 0x%08x.\n",
-					(resource_size_t)&start[offset],
-					(offset + 1), temp);
+			printf("\n");
+			mem_test_report_failure("read/write",
+						(offset + 1),
+						temp, &start[offset]);
 			return -EIO;
 		}

@@ -306,10 +309,10 @@ int mem_test_dram(resource_size_t _start,
 		temp = start[offset];

 		if (temp != anti_pattern) {
-			printf("\nFAILURE (read/write): @ 0x%08x:"
-					" expected 0x%08x, actual 0x%08x.\n",
-					(resource_size_t)&start[offset],
-					anti_pattern, temp);
+			printf("\n");
+			mem_test_report_failure("read/write",
+						anti_pattern,
+						temp, &start[offset]);
 			return -EIO;
 		}

--
2.1.4

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

[PATCH 05/10] serial: i.MX: Write settings to a correct register

[Andrey Smirnov]

Fix what looks like a copy and past error, where settings for USR1
register were being written to USR2.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/serial/serial_imx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_imx.c b/drivers/serial/serial_imx.c
index 12155ad..b7ea14a 100644
--- a/drivers/serial/serial_imx.c
+++ b/drivers/serial/serial_imx.c
@@ -113,10 +113,10 @@ static int imx_serial_init_port(struct console_device *cdev)
 	writel(val, regs + USR2);

   	/* Clear status flags */
-	val = readl(regs + USR2);
+	val = readl(regs + USR1);
 	val |= USR1_PARITYERR | USR1_RTSD | USR1_ESCF | USR1_FRAMERR | USR1_AIRINT |
 	       USR1_AWAKE;
-	writel(val, regs + USR2);
+	writel(val, regs + USR1);

 	return 0;
 }
--
2.1.4

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

[PATCH 06/10] common: pbl: Allow boards to override hang()

[Andrey Smirnov]

Add provisions such that board code can re-define the behaviour of
hang() to implement a behaviour better suited for a particular
hardware platform.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/startup.c | 15 ++++++++++++++-
 include/common.h | 22 ++++++++++++++++++++++
 pbl/misc.c       | 19 ++++++++++++++++++-
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/common/startup.c b/common/startup.c
index e59b06d..d701fab 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -118,12 +118,25 @@ void __noreturn start_barebox(void)
 	/* NOTREACHED - no way out of command loop except booting */
 }

-void __noreturn hang (void)
+void __noreturn __hang (void)
 {
 	puts ("### ERROR ### Please RESET the board ###\n");
 	for (;;);
 }

+static hang_handler_t hang_handler = __hang;
+
+void __noreturn hang(void)
+{
+	hang_handler();
+}
+
+void set_hang_handler(hang_handler_t handler)
+{
+	hang_handler = handler;
+}
+
+
 void (*board_shutdown)(void);

 /* Everything needed to cleanly shutdown barebox.
diff --git a/include/common.h b/include/common.h
index eef371c..86e755d 100644
--- a/include/common.h
+++ b/include/common.h
@@ -56,7 +56,11 @@
  */
 void reginfo(void);

+typedef void __noreturn (*hang_handler_t)(void);
+
 void __noreturn hang (void);
+void __noreturn __hang (void);
+void set_hang_handler(hang_handler_t handler);

 char *size_human_readable(unsigned long long size);

@@ -193,6 +197,24 @@ void barebox_set_hostname(const char *);
 #define IOMEM(addr)	((void __force __iomem *)(addr))
 #endif

+#if defined(CONFIG_ARM)
+#include <asm/barebox-arm.h>
+
+static inline void *get_true_address(const void *ptr)
+{
+	resource_size_t address = (resource_size_t)ptr;
+
+	address -= get_runtime_offset();
+
+	return (void *)address;
+}
+#else
+static inline void *get_true_address(const void *ptr)
+{
+	return (void *)ptr;
+}
+#endif
+
 /*
  * Check if two regions overlap. returns true if they do, false otherwise
  */
diff --git a/pbl/misc.c b/pbl/misc.c
index 7e76120..3d5a881 100644
--- a/pbl/misc.c
+++ b/pbl/misc.c
@@ -4,11 +4,28 @@
 #include <linux/string.h>
 #include <linux/ctype.h>

-void __noreturn hang(void)
+void __noreturn __hang(void)
 {
 	while (1);
 }

+static hang_handler_t hang_handler = __hang;
+
+void __noreturn hang(void)
+{
+	hang_handler_t *__hand_handler_p = get_true_address(&hang_handler);
+	hang_handler_t __hang_handler    = get_true_address(*__hand_handler_p);
+
+	__hang_handler();
+}
+
+void set_hang_handler(hang_handler_t handler)
+{
+	hang_handler_t *__hang_handler_p = get_true_address(&hang_handler);
+
+	*__hang_handler_p = handler;
+}
+
 void __noreturn panic(const char *fmt, ...)
 {
 	while(1);
--
2.1.4

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

[PATCH 07/10] debug_ll: i.MX: Add support for input to DEBUG_LL

[Andrey Smirnov]

Add input support to DEBUG_LL infrastructure and implement it for i.MX

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/Kconfig                          |  1 +
 arch/arm/mach-imx/include/mach/debug_ll.h | 26 ++++++++++++++++++++++++++
 common/Kconfig                            | 11 +++++++++++
 include/debug_ll.h                        | 30 ++++++++++++++++++++++++++++++
 4 files changed, 68 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 50f3095..47d792b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -103,6 +103,7 @@ config ARCH_IMX
 	select CLKDEV_LOOKUP
 	select WATCHDOG_IMX_RESET_SOURCE
 	select HAS_DEBUG_LL
+	select HAS_DEBUG_LL_INPUT

 config ARCH_MVEBU
 	bool "Marvell EBU platforms"
diff --git a/arch/arm/mach-imx/include/mach/debug_ll.h b/arch/arm/mach-imx/include/mach/debug_ll.h
index 8eb59f6..3f5a102 100644
--- a/arch/arm/mach-imx/include/mach/debug_ll.h
+++ b/arch/arm/mach-imx/include/mach/debug_ll.h
@@ -114,6 +114,32 @@ static inline void PUTC_LL(int c)

 	writel(c, base + URTX0);
 }
+
+#ifdef CONFIG_DEBUG_LL_INPUT
+static inline bool TSTC_LL(void)
+{
+	void __iomem *base = IOMEM(IMX_UART_BASE(IMX_DEBUG_SOC,
+						 CONFIG_DEBUG_IMX_UART_PORT));
+
+	return readl(base + USR2) & USR2_RDR;
+}
+
+static inline int GETC_LL(void)
+{
+	void __iomem *base = IOMEM(IMX_UART_BASE(IMX_DEBUG_SOC,
+						 CONFIG_DEBUG_IMX_UART_PORT));
+
+	if (!base)
+		return -EINVAL;
+
+	if (!(readl(base + UCR1) & UCR1_UARTEN))
+		return -EINVAL;
+
+	while (!TSTC_LL());
+
+	return 0xff & readl(base + URXD0);
+}
+#endif
 #else

 static inline void imx_uart_setup_ll(void __iomem *uartbase,
diff --git a/common/Kconfig b/common/Kconfig
index 1c5d14c..4032dcd 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -774,6 +774,15 @@ config DEBUG_LL
 	  This requires SoC specific support. Most SoCs require the debug UART to be
 	  initialized by a debugger or first stage bootloader.

+config DEBUG_LL_INPUT
+        bool
+	depends on DEBUG_LL
+	depends on HAS_DEBUG_LL_INPUT
+	prompt "enable data reception on low level debug port"
+	help
+	  Enable this option if you want to be able to receive data using low
+	  level debug port as well as sending data over it.
+
 choice
 	prompt "Kernel low-level debugging port"
 	depends on DEBUG_LL
@@ -930,3 +939,5 @@ endmenu

 config HAS_DEBUG_LL
 	bool
+config HAS_DEBUG_LL_INPUT
+        bool
diff --git a/include/debug_ll.h b/include/debug_ll.h
index b0eb7cd..4272830 100644
--- a/include/debug_ll.h
+++ b/include/debug_ll.h
@@ -38,6 +38,27 @@ static inline void putc_ll(char value)
 	PUTC_LL(value);
 }

+#if defined (CONFIG_DEBUG_LL_INPUT)
+static inline int tstc_ll(void)
+{
+	return TSTC_LL();
+}
+
+static inline int getc_ll(void)
+{
+	return GETC_LL();
+}
+#else
+static inline int tstc_ll(void)
+{
+	return 0;
+}
+static inline int getc_ll(void)
+{
+	return -ENOTSUPP;
+}
+#endif
+
 static inline void puthex_ll(unsigned long value)
 {
 	int i; unsigned char ch;
@@ -71,6 +92,15 @@ static inline void putc_ll(char value)
 {
 }

+static inline int tstc_ll(void)
+{
+	return 0;
+}
+static inline int getc_ll(void)
+{
+	return -ENOTSUPP;
+}
+
 static inline void puthex_ll(unsigned long value)
 {
 }
--
2.1.4

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

[PATCH 08/10] i.MX51: babbage: Add UART RXD pin configuration

[Andrey Smirnov]

Add UART RXD pin pinmux configuration to support input in DEBUG_LL
functions.

Also use appropriate pad configuration constant in TXD pin
configuraion.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boards/freescale-mx51-babbage/lowlevel.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boards/freescale-mx51-babbage/lowlevel.c b/arch/arm/boards/freescale-mx51-babbage/lowlevel.c
index 57a324e..09a7a67 100644
--- a/arch/arm/boards/freescale-mx51-babbage/lowlevel.c
+++ b/arch/arm/boards/freescale-mx51-babbage/lowlevel.c
@@ -1,5 +1,6 @@
 #include <debug_ll.h>
 #include <mach/clock-imx51_53.h>
+#include <mach/iomux-mx51.h>
 #include <common.h>
 #include <mach/esdctl.h>
 #include <mach/generic.h>
@@ -27,11 +28,14 @@ static inline void setup_uart(void)
 	 * The code below should be more or less a "moral equivalent"
 	 * of:
 	 *	 MX51_PAD_UART1_TXD__UART1_TXD 0x1c5
-	 *
+	 *       MX51_PAD_UART1_TXD__UART1_RXD 0x1c5
 	 * in device tree
 	 */
 	writel(0x00000000, iomuxbase + 0x022c);
-	writel(0x000001c5, iomuxbase + 0x061c);
+	writel(MX51_UART_PAD_CTRL, iomuxbase + 0x061c);
+	writel(0, iomuxbase + 0x0228);
+	writel(0, iomuxbase + 0x09e4);
+	writel(MX51_UART_PAD_CTRL, iomuxbase + 0x0618);

 	imx51_uart_setup_ll();

--
2.1.4

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

[PATCH 09/10] pbl: Implement ctrlc() using getc_ll()

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 pbl/console.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/pbl/console.c b/pbl/console.c
index a9859ad..e34633f 100644
--- a/pbl/console.c
+++ b/pbl/console.c
@@ -30,12 +30,19 @@ int pr_print(int level, const char *fmt, ...)

 	return i;
 }
-
+#ifdef CONFIG_DEBUG_LL_INPUT
 int ctrlc(void)
 {
+	if (tstc_ll() && getc_ll() == 3)
+		return 1;
 	return 0;
 }
-
+#else
+int ctrlc(void)
+{
+	return 0;
+}
+#endif
 void console_putc(unsigned int ch, char c)
 {
 	putc_ll(c);
--
2.1.4

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

[PATCH 10/10] ARM: pbl: Add an option to validate DRAM

[Andrey Smirnov]

Add an option to perform DRAM region validation before using it. The
framework allows individual boards to set up a memory validaion hook
that would be called prior to Barebox using that region of memory.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/start.c               | 14 +-----------
 arch/arm/cpu/uncompress.c          | 32 ++++++++++++++++++++++++---
 arch/arm/include/asm/barebox-arm.h | 28 ++++++++++++++++++++++++
 common/Kconfig                     |  8 +++++++
 common/Makefile                    |  2 +-
 include/common.h                   |  1 +
 include/memtest.h                  | 19 ++++++++++++++++
 lib/Makefile                       |  6 ++++++
 pbl/misc.c                         | 44 ++++++++++++++++++++++++++++++++++++++
 9 files changed, 137 insertions(+), 17 deletions(-)

diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 304ed0c..ee7eb00 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -105,19 +105,7 @@ static noinline __noreturn void __start(unsigned long membase,
 	else
 		malloc_end = (unsigned long)_text;

-	/*
-	 * Maximum malloc space is the Kconfig value if given
-	 * or 1GB.
-	 */
-	if (MALLOC_SIZE > 0) {
-		malloc_start = malloc_end - MALLOC_SIZE;
-		if (malloc_start < membase)
-			malloc_start = membase;
-	} else {
-		malloc_start = malloc_end - (malloc_end - membase) / 2;
-		if (malloc_end - malloc_start > SZ_1G)
-			malloc_start = malloc_end - SZ_1G;
-	}
+	malloc_start = arm_get_malloc_start(membase, malloc_end);

 	pr_debug("initializing malloc pool at 0x%08lx (size 0x%08lx)\n",
 			malloc_start, malloc_end - malloc_start);
diff --git a/arch/arm/cpu/uncompress.c b/arch/arm/cpu/uncompress.c
index b0b7c6d..853d2a3 100644
--- a/arch/arm/cpu/uncompress.c
+++ b/arch/arm/cpu/uncompress.c
@@ -20,6 +20,7 @@
 #define pr_fmt(fmt) "uncompress.c: " fmt

 #include <common.h>
+#include <memtest.h>
 #include <init.h>
 #include <linux/sizes.h>
 #include <pbl.h>
@@ -47,6 +48,7 @@ static void __noreturn noinline uncompress_start_payload(unsigned long membase,
 	uint32_t pg_len;
 	void __noreturn (*barebox)(unsigned long, unsigned long, void *);
 	uint32_t endmem = membase + memsize;
+	unsigned long malloc_start, malloc_end;
 	unsigned long barebox_base;
 	uint32_t *image_end;
 	void *pg_start;
@@ -64,10 +66,14 @@ static void __noreturn noinline uncompress_start_payload(unsigned long membase,
 		 * to the current address. Otherwise it may be a readonly location.
 		 * Copy and relocate to the start of the memory in this case.
 		 */
-		if (pc > membase && pc < membase + memsize)
+		if (pc > membase && pc < membase + memsize) {
 			relocate_to_current_adr();
-		else
+		} else {
+			validate_memory_range("validating pbl relocation area",
+					      membase,
+					      membase + arm_barebox_image_size() - 1);
 			relocate_to_adr(membase);
+		}
 	}

 	if (IS_ENABLED(CONFIG_RELOCATABLE))
@@ -77,6 +83,21 @@ static void __noreturn noinline uncompress_start_payload(unsigned long membase,

 	setup_c();

+	validate_memory_range("validating barebox decompression area",
+			      barebox_base, endmem - 1);
+
+	/*
+	  We know that when we give control to __start function of
+	  uncompressed image _text would be within the boundaries of
+	  (membase, membase + memsize) so the malloc_end variable
+	  would be set to _text
+	 */
+	malloc_end   = barebox_base;
+	malloc_start = arm_get_malloc_start(membase, malloc_end);
+
+	validate_memory_range("validating malloc area",
+			      malloc_start, malloc_end - 1);
+
 	pr_debug("memory at 0x%08lx, size 0x%08lx\n", membase, memsize);

 	if (IS_ENABLED(CONFIG_MMU_EARLY)) {
@@ -122,7 +143,12 @@ static void __noreturn noinline uncompress_start_payload(unsigned long membase,
 void __naked __noreturn barebox_arm_entry(unsigned long membase,
 		unsigned long memsize, void *boarddata)
 {
-	arm_setup_stack(membase + memsize - 16);
+	unsigned long stack_end = membase + memsize - 16;
+
+	validate_memory_range("validating stack area",
+			      membase + memsize - STACK_SIZE, stack_end);
+
+	arm_setup_stack(stack_end);

 	uncompress_start_payload(membase, memsize, boarddata);
 }
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index dbc8aaa..f577df7 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -83,6 +83,34 @@ static inline unsigned long arm_barebox_image_place(unsigned long endmem)
 	return endmem;
 }

+static inline unsigned long arm_barebox_image_size(void)
+{
+	/* We assume that 1G is significantly more that Barebox will
+	 * ever try to reserve */
+	return SZ_1G - arm_barebox_image_place(SZ_1G);
+}
+
+static inline unsigned long arm_get_malloc_start(unsigned long membase,
+						 unsigned long malloc_end)
+{
+	unsigned long malloc_start;
+	/*
+	 * Maximum malloc space is the Kconfig value if given
+	 * or 1GB.
+	 */
+	if (MALLOC_SIZE > 0) {
+		malloc_start = malloc_end - MALLOC_SIZE;
+		if (malloc_start < membase)
+			malloc_start = membase;
+	} else {
+		malloc_start = malloc_end - (malloc_end - membase) / 2;
+		if (malloc_end - malloc_start > SZ_1G)
+			malloc_start = malloc_end - SZ_1G;
+	}
+
+	return malloc_start;
+}
+
 #define ENTRY_FUNCTION(name, arg0, arg1, arg2)				\
 	static void __##name(uint32_t, uint32_t, uint32_t);		\
 									\
diff --git a/common/Kconfig b/common/Kconfig
index 4032dcd..5d40dee 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -246,6 +246,14 @@ config MALLOC_SIZE
 	prompt "malloc area size"
 endmenu

+config MEMORY_VALIDATION
+       bool "Validate regions of memory prior to using them"
+       select PBL_CONSOLE
+       help
+         select this option if you want Barebox to perform memory test
+         on regions of DRAM memory that it is about to use for the
+         first time
+
 config BROKEN
 	bool

diff --git a/common/Makefile b/common/Makefile
index 2738238..a634f0d 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_BLSPEC)		+= blspec.o
 obj-$(CONFIG_BOOTM)		+= bootm.o
 obj-$(CONFIG_CMD_LOADS)		+= s_record.o
 obj-$(CONFIG_CMD_MEMTEST)	+= memtest.o
+pbl-$(CONFIG_MEMORY_VALIDATION)	+= memtest.o
 obj-$(CONFIG_COMMAND_SUPPORT)	+= command.o
 obj-$(CONFIG_CONSOLE_FULL)	+= console.o
 obj-$(CONFIG_CONSOLE_SIMPLE)	+= console_simple.o
@@ -90,4 +91,3 @@ include/generated/compile.h: FORCE
 	@$($(quiet)chk_compile.h)
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/mkcompile_h $@ \
 	"$(UTS_MACHINE)" "$(CC) $(KBUILD_CFLAGS)"
-
diff --git a/include/common.h b/include/common.h
index 86e755d..021637a 100644
--- a/include/common.h
+++ b/include/common.h
@@ -209,6 +209,7 @@ static inline void *get_true_address(const void *ptr)
 	return (void *)address;
 }
 #else
+
 static inline void *get_true_address(const void *ptr)
 {
 	return (void *)ptr;
diff --git a/include/memtest.h b/include/memtest.h
index a337be8..4a426c3 100644
--- a/include/memtest.h
+++ b/include/memtest.h
@@ -8,7 +8,26 @@ struct mem_test_resource {
 	struct list_head list;
 };

+typedef int (*mem_test_handler_t)(resource_size_t _start, resource_size_t _end);
+
 int mem_test(resource_size_t _start,
 		resource_size_t _end, int bus_only);

+#ifdef CONFIG_MEMORY_VALIDATION
+void validate_memory_range(const char *message,
+			   resource_size_t start,
+			   resource_size_t end);
+void set_memory_validation_handler(mem_test_handler_t handler);
+#else
+static inline void validate_memory_range(const char *message,
+					 resource_size_t start,
+					 resource_size_t end)
+{
+}
+
+static inline void set_memory_validation_handler(mem_test_handler_t handler)
+{
+}
+#endif
+
 #endif /* __MEMTEST_H */
diff --git a/lib/Makefile b/lib/Makefile
index 6a3e9fd..3da2c27 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -35,6 +35,12 @@ obj-y			+= random.o
 obj-y			+= lzo/
 obj-$(CONFIG_LZ4_DECOMPRESS) += lz4/
 obj-y			+= show_progress.o
+pbl-$(CONFIG_MEMORY_VALIDATION) += show_progress.o
+# The code is going to be used before MMU is set up and unaligned
+# access will cause an exception(there is an unaligned access to the
+# contenst of 'spinchr' in show_progress())
+CFLAGS_pbl-show_progress.o = -mno-unaligned-access
+
 obj-$(CONFIG_LZO_DECOMPRESS)		+= decompress_unlzo.o
 obj-$(CONFIG_LZ4_DECOMPRESS) += decompress_unlz4.o
 obj-$(CONFIG_PROCESS_ESCAPE_SEQUENCE)	+= process_escape_sequence.o
diff --git a/pbl/misc.c b/pbl/misc.c
index 3d5a881..2b446f5 100644
--- a/pbl/misc.c
+++ b/pbl/misc.c
@@ -1,8 +1,10 @@
 #include <common.h>
 #include <init.h>
+#include <memtest.h>
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
+#include <debug_ll.h>

 void __noreturn __hang(void)
 {
@@ -26,6 +28,48 @@ void set_hang_handler(hang_handler_t handler)
 	*__hang_handler_p = handler;
 }

+#ifdef CONFIG_MEMORY_VALIDATION
+
+static mem_test_handler_t mem_test_handler = NULL;
+
+void validate_memory_range(const char *message,
+			   resource_size_t start,
+			   resource_size_t end)
+{
+	int ret;
+	mem_test_handler_t *__mem_test_handler_p = get_true_address(&mem_test_handler);
+	mem_test_handler_t __mem_test_handler    = get_true_address(*__mem_test_handler_p);
+
+	if (__mem_test_handler) {
+		message = get_true_address(message);
+
+		puts_ll(message);
+		putc_ll(' ');
+		putc_ll('[');
+		puthex_ll(start);
+		putc_ll(' ');
+		putc_ll('-');
+		putc_ll(' ');
+		puthex_ll(end);
+		putc_ll(']');
+		putc_ll('\r');
+		putc_ll('\n');
+
+		ret = __mem_test_handler(start, end);
+
+		if (ret < 0)
+			hang();
+	}
+}
+
+void set_memory_validation_handler(mem_test_handler_t handler)
+{
+	mem_test_handler_t *__mem_test_handler_p = get_true_address(&mem_test_handler);
+
+	*__mem_test_handler_p = handler;
+}
+#endif
+
 void __noreturn panic(const char *fmt, ...)
 {
 	while(1);
--
2.1.4

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

Re: [PATCH 06/10] common: pbl: Allow boards to override hang()

[Sascha Hauer]

Hi Andrey,

On Wed, May 13, 2015 at 07:54:23PM -0700, Andrey Smirnov wrote:
> Add provisions such that board code can re-define the behaviour of
> hang() to implement a behaviour better suited for a particular
> hardware platform.

I'm generally fine with being able to overwrite hang(), but the ARM
specific changes here should be an extra patch with an explanation why
this change is necessary. I assume you want to make hang() usable
for early debugging in the PBL when the code is not running at the
address it's linked at. But that's only an assumption.

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 01/10] common/memtest.c: Fix incorrect array boundary check

[Sascha Hauer]

On Wed, May 13, 2015 at 07:54:18PM -0700, Andrey Smirnov wrote:
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  common/memtest.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)


I have applied the memtest patches for now. I'll come to the PBL console
input patches next week when I find some time. I think I haven't made my
mind yet whether we really want to have input support in PBL and where
this all leads to. We use functions which were originally meant for
debugging to do regular output with PBL console support. Maybe we should
clean this up and introduce some real early console backend rather than
abusing the debugging functions before we add more features to it.

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 10/10] ARM: pbl: Add an option to validate DRAM

[Sascha Hauer]

Hi Andrey,

On Wed, May 13, 2015 at 07:54:27PM -0700, Andrey Smirnov wrote:
> Add an option to perform DRAM region validation before using it. The
> framework allows individual boards to set up a memory validaion hook
> that would be called prior to Barebox using that region of memory.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

What usecase do you have for this patch? Is it debugging or something
you always want to enable on your hardware? Why must the
validate_memory_range function be board specific?

I see that you call validate_memory_range on potentially large areas of
memory, so I wonder if you can't call validate_memory_range from your
board setup code with the complete memory instead.
I am not very fond of overly using get_runtime_offset to calculate
pointers. Setting callback functions from early code which does not run
on its link address is something I really want to avoid.

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 06/10] common: pbl: Allow boards to override hang()

[Andrey Smirnov]

On Thu, May 14, 2015 at 10:25 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi Andrey,
>
> On Wed, May 13, 2015 at 07:54:23PM -0700, Andrey Smirnov wrote:
>> Add provisions such that board code can re-define the behaviour of
>> hang() to implement a behaviour better suited for a particular
>> hardware platform.
>
> I'm generally fine with being able to overwrite hang(), but the ARM
> specific changes here should be an extra patch with an explanation why
> this change is necessary. I assume you want to make hang() usable
> for early debugging in the PBL when the code is not running at the
> address it's linked at. But that's only an assumption.
>

You are correct in your assumption. I'll split the patch in the next
version of the series.


> 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 01/10] common/memtest.c: Fix incorrect array boundary check

[Andrey Smirnov]

On Thu, May 14, 2015 at 10:33 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, May 13, 2015 at 07:54:18PM -0700, Andrey Smirnov wrote:
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  common/memtest.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>
>
> I have applied the memtest patches for now. I'll come to the PBL console
> input patches next week when I find some time. I think I haven't made my
> mind yet whether we really want to have input support in PBL and where
> this all leads to. We use functions which were originally meant for
> debugging to do regular output with PBL console support. Maybe we should
> clean this up and introduce some real early console backend rather than
> abusing the debugging functions before we add more features to it.
>

I agree that it seems to start to look like too much of a code
duplication, FWIW, my main intention with adding input support to PBL
was to allow for more interactivity in hang() handler(like being able
to send a very primitive command to reset the board, instead of
relying on external reset circuitry). I am not sure how this can be
improved, so if you have any ideas of how to better introduce the same
functionality I'll be happy to help on implementing it.

> 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 10/10] ARM: pbl: Add an option to validate DRAM

[Andrey Smirnov]

On Tue, May 19, 2015 at 12:06 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi Andrey,
>
> On Wed, May 13, 2015 at 07:54:27PM -0700, Andrey Smirnov wrote:
>> Add an option to perform DRAM region validation before using it. The
>> framework allows individual boards to set up a memory validaion hook
>> that would be called prior to Barebox using that region of memory.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>
> What usecase do you have for this patch?

The usecase is to be able to have a hardware verification build of the
boot-loader which can be used to test the boards in manufacturing.

> Is it debugging or something  you always want to enable on your hardware?

I need it to be always enabled only in a special build of the BL.

> Why must the validate_memory_range function be board specific?

Because the choice of a memory validation algorithm depends on many factors:
- Speed vs. how extensive you want your tests to be
- Chosen memory fault model (DDR vs. SRAM would be different)
- etc.

Also various memory controllers also might various degree of low level
control so some might allow the developer to flip more switches and
test more corner cases.

>
> I see that you call validate_memory_range on potentially large areas of
> memory, so I wonder if you can't call validate_memory_range from your
> board setup code with the complete memory instead.

Even with the current algorithm implemented in mem_test() (which ,
having read a number of academic papers on memory testing, I don't
believe is comprehensible enough) testing any significant of memory
takes a very noticeable amount of time. I wanted to spend as little
amount of time without having access to extended Barebox functionality
to communicate with the rest of the world(like networking, proper
serial) as possible so I set up the algorithm the way it is and
configured Barebox to have a small(3MB) heap.

Also, testing all of the memory in PBL code brings up the question of
what is the point of 'memtest' command? If the only comprehensive way
of testing memory is in PBL code than, IMHO, that function is not very
useful.

> I am not very fond of overly using get_runtime_offset to calculate
> pointers. Setting callback functions from early code which does not run
> on its link address is something I really want to avoid.

I agree with you on this point. I don't like that code very much either.

>
> 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 10/10] ARM: pbl: Add an option to validate DRAM

[Alexander Aring]

Hi,

On Sat, May 23, 2015 at 11:48:41AM -0700, Andrey Smirnov wrote:
> On Tue, May 19, 2015 at 12:06 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Hi Andrey,
> >
> > On Wed, May 13, 2015 at 07:54:27PM -0700, Andrey Smirnov wrote:
> >> Add an option to perform DRAM region validation before using it. The
> >> framework allows individual boards to set up a memory validaion hook
> >> that would be called prior to Barebox using that region of memory.
> >>
> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >
> > What usecase do you have for this patch?
> 
> The usecase is to be able to have a hardware verification build of the
> boot-loader which can be used to test the boards in manufacturing.
> 
> > Is it debugging or something  you always want to enable on your hardware?
> 
> I need it to be always enabled only in a special build of the BL.
> 
> > Why must the validate_memory_range function be board specific?
> 
> Because the choice of a memory validation algorithm depends on many factors:
> - Speed vs. how extensive you want your tests to be
> - Chosen memory fault model (DDR vs. SRAM would be different)
> - etc.
> 
> Also various memory controllers also might various degree of low level
> control so some might allow the developer to flip more switches and
> test more corner cases.
> 
> >
> > I see that you call validate_memory_range on potentially large areas of
> > memory, so I wonder if you can't call validate_memory_range from your
> > board setup code with the complete memory instead.
> 
> Even with the current algorithm implemented in mem_test() (which ,
> having read a number of academic papers on memory testing, I don't
> believe is comprehensible enough) testing any significant of memory
> takes a very noticeable amount of time. I wanted to spend as little
> amount of time without having access to extended Barebox functionality
> to communicate with the rest of the world(like networking, proper
> serial) as possible so I set up the algorithm the way it is and
> configured Barebox to have a small(3MB) heap.
> 
> Also, testing all of the memory in PBL code brings up the question of
> what is the point of 'memtest' command? If the only comprehensive way

The memtest command enable/disable caches and running the memtest
function. The memtest function is moslty the same like it was when I
touched the code, it's original taken from u-boot code [0]. The memtest
will run on all memory space which is not allocated by barebox
automatically, the command before had some simple "start" and "end"
address parameters and you had some luck if you doesn't hit any core
functionality of barebox.

> of testing memory is in PBL code than, IMHO, that function is not very
> useful.
> 

The memtest function or the memtest command? If I remember correctly
then the memtest function was exported out of memtest command exactly
for the reason to use it in PBL or anywhere for debugging. The memtest
command is just simple to use.

- Alex

[0] http://git.denx.de/?p=u-boot.git;a=blob;f=common/cmd_mem.c;h=2e85d53dd23c02902b6e4973ad3cb2e98bbda678;hb=HEAD#l711

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

Re: [PATCH 10/10] ARM: pbl: Add an option to validate DRAM

[Andrey Smirnov]

>>
>> Also, testing all of the memory in PBL code brings up the question of
>> what is the point of 'memtest' command? If the only comprehensive way
>
> The memtest command enable/disable caches and running the memtest
> function. The memtest function is moslty the same like it was when I
> touched the code, it's original taken from u-boot code [0]. The memtest
> will run on all memory space which is not allocated by barebox
> automatically, the command before had some simple "start" and "end"
> address parameters and you had some luck if you doesn't hit any core
> functionality of barebox.

I am sorry it looks like I didn't convey my point well enough. I do
understand what the 'memtest' command is doing and I agree that having
arbitrary "start" and "end' points without taking into account what
memory areas are being used internally by Barebox is a recipe for
disaster. At the same time I expects/assume that the majority of the
potential users of that command would be using that command because
they want to perform a full memory test. So what I am trying to say is
because 'memtest' can't really touch reserved memory during
execution(for obvious reasons), unless those regions are explicitly
tested before Barebox starts using them, calling it offers very
limited value(it wouldn't be completely useless, but having big chunks
of untested memory is far from ideal).

>
>> of testing memory is in PBL code than, IMHO, that function is not very
>> useful.
>>
>
> The memtest function or the memtest command? If I remember correctly
> then the memtest function was exported out of memtest command exactly
> for the reason to use it in PBL or anywhere for debugging. The memtest
> command is just simple to use.

I agree that it is very easy to use, but why would you want to use it?
I am having hard time imagining use-case where the user would want to
test only unreserved areas of memory and not need to test anything
else.


>
> - Alex
>
> [0] http://git.denx.de/?p=u-boot.git;a=blob;f=common/cmd_mem.c;h=2e85d53dd23c02902b6e4973ad3cb2e98bbda678;hb=HEAD#l711

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

Re: [PATCH 10/10] ARM: pbl: Add an option to validate DRAM

[Sascha Hauer]

On Sat, May 23, 2015 at 11:48:41AM -0700, Andrey Smirnov wrote:
> On Tue, May 19, 2015 at 12:06 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Hi Andrey,
> >
> > On Wed, May 13, 2015 at 07:54:27PM -0700, Andrey Smirnov wrote:
> >> Add an option to perform DRAM region validation before using it. The
> >> framework allows individual boards to set up a memory validaion hook
> >> that would be called prior to Barebox using that region of memory.
> >>
> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >
> > What usecase do you have for this patch?
> 
> The usecase is to be able to have a hardware verification build of the
> boot-loader which can be used to test the boards in manufacturing.
> 
> > Is it debugging or something  you always want to enable on your hardware?
> 
> I need it to be always enabled only in a special build of the BL.
> 
> > Why must the validate_memory_range function be board specific?
> 
> Because the choice of a memory validation algorithm depends on many factors:
> - Speed vs. how extensive you want your tests to be
> - Chosen memory fault model (DDR vs. SRAM would be different)
> - etc.
> 
> Also various memory controllers also might various degree of low level
> control so some might allow the developer to flip more switches and
> test more corner cases.
> 
> >
> > I see that you call validate_memory_range on potentially large areas of
> > memory, so I wonder if you can't call validate_memory_range from your
> > board setup code with the complete memory instead.
> 
> Even with the current algorithm implemented in mem_test() (which ,
> having read a number of academic papers on memory testing, I don't
> believe is comprehensible enough) testing any significant of memory
> takes a very noticeable amount of time. I wanted to spend as little
> amount of time without having access to extended Barebox functionality
> to communicate with the rest of the world(like networking, proper
> serial) as possible so I set up the algorithm the way it is and
> configured Barebox to have a small(3MB) heap.
> 
> Also, testing all of the memory in PBL code brings up the question of
> what is the point of 'memtest' command? If the only comprehensive way
> of testing memory is in PBL code than, IMHO, that function is not very
> useful.
> 
> > I am not very fond of overly using get_runtime_offset to calculate
> > pointers. Setting callback functions from early code which does not run
> > on its link address is something I really want to avoid.
> 
> I agree with you on this point. I don't like that code very much either.

How about testing only a small fragment of DRAM, say 8MB, in your
lowlevel board code and calling barebox_arm_entry() with the
membase/memsize you previously tested? This way you can make sure that
barebox only uses tested memory without having to test all memory before
calling barebox_arm_entry() and without having to call back into some
testing function.

The newest TLSF implementation also supports pools, so we could add the
full amount of memory later if we want to.

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 10/10] ARM: pbl: Add an option to validate DRAM

[Andrey Smirnov]

> How about testing only a small fragment of DRAM, say 8MB, in your
> lowlevel board code and calling barebox_arm_entry() with the
> membase/memsize you previously tested? This way you can make sure that
> barebox only uses tested memory without having to test all memory before
> calling barebox_arm_entry() and without having to call back into some
> testing function.

I like that solution and it does allow to get rid of all of that nasty
linking address agnostic code. The only problem that I see with it is
that if I start Barebox with 8MB of memory and then try to boot Linux
of_memory_fixup() will modify the devicetree file and cause Linux to
think the machine only has 8MB of RAM as well. Any ideas on how to
deal with that?

Thank you,
Andrey

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

Re: [PATCH 10/10] ARM: pbl: Add an option to validate DRAM

[Sascha Hauer]

On Mon, Jun 01, 2015 at 06:09:38AM -0700, Andrey Smirnov wrote:
> > How about testing only a small fragment of DRAM, say 8MB, in your
> > lowlevel board code and calling barebox_arm_entry() with the
> > membase/memsize you previously tested? This way you can make sure that
> > barebox only uses tested memory without having to test all memory before
> > calling barebox_arm_entry() and without having to call back into some
> > testing function.
> 
> I like that solution and it does allow to get rid of all of that nasty
> linking address agnostic code. The only problem that I see with it is
> that if I start Barebox with 8MB of memory and then try to boot Linux
> of_memory_fixup() will modify the devicetree file and cause Linux to
> think the machine only has 8MB of RAM as well. Any ideas on how to
> deal with that?

You should pass a size of 8MB to barebox_arm_entry(). This should not
influence the memory banks which are for determing the memory passed to
Linux. You can instrument barebox_add_memory_bank() which should be
called with the real amount of SDRAM, not with the 8MB. If not, add a
dump_stack() to see who calls this function with the wrong amount.

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