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

[Darius Augulis]

Hi Albert,

On 12/01/2010 08:42 AM, Albert ARIBAUD wrote:
> Hi Darius,
>
> Le 30/11/2010 10:19, Darius Augulis a écrit :
>
>> 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.
>
> These rules apply to new code; existing code can just remain the same,
> precisely to avoid rewrites.
>
>> 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.
>
> There are exceptions for code ported from Linux, IIRC.
>
>> 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.
>
> That's only one dummy variable, an array of 100 elements. Plus it
> *shows* there is a gap, which when dealing with datasheets is a plus.
>
>> 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...
>
> One of the interests of C structs is that they guarantee against errors
> in typing register offsets, for instance.

really? All is fine until there are no lot of gaps between registers.
For example - s3c6410 gpio register bank has lot of random gaps. Size of 
gaps vary from 8 to 1744 bytes randomly. I tried to make such structure, 
but I gave up calculating(!) how many dummy variables I have to put in 
particular gap. Also gpio port registers are mixed with other special 
registers, port alphabetical order do not fit offset order, etc... There 
is 1000x times bigger possibility to do an error. Because most 
datasheets provide simple table of register names and offsets near them. 
You must be almost blind to make error during this simple copy paste 
procedure from pdf to C file. And I got crazy and spend 2 hours trying 
to build one single structure for gpio registers. It's absolutely 
stupid. And it's only gpio - what about other xx SoC modules? I've 
ported fb driver in shorter time than I wasted for this worthless work.
I think u-boot maintainers make big mistake with this rule. I'm afraid I 
will stop my work adding mini6410 and other boards to mainline, because 
I don't have time for such games with C structures for SoC registers.
You suggest to spend lot of hours in such place, where it's enough 2 
minutes. I need to write few registers, but instead of defining them 
(few lines of code) I must calculate, and build entire structure with 
hundreds of registers... Sorry, I wont do this.
Perhaps this and similar strict rules, especially those which do not add 
any improvement, forces lot of people to have their own modified copies 
of u-boot. You lose community...

regards,
Darius


>
>> 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.
>
> Please bear with it, just like we all bear with e.g. coding style rules
> from checkpatch.pl even though we would personally do it another way.
>
>> thanks,
>> Darius
>
> Amicalement,