[U-Boot] [PATCH] Feature Removal: disable "mtest" command by default
Ira W. Snyder
iws at ovro.caltech.edu
Fri Mar 8 20:41:31 CET 2013
On Fri, Mar 08, 2013 at 08:08:24PM +0100, Wolfgang Denk wrote:
> The "mtest" command is of little practical use (if any), and
> experience has shown that a large number of board configurations
> define useless or even dangerous start and end addresses. If not even
> the board maintainers are able to figure out which memory range can be
> reliably tested, how can we expect such from the end users? As this
> problem comes up repeatedly, we rather do not enable this command by
> default, so only people who know what they are doing will be
> confronted with it.
>
> As this changes the user interface, we allow for a grace period
> before this change takes effect. For now, we make "mtest"
> configurable through the CONFIG_CMD_MEMTEST variable, which is defined
> in include/config_cmd_default.h; we also add an entry to
> doc/feature-removal-schedule.txt which announces the removal of this
> default setting in two releases from now, i. e. with v2013.07.
>
> Signed-off-by: Wolfgang Denk <wd at denx.de>
> Cc: Tom Rini <trini at ti.com>
Hi Wolfgang Denk,
I noticed a couple of small typos, and thought I'd point them out.
Hope it helps,
Ira
> ---
> README | 3 +-
> common/cmd_mem.c | 5 +-
> doc/README.memory-test | 98 ++++++++++++++++++++++++++++++++++++++++
> doc/feature-removal-schedule.txt | 17 +++++++
> include/config_cmd_all.h | 3 +-
> include/config_cmd_default.h | 3 +-
> 6 files changed, 125 insertions(+), 4 deletions(-)
> create mode 100644 doc/README.memory-test
>
> diff --git a/README b/README
> index 42544ce..869694f 100644
> --- a/README
> +++ b/README
> @@ -860,7 +860,8 @@ The following options need to be configured:
> (requires CONFIG_CMD_MEMORY and CONFIG_MD5)
> CONFIG_CMD_MEMINFO * Display detailed memory information
> CONFIG_CMD_MEMORY md, mm, nm, mw, cp, cmp, crc, base,
> - loop, loopw, mtest
> + loop, loopw
> + CONFIG_CMD_MEMTEST mtest
> CONFIG_CMD_MISC Misc functions like sleep etc
> CONFIG_CMD_MMC * MMC memory mapped support
> CONFIG_CMD_MII * MII utility commands
> diff --git a/common/cmd_mem.c b/common/cmd_mem.c
> index 042c994..53572f7 100644
> --- a/common/cmd_mem.c
> +++ b/common/cmd_mem.c
> @@ -910,6 +910,7 @@ static ulong mem_test_quick(vu_long *buf, ulong start_addr, ulong end_addr,
> return 0;
> }
>
> +#ifdef CONFIG_CMD_MEMTEST
> /*
> * Perform a memory test. A more complete alternative test can be
> * configured using CONFIG_SYS_ALT_MEMTEST. The complete test loops until
> @@ -1002,7 +1003,7 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int argc,
>
> return ret; /* not reached */
> }
> -
> +#endif /* CONFIG_CMD_MEMTEST */
>
> /* Modify memory.
> *
> @@ -1240,11 +1241,13 @@ U_BOOT_CMD(
> );
> #endif /* CONFIG_LOOPW */
>
> +#ifdef CONFIG_CMD_MEMTEST
> U_BOOT_CMD(
> mtest, 5, 1, do_mem_mtest,
> "simple RAM read/write test",
> "[start [end [pattern [iterations]]]]"
> );
> +#endif /* CONFIG_CMD_MEMTEST */
>
> #ifdef CONFIG_MX_CYCLIC
> U_BOOT_CMD(
> diff --git a/doc/README.memory-test b/doc/README.memory-test
> new file mode 100644
> index 0000000..1e9520b
> --- /dev/null
> +++ b/doc/README.memory-test
> @@ -0,0 +1,98 @@
> +The most frequent cause of problems when porting U-Boot to new
> +hardware, or when using a sloppy port on some board, is memory errors.
> +In most cases these are not caused by failing hardware, but by
> +incorrect initialization of the memory controller. So it appears to
> +be a good idea to always test if the memory is working correctly,
> +before looking for any other potential causes of any problems.
> +
> +U-Boot implements 3 different approaches to perform memory tests:
> +
> +1. The get_ram_size() function (see "common/memsize.c").
> +
> + This function is supposed to be used in each and every U-Boot port
> + determine the presence and actual size of each of the potential
> + memory banks on this piece of hardware. The code is supposed to be
> + very fast, so running it for each reboot does not hurt. It is a
> + little known and generally underrated fact that this code will also
> + catch 99% of hardware related (i. e. reliably reproducable) memory
> + errors. It is strongly recommended to always use this function, in
> + each and every port of U-Boot.
> +
> +2. The "mtest" command.
> +
> + This is probably the best known memory test utility in U-Boot.
> + Unfortunately, it is also the most problematic, and the most
> + useless one.
> +
> + There are a number of serious problems with this command:
> +
> + - It is terribly slow. Running "mtest" on the whole system RAM
> + takes a _long_ time before there is any significance in the fact
> + that no errors have been found so far.
> +
> + - It is difficult to configure, and to use. And any errors here
> + will reliably crash or hang your system. "mtest" is dump and has
^dumb
> + no knowledge about memory ranges that may be in use for other
> + purposes, like exception code, U-Boot code and data, stack,
> + malloc arena, video buffer, log buffer, etc. If you let it, it
> + will happily "test" all such areas, which of corse will cause
^course
> + some problems.
> +
> + - It is not easy to configure and use, and a large number of
> + systems are seriously misconfigured. The original idea was to
> + test basically the whole system RAM, with only excempting the
^exempting
> + areas used by U-Bot itself - on most systems these are the areas
^U-Boot
> + used for the exception vectors (usually at the very lower end of
> + system memory) and for U-Boot (code, data, etc. - see above;
> + these are usually at the very upper end of system memory). But
> + experience has shown that a very large number of ports use
> + pretty much bogus settings of CONFIG_SYS_MEMTEST_START and
> + CONFIG_SYS_MEMTEST_END; this results in useless tests (because
> + the ranges is too small and/or badly located) or in critical
> + failures (system crashes).
> +
> + Because of these issues, the "mtest" command is considered depre-
> + cated. It should not be enabled in most normal ports of U-Boot,
> + especially not in production. If you really need a memory test,
> + then see 1. and 3. above resp. below.
> +
> +3. The most thorough memory test facility is available as part of the
> + POST (Power-On Self Test) sub-system, see "post/drivers/memory.c".
> +
> + If you areally need to perform memory tests (for example, because
^really
> + it is mandatory part of your requirement specification), then
> + enable this test which is generic and should work on all archi-
> + tectures.
> +
> +WARNING:
> +
> +It should pointed out that _all_ these memory tests have one
> +fundamental, unfixable design flaw: they are based on the assumption
> +that memory errors can be found by writing to and reading from memory.
> +Unfortunaltely, this is only true for the relatively harmless, usually
^Unfortunately
> +static errors like shorts between data or address lines, unconnected
> +pins, etc. All the really nasty errors which will first turn your
> +hair grey, only to make you tear it out later, are dynamical errors,
> +which usually happen not with simple read or write cycles on the bus,
> +but when performing back-to-back data transfers in burst mode. Such
> +accesses usually happen only for certain DMA operations, or for heavy
> +cache use (instruction fetching, cache flushing). So far I am not
> +aware of any freely available code that implements a generic, and
> +efficient, memory test like that. The best known test case to stress
> +a system like that is to boot Linux with root file system mounted over
> +NFS, and then build some larger software package natively (say,
> +compile a Linux kernel on the system) - this will cause enough context
> +switches, network traffic (and thus DMA transfers from the network
> +controller), varying RAM use, etc. to trigger any weak spots in this
> +area.
> +
> +Note: An attempt was made once to implement such a test to catch
> +memory problems on a specific board. The code is pretty much board
> +specific (for example, it includes setting specific GPIO signals to
> +provide triggers for an attached logic analyzer), but you can get an
> +idea how it works: see "examples/standalone/test_burst*".
> +
> +Note 2: Ironically enough, the "test_burst" did not catch any RAM
> +errors, not a single one ever. The problems this code was supposed
> +to catch did not happen when accessing the RAM, but when reading from
> +NOR flash.
> diff --git a/doc/feature-removal-schedule.txt b/doc/feature-removal-schedule.txt
> index e04ba2d..d9a0cc2 100644
> --- a/doc/feature-removal-schedule.txt
> +++ b/doc/feature-removal-schedule.txt
> @@ -7,6 +7,23 @@ file.
>
> ---------------------------
>
> +What: Remove CONFIG_CMD_MEMTEST from default list
> +When: Release v2013.07
> +
> +Why: The "mtest" command is of little practical use (if any), and
> + experience has shown that a large number of board configu-
> + rations define useless or even dangerous start and end
> + addresses. If not even the board maintainers are able to
> + figure out which memory range can be reliably tested, how can
> + we expect such from the end users? As this problem comes up
> + repeatedly, we rather do not enable this command by default,
> + so only people who know what they are doing will be confronted
> + with it.
> +
> +Who: Wolfgang Denk <wd at denx.de>
> +
> +---------------------------
> +
> What: Users of the legacy miiphy_* code
> When: undetermined
>
> diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
> index 0930781..53a2f05 100644
> --- a/include/config_cmd_all.h
> +++ b/include/config_cmd_all.h
> @@ -57,7 +57,8 @@
> #define CONFIG_CMD_LOADB /* loadb */
> #define CONFIG_CMD_LOADS /* loads */
> #define CONFIG_CMD_MEMINFO /* meminfo */
> -#define CONFIG_CMD_MEMORY /* md mm nm mw cp cmp crc base loop mtest */
> +#define CONFIG_CMD_MEMORY /* md mm nm mw cp cmp crc base loop */
> +#define CONFIG_CMD_MEMTEST /* mtest */
> #define CONFIG_CMD_MFSL /* FSL support for Microblaze */
> #define CONFIG_CMD_MII /* MII support */
> #define CONFIG_CMD_MISC /* Misc functions like sleep etc*/
> diff --git a/include/config_cmd_default.h b/include/config_cmd_default.h
> index 6e3903c..a521103 100644
> --- a/include/config_cmd_default.h
> +++ b/include/config_cmd_default.h
> @@ -30,7 +30,8 @@
> #endif
> #define CONFIG_CMD_LOADB /* loadb */
> #define CONFIG_CMD_LOADS /* loads */
> -#define CONFIG_CMD_MEMORY /* md mm nm mw cp cmp crc base loop mtest */
> +#define CONFIG_CMD_MEMORY /* md mm nm mw cp cmp crc base loop */
> +#define CONFIG_CMD_MEMTEST /* mtest */
> #define CONFIG_CMD_MISC /* Misc functions like sleep etc*/
> #define CONFIG_CMD_NET /* bootp, tftpboot, rarpboot */
> #define CONFIG_CMD_NFS /* NFS support */
> --
> 1.7.11.7
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list