hi patrick, thanks for your kindly suggestions. That's will be much helpful for me. I would take that into consideration.
Signed-off-by: Cai Bai Yin caibaiyin.pku@gmail.com
2010/7/14 Patrick Georgi patrick@georgi-clan.de
Am 13.07.2010 16:51, schrieb baiyin cai:
- load libpayload kconfig while configuring filo.
- build libpayload before filo building.
it can be used by : $MAKE LIBCONFIG_PATH=/path/to/libpayload" Any suggestion will be welcome. thanks
Marc asked me to take a closer look, as I worked a lot on coreboot related Makefiles in the past (though not on these) and he mentioned that you plan to use that work for more libpayload-based payload.
If that is the case, consider pushing the complex and libpayload specific parts into the libpayload tree, and then use "include" to load it from the payloads' Makefiles.
But that can (and should) be done in a separate patch, to avoid that you lose your work to mistakes.
+ifneq ($(strip $(HAVE_FILO_CONFIG)),) +ifneq ($(strip $(HAVE_LIB_CONFIG)),) xconfig: prepare $(objk)/qconf
$(Q)printf "Libpayload config for FILO.\n"$(Q)mv $(FILO_CONFIG) $(FILO_CONFIG)."temp"$(Q)mv $(LIB_CONFIG) $(FILO_CONFIG)$(Q)$(objk)/qconf $(LIBCONFIG_PATH)/Config.in$(Q)mv $(FILO_CONFIG) $(LIB_CONFIG)$(Q)printf "Libpayload config done.\n"$(Q)mv $(FILO_CONFIG)."temp" $(FILO_CONFIG) $(Q)$(objk)/qconf $(Kconfig)+else +xconfig: prepare $(objk)/qconf
$(Q)printf "Lost libpayload config file.\n"$(Q)rm -f $(FILO_CONFIG)What's the intended behaviour if HAVE_LIB_CONFIG is not set? Maybe it's better to just $(error ... ) before doing all these nested ifs and elses? That would spare you the various copies below.
-ifeq ($(strip $(HAVE_LIBPAYLOAD)),) -all:
@printf "\nError: libpayload is not installed!\nexpected:$(LIBPAYLOAD).\n"
+ifneq ($(strip $(HAVE_LIBPAYLOAD)),) +libpayload:
@printf "Found Libpayload $(LIBPAYLOAD).\n"else -all: prepare $(obj)/version.h $(TARGET) +libpayload: $(src)/$(LIB_CONFIG)
libpayload should be marked as a "phony" target, I think. Alternatively, libpayload's install target could be changed to install a certain file last, and then you could rely on its presence to determine if there's a complete installation.
Also be careful with the actions of the libpayload target: libpayload is referenced from multiple places, so if you're doing parallel builds (make -j), make is free to execute this rule parallely multiple times (not that this makes sense, but it can do so, and it does, sometimes). Whatever you do here must be stable against races in such situations.
Apart from these little issues, I think your patch is a real improvement for simplifying the build of a complete image. Thank you!
With the above taken into account, this is Acked-by: Patrick Georgi patrick.georgi@coresystems.de
Patrick