Patchwork filo kconfig patch of libpayload

login
register
about
Submitter baiyin cai
Date 2010-07-13 14:51:17
Message ID <AANLkTik7Dr0bf83yU7HquhEBc-kcsAtaSKVwSZ-ihBar@mail.gmail.com>
Download mbox | patch
Permalink /patch/1621/
State Accepted
Headers show

Comments

baiyin cai - 2010-07-13 14:51:17
hi all,
   the patch did the following things:
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
Patrick Georgi - 2010-07-13 21:01:45
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@coresystems.de>

Patrick
baiyin cai - 2010-07-14 23:26:22
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:
> > 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@coresystems.de>
>
> Patrick
>
Marc Jones - 2010-07-19 18:33:58
On Wed, Jul 14, 2010 at 5:26 PM, baiyin cai <caibaiyin.pku@gmail.com> wrote:
> 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>

>>Acked-by: Patrick Georgi <patrick.georgi@coresystems.de>

Acked-by: Marc Jones <marcj303@gmail.com>

r135

Patch

Index: util/kconfig/Makefile
===================================================================
--- util/kconfig/Makefile	(revision 133)
+++ util/kconfig/Makefile	(working copy)
@@ -12,24 +12,162 @@ 
 
 Kconfig := Config.in
 
+FILO_CONFIG := $(src)/.config
+LIB_CONFIG  := $(src)/lib.config
+HAVE_FILO_CONFIG := $(wildcard $(FILO_CONFIG))
+HAVE_LIB_CONFIG := $(wildcard $(LIB_CONFIG))
+
+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)
+endif
+else
+xconfig: prepare $(objk)/qconf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)$(objk)/qconf $(LIBCONFIG_PATH)/Config.in
+	$(Q)mv $(FILO_CONFIG) $(LIB_CONFIG)
+	$(Q)printf "Libpayload config done.\n"
+	$(Q)$(objk)/qconf $(Kconfig)
+endif
 
+ifneq ($(strip $(HAVE_FILO_CONFIG)),)
+ifneq ($(strip $(HAVE_LIB_CONFIG)),)
 gconfig: prepare $(objk)/gconf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)mv $(FILO_CONFIG) $(FILO_CONFIG)."temp"
+	$(Q)mv $(LIB_CONFIG) $(FILO_CONFIG)
+	$(Q)$(objk)/gconf $(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)/gconf $(Kconfig)
+else
+gconfig: prepare $(objk)/gconf
+	$(Q)printf "Lost libpayload config file.\n"
+	$(Q)rm -f $(FILO_CONFIG)
+endif
+else
+gconfig: prepare $(objk)/gconf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)$(objk)/gconf $(LIBCONFIG_PATH)/Config.in
+	$(Q)mv $(FILO_CONFIG) $(LIB_CONFIG)
+	$(Q)printf "Libpayload config done.\n"
+	$(Q)$(objk)/gconf $(Kconfig)
+endif
 
+ifneq ($(strip $(HAVE_FILO_CONFIG)),)
+ifneq ($(strip $(HAVE_LIB_CONFIG)),)
 menuconfig: prepare $(objk)/mconf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)mv $(FILO_CONFIG) $(FILO_CONFIG)."temp"
+	$(Q)mv $(LIB_CONFIG) $(FILO_CONFIG)
+	$(Q)$(objk)/mconf $(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)/mconf $(Kconfig)
+else
+menuconfig: prepare $(objk)/mconf
+	$(Q)printf "Lost libpayload config file.\n"
+	$(Q)rm -f $(FILO_CONFIG)
+endif
+else
+menuconfig: prepare $(objk)/mconf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)$(objk)/mconf $(LIBCONFIG_PATH)/Config.in
+	$(Q)mv $(FILO_CONFIG) $(LIB_CONFIG)
+	$(Q)printf "Libpayload config done.\n"
+	$(Q)$(objk)/mconf $(Kconfig)
+endif
 
+ifneq ($(strip $(HAVE_FILO_CONFIG)),)
+ifneq ($(strip $(HAVE_LIB_CONFIG)),)
 config: prepare $(objk)/conf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)mv $(FILO_CONFIG) $(FILO_CONFIG)."temp"
+	$(Q)mv $(LIB_CONFIG) $(FILO_CONFIG)
+	$(Q)$(objk)/conf $(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)/conf $(Kconfig)
+else
+config: prepare $(objk)/conf
+	$(Q)printf "Lost libpayload config file.\n"
+	$(Q)rm -f $(FILO_CONFIG)
+endif
+else
+config: prepare $(objk)/conf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)$(objk)/conf $(LIBCONFIG_PATH)/Config.in
+	$(Q)mv $(FILO_CONFIG) $(LIB_CONFIG)
+	$(Q)printf "Libpayload config done.\n"
+	$(Q)$(objk)/conf $(Kconfig)
+endif
 
+ifneq ($(strip $(HAVE_FILO_CONFIG)),)
+ifneq ($(strip $(HAVE_LIB_CONFIG)),)
 oldconfig: prepare $(objk)/conf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)mv $(FILO_CONFIG) $(FILO_CONFIG)."temp"
+	$(Q)mv $(LIB_CONFIG) $(FILO_CONFIG)
+	$(Q)$(objk)/conf -o $(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)/conf -o $(Kconfig)
+else
+oldconfig: prepare $(objk)/conf
+	$(Q)printf "Lost libpayload config file.\n"
+	$(Q)rm -f $(FILO_CONFIG)
+endif
+else
+oldconfig: prepare $(objk)/conf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)$(objk)/conf -o $(LIBCONFIG_PATH)/Config.in
+	$(Q)mv .config $(LIB_CONFIG)
+	$(Q)printf "Libpayload config done.\n"
+	$(Q)$(objk)/conf -o $(Kconfig)
+endif
 
+ifneq ($(strip $(HAVE_FILO_CONFIG)),)
+ifneq ($(strip $(HAVE_LIB_CONFIG)),)
 silentoldconfig: prepare $(objk)/conf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)mv $(FILO_CONFIG) $(FILO_CONFIG)."temp"
+	$(Q)mv $(LIB_CONFIG) $(FILO_CONFIG)
+	$(Q)$(objk)/conf -s $(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)/conf -s $(Kconfig)
+else
+silentoldconfig: prepare $(objk)/conf
+	$(Q)printf "Lost libpayload config file.\n"
+	$(Q)rm -f $(FILO_CONFIG)
+endif
+else
+silentoldconfig: prepare $(objk)/conf
+	$(Q)printf "Libpayload config for FILO.\n"
+	$(Q)$(objk)/conf -s $(LIBCONFIG_PATH)/Config.in
+	$(Q)mv .config $(LIB_CONFIG)
+	$(Q)printf "Libpayload config done.\n"
+	$(Q)$(objk)/conf -s $(Kconfig)
 
+endif
+
 # --- UNUSED, ignore ----------------------------------------------------------
 # Create new linux.pot file
 # Adjust charset to UTF-8 in .po file to accept UTF-8 in Kconfig files
Index: Makefile
===================================================================
--- Makefile	(revision 133)
+++ Makefile	(working copy)
@@ -24,6 +24,7 @@ 
 export srck := $(src)/util/kconfig
 export obj := $(src)/build
 export objk := $(src)/build/util/kconfig
+export LIBCONFIG_PATH := $(src)/../libpayload
 
 export KERNELVERSION      := $(PROGRAM_VERSION)
 export KCONFIG_AUTOHEADER := $(obj)/config.h
@@ -102,19 +103,29 @@ 
 
 TARGET  = $(obj)/filo.elf
 
+HAVE_LIBCONFIG := $(wildcard $(LIBCONFIG_PATH))
+
+all: prepare $(obj)/version.h $(TARGET)
+
+
 HAVE_LIBPAYLOAD := $(wildcard $(LIBPAYLOAD))
-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)
+	$(Q)printf "building libpayload.\n"
+	$(Q)make -C $(LIBCONFIG_PATH) distclean
+	$(Q)cp lib.config $(LIBCONFIG_PATH)/.config
+	$(Q)make -C $(LIBCONFIG_PATH) oldconfig
+	$(Q)make -C $(LIBCONFIG_PATH) DESTDIR=$(src)/build install
 endif
 
-$(obj)/filo: $(src)/.config $(OBJS)
+$(obj)/filo: $(src)/.config $(OBJS)  libpayload
 	$(Q)printf "  LD      $(subst $(shell pwd)/,,$(@))\n"
 	$(Q)$(LD) -N -T $(ARCHDIR-y)/ldscript -o $@ $(OBJS) $(LIBPAYLOAD) $(LIBGCC)
 
-$(TARGET): $(obj)/filo
+$(TARGET): $(obj)/filo libpayload
 	$(Q)cp $(obj)/filo $@
 	$(Q)$(NM) $(obj)/filo | sort > $(obj)/filo.map
 	$(Q)printf "  STRIP   $(subst $(shell pwd)/,,$(@))\n"
@@ -122,7 +133,7 @@ 
 
 include util/kconfig/Makefile
 
-$(obj)/%.o: $(src)/%.c
+$(obj)/%.o: $(src)/%.c libpayload
 	$(Q)printf "  CC      $(subst $(shell pwd)/,,$(@))\n"
 	$(Q)$(CC) -MMD $(CFLAGS) $(CPPFLAGS) -c -o $@ $<
 
@@ -148,7 +159,7 @@ 
 
 distclean: clean
 	$(Q)rm -rf build
-	$(Q)rm -f .config .config.old ..config.tmp .kconfig.d .tmpconfig*
+	$(Q)rm -f .config lib.config .config.old ..config.tmp .kconfig.d .tmpconfig*
 
 FORCE: