Dear Wolfgang,
On 31 May 2010 19:38, Wolfgang Denk wd@denx.de wrote:
Dear Minkyu Kang,
In message 4C0309E2.2030206@samsung.com you wrote:
This patch adds support for the Samsung Goni board (S5PC110 SoC)
Signed-off-by: Minkyu Kang mk7.kang@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
--- a/MAINTAINERS +++ b/MAINTAINERS @@ -628,6 +628,7 @@ Simon Kagstrom simon.kagstrom@netinsight.net
Minkyu Kang mk7.kang@samsung.com
- s5p_goni ARM CORTEX-A8 (S5PC110 SoC)
SMDKC100 ARM CORTEX-A8 (S5PC100 SoC)
Nishant Kamat nskamat@ti.com
Please keep lists sorted (kang > Kamat).
I'll fix it.
...
+#ifdef CONFIG_DISPLAY_BOARDINFO +int checkboard(void) +{
- printf("Board:\tGoni\n");
Please consider using puts() when you don't do any print formatting.
OK.
index 0000000..4b72992 --- /dev/null +++ b/board/samsung/goni/lowlevel_init.S @@ -0,0 +1,585 @@ +/*
- Memory Setup stuff - taken from blob memsetup.S
Is it really necessary to code all this in assembler?
+/*
- uart_asm_init: Initialize UART's pins
- */
+uart_asm_init:
For example, why cannot this be part of the UART driver and written in C instead?
UART setting is possible to write the C code, but sometimes it is useful for debugging before jump the C codes.
index 0000000..6239444 --- /dev/null +++ b/include/configs/s5p_goni.h
...
+#undef CONFIG_CMD_BOOTD +#undef CONFIG_CMD_CONSOLE +#undef CONFIG_CMD_ECHO +#undef CONFIG_CMD_FPGA +#undef CONFIG_CMD_ITEST +#undef CONFIG_CMD_FLASH +#undef CONFIG_CMD_IMLS +#undef CONFIG_CMD_LOADB +#undef CONFIG_CMD_LOADS +#undef CONFIG_CMD_NAND +#undef CONFIG_CMD_MISC +#undef CONFIG_CMD_NFS +#undef CONFIG_CMD_SOURCE +#undef CONFIG_CMD_XIMG +#undef CONFIG_CMD_NET +#undef CONFIG_XYZMODEM
Many of these commands are pretty useful to the end user. Is there a special reason for disabling these here? It seems you are not especially restricted in terms of resources, so I don't understand why you disable these.
Goni doesn't have NOR flash, NAND flash and Network device. So, I disabled about flash, nand and network commands. Others are for reducing image size. (about 10KiB) Anyway, I'll enable some useful commands.
+/*-----------------------------------------------------------------------
- Stack sizes
- The stack sizes are set up in start.S using the settings below
- */
Incorrect multiline comment style; please fix globally.
OK.
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: wd@denx.de "...and the fully armed nuclear warheads, are, of course, merely a courtesy detail." _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks. Minkyu Kang