Hi Mike,
On 08/11/2012 19:50, Mike Frysinger wrote:
On Saturday 11 August 2012 10:17:04 Benoît Thébaudeau wrote:
On 08/11/2012 05:18 AM, Mike Frysinger wrote:
On Friday 10 August 2012 15:16:15 Benoît Thébaudeau wrote:
mtest is supposed to test many types of memory accesses in many different conditions. If dcache is enabled, memory accesses are likely bursts, and some memory accesses are simply skipped. Hence, dcache should be disabled during mtest operation so that what mtest actually tests is not masked by dcache.
if you want dcache disabled, then why don't you run `dcache off` first ? i think it's useful to be able to do both, and forcing it one way is wrong.
thus, NAK from me.
Because you will very likely trust mtest and forget about running `dcache off` first, so you may then be happy about falsely positive mtest results.
there are a lot of commands in u-boot that people could "easily forget" to tweak and have it do the wrong thing. imo, mtest running in the active cache/memory settings is the *expected* behavior rather than having it silently reconfigure the system behind the back of the user.
IMO, mtest is supposed to test the underlying memory as it advertises, not the dcache system, so this is what people will expect from it, and this should be the default behavior. If dcache is needed in some cases, it should be an explicit choice, not the default one. People should not have to think about dcache being supported or enabled on their platform when running mtest. If dcache is added as a command line option to mtest, it will also appear in its help message, which will be a reminder, so there will be nothing made really silently. On the contrary, if dcache is left enabled by default, this will be completely unknown from the user, and there will not even be a reminder that disabling dcache manually should be considered.
Moreover, I can't find any sense or usefulness in running mtest with dcache enabled.
it can be useful to stress test the system with cache enabled. i recall during some early porting work that being able to test both caught bugs that we might have missed otherwise.
OK, but this is not the main purpose of mtest. Here, you used it as a system stress test tool, not really as a memory stress test tool. So I agree that leaving dcache enabled should be an option, but not the default behavior.
If there is really a need for running mtest with dcache enabled, an option (disabled by default) could be added to the command line to tell mtest not to disable dcache.
we already have such an option: dcache off; mtest; dcache on -mike
See my comment above. IMO, dcache off should be the default for mtest.
If you really don't want that, the least should be to have a big warning printed both in mtest help message and when running mtest with dcache enabled.
Best regards, Benoît