On 29.02.2008 17:52, ron minnich wrote:
On Fri, Feb 29, 2008 at 5:41 AM, Peter Stuge peter@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).
Perhaps call it "launcher" stage? On the other hand, it is part of the bootblock (so either stage0 or stage1) and it is after CAR has been switched on (so definitely stage1). I vote to keep the name until we figure out something better.
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.
- 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.
What we have now is OK, but there is room for improvement.
- 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.
5. Maybe the name of mainboard_pre_payload() is unfortunate and we should have pre_payload() which calls cpu_pre_payload(), chipset_pre_payload() and mainboard_pre_payload() in a row. That way, we don't have to put any GeodeLX ROM cache disabling below mainboard() and keep that stuff local to the southbridge code.
+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.
Peter's idea was probably to separate code from data and I think there is some potential in that idea. write_pirq_routing_table() is identical for alix1c and dbe62 and could in theory really live in the south directory. AFAICS this wouldn't even require any changes in the function.
Regards, Carl-Daniel