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

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

[Benoît Thébaudeau]

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.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
Cc: Wolfgang Denk <wd@denx.de>
---
 .../common/cmd_mem.c                               |   39 ++++++++++++++------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git u-boot-4d3c95f.orig/common/cmd_mem.c u-boot-4d3c95f/common/cmd_mem.c
index 18f0a3f..5d2b735 100644
--- u-boot-4d3c95f.orig/common/cmd_mem.c
+++ u-boot-4d3c95f/common/cmd_mem.c
@@ -600,6 +600,8 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	ulong	errs = 0;
 	int iterations = 1;
 	int iteration_limit;
+	int dcache;
+	int ret = 1;
 
 #if defined(CONFIG_SYS_ALT_MEMTEST)
 	vu_long	len;
@@ -651,6 +653,13 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	else
 		iteration_limit = 0;
 
+	/* Perform tests on the underlying memory rather than on the D-cache. */
+	dcache = dcache_status();
+	if (dcache) {
+		dcache_disable();
+		invalidate_dcache_all();
+	}
+
 #if defined(CONFIG_SYS_ALT_MEMTEST)
 	printf ("Testing %08x ... %08x:\n", (uint)start, (uint)end);
 	debug("%s:%d: start 0x%p end 0x%p\n",
@@ -659,14 +668,15 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	for (;;) {
 		if (ctrlc()) {
 			putc ('\n');
-			return 1;
+			goto end;
 		}
 
 
 		if (iteration_limit && iterations > iteration_limit) {
 			printf("Tested %d iteration(s) with %lu errors.\n",
 				iterations-1, errs);
-			return errs != 0;
+			ret = errs != 0;
+			goto end;
 		}
 
 		printf("Iteration: %6d\r", iterations);
@@ -704,7 +714,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			    errs++;
 			    if (ctrlc()) {
 				putc ('\n');
-				return 1;
+				goto end;
 			    }
 			}
 			*addr  = ~val;
@@ -717,7 +727,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			    errs++;
 			    if (ctrlc()) {
 				putc ('\n');
-				return 1;
+				goto end;
 			    }
 			}
 		    }
@@ -787,7 +797,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			errs++;
 			if (ctrlc()) {
 			    putc ('\n');
-			    return 1;
+			    goto end;
 			}
 		    }
 		}
@@ -809,7 +819,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			    errs++;
 			    if (ctrlc()) {
 				putc ('\n');
-				return 1;
+				goto end;
 			    }
 			}
 		    }
@@ -851,7 +861,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			errs++;
 			if (ctrlc()) {
 			    putc ('\n');
-			    return 1;
+			    goto end;
 			}
 		    }
 
@@ -873,7 +883,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			errs++;
 			if (ctrlc()) {
 			    putc ('\n');
-			    return 1;
+			    goto end;
 			}
 		    }
 		    start[offset] = 0;
@@ -885,13 +895,14 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	for (;;) {
 		if (ctrlc()) {
 			putc ('\n');
-			return 1;
+			goto end;
 		}
 
 		if (iteration_limit && iterations > iteration_limit) {
 			printf("Tested %d iteration(s) with %lu errors.\n",
 				iterations-1, errs);
-			return errs != 0;
+			ret = errs != 0;
+			goto end;
 		}
 		++iterations;
 
@@ -918,7 +929,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 				errs++;
 				if (ctrlc()) {
 					putc ('\n');
-					return 1;
+					goto end;
 				}
 			}
 			val += incr;
@@ -939,7 +950,11 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		incr = -incr;
 	}
 #endif
-	return 0;	/* not reached */
+
+end:
+	if (dcache)
+		dcache_enable();
+	return ret;
 }
 
 

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

[Mike Frysinger]

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.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120810/0de99111/attachment.pgp>

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

[Benoît Thébaudeau]

Hi Mike,

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.
> -mike

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. Moreover,
I can't find any sense or usefulness in running mtest with dcache enabled.

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.

Best regards,
Beno?t

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

[Mike Frysinger]

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.

> 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.

> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120811/f85ea029/attachment.pgp>

[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

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

[Benoît Thébaudeau]

On 08/11/2012 21:47, Andrew Dyer wrote:
> I agree with Mike, use the current dcache settings. U-boot has always
> assumed the user knew what they were doing.
> If you want to print a small message with the dcache setting that
> makes sense to me, but no big warning.

Then, something like the following at runtime in the first lines printed my
mtest?

dcache state: on

Best regards,
Beno?t

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

[Mike Frysinger]

On Saturday 11 August 2012 16:05:36 Beno?t Th?baudeau wrote:
> On 08/11/2012 21:47, Andrew Dyer wrote:
> > I agree with Mike, use the current dcache settings. U-boot has always
> > assumed the user knew what they were doing.
> > If you want to print a small message with the dcache setting that
> > makes sense to me, but no big warning.
> 
> Then, something like the following at runtime in the first lines printed my
> mtest?
> 
> dcache state: on

i'd be fine with that.  something like below (if you want to test & post,
that'd be good).
-mike

diff --git a/common/cmd_mem.c b/common/cmd_mem.c
index 18f0a3f..5628f6a 100644
--- a/common/cmd_mem.c
+++ b/common/cmd_mem.c
@@ -651,8 +651,10 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	else
 		iteration_limit = 0;
 
+	printf("Testing %08x ... %08x (dcache: %s):\n", (uint)start, (uint)end,
+		dcache_status() ? "on" : "off");
+
 #if defined(CONFIG_SYS_ALT_MEMTEST)
-	printf ("Testing %08x ... %08x:\n", (uint)start, (uint)end);
 	debug("%s:%d: start 0x%p end 0x%p\n",
 		__FUNCTION__, __LINE__, start, end);
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120811/5c64e75b/attachment.pgp>

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

[Benoît Thébaudeau]

On Saturday 11 August 2012 22:15:16 Mike Frysinger wrote:
> On Saturday 11 August 2012 16:05:36 Beno?t Th?baudeau wrote:
> > On 08/11/2012 21:47, Andrew Dyer wrote:
> > > I agree with Mike, use the current dcache settings. U-boot has
> > > always
> > > assumed the user knew what they were doing.
> > > If you want to print a small message with the dcache setting that
> > > makes sense to me, but no big warning.
> > 
> > Then, something like the following at runtime in the first lines
> > printed my
> > mtest?
> > 
> > dcache state: on
> 
> i'd be fine with that.  something like below (if you want to test &
> post,
> that'd be good).
> -mike
> 
> diff --git a/common/cmd_mem.c b/common/cmd_mem.c
> index 18f0a3f..5628f6a 100644
> --- a/common/cmd_mem.c
> +++ b/common/cmd_mem.c
> @@ -651,8 +651,10 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag,
> int argc, char * const argv[])
>  	else
>  		iteration_limit = 0;
>  
> +	printf("Testing %08x ... %08x (dcache: %s):\n", (uint)start,
> (uint)end,
> +		dcache_status() ? "on" : "off");
> +
>  #if defined(CONFIG_SYS_ALT_MEMTEST)
> -	printf ("Testing %08x ... %08x:\n", (uint)start, (uint)end);
>  	debug("%s:%d: start 0x%p end 0x%p\n",
>  		__FUNCTION__, __LINE__, start, end);

OK, I'll do that next week.

Best regards,
Beno?t

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

[Albert ARIBAUD]

Hi Mike,

On Sat, 11 Aug 2012 16:15:16 -0400, Mike Frysinger <vapier@gentoo.org>
wrote:
> On Saturday 11 August 2012 16:05:36 Beno?t Th?baudeau wrote:
> > On 08/11/2012 21:47, Andrew Dyer wrote:
> > > I agree with Mike, use the current dcache settings. U-boot has
> > > always assumed the user knew what they were doing.
> > > If you want to print a small message with the dcache setting that
> > > makes sense to me, but no big warning.
> > 
> > Then, something like the following at runtime in the first lines
> > printed my mtest?
> > 
> > dcache state: on
> 
> i'd be fine with that.  something like below (if you want to test &
> post, that'd be good).
> -mike

I'd disagree with this solution; one can easily overlook casual test
messages like these. To me it is better that the person doing mtest not
trust the cache state at all and check / set it.

Amicalement,
-- 
Albert.

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

[Wolfgang Denk]

Dear Beno?t Th?baudeau,

In message <1725235724.2300239.1344694624384.JavaMail.root@advansee.com> you wrote:
> 
> On 08/11/2012 05:18 AM, Mike Frysinger wrote:
...
> > 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.
> > -mike
> 
> 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. Moreover,
> I can't find any sense or usefulness in running mtest with dcache enabled.

I agree with Mike.

"UNIX was not designed to stop you from doing stupid things,  because
that would also stop you from doing clever things."       - Doug Gwyn

So NAK from me, too.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offence.
          -- Edsger W. Dijkstra, SIGPLAN Notices, Volume 17, Number 5

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

[Benoît Thébaudeau]

Dear Wolfgang Denk,

On Sunday, September 2, 2012 6:30:23 PM, Wolfgang Denk wrote:
> Dear Beno?t Th?baudeau,
> 
> In message
> <1725235724.2300239.1344694624384.JavaMail.root@advansee.com> you
> wrote:
> > 
> > On 08/11/2012 05:18 AM, Mike Frysinger wrote:
> ...
> > > 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.
> > > -mike
> > 
> > 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. Moreover,
> > I can't find any sense or usefulness in running mtest with dcache
> > enabled.
> 
> I agree with Mike.
> 
> "UNIX was not designed to stop you from doing stupid things,  because
> that would also stop you from doing clever things."       - Doug Gwyn
> 
> So NAK from me, too.

OK, but do you agree with the following that Mike and me agreed on after that?
http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
http://patchwork.ozlabs.org/patch/176909/

Best regards,
Beno?t

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

[Albert ARIBAUD]

Hi Beno?t,

On Mon, 3 Sep 2012 16:25:15 +0200 (CEST), Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> Dear Wolfgang Denk,
> 
> On Sunday, September 2, 2012 6:30:23 PM, Wolfgang Denk wrote:
> > Dear Beno?t Th?baudeau,
> > 
> > In message
> > <1725235724.2300239.1344694624384.JavaMail.root@advansee.com> you
> > wrote:
> > > 
> > > On 08/11/2012 05:18 AM, Mike Frysinger wrote:
> > ...
> > > > 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.
> > > > -mike
> > > 
> > > 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. Moreover,
> > > I can't find any sense or usefulness in running mtest with dcache
> > > enabled.
> > 
> > I agree with Mike.
> > 
> > "UNIX was not designed to stop you from doing stupid things,
> > because that would also stop you from doing clever things."       -
> > Doug Gwyn
> > 
> > So NAK from me, too.
> 
> OK, but do you agree with the following that Mike and me agreed on
> after that?
> http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
> http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
> http://patchwork.ozlabs.org/patch/176909/

I did already reply to this, but since the agreement is brought back, I
think I should re-state my opinion: such a warning line will most likely
be overlooked, thus has little value. People using mtest should know
that they must check/set dcache state before running mtest.

> Best regards,
> Beno?t

Amicalement,
-- 
Albert.

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

[Benoît Thébaudeau]

Hi Albert,

On Monday, September 3, 2012 6:50:14 PM, Albert ARIBAUD wrote:
> Hi Beno?t,
> 
> On Mon, 3 Sep 2012 16:25:15 +0200 (CEST), Beno?t Th?baudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > Dear Wolfgang Denk,
> > 
> > On Sunday, September 2, 2012 6:30:23 PM, Wolfgang Denk wrote:
> > > Dear Beno?t Th?baudeau,
> > > 
> > > In message
> > > <1725235724.2300239.1344694624384.JavaMail.root@advansee.com> you
> > > wrote:
> > > > 
> > > > On 08/11/2012 05:18 AM, Mike Frysinger wrote:
> > > ...
> > > > > 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.
> > > > > -mike
> > > > 
> > > > 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. Moreover,
> > > > I can't find any sense or usefulness in running mtest with
> > > > dcache
> > > > enabled.
> > > 
> > > I agree with Mike.
> > > 
> > > "UNIX was not designed to stop you from doing stupid things,
> > > because that would also stop you from doing clever things."
> > >       -
> > > Doug Gwyn
> > > 
> > > So NAK from me, too.
> > 
> > OK, but do you agree with the following that Mike and me agreed on
> > after that?
> > http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
> > http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
> > http://patchwork.ozlabs.org/patch/176909/
> 
> I did already reply to this, but since the agreement is brought back,
> I
> think I should re-state my opinion: such a warning line will most
> likely
> be overlooked, thus has little value. People using mtest should know
> that they must check/set dcache state before running mtest.

If this line is overlooked, it's the same as not having it. If it is not
overlooked, it is useful both to detail the test conditions and as a reminder
not to do stupid things. This line does not prevent users from doing any manual
dcache check/enable/disable operation they want before running mtest. All in
all, adding this line can only be beneficial.

Best regards,
Beno?t

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

[Graeme Russ]

Hi Beno?t,

On Tue, Sep 4, 2012 at 7:14 AM, Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:
> Hi Albert,
>
> On Monday, September 3, 2012 6:50:14 PM, Albert ARIBAUD wrote:
>> Hi Beno?t,
>>
>> On Mon, 3 Sep 2012 16:25:15 +0200 (CEST), Beno?t Th?baudeau
>> <benoit.thebaudeau@advansee.com> wrote:
>>
>> > Dear Wolfgang Denk,
>> >
>> > On Sunday, September 2, 2012 6:30:23 PM, Wolfgang Denk wrote:
>> > > Dear Beno?t Th?baudeau,
>> > >
>> > > In message
>> > > <1725235724.2300239.1344694624384.JavaMail.root@advansee.com> you
>> > > wrote:
>> > > >
>> > > > On 08/11/2012 05:18 AM, Mike Frysinger wrote:
>> > > ...
>> > > > > 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.
>> > > > > -mike
>> > > >
>> > > > 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. Moreover,
>> > > > I can't find any sense or usefulness in running mtest with
>> > > > dcache
>> > > > enabled.
>> > >
>> > > I agree with Mike.
>> > >
>> > > "UNIX was not designed to stop you from doing stupid things,
>> > > because that would also stop you from doing clever things."
>> > >       -
>> > > Doug Gwyn
>> > >
>> > > So NAK from me, too.
>> >
>> > OK, but do you agree with the following that Mike and me agreed on
>> > after that?
>> > http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
>> > http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
>> > http://patchwork.ozlabs.org/patch/176909/
>>
>> I did already reply to this, but since the agreement is brought back,
>> I
>> think I should re-state my opinion: such a warning line will most
>> likely
>> be overlooked, thus has little value. People using mtest should know
>> that they must check/set dcache state before running mtest.
>
> If this line is overlooked, it's the same as not having it. If it is not
> overlooked, it is useful both to detail the test conditions and as a reminder
> not to do stupid things. This line does not prevent users from doing any manual
> dcache check/enable/disable operation they want before running mtest. All in
> all, adding this line can only be beneficial.

I agree - particularly when somebody else is looking at the output (on
the ML for example) and notices it.

I'm also inclined to not 100% trust dcache operations (we all know
that cache support is in a state of flux) so something like:

# dcache on
# mtest 0x80100000 0x90000000 0xaabb

If you get back:

Testing 0x80100000 ... 0x90000000 (dcache: off):

You know something is wrong with the dcache on implementation

Regards,

Graeme

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

[Wolfgang Denk]

Dear Graeme Russ,

In message <CALButCLVW0Vv9vc+MZ0wpau2xw7FPv+ZqpD7MZpxn=K_400HVA@mail.gmail.com> you wrote:
> 
> > If this line is overlooked, it's the same as not having it. If it is not
> > overlooked, it is useful both to detail the test conditions and as a reminder
> > not to do stupid things. This line does not prevent users from doing any manual
> > dcache check/enable/disable operation they want before running mtest. All in
> > all, adding this line can only be beneficial.

I disagree. It add line noise, printing information that is available
otherwise as well.

If you are interested in the state of the data cache support, just run
the "dcache" command.

> I'm also inclined to not 100% trust dcache operations (we all know
> that cache support is in a state of flux) so something like:
>
> # dcache on
> # mtest 0x80100000 0x90000000 0xaabb
...
> You know something is wrong with the dcache on implementation

Well, it appears awkward to me to use a completely unrelated command
for such "testing".  The same result should be available (much more
straightforward) with:

	# dcache on
	# dcache


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The word "fit", as I understand it, means "appropriate to a purpose",
and I would say the body of the Dean is supremely appropriate to  the
purpose of sitting around all day and eating big heavy meals.
                                 - Terry Pratchett, _Moving Pictures_

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

[Graeme Russ]

Hi Wolfgang,

On Tue, Sep 4, 2012 at 4:41 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> Well, it appears awkward to me to use a completely unrelated command
> for such "testing".  The same result should be available (much more
> straightforward) with:
>
>         # dcache on
>         # dcache

Good point :)

Regards,

Graeme

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

[Wolfgang Denk]

Dear Beno?t Th?baudeau,

In message <356588499.3415554.1346682315106.JavaMail.root@advansee.com> you wrote:
> 
> > So NAK from me, too.
>
> OK, but do you agree with the following that Mike and me agreed on after that?
> http://lists.denx.de/pipermail/u-boot/2012-August/130650.html
> http://lists.denx.de/pipermail/u-boot/2012-August/130726.html
> http://patchwork.ozlabs.org/patch/176909/

No, I don't.  If you anybody is interested in the state of the cache
support, he can use the "dcache" (and/or "icache") command(s).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Technology is dominated by those who manage what they do  not  under-
stand.