[coreboot] filo kconfig patch of libpayload

baiyin cai caibaiyin.pku at gmail.com
Thu Jul 15 01:26:22 CEST 2010


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 at gmail.com>

2010/7/14 Patrick Georgi <patrick at georgi-clan.de>

> 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 <patrick.georgi at coresystems.de>
>
> Patrick
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20100715/4e7a4881/attachment.html>


More information about the coreboot mailing list