Dear York Sun,
In message 1285691891-32700-1-git-send-email-yorksun@freescale.com you wrote:
Add weak functions to enable architecture depended preparation, address advancing, cleaning up and error handling.
Signed-off-by: York Sun yorksun@freescale.com
This needs some comments to explain the interfaces you are providing.
-int memory_post_test (int flags) +__attribute__((weak)) +int arch_memory_test_prepare(u32 *vstart, u32 *size, phys_addr_t *phys_offset)
phys_offset is unused here. Drop it?
- int ret = 0; bd_t *bd = gd->bd;
- unsigned long memsize = (bd->bi_memsize >= 256 << 20 ?
256 << 20 : bd->bi_memsize) - (1 << 20);
- *vstart = CONFIG_SYS_SDRAM_BASE;
Blank line after declarations, please.
- *size = (bd->bi_memsize >= 256 << 20 ?
256 << 20 : bd->bi_memsize) - (1 << 20);
- if (CONFIG_SYS_SDRAM_BASE + memsize > (ulong)bd)
memsize = (ulong)bd - CONFIG_SYS_SDRAM_BASE;
- if ((*vstart) + (*size) > (ulong)bd)
*size = (ulong)bd - CONFIG_SYS_SDRAM_BASE;
Should this eventually be
*size = (ulong)bd - *vstart; ??
+__attribute__((weak)) +int arch_memory_test_advance(u32 *vstart, u32 *size, phys_addr_t *phys_offset) +{
- return 1;
+}
Unused arguments?
Don't you get compiler warnings for these?
+__attribute__((weak)) +int arch_memory_test_cleanup(u32 *vstart, u32 *size, phys_addr_t *phys_offset) +{
- return 0;
+}
Ditto ?
+int memory_post_test(int flags) +{
- int ret = 0;
- phys_addr_t phys_offset = 0;
- u32 memsize, vstart;
- arch_memory_test_prepare(&vstart, &memsize, &phys_offset);
- do {
Empty line before the do, please.
if (flags & POST_SLOWTEST) {ret = memory_post_tests(vstart, memsize);} else { /* POST_NORMAL */unsigned long i;for (i = 0; i < (memsize >> 20) && ret == 0; i++) {if (ret == 0)ret = memory_post_tests(i << 20, 0x800);if (ret == 0)ret = memory_post_tests((i << 20) + 0xff800, 0x800);
Line too long.
}}- } while (!ret && !arch_memory_test_advance(&vstart, &memsize, &phys_offset));
- arch_memory_test_cleanup(&vstart, &memsize, &phys_offset);
Empty line before and after, please.
- if (ret)
arch_memory_failure_handle();
Empty line before.
Best regards,
Wolfgang Denk