Dear York Sun,
In message <[email protected]> you wrote:
> Add weak functions to enable architecture depended preparation, address
> advancing, cleaning up and error handling.
>
> Signed-off-by: York Sun <[email protected]>
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);
> /* Limit area to be tested with the board info struct */
> - 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();
> return ret;
Empty line before.
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: [email protected]
It is much easier to suggest solutions when you know nothing
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
