[U-Boot] [RFC PATCH] ARM: S3C64XX: add support for mini6410

[Darius Augulis]

Hi,

On Tue, Nov 30, 2010 at 10:50 AM, Minkyu Kang <promsoft at gmail.com> wrote:
> Dear Darius Augulis,
>
> On 30 November 2010 17:17, Darius Augulis <augulis.darius at gmail.com> wrote:
>> Hi,
>>
>> thank you for review. Please find my questions inline:
>>
>>>> +
>>>> +static void dm9000_pre_init(void)
>>>> +{
>>>> +       SROM_BW_REG &= ~(0xf << 4);
>>>
>>> u-boot don't allow it.
>>> Please use read/write function.
>>> And please access the register by C structure.
>>
>> it's clear about read/write. But why C structure? We have all register
>> definitions in header file.
>
> It's old style.

but not bad.

>
>> What are advantages of C structure? Many boards use definitions and I
>> like it. Why to change?
>
> It's mandatory.
> Please see below link. (explained well)
> http://www.mail-archive.com/u-boot@lists.denx.de/msg11583.html

I understand, that there is such rule in u-boot, but it's ... at least strange.
Even linux kernel doesn't use such approach and uses simple defines instead.
One big disadvantage for already existing platforms - one must convert
all defines to C structure.
Remove almost everything from header file, because no need to have
duplicated code.
And what about code porting between linux kernel and u-boot? With
definitions in header file it's easy copy paste procedure.
What if I need two registers separated with big gap from one big 100
register bank? Then I need to build huge C structure with many dummy
variables. Lot of lines... In this case, possibility to make a dummy
error is bigger. IMHO C structure for register definition is not
better than definitions in header file. It's worse.
We could have much tiny, simply and more readable code with definition
in header file. And this approach has been used for years and still
used in linux kernel. Perhaps, not without reason...
If u-boot maintainers are really going not accept this patch because
of this, I will change it. But it's meaningless and very annoying.

thanks,
Darius

>
>>
>>>> +#define CONFIG_CMDLINE_EDITING
>>>> +#define CONFIG_BAUDRATE                        115200
>>>> +#define CONFIG_SYS_BAUDRATE_TABLE      { 9600, 19200, 38400, 57600, 115200 }
>>>> +#define CONFIG_BOOTCOMMAND             "nand read 50100000 100000 300000; bootm 50100000"
>>>
>>> If you load the uImage to 0x500fffc0 (0x50100000 - 0x40), then you can
>>> reduce the boot time (about 0.5 sec?).
>>
>> why? please explain.
>
> Please see this patch.
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=fca0cecff73db99d99ad094cca7980472b8a11b5
>
> If  load address and image start address are same address, then
> memmove is unnecessary.
> Because of u-boot header, we should load the image to start address -
> 0x40 (size of u-boot header).
> This is not mandatory.
> I just gave you some tips
>
>>
>> thanks,
>> Darius
>>
>
> Thanks
> Minkyu Kang
> --
> from. prom.
> www.promsoft.net
>