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.
Moves the inclusion of the superio early code from romstage.c in the mainboard directory to Makefile.inc in the superio directory. Adapted amd/persimmon board to fit this model. Designed to serve as an example on how to deal with all superio #include "*.c" directives in romstage.c Signed-off-by: Alexandru Gagniuc <[email protected]> --- Here's what I did to my svn: svn add src/include/superio svn mv src/superio/fintek/f81865/f81865f_early_serial.c src/superio/fintek/f81865/early_serial.c Index: src/include/superio/early_serial.h =================================================================== --- src/include/superio/early_serial.h (revision 0) +++ src/include/superio/early_serial.h (revision 0) @@ -0,0 +1,39 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2011 Alexandru Gagniuc <[email protected]> + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef SUPERIO_EARLY_SERIAL_H +#define SUPERIO_EARLY_SERIAL_H + +#include <arch/romcc_io.h> +#include <stdint.h> + +/** + * \brief Pre-RAM (romstage) serial port initialization + * + * Initializes the serial port early in the boot sequence. + * \n + * The actual definition is in each superio chip's early_serial.c. + * early_serial.c should be included by the Makefile.inc of the respective + * superio See \n + * src/superio/fintek/f81865/Makefile.inc \n + * for details + * + */ +void superio_enable_early_serial(device_t dev, u16 iobase); + +#endif /* SUPERIO_EARLY_SERIAL_H */ \ No newline at end of file Index: src/superio/fintek/f81865f/f81865f_early_serial.c =================================================================== --- src/superio/fintek/f81865f/f81865f_early_serial.c (revision 6380) +++ src/superio/fintek/f81865f/f81865f_early_serial.c (working copy) @@ -1,47 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2011 Advanced Micro Devices, Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA - */ - -/* Pre-RAM driver for the Fintek F81865F/FG Super I/O chip. */ - -#include <arch/romcc_io.h> -#include "f81865f.h" - -static void pnp_enter_conf_state(device_t dev) -{ - u16 port = dev >> 8; - outb(0x87, port); - outb(0x87, port); -} - -static void pnp_exit_conf_state(device_t dev) -{ - u16 port = dev >> 8; - outb(0xaa, port); -} - -static void f81865f_enable_serial(device_t dev, u16 iobase) -{ - pnp_enter_conf_state(dev); - pnp_set_logical_device(dev); - pnp_set_enable(dev, 0); - pnp_set_iobase(dev, PNP_IDX_IO0, iobase); - pnp_set_enable(dev, 1); - pnp_exit_conf_state(dev); -} Index: src/superio/fintek/f81865f/Makefile.inc =================================================================== --- src/superio/fintek/f81865f/Makefile.inc (revision 6380) +++ src/superio/fintek/f81865f/Makefile.inc (working copy) @@ -2,6 +2,7 @@ ## This file is part of the coreboot project. ## ## Copyright (C) 2011 Advanced Micro Devices, Inc. +## Copyright (C) 2011 Alexandru Gagniuc <[email protected]> ## ## This program is free software; you can redistribute it and/or modify ## it under the terms of the GNU General Public License as published by @@ -19,3 +20,8 @@ ## ramstage-$(CONFIG_SUPERIO_FINTEK_F81865F) += superio.c + +# Include early_serial.c automatically in romstage, so that including it in +# romstage.c as a "#include .c" is no longer necesarry + +romstage-$(CONFIG_SUPERIO_FINTEK_F81865F) += early_serial.c Index: src/superio/fintek/f81865f/early_serial.c =================================================================== --- src/superio/fintek/f81865f/early_serial.c (revision 6380) +++ src/superio/fintek/f81865f/early_serial.c (working copy) @@ -19,10 +19,14 @@ */ /* Pre-RAM driver for the Fintek F81865F/FG Super I/O chip. */ +#include <arch/io.h> /* <== Must be included before early_serial.h */ +#include <superio/early_serial.h> -#include <arch/romcc_io.h> +#include <device/pnp_def.h> + #include "f81865f.h" + static void pnp_enter_conf_state(device_t dev) { u16 port = dev >> 8; @@ -36,7 +40,7 @@ outb(0xaa, port); } -static void f81865f_enable_serial(device_t dev, u16 iobase) +void superio_enable_early_serial(device_t dev, u16 iobase) { pnp_enter_conf_state(dev); pnp_set_logical_device(dev); Index: src/mainboard/amd/persimmon/romstage.c =================================================================== --- src/mainboard/amd/persimmon/romstage.c (revision 6380) +++ src/mainboard/amd/persimmon/romstage.c (working copy) @@ -2,6 +2,7 @@ * This file is part of the coreboot project. * * Copyright (C) 2011 Advanced Micro Devices, Inc. + * Copyright (C) 2011 Alexandru Gagniuc <[email protected]> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -31,7 +32,8 @@ #include <console/loglevel.h> #include "agesawrapper.h" #include "cpu/x86/bist.h" -#include "superio/fintek/f81865f/f81865f_early_serial.c" +#include <superio/early_serial.h> +#include "superio/fintek/f81865f/f81865f.h" #include "cpu/x86/lapic/boot_cpu.c" #include "pc80/i8254.c" #include "pc80/i8259.c" @@ -52,7 +54,7 @@ sb_poweron_init(); post_code(0x31); - f81865f_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); + superio_enable_early_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); uart_init(); console_init(); } Index: README =================================================================== --- README (revision 6380) +++ README (working copy) @@ -1,3 +1,6 @@ +svn add src/include/superio +svn mv src/superio/fintek/f81865/f81865f_early_serial.c src/superio/fintek/f81865/early_serial.c + ------------------------------------------------------------------------------- coreboot README -------------------------------------------------------------------------------
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot

