On 03/01/2011 07:20 AM, Keith Hui wrote:
And here is the patch. abuild-tested. I will boot test it with P2B-LS and P3B-F tomorrow but I want this patch out there to generate some discussions and get more boot test coverage.
OK.
This I believe falls under "infrastructure projects" [1].
Indeed, and for that reason people should be _more_ involved and open to discuss this issue, not just leave it aside and ignore it. I'm glad you're at least thinking of that.
This patch facilitates, for boards using Winbond W83977TF superio, dropping early_serial.c from #includes of their romstage, instead linking to them; and converts 12 of 13 mainboards using this superio to do exactly this. The lone board out, iei/nova4899r, is a Geode/CS5530 board that has not yet been converted to CAR or tiny bootblock. The rest are all slot 1/440BX/i82371eb boards that have been converted. At this stage I should leave converting CS5530 to tiny bootblock to someone better versed in this platform.
Definitely leave it out for now. Converting that to CAR is for a different patch, though I think the tanks will expect you to convert it.
The pnp_... functions defined in romcc_io.h now have a new home in arch/x86/lib/pnp.c, and is compiled and linked like any other C files meant for romstage.
I do not like this. I would prefer to have the pnp functions declared inline in a header file. It's more elegant for functions this small, and avoids call/ret.
This patch puts the W83977TF early serial code in a state where it can be incorporated both through the old setup (ie. #included in mainboard romstage), or be compiled separately and linked into romstage. Once this conversion is done on all superios and all southbridges/boards have been converted, the few #ifdefs that made this possible can be cleaned out.
Don't worry about new board using this superio. If you broke the one non-CAR board using this superio, leave it broken and ignore all the torro-fecal matter surrounding the issue of the old build system. If you think you can convert that board to CAR (and have the time), definitely go for it. :)
Index: src/include/device/device.h
--- src/include/device/device.h (revision 6411) +++ src/include/device/device.h (working copy) @@ -7,7 +7,15 @@
struct device;
+// In ramstage, device_t will be a structure. +#if defined(__PRE_RAM__) && !defined(__ROMCC__) +typedef unsigned device_t;
OK. I can see where this is going. I'm still not convinced it is the best option, but I don't have a suggestion on this.
Index: src/superio/winbond/w83977tf/Makefile.inc
--- src/superio/winbond/w83977tf/Makefile.inc (revision 6411) +++ src/superio/winbond/w83977tf/Makefile.inc (working copy) @@ -22,3 +22,5 @@
ramstage-$(CONFIG_SUPERIO_WINBOND_W83977TF) += superio.c
+romstage-$(CONFIG_SUPERIO_WINBOND_W83977TF) += early_serial.c +romstage-$(CONFIG_SUPERIO_WINBOND_W83977TF) += ../../../arch/x86/lib/pnp.c
This last line is ugly. Just keep the functions inlined. pnp.c is unnecessarry.
Index: src/superio/winbond/w83977tf/early_serial.c
--- src/superio/winbond/w83977tf/early_serial.c (revision 6411) +++ src/superio/winbond/w83977tf/early_serial.c (working copy) @@ -20,7 +20,12 @@
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#if defined(__ROMCC__) #include <arch/romcc_io.h> +#else +#include <device/pnp.h> +#include <arch/io.h> +#endif
Keith, nothing here for you, move along. For others, why can't we just get rid of romcc altogether?.
Index: src/arch/x86/lib/pnp.c
AAAAAAAAAAAAAAAAARGH!
Another thing we need to discuss (THAT MEANS __*EVERYBODY*__, NOT JUST ME AND KEITH) is whether or not we should declare the enable_early_serial() in a common place, and just have the superio code implement it. I prefer common name option, as it makes the superio code more transparent to the developer.
Alex