[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

Hi guys!

I present a proposal for dealing with the annoying:
#include "superio/vendor/model/early_serial.c"
present in virtually all romstage.c files.

The steps are as follows:
1) Declare a generic prototype: superio_enable_early_serial();
2) Remove #include */early_serial.c from romstage
3) add a romstage-$(CONFIG_THIS_SUPERIO) += early_serial.c to the
superio's Makefile.inc
4) Include the generic "superio/early_serial.h" definition in romstage.c
5) Include the superio specific header in romstage.c
6) Done

I'm demonstrating this with the fintek/f81865f superio, and the
amd/persimmon board. Besides discussion, this patch is also intended for
inclusion.

See patch for more details.

[PATCH] Proposal for dealing with superio *.c includes

[Patrick Georgi]

Am Samstag, 26. Februar 2011, 14:17:37 schrieb Alex G.:
> add a romstage-$(CONFIG_THIS_SUPERIO) += early_serial.c to the
>
> superio's Makefile.inc

This will fail for romcc boards, as for them, romstage must be compiled by a 
single romcc invocation.

I planned to work on this by having the build system generate a wrapper for 
romcc boards automatically  (essentially a file in $(obj) with romstage.c and 
other .c files #included, properly hidden from the dev), but that has some 
issues with the right order in which things must be included (romcc doesn't 
support forward declarations).

Patrick

[PATCH] Proposal for dealing with superio *.c includes

[Peter Stuge]

Alex G. wrote:
> Moves the inclusion of the superio early code from romstage.c in 
> the mainboard directory to Makefile.inc in the superio directory.

Will it work also for boards with more than one superio?

//Peter

[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

On 02/26/2011 11:16 PM, Peter Stuge wrote:
> Alex G. wrote:
> > Moves the inclusion of the superio early code from romstage.c in 
> > the mainboard directory to Makefile.inc in the superio directory.
>
> Will it work also for boards with more than one superio?

I'm very tempted (and sure) to say "no". That's why this is largely a
proposal, and you are welcome to extend it. I can only do one superio at
a time, and I definitely cannot do it all by myself.

A:
One other solution I had considered was to put the include in the
board's Makefile.inc, but, as you might have imagined, proved too messy.

B:
A fuzzy extension of this option is to declare
superio_enable_early_serial() as weak, and have the board select which
superio to use for serial output.

C:
Or, besides SUPERIO_VENDOR_NAME we can also have a Kconfig option
SUPERIO_VENDOR_NAME_HAS_EARLY_SERIAL, and base our decision of including
the early serial code in romstage based on the latter.

======================================================================
config BOARD_SPECIFIC_OPTIONS # dummy
    def_bool y
    [...]
    select SUPERIO_FINTEK_F81865F
    select SUPERIO_OTHERVENDOR_OTHERMODEL
    select SUPERIO_FINTEK_F81865F_HAS_EARLY_SERIAL
    [...]
======================================================================

romstage-$(CONFIG_SUPERIO_FINTEK_F81865F_HAS_EARLY_SERIAL) += early_serial.c

If the board has two identical superios, then the same file will get
included (only once). In this case, handling both superios will have to
be done in mainboard logic anyways.

D:
Just forget about it and never again worry about .c includes.

Alex

[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

Signed-off-by: Alexandru Gagniuc 
---
> C:
> Or, besides SUPERIO_VENDOR_NAME we can also have a Kconfig option
> SUPERIO_VENDOR_NAME_HAS_EARLY_SERIAL, and base our decision of including
> the early serial code in romstage based on the latter.

I don't like options A, B, and D, so I created a patch to demonstrate
option C. I wasn't able to pick on a multi-superio board, as I don't
know which of the supported boards has more than one, and I'm not too
enthusiasti about looking thorugh 200+ Kconfig files.

Let me know what you think.

Alex

[PATCH] Proposal for dealing with superio *.c includes

[Alex G.]

Option C with GPLv2+ licensing.

Alex