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 <[email protected]> 2010/7/14 Patrick Georgi <[email protected]> > Am 13.07.2010 16:51, schrieb baiyin cai: > > 1) load libpayload kconfig while configuring filo. > > 2) 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 <[email protected]> > > Patrick >
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
