[coreboot] [PATCH][RFC] First step in converting W83977TF early serial from included to linked
mr.nuke.me at gmail.com
Tue Mar 1 10:19:09 CET 2011
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.
> This I believe falls under "infrastructure projects" .
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
> 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
> 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>
> +#include <device/pnp.h>
> +#include <arch/io.h>
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
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.
More information about the coreboot