From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Alexander Aring <alex.aring@gmail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 5/6] common: add mem_test routine
Date: Thu, 07 Feb 2013 11:52:47 +0100 [thread overview]
Message-ID: <5113877F.1070406@pengutronix.de> (raw)
In-Reply-To: <1360233900-26486-6-git-send-email-alex.aring@gmail.com>
[-- 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
next prev parent reply other threads:[~2013-02-07 10:52 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-07 10:44 [PATCH v3 0/6] add new memtest command Alexander Aring
2013-02-07 10:44 ` [PATCH 1/6] common: fix codestyle in ALIGN macros Alexander Aring
2013-02-07 10:44 ` [PATCH 2/6] common: add ALIGN_DOWN macro Alexander Aring
2013-02-07 10:44 ` [PATCH 3/6] memory: add function address_in_sdram_regions Alexander Aring
2013-02-07 10:44 ` [PATCH 4/6] memtest: remove memtest command Alexander Aring
2013-02-07 10:44 ` [PATCH 5/6] common: add mem_test routine Alexander Aring
2013-02-07 10:52 ` Marc Kleine-Budde [this message]
2013-02-07 11:16 ` Alexander Aring
2013-02-07 11:00 ` Sascha Hauer
2013-02-07 11:40 ` Alexander Aring
2013-02-07 11:54 ` Sascha Hauer
2013-02-07 15:41 ` Alexander Aring
2013-02-07 10:45 ` [PATCH 6/6] commands: add new memtest command Alexander Aring
2013-02-07 10:56 ` Marc Kleine-Budde
2013-02-07 11:20 ` Alexander Aring
2013-02-07 12:01 ` Sascha Hauer
2013-02-07 15:42 ` Alexander Aring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5113877F.1070406@pengutronix.de \
--to=mkl@pengutronix.de \
--cc=alex.aring@gmail.com \
--cc=barebox@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox