[coreboot] patch: dbe62

ron minnich rminnich at gmail.com
Fri Feb 29 17:52:28 CET 2008


On Fri, Feb 29, 2008 at 5:41 AM, Peter Stuge <peter at stuge.se> wrote:

>  > >  > +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.


Yep, but here you've hit a limitation in the design which we never
thought of. Device stuff is in stage 2. But this mainboard_early stuff
is in main (btw, we're going to need to rename arch/x86/stage1.c again
-- since it calls stage2 and stage2 returns to it, it's not really a
stage1 at all. I believe the name change was my mistake).

The device tree is in stage 2. It goes away at the end of stage2 since
we didn't see a need for it. The data that it creates is gone at that
point.

OK, it gets worse. You REALLY don't want to do this call until just
about the last second, after payload is up and ready to go, since it
disables ROM caching. That's a long time after stage 2. You can't do
this call in stage2.

There are several options here but most of them are not that great.
1. add phase7 to stage 2 in device tree and return dynamic device tree
from stage 2 to caller.
stage2 phase 7 is now called AFTER payload is loaded and BEFORE
payload is jumped to. This breaks our nice stage design but that's
life. Big problem -- where does device tree go in memory and what do
we do if it interferes with payload location?
2. what we have now -- it works.
3. name a pre-payload in the dts, have dtc build the
mainboard_pre_payload function for us, call stays as it is now.
4. Linker sets -- Oh, yuck, a lot of work went into v3 to AVOID linker sets.

Let the comments begin!

>
>
>
>  > >  > +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?

Sure, but it's almost always a southbridge thing. But you need to
rewrite the code quite a bit to move it to the south directory, and
I'm not sure it's worth it in the end -- having it in mainboard allows
you to deal with mainboard bugs and problems very easily.

ron




More information about the coreboot mailing list