Submitter | Stefan Reinauer |
---|---|
Date | 2010-09-01 11:30:55 |
Message ID | <4C7E396F.5080307@coresystems.de> |
Download | mbox | patch |
Permalink | /patch/1843/ |
State | Superseded |
Headers | show |
Comments
The only thing that worries me is this include.
from src/include/pc80/mc146818rtc.h:
#include <pc80/mc146818rtc_early.c>
It seems like usually when we make initobj, we stop including the c file.
Acked-by: Myles Watson <mylesgw@gmail.com>
Thanks,
Myles
On 9/1/10 5:26 PM, Myles Watson wrote: > The only thing that worries me is this include. > > from src/include/pc80/mc146818rtc.h: > #include <pc80/mc146818rtc_early.c> > > It seems like usually when we make initobj, we stop including the c file. Yes... Another problem though. It's guarded by #ifdef __ROMCC__ so it only gets included on romcc targets. We could remove the C include from there and move it to the romstage.c files, to make it more obvious which .c files get sucked in on the targets. Not sure what else we could do.. There are 4 other places where a .c file is included in a .h file: ./src/arch/i386/include/bootblock_common.h:#include <cpu/x86/lapic/boot_cpu.c> ./src/include/console/console.h:#include <pc80/serial.c> ./src/include/console/console.h:#include "lib/ne2k.c" ./src/include/console/console.h:#include <console/console.c> ./src/include/pc80/mc146818rtc.h:#include <pc80/mc146818rtc_early.c> In addition: - We have an unknown number of .h files that contain functions - We include .c files in 148 romstage.c files and in these: ./cpu/amd/car/post_cache_as_ram.c ./cpu/amd/dualcore/amd_sibling.c ./cpu/amd/dualcore/dualcore.c ./cpu/amd/model_10xxx/init_cpus.c ./cpu/amd/quadcore/amd_sibling.c ./cpu/amd/quadcore/quadcore.c ./cpu/amd/sc520/raminit.c ./drivers/ati/ragexl/xlinit.c ./lib/lzma.c ./mainboard/getac/p470/mainboard.c ./mainboard/getac/p470/mainboard_smi.c ./northbridge/amd/amdfam10/bootblock.c ./northbridge/amd/amdfam10/debug.c ./northbridge/amd/amdfam10/northbridge.c ./northbridge/amd/amdfam10/raminit_amdmct.c ./northbridge/amd/amdht/h3finit.c ./northbridge/amd/amdht/ht_wrapper.c ./northbridge/amd/amdk8/raminit_f.c ./northbridge/amd/amdk8/raminit_test.c ./northbridge/intel/i82830/raminit.c ./northbridge/intel/i945/early_init.c ./northbridge/intel/i945/raminit.c ./northbridge/via/vx800/raminit.c ./pc80/usbdebug_serial.c ./pc80/vga/vga.c ./southbridge/amd/amd8111/amd8111_reset.c ./southbridge/amd/amd8111/bootblock.c ./southbridge/amd/cs5530/cs5530_vga.c ./southbridge/amd/sb600/sb600_early_setup.c ./southbridge/amd/sb600/sb600_reset.c ./southbridge/amd/sb600/sb600_sm.c ./southbridge/amd/sb700/sb700_early_setup.c ./southbridge/amd/sb700/sb700_reset.c ./southbridge/amd/sb700/sb700_sm.c ./southbridge/broadcom/bcm5785/bcm5785_early_setup.c ./southbridge/broadcom/bcm5785/bcm5785_reset.c ./southbridge/broadcom/bcm5785/bootblock.c ./southbridge/intel/i82801dx/i82801dx_smihandler.c ./southbridge/intel/i82801gx/i82801gx_smihandler.c ./southbridge/nvidia/ck804/ck804_reset.c ./southbridge/nvidia/mcp55/mcp55_reset.c ./southbridge/sis/sis966/sis966_reset.c Stefan
On Wed, Sep 1, 2010 at 10:20 AM, Stefan Reinauer <stefan.reinauer@coresystems.de> wrote: > On 9/1/10 5:26 PM, Myles Watson wrote: >> The only thing that worries me is this include. >> >> from src/include/pc80/mc146818rtc.h: >> #include <pc80/mc146818rtc_early.c> >> >> It seems like usually when we make initobj, we stop including the c file. > Yes... Another problem though. Yes. A problem for another day. I was just noting that it made it harder to follow. Thanks, Myles
Patch
Index: src/include/pc80/mc146818rtc.h =================================================================== --- src/include/pc80/mc146818rtc.h (revision 5761) +++ src/include/pc80/mc146818rtc.h (working copy) @@ -81,14 +81,6 @@ #define PC_CKS_RANGE_END 45 #define PC_CKS_LOC 46 -/* coreboot cmos checksum is usually only built over bytes 49..125 - * LB_CKS_RANGE_START, LB_CKS_RANGE_END and LB_CKS_LOC are defined - * in option_table.h - */ -#if CONFIG_HAVE_OPTION_TABLE -#include <option_table.h> -#endif - #ifndef UTIL_BUILD_OPTION_TABLE #include <arch/io.h> static inline unsigned char cmos_read(unsigned char addr) Index: src/mainboard/amd/dbm690t/Kconfig =================================================================== --- src/mainboard/amd/dbm690t/Kconfig (revision 5761) +++ src/mainboard/amd/dbm690t/Kconfig (working copy) @@ -25,12 +25,6 @@ string default amd/dbm690t -# This is a temporary fix, and should be removed when the race condition for -# building option_table.h is fixed. -config WARNINGS_ARE_ERRORS - bool - default n - config DCACHE_RAM_BASE hex default 0xc8000 Index: src/pc80/mc146818rtc_early.c =================================================================== --- src/pc80/mc146818rtc_early.c (revision 5761) +++ src/pc80/mc146818rtc_early.c (working copy) @@ -1,6 +1,10 @@ #include <pc80/mc146818rtc.h> #include <fallback.h> +#if CONFIG_USE_OPTION_TABLE +#include <option_table.h> +#endif + #ifndef CONFIG_MAX_REBOOT_CNT #error "CONFIG_MAX_REBOOT_CNT not defined" #endif Index: src/pc80/Makefile.inc =================================================================== --- src/pc80/Makefile.inc (revision 5761) +++ src/pc80/Makefile.inc (working copy) @@ -8,3 +8,4 @@ subdirs-y += vga $(obj)/pc80/mc146818rtc.o : $(OPTION_TABLE_H) +$(obj)/pc80/mc146818rtc_early.initobj.o : $(OPTION_TABLE_H) Index: src/pc80/mc146818rtc.c =================================================================== --- src/pc80/mc146818rtc.c (revision 5761) +++ src/pc80/mc146818rtc.c (working copy) @@ -2,6 +2,9 @@ #include <pc80/mc146818rtc.h> #include <boot/coreboot_tables.h> #include <string.h> +#if CONFIG_USE_OPTION_TABLE +#include <option_table.h> +#endif /* control registers - Moto names */
See patch. Fix race condition in option_table.h generation by moving the include statement to those files that actually need it. This significantly reduces the number of dependencies, so it's no longer extremely ugly to specify them manually (see the src/pc80/Makefile.inc portion) Also, drop the AMD DBM690T work around for the issue. Signed-off-by: Stefan Reinauer <stepan@coresystems.de>