[U-Boot] [PATCH] mtest: Disable dcache during test

[Benoît Thébaudeau]

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