[PATCH] First step in converting W83977TF early serial from included to linked

[Keith Hui]

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" [1].

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.

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.

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.

Signed-off-by: Keith Hui

[1] http://www.coreboot.org/Infrastructure_Projects#Remove_.c_includes

On Mon, Feb 28, 2011 at 1:00 PM, Keith Hui  wrote:
> Hi all,
>
> In my pipeline is something more infrastructural, something different
> from what I usually do -
>
> I am converting early serial code for Winbond W83977TF from code
> included in romstage to compiled unit to be linked into romstage.
>
> 13 boards use this superio, some apparently still not converted to CAR
> (including one Intel server board). I have yet to complete running
> abuild on my change. Should I let it break when I submit it for review
> given earlier buzz suggesting we leave non-CAR boards behind?
>
> The reason I'm doing this is I want to do this for ITE IT8705F, which
> happens to have no board using it at the moment, but I want to test
> this approach on a board that I can boot test right now. Enter the
> P2B-LS and W83977TF. If this proof of concept works, we can start this
> conversion one superio at a time and nail down one more goal of the
> infrastructure refactoring.
>
> Thoughts and comments?

Thanks
Keith

[PATCH][RFC] First step in converting W83977TF early serial from included to linked

[Alex G.]

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

how to deal with large romstage size?

[zxy__1127]

Hi all,

After I add some memory initialisize code, the romstage is over 64K byte,and tne code can't run properly.
Then what should I do when romstage is over 64K byte? Is romstage's size limits to 64KB?

Thanks a lot!
2011-03-01

zxy__1127