[U-Boot] [PATCH v2] s5pc1xx: Add support for Samsung Goni board

[Minkyu Kang]

Dear Wolfgang,

On 31 May 2010 19:38, Wolfgang Denk <wd at denx.de> wrote:
> Dear Minkyu Kang,
>
> In message <4C0309E2.2030206 at samsung.com> you wrote:
>> This patch adds support for the Samsung Goni board (S5PC110 SoC)
>>
>> Signed-off-by: Minkyu Kang <mk7.kang at samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -628,6 +628,7 @@ Simon Kagstrom <simon.kagstrom at netinsight.net>
>>
>>  Minkyu Kang <mk7.kang at samsung.com>
>>
>> +     s5p_goni        ARM CORTEX-A8 (S5PC110 SoC)
>>       SMDKC100        ARM CORTEX-A8 (S5PC100 SoC)
>>
>>  Nishant Kamat <nskamat at 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 at denx.de
> "...and the fully armed nuclear warheads, are, of  course,  merely  a
> courtesy detail."
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Thanks.
Minkyu Kang
-- 
from. prom.
www.promsoft.net