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

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

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);

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

mtest tests 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, mtest results may change depending on dcache state.
This patch prints the dcache state at the beginning of mtest so as to keep the user informed of the test conditions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de Cc: Mike Frysinger vapier@gentoo.org --- .../common/cmd_mem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git u-boot-4d3c95f.orig/common/cmd_mem.c u-boot-4d3c95f/common/cmd_mem.c index 18f0a3f..5628f6a 100644 --- u-boot-4d3c95f.orig/common/cmd_mem.c +++ u-boot-4d3c95f/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);

On Mon, Aug 13, 2012 at 12:59:12AM -0000, Beno??t Th??baudeau wrote:
mtest tests 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, mtest results may change depending on dcache state.
This patch prints the dcache state at the beginning of mtest so as to keep the user informed of the test conditions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Beno??t Th??baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de Cc: Mike Frysinger vapier@gentoo.org
NAK, this breaks x86 which has no dcache_status.

Hi Tom,
On Wed, 3 Oct 2012 15:05:21 -0700, Tom Rini trini@ti.com wrote:
On Mon, Aug 13, 2012 at 12:59:12AM -0000, Beno??t Th??baudeau wrote:
mtest tests 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, mtest results may change depending on dcache state.
This patch prints the dcache state at the beginning of mtest so as to keep the user informed of the test conditions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Beno??t Th??baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de Cc: Mike Frysinger vapier@gentoo.org
NAK, this breaks x86 which has no dcache_status.
IIRC the general idea of printing the cache status in mtest had already been NAKed (though not formally) by Wolfgang.
Amicalement,

Hi Albert, Tom,
On Thursday, October 4, 2012 1:03:48 PM, Albert ARIBAUD wrote:
Hi Tom,
On Wed, 3 Oct 2012 15:05:21 -0700, Tom Rini trini@ti.com wrote:
On Mon, Aug 13, 2012 at 12:59:12AM -0000, Beno??t Th??baudeau wrote:
mtest tests 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, mtest results may change depending on dcache state.
This patch prints the dcache state at the beginning of mtest so as to keep the user informed of the test conditions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Beno??t Th??baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de Cc: Mike Frysinger vapier@gentoo.org
NAK, this breaks x86 which has no dcache_status.
IIRC the general idea of printing the cache status in mtest had already been NAKed (though not formally) by Wolfgang.
Indeed. This subject has already been closed. Perhaps the patch status had just not been updated in Patchwork.
Best regards, Benoît

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/04/12 04:13, Benoît Thébaudeau wrote:
Hi Albert, Tom,
On Thursday, October 4, 2012 1:03:48 PM, Albert ARIBAUD wrote:
Hi Tom,
On Wed, 3 Oct 2012 15:05:21 -0700, Tom Rini trini@ti.com wrote:
On Mon, Aug 13, 2012 at 12:59:12AM -0000, Beno??t Th??baudeau wrote:
mtest tests 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, mtest results may change depending on dcache state.
This patch prints the dcache state at the beginning of mtest so as to keep the user informed of the test conditions.
Signed-off-by: Mike Frysinger vapier@gentoo.org Tested-by: Beno??t Th??baudeau benoit.thebaudeau@advansee.com Cc: Wolfgang Denk wd@denx.de Cc: Mike Frysinger vapier@gentoo.org
NAK, this breaks x86 which has no dcache_status.
IIRC the general idea of printing the cache status in mtest had already been NAKed (though not formally) by Wolfgang.
Indeed. This subject has already been closed. Perhaps the patch status had just not been updated in Patchwork.
OK, thanks guys.
- -- Tom

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,
participants (4)
-
Albert ARIBAUD
-
Benoît Thébaudeau
-
Mike Frysinger
-
Tom Rini