[U-Boot-Users] PATCH for cmd_mem.c:do_mem_mtest()
Sorry the patch below isn't from git - I hope it's still acceptable. I've put a copy in the body and attached it incase gmail decides to line wrap the text.
ChangeLog: * Patch by Andrew Dyer, 13 September 2005: if CFG_ALT_MEMTEST is defined without CFG_MEMTEST_SCRATCH, cmd_mem.c code will dereference a null pointer. Change the code to use the last word of the memory test area as the scratch location in this case. Print the scratch memory location used.
Signed-off-by: Andrew Dyer amdyer@gmail.com
Index: common/cmd_mem.c =================================================================== RCS file: /home/cvsroot/Projects/u-boot/common/cmd_mem.c,v retrieving revision 1.1.1.1 diff -p -u -r1.1.1.1 cmd_mem.c --- common/cmd_mem.c 28 Jun 2005 17:32:26 -0000 1.1.1.1 +++ common/cmd_mem.c 13 Sep 2005 10:40:26 -0000 @@ -707,7 +707,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int #if defined(CFG_MEMTEST_SCRATCH) vu_long *dummy = (vu_long*)CFG_MEMTEST_SCRATCH; #else - vu_long *dummy = NULL; + vu_long *dummy; #endif int j; int iterations = 1; @@ -747,7 +747,17 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int }
#if defined(CFG_ALT_MEMTEST) + +#if !defined(CFG_MEMTEST_SCRATCH) + /* if no fixed scratch area is defined use + * the last location of the test area + */ + end--; + dummy = end; +#endif + printf ("Testing %08x ... %08x:\n", (uint)start, (uint)end); + printf ("using address %08x as scratch\n",dummy); PRINTF("%s:%d: start 0x%p end 0x%p\n", __FUNCTION__, __LINE__, start, end);
In message c166aa9f0509130909544387fd@mail.gmail.com you wrote:
Sorry the patch below isn't from git - I hope it's still acceptable.
No problem.
I've put a copy in the body and attached it incase gmail decides to line wrap the text.
Ummm.. one copy is enough.
ChangeLog:
- Patch by Andrew Dyer, 13 September 2005:
if CFG_ALT_MEMTEST is defined without CFG_MEMTEST_SCRATCH, cmd_mem.c code will dereference a null pointer. Change
No, it does not. It just points to address 0x0000. This is a perfectly valid address on many systems.
the code to use the last word of the memory test area as the scratch location in this case. Print the scratch memory location used.
Which problem is this patch supposed to fix in the first place? The existing code compiles and works fine on many, many systems.
Best regards,
Wolfgang Denk
On Tue, Sep 13, 2005 at 06:18:56PM +0200, Wolfgang Denk wrote:
ChangeLog:
- Patch by Andrew Dyer, 13 September 2005:
if CFG_ALT_MEMTEST is defined without CFG_MEMTEST_SCRATCH, cmd_mem.c code will dereference a null pointer. Change
No, it does not. It just points to address 0x0000. This is a perfectly valid address on many systems.
I sent patch to mailing list few months ago which changes NULL to 0x00000000 and adds some comment to avoid such confusion (I burned my fingers the same way :-)).
ftp://ftp.linux-mips.org/pub/linux/mips/people/ladis/u-boot/cmd_mem_dummy_addr.diff
Best regards, ladis
In message 20050913163114.GA19366@orphique you wrote:
I sent patch to mailing list few months ago which changes NULL to 0x00000000 and adds some comment to avoid such confusion (I burned my fingers the same way :-)).
We can write 0 or 0x0 or 000 or 0x00000 or (1 - 1) or NULL - all this does not change the behaviour of the code at all.
Best regards,
Wolfgang Denk
I am using target :ppc405 netbsd host i386, netbsd I can use u-boot to boot up. I use load to load netbsd.bin (kernel) to bootup. I read about bootm, that netbsd image needs bootloader to load kernel and rootfile system.
I don't have Stage 2 Netbsd loader so I downloaded from website, but don't know how to build it. Can anyone help ? or any other bootloader for netbsd images
I am really stuck at this and cannot proceed.
thanks
__________________________________ Yahoo! Mail - PC Magazine Editors' Choice 2005 http://mail.yahoo.com
I've been unable to download the stage 2 NetBSD loader from this link: ftp://ftp.denx.de/pub/u-boot/ppcboot_stage2.tar.gz. Are there any other alternatives, or can anyone send me this loader? Also, is there any documentation of how to use it?
Thanks.
Sam
--- manju mahajan manjumail@yahoo.com wrote:
I am using target :ppc405 netbsd host i386, netbsd I can use u-boot to boot up. I use load to load netbsd.bin (kernel) to bootup. I read about bootm, that netbsd image needs bootloader to load kernel and rootfile system.
I don't have Stage 2 Netbsd loader so I downloaded from website, but don't know how to build it. Can anyone help ? or any other bootloader for netbsd images
I am really stuck at this and cannot proceed.
thanks
__________________________________ Yahoo! Mail - PC Magazine Editors' Choice 2005 http://mail.yahoo.com
-------------------------------------------------------
SF.Net email is sponsored by: Tame your development challenges with Apache's Geronimo App Server. Download it for free - -and be entered to win a 42" plasma tv or your very own Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
__________________________________________________ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com
In message 20051003165200.98983.qmail@web54107.mail.yahoo.com you wrote:
I've been unable to download the stage 2 NetBSD loader from this link: ftp://ftp.denx.de/pub/u-boot/ppcboot_stage2.tar.gz.
Why not? The link is correct and the server is up and running.
Are there any other alternatives, or can anyone send me this loader? Also, is there any documentation of how to use it?
Except from the README? Ask the NetBSD folks, pelase.
Best regards,
Wolfgang Denk
--- Wolfgang Denk wd@denx.de wrote:
In message 20051003165200.98983.qmail@web54107.mail.yahoo.com you wrote:
I've been unable to download the stage 2 NetBSD
loader
from this link:
ftp://ftp.denx.de/pub/u-boot/ppcboot_stage2.tar.gz.
Why not? The link is correct and the server is up and running.
You're right. I was able to get from home. I guess it has to do with the firewall at my workplace.
Are there any other alternatives, or can anyone
send
me this loader? Also, is there any documentation
of
how to use it?
Except from the README? Ask the NetBSD folks, pelase.
Will do. Thanks.
Best regards,
Wolfgang Denk
-- Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Niklaus Wirth has lamented that, whereas Europeans pronounce his name correctly (Ni-klows Virt), Americans invariably mangle it into (Nick- les Worth). Which is to say that Europeans call him by name, but Americans call him by value.
__________________________________ Yahoo! Mail - PC Magazine Editors' Choice 2005 http://mail.yahoo.com
On Tue, Sep 13, 2005 at 08:04:25PM +0200, Wolfgang Denk wrote:
We can write 0 or 0x0 or 000 or 0x00000 or (1 - 1) or NULL - all this does not change the behaviour of the code at all.
No doubt, but practice is to use NULL to mark invalid pointer, while address zero is simply "0". It won't affect generated code, but will make understanding it better.
Best regards, ladis
In message 20050914080947.GA22280@orphique you wrote:
We can write 0 or 0x0 or 000 or 0x00000 or (1 - 1) or NULL - all this does not change the behaviour of the code at all.
No doubt, but practice is to use NULL to mark invalid pointer, while address zero is simply "0". It won't affect generated code, but will make understanding it better.
OK, applied.
Best regards,
Wolfgang Denk
On 9/13/05, Wolfgang Denk wd@denx.de wrote:
ChangeLog:
- Patch by Andrew Dyer, 13 September 2005:
if CFG_ALT_MEMTEST is defined without CFG_MEMTEST_SCRATCH, cmd_mem.c code will dereference a null pointer. Change
No, it does not. It just points to address 0x0000. This is a perfectly valid address on many systems.
Not on MIPS (unless someone was daft enough to map it with the TLB). The code causes a TLB store miss exception. Since this is in 'common' code, I think it's a bug.
the code to use the last word of the memory test area as the scratch location in this case. Print the scratch memory location used.
Which problem is this patch supposed to fix in the first place? The existing code compiles and works fine on many, many systems.
There are two problems -
1) writing to address 0 is bad on MIPS and causes a crash.
2) I have a system with two banks of DRAM, 1 attached to the CPU directly and 1 hooked through an FPGA to do video stuff. Both are entirely accessable through the CPU, but via different external busses. I would like to be able to test either one with the same basic command.
With a static define of the scratch address I can't test one of the memories correctly becuase the scratch address is pointing to the other memory, so the scratch writes don't serve their purpose.
My solution was to just allocate a location in the test range as scratch if the static define isn't present. This guarantees the scratch address used is valid and in the bank of ram required.
In message c166aa9f05091309552efac7d@mail.gmail.com you wrote:
Not on MIPS (unless someone was daft enough to map it with the TLB). The code causes a TLB store miss exception. Since this is in 'common' code, I think it's a bug.
OK.
But all you need to do is #define CFG_MEMTEST_SCRATCH in your board config file, right?
With a static define of the scratch address I can't test one of the memories correctly becuase the scratch address is pointing to the other memory, so the scratch writes don't serve their purpose.
OK, but this is a special case, isn't it?
Would it be acceptable for you if we just move the initialization
dummy = (vu_long*)CFG_MEMTEST_SCRATCH;
below the computation of "end", so that you can use
#define CFG_MEMTEST_SCRATCH (--end)
in your board config file?
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message c166aa9f05091309552efac7d@mail.gmail.com you wrote:
Not on MIPS (unless someone was daft enough to map it with the TLB). The code causes a TLB store miss exception. Since this is in 'common' code, I think it's a bug.
OK.
But all you need to do is #define CFG_MEMTEST_SCRATCH in your board config file, right?
With a static define of the scratch address I can't test one of the memories correctly becuase the scratch address is pointing to the other memory, so the scratch writes don't serve their purpose.
OK, but this is a special case, isn't it?
Would it be acceptable for you if we just move the initialization
dummy = (vu_long*)CFG_MEMTEST_SCRATCH;
below the computation of "end", so that you can use
#define CFG_MEMTEST_SCRATCH (--end)
in your board config file?
Best regards,
Wolfgang Denk
I'm not sure you want that, exactly. You are affecting the variable "end" as a side effect in a macro which is generally a bad idea. A no side effect version would be: #define CFG_MEMTEST_SCRATCH (end - 1)
You are also making assumptions on the type of end (the variable, that is). I'm assuming this is a 32 bit wide memory test, sorry for not verifying that. You may want to coerce "end" to make sure the "- 1" subtracts the proper number of bytes: #define CFG_MEMTEST_SCRATCH ((void *)end - 1)
gvb
In message 43271DD7.1050003@smiths-aerospace.com you wrote:
#define CFG_MEMTEST_SCRATCH (--end)
...
I'm not sure you want that, exactly. You are affecting the variable "end" as a side effect in a macro which is generally a bad idea. A no side effect version would be: #define CFG_MEMTEST_SCRATCH (end - 1)
That would mean that *end is included in testing, and used as scratch cell at the same time --> will not work.
You are also making assumptions on the type of end (the variable, that
No. Just, that it has the same type as the "dummy" variable initilaized by CFG_MEMTEST_SCRATCH
#define CFG_MEMTEST_SCRATCH ((void *)end - 1)
That would be definitely worse.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 43271DD7.1050003@smiths-aerospace.com you wrote:
#define CFG_MEMTEST_SCRATCH (--end)
...
I'm not sure you want that, exactly. You are affecting the variable "end" as a side effect in a macro which is generally a bad idea. A no side effect version would be: #define CFG_MEMTEST_SCRATCH (end - 1)
That would mean that *end is included in testing, and used as scratch cell at the same time --> will not work.
You are also making assumptions on the type of end (the variable, that
No. Just, that it has the same type as the "dummy" variable initilaized by CFG_MEMTEST_SCRATCH
#define CFG_MEMTEST_SCRATCH ((void *)end - 1)
That would be definitely worse.
Best regards,
Wolfgang Denk
Ahh, you _did_ intend the side effect. You are being trickier than I gave you credit for. Please disregard my interference...
gvb
On 9/13/05, Wolfgang Denk wd@denx.de wrote:
Not on MIPS (unless someone was daft enough to map it with the TLB). The code causes a TLB store miss exception. Since this is in 'common' code, I think it's a bug.
OK.
But all you need to do is #define CFG_MEMTEST_SCRATCH in your board config file, right?
Since this is common code I would suggest requiring all boards that want to use CFG_ALT_MEMTEST be required to define CFG_MEMTEST_SCRATCH in their config and a #error if it's not defined. It seems like something likely to trip up the unaware. Attached is a revised patch that does this.
ChangeLog: * Patch by Andrew Dyer, 13 September 2005: in common/cmd_mem.c:do_mem_mtest():
Require CFG_MEMTEST_SCRATCH to be defined if CFG_ALT_MEMTEST is enabled instead of using the unsafe default of 0x00000000
Define CFG_MEMTEST_SCRATCH for all targets with CFG_ALT_MEMTEST enabled.
evaluate CFG_MEMTEST_SCRATCH after start and end are computed so CFG_MEMTEST_SCRATCH can use those values
Update ./README
Signed-off-by: Andrew Dyer amdyer@gmail.com
In message c166aa9f0509131326eaa6caf@mail.gmail.com you wrote:
Since this is common code I would suggest requiring all boards that want to use CFG_ALT_MEMTEST be required to define CFG_MEMTEST_SCRATCH in their config and a #error if it's not defined. It seems like
No. I reject this. The overwhelming majority of boards does not need this, so why enforce it?
something likely to trip up the unaware. Attached is a revised patch that does this.
I also reject the patch because you define a variable in the middle of the code. I will not accept such code.
Best regards,
Wolfgang Denk
participants (6)
-
Andrew Dyer -
Jerry Van Baren -
Ladislav Michl -
manju mahajan -
Sam Pham -
Wolfgang Denk