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/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
