[PATCH 8/8] memtest: add rewritten memtest command
Alexander Aring
alex.aring at gmail.com
Wed Jan 23 15:01:36 EST 2013
Hi,
thank you for reviewing this patch.
On Thu, Jan 17, 2013 at 10:54:31AM +0100, Sascha Hauer wrote:
> On Tue, Jan 15, 2013 at 02:48:50PM +0100, Alexander Aring wrote:
> > Rewrite memtest command:
> > - Skip barebox sdram regions.
> > - Uncache unused mem regions while testing.
> > - Add iteration parameter.
> > - Add parameter to do only bus testing.
> > - Check start and end addresses.
> > - Testing all banks if no start and end
> > address are given.
> >
> > - Add sha changes (thanks):
> > - fix calculation of regions to test. When we use PAGE_ALIGN on
> > size, size can be to high.
> > - start address has to be aligned up
> > - end address has to be aligned down
> > - then size can be calculated as end - start + 1
> > - Add ctrlc() to the longer loops
> > - Add a progress bar to give some visual feedback that something
> > issues
> > still going on.
> >
> > - Change to use end element instead of size in region struct.
> > - Fix some newline issues.
> >
> > - Add '-c' parameter if CONFIG_MMU enabled
> > to test with enabled cache.
> > - Fix some size calculation.
> > - set start address to 0xffffffff
> >
> > Signed-off-by: Alexander Aring <alex.aring at gmail.com>
> > ---
> > commands/Kconfig | 9 +
> > commands/Makefile | 1 +
> > commands/memtest.c | 698 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 708 insertions(+)
> > create mode 100644 commands/memtest.c
> >
> > diff --git a/commands/Kconfig b/commands/Kconfig
> > index fc0e448..f027a7e 100644
> > --- a/commands/Kconfig
> > +++ b/commands/Kconfig
> > @@ -496,6 +496,15 @@ config CMD_NANDTEST
> > select PARTITION_NEED_MTD
> > prompt "nandtest"
> >
> > +config CMD_MEMTEST
> > + tristate
> > + 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 3145685..6b4d9cb 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..c31e553
> > --- /dev/null
> > +++ b/commands/memtest.c
> > @@ -0,0 +1,698 @@
> > +/*
> > + * memtest - Perform a memory test
> > + *
> > + * (C) Copyright 2013
> > + * Alexander Aring <a.aring at gmail.com>
> > + *
> > + * (C) Copyright 2000
> > + * Wolfgang Denk, DENX Software Engineering, wd at 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 <common.h>
> > +#include <command.h>
> > +#include <types.h>
> > +#include <getopt.h>
> > +#include <memory.h>
> > +#include <errno.h>
> > +#include <progress.h>
> > +#include <asm/mmu.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 */
> > +};
> > +
> > +/*
> > + * 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;
>
> Please no uppercase letters for variable names.
>
Ok. I took it from mach/arm/cpu/mmu.c, should I create a patch which
rename it in arch/arm/cpu/mmu.c lower case, too?
> > +#else
> > +static char optstr[] = "s:e:i:b";
> > +#endif
> > +
> > +/*
> > + * Perform a memory test. The complete test
> > + * loops until interrupted by ctrl-c.
> > + */
> > +static int mem_test(vu_long _start, vu_long _end,
> > + int bus_only)
>
> It would be good to move this function to common/memory_test.c. This way
> it could be called from C. Especially testing memory might be called
> from some early small (no shell) development binaries which are running from some
> internal SRAM.
>
Ok.
Maybe we can do a menu entry via Kconfig or something else to run a
memtest after booting automatically.
> > +{
> > + vu_long *start = (vu_long *)_start;
> > + /* Point the dummy to start[1] */
> > + vu_long *dummy = start+1;
> > +
> > + vu_long val;
> > + vu_long readback;
> > + vu_long offset;
> > + vu_long pattern;
> > + vu_long test_offset;
> > + vu_long temp;
> > + vu_long anti_pattern;
> > + vu_long num_words;
> > +
> > + int i;
> > + int ret;
> > +
> > + if (!IS_ALIGNED(_end - _start + 1, sizeof(vu_long))) {
> > + printf("Testing memarea size (0x%08lx) not a multiple"
> > + "of size 0x%x, please change start or end address.",
> > + _end - _start + 1, sizeof(vu_long));
> > + return -1;
> > + }
>
> I think this is both too restrictive and not restitrictive enough. How
> about quietly aligning up the start to a multiple-of-4 boundary and the
> end down to a multiple-of-4 boundary?
Ok, I change it to do a align.
>
> The above requires me to enter a address containing a lot of 'f's and it
> will happily crash my system if I pass 0xa0000001 as start address.
>
> > +
> > + num_words = (_end - _start + 1)/sizeof(vu_long);
> > +
> > + 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++) {
> > + ret = address_in_sdram_regions((vu_long)start);
>
> Can't you just call request_sdram_region at the beginning of this
> function? Then you can be sure that you exclusivly own the region and do
> not have to test for it on and on again here.
Ok, sure I didn't see that.
>
> 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 |
More information about the barebox
mailing list