Dear Wolfgang,

thanks for the review,

On Fri, Jan 25, 2013 at 01:30:54PM +0100, Wolfgang Denk wrote:

Dear Angelo Dureghello,

In message 20130125104307.GA3223@sion.sysam you wrote:

...

Add support for Sysam AMCORE mcf5307 (coldfire) based board.

Sorry for the late catch - this escaped me so far...

...

Changes for v5:

• Fix MAINTAINERS bad sorted entry
• Fix incorrect indentation
• Remove #undef where not needed

Did you? I still see some...

...

+#if defined(CONFIG_SYS_DRAM_TEST) +/* memory test */ +int testdram(void)

...

...

+#endif

This is:

1. dead code, as you don't define CONFIG_SYS_DRAM_TEST
2. an obsolete approach that you should not use. We have a several different (and much more reliable) memory tests already - if yuou really need one, then please use the existing code. On the other hand, you are using get_ram_size, which already includes a simple memory test - so re you sure you really need an extra test?

...

+#undef CONFIG_SYS_DRAM_TEST /* default undef */

I used the sdram test for some help when soldering here the prototypes. Then i disabled the test. As you already asked me, i removed my own sdram test function and used one already used from other boards, and added this in the amcore.c top coments.

* Copy memory testdram() from sandburst/common/sb_common.c

Anyway, as you said, get_ram_size is enough for me. I will remove CONFIG_SYS_DRAM_TEST and so "int testdram(void)" content.

==> remove.

...

+/* bypass PLL for test purpose */ +#undef CONFIG_SYS_PLL_BYPASS

???

I apologize, forget out, fixed.

If you haven't seen anything else, i'll post next V6 shortly.

Best Regards, Angelo Dureghello