[coreboot] [commit] r5304 - in trunk: . src/arch/i386 src/arch/i386/lib src/cpu/x86/smm util/abuild util/cbfstool

[Stefan Reinauer]

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