[coreboot] [PATCH 3/5] artecgroup/dbe61: Gather RAM initialization function calls to one helper function.

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Nov 13 04:32:30 CET 2008


On 13.11.2008 04:12, Mart Raudsepp wrote:
> On N, 2008-11-13 at 03:11 +0100, Carl-Daniel Hailfinger wrote:
>   
>> On 13.11.2008 02:41, Peter Stuge wrote:
>>     
>>> If a board is unique and we can't abstract in general code then
>>> mainboard/ code is fine.
>>>   
>>>       
>> If the abstraction works for all the other boards, there is no reason to
>> have different codebases except to confuse people who want to port a new
>> similar board.
>>
>> We have lots of places in v2 where people made some change to one
>> specific board, but the change would have applied to lots of boards.
>> Later on, nobody could recall offhand why the files were different. To
>> be honest, the amount of code duplication we have in v2 with little
>> arbitrary changes sprinkled all over the map is one of the biggest
>> reasons why I try to avoid v2 wherever possible. Diffing two boards will
>> usually give you lots of differences which are in no way related to the
>> board configuration/hardware.
>>     
>
> The difference here is that in DBE61 I call that sequence possibly
> multiple times, while for all others that is not the case, and it's just
> an unnecessary function call.
>
> I suppose we could add such a helper function for all boards if an
> inline keyword is added for it.
>
> That said, there's actually some things to improve there regarding what
> is called.
> For instance cpu_reg_init does many things, and only one part is memory
> related - at least where the SPD matters. Some other bits and pieces are
> also re-done on second call that might not be necessary.
> So I'm not sure if in the long run it's suitable for all boards.
> Perhaps that initialize_ram deal could end up in generic code even if
> the order of those operations has to always be like that (does it?).
>   

You're right AFAICS.

The GeodeLX RAM init code also suffers from ROMCC mentality. Stuff is
recalculated instead of being passed as function parameters. A prime
example is the check whether a SPD is in a given slot. Moving that check
outside the functions would probably kill a dozen lines of code, if not
more.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list