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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.