[coreboot] patch: dbe62

Peter Stuge peter at stuge.se
Fri Feb 29 14:41:31 CET 2008


On Thu, Feb 28, 2008 at 06:53:39PM -0800, ron minnich wrote:
> On Thu, Feb 28, 2008 at 5:00 PM, Peter Stuge <peter at stuge.se> wrote:
> 
> >  > +     help
> >  > +       This is the default mainboard name.
> >
> >  What's a default name?
> 
> you'll have to ask the guy who wrote that Kconfig file. I'm just a
> humble intern.

:) Change the help text to something better then.


> >  > +void mainboard_pre_payload(void)
> >  > +{
> >  > +     geode_pre_payload();
> >  > +     banner(BIOS_DEBUG, "mainboard_pre_payload: done");
> >  > +}
> >
> >  Why do we need this mainboard code when it is only calling a
> >  function that can be determined using the dts?
> 
> Show me how. I don't know.

arch/x86/stage1.c currently calls mainboard_pre_payload() - for
Geode LX the call has nothing to do with the mainboard and
everything to do with the north.

geode_pre_payload() is defined in geodelx/geodelxinit.c, so one easy
way is to rename it to northbridge_amd_geodelx_pre_payload() and
generate the call from data in the dts.

Another way would be to add it to struct device_operations. I like
that better actually.


> >  > +unsigned long write_pirq_routing_table(unsigned long addr)
> >
> >  What an abomination. hint hint ;)
> >
> >  Can this code really not live in northbridge/ ?
> 
> no, it's a south function, with mainboard-dependent bits. I'm happy
> to see a way to make it generic, I just don't know how yet.

Is the _code_ really mainboard dependent? I would think it's device
dependent, and the board only changes input values. Am I off?


> >  Yes! So completely on track. :)
> 
> are you happy or unhappy here? Just want to make sure ...

Very happy! I think v3 is really excellent save for a few things
that I make much noise about.


//Peter




More information about the coreboot mailing list