[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.:
  3) 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 <mr.nuke.me(a)gmail.com>
---
  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