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 <w...@denx.de> > Cc: Tom Rini <tr...@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 <w...@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@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot