While I think this patch is great and we definitely need it, there are some things I'd like to discuss and improve, or back out if possible...
On 3/27/10 6:18 PM, repository service wrote:
-all: $(obj)/config.h coreboot +all: $(obj)/config.h $(obj)/build.h coreboot endif
This will make $(obj)/build.h and coreboot in parallel I think... is that intended?
-$(obj)/$$(1)%$(3).o: src/$$(1)%.$(2) $(obj)/config.h +$(obj)/$$(1)%$(3).o: src/$$(1)%.$(2) | $(obj)/build.h $(obj)/config.h printf " CC $$$$(subst $$$$(obj)/,,$$$$(@))\n"
Uh what's that | $(obj)/build.h ?
OBJS := $(patsubst %,$(obj)/%,$(TARGETS-y)) -INCLUDES := -I$(top)/src -I$(top)/src/include -I$(obj) -I$(top)/src/arch/$(ARCHDIR-y)/include -INCLUDES += -I$(top)/src/devices/oprom/include -INCLUDES += -include $(obj)/config.h +INCLUDES := -Isrc -Isrc/include -I$(obj) -Isrc/arch/$(ARCHDIR-y)/include +INCLUDES += -Isrc/devices/oprom/include +# abspath is a workaround for romcc +INCLUDES += -include $(abspath $(obj)/config.h) -include $(abspath $(obj)/build.h)
Why is build.h added to INCLUDES? This basically renders the dependency system unusable because all of the C files are depending on build.h (which they are not) Only two files should depend on build.h and that's console.o and romstage.inc
@@ -195,7 +193,7 @@
$(obj)/mainboard/$(MAINBOARDDIR)/romstage.inc: $(src)/mainboard/$(MAINBOARDDIR)/romstage.c $(obj)/romcc $(OPTION_TABLE_H) $(obj)/build.h printf " ROMCC romstage.inc\n"
- $(ROMCC) -c -S $(ROMCCFLAGS) -include $(obj)/build.h -I. $(INCLUDES) $< -o $@
- $(ROMCC) -c -S $(ROMCCFLAGS) -I. $(INCLUDES) $< -o $@
Why is build.h dropped here?
@@ -205,7 +203,7 @@
$(obj)/mainboard/$(MAINBOARDDIR)/romstage.pre.inc: $(src)/mainboard/$(MAINBOARDDIR)/romstage.c $(OPTION_TABLE_H) $(obj)/build.h printf " CC romstage.inc\n"
- $(CC) -MMD $(CFLAGS) -include $(obj)/build.h -I$(src) -I. -c -S $< -o $@
- $(CC) -MMD $(CFLAGS) -I$(src) -I. -c -S $< -o $@
Why is build.h dropped here?
Modified: trunk/src/arch/i386/lib/Makefile.inc
--- trunk/src/arch/i386/lib/Makefile.inc Fri Mar 26 19:31:12 2010 (r5303) +++ trunk/src/arch/i386/lib/Makefile.inc Sat Mar 27 18:18:39 2010 (r5304) @@ -8,8 +8,3 @@
initobj-y += printk_init.o initobj-y += cbfs_and_run.o
-ifdef POST_EVALUATION -$(obj)/arch/i386/lib/console.o :: $(obj)/build.h -endif
I don't think it's OK to drop this dependency, as it needs to be known before gcc is run on the binary.
Stefan