
Hi, On 20 Jul 2007 at 14:31, Stefan Roese wrote:
On Thursday 12 July 2007, Stefan Roese wrote:
http://opensource.freescale.com/git?p=u-boot-coldfire.git;a=summary
Thanks. Looks very promising. All ColdFire users out there, please take a look at it and send comments. If I don't hear anything negative, I'll merge the repository into the u-boot-coldfire repository soon.
The merge is now done. It's now available in the u-boot-coldfire repository. Please give it a try and let me know if you see any problems.
Thanks.
After struggling with git again to get the correct tree, I finally could get the source but am still struggling to get it ported to my board. As I do not know when this will be done, here my comments "during porting".
First: the cleaned-up immap_5329.h and m5329.h look very nice! (although I think naming all registers according to their naming in the data sheet would make things easier - I am talking about SDRAM controller here, and e.g. GPIO ppdsdr)
However, I also have some comments:
First, I still think it would be possible to put together MCF532x and MCF537x into one source tree (and header files), as they only differ in some peripherals being present or not. TsiChung, you mentioned there are differences in cache handling - I can not find those? Please note that MCF5307 is indeed completely different, I am not relating to this one! In case we do so, the cpu "id" in cpu/mcf532x/cpu.c should be changed, because we would have to distinguish MCF5372/MCF5372L and MCF5373/MCF5373L.
Then, concerning UART: - is CFG_UART_PORT (in cpu_init.c) meant to be there? Configurable console port is great, I need it myself, but shouldn´t it be named CONFIG_CONSOLE_PORT or something? - is it useful to have the GPIO part of UART configuration in cpu_init.c? IIRC the console port should be changeable from the environment, then the GPIO setting for the UART should be in mcfuart, or at least somewhere accessible from there - why do we need #ifdef CONFIG_MCFSERIAL in mcfuart.c? - IMHO the GPIO initialization for UART3 is incorrect in cpu_init.c. This should be case 2: gpio->par_timer &= 0x0F; gpio->par_timer |= (GPIO_PAR_TIN3_URXD2 | GPIO_PAR_TIN2_UTXD2);
For the sync() function (needed for CFI): is it useful to have it for each board, or shouldn´t this be handled somewhere in lib_m68k?
in cpu/mcf532x/Makefile, should START = be START = start.o for start.o being correctly re-made if needed?
What is the sense of having CFG_SDRAM_CFG1 and all this stuff defined in the configuration file when you have the SDRAM configuration itself in the board-specific source code? In case of switching between DDR/SDR or mobile DDR you have to change the initialization sequence anyways...
Last but not least, there is a missing #endif at the bottom of mcfuart.h?!
I hope I got most of it correct now. Any comments welcome!
Best regards, Wolfgang