[coreboot] buildrom bug

[Peter Stuge]

On Fri, Apr 25, 2008 at 03:52:43PM -0600, Jordan Crouse wrote:
> +		b=`echo $$file | sed -e s:$(ROM_DIR)\/*::`; \

Should the expression include ^ somehow so as to not fail on subdirs
with matching names?

Or just use basename?


> +# FIXME:  Get rid of this ifeq somehow
>  
>  ifeq ($(CONFIG_COREBOOT_V2),y)
> -include $(CBV2_MK)
> +INCMK += $(PACKAGE_DIR)/coreboot-v2/coreboot-v2.mk
>  else
> -include $(PACKAGE_DIR)/coreboot-v3/coreboot-v3.mk
> +INCMK += $(PACKAGE_DIR)/coreboot-v3/coreboot-v3.mk
>  endif

This will have to exist _somewhere_ though.


> +# Platform specific dependencies
> +DEPENDS-$(CONFIG_PLATFORM_GEODE) += geodevsa

I like it!


> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ buildrom-devel/packages/coreboot-v2/coreboot-v2.mk	2008-04-25 15:28:59.000000000 -0600
> @@ -0,0 +1,24 @@
> +# "toplevel" coreboot-v2.mk - this is where we decide
> +# which of the platform specific files to actually
> +# include
> +
> +CBV2MK-y=$(PACKAGE_DIR)/coreboot-v2/generic.mk
> +CBV2MK-$(CONFIG_PLATFORM_NORWICH) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
> +CBV2MK-$(CONFIG_PLATFORM_MSM800SEV) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
> +CBV2MK-$(CONFIG_PLATFORM_ALIX1C) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
> +CBV2MK-$(CONFIG_PLATFORM_DB800) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
> +CBV2MK-$(CONFIG_PLATFORM_DBE61) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk

Why is this mapping needed here? Coreboot already knows that they are
lx boards? Sorry, I am a total buildrom noob so will ask many
questions and try to point out not obvious things.


> +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2881) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2881.mk
> +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2882) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2882.mk
> +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2891) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2891.mk
> +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2892) = $(PACKAGE_DIR)/coreboot-v2/tyan-generic.mk
> +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2895) = $(PACKAGE_DIR)/coreboot-v2/tyan-generic.mk

Err.. When is tyan-generic good and when is tyan-<$newboard> good?


> @@ -31,13 +35,19 @@
>  $(GEODE_PADDED_VSA): $(GEODE_COMPRESSED_VSA)
>  	@ cp $< $@
>  	@ (size=`stat -c %s $<`; count=`expr $(GEODE_VSA_SIZE) - $$size`; \
> -	@ dd if=/dev/zero bs=1 count=$$count  >> $@ 2> /dev/null)
> +	dd if=/dev/zero bs=1 count=$$count  >> $@ 2> /dev/null)

On purpose?


> +ifeq($(KERNELMK-y),)
> +$(error "You do not have a kernel .mk file defined for this platform")
> +endif

Awesome!


So many removed lines of code must make a good patch! :)

Acked-by: Peter Stuge <peter at stuge.se>

..but please wait for another.


//Peter