[U-Boot] [PATCH] [ARM] Move machine specific code to board at s3c64xx

[Kyungmin Park]

On Thu, Oct 23, 2008 at 4:39 PM, Guennadi Liakhovetski <lg at denx.de> wrote:
> On Wed, 22 Oct 2008, Kyungmin Park wrote:
>
>> Move machine specific code to smdk6400.
>> Some board use OneNAND instead of NAND.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
>> ---
>> diff --git a/board/samsung/smdk6400/lowlevel_init.S b/board/samsung/smdk6400/lowlevel_init.S
>> index e0119a7..53d9125 100644
>> --- a/board/samsung/smdk6400/lowlevel_init.S
>> +++ b/board/samsung/smdk6400/lowlevel_init.S
>> @@ -104,6 +104,13 @@ lowlevel_init:
>>       bl nand_asm_init
>>  #endif
>>
>> +     /* Memory subsystem address 0x7e00f120 */
>> +     ldr     r0, =ELFIN_MEM_SYS_CFG
>> +
>> +     /* Xm0CSn2 = NFCON CS0, Xm0CSn3 = NFCON CS1 */
>> +     mov     r1, #0xd
>> +     str     r1, [r0]
>> +
>
> Hm, no, I don't quite agree. In principle, yes, this configuration can be
> considered platform-specific and moving it this way of course works. But:
>
> 1. The patch comment is not correct. This code doesn't select between NAND
>   and OneNAND. It selects between (one of) NANDs and SROMs.

Yes I just move to platform since it's board specific.

>
> 2. While at it, we could fix the value being written to the MEM_SYS_CFG
>   register too. Currently it writes 0xd =
>
>  (1 << 0) - ignored, default 0, so, better set it to 0
> | (0 << 1) - set Xm0CSn[2] to OneNANDC CS0 or NFCON CS0
> | (1 << 2) - ignored, default 0, so, better set it to 0
> | (1 << 3) - set Xm0CSn[3] to SROMC CS3
>
> So, we should just write an 8 in it:
>
> +       mov     r1, #0x8
> +       str     r1, [r0]
>
> 3. The comment in the code doesn't look right. According to the above it
>   should read
>
> +       /* Xm0CSn[2] = OneNANDC CS0 or NFCON CS0, Xm0CSn[3] = SROMC CS3 */

Right, and also add OneNAND & NFCON is depends on XNANDSEL.

As you know mem_ctrl_asm_init is common code and other boards can use
it without board specific codes.

In OneNAND board, it should be set as 0x1002

>
> The only thing that confuses me, is why the author, belonging to the
> manufacturer of the chip, hasn't done this. Maybe the documentation is
> wrong?
>

Maybe he missed it. Document is right.

Thank you,
Kyungmin Park