[U-Boot] [PATCH 3/3] fw_env: fix building w/out a config.h

From: Joe Hershberger <joe.hershberger@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/3] fw_env: fix building w/out a config.h
Date: Sat, 15 Dec 2012 12:04:40 -0600	[thread overview]
Message-ID: <CANr=Z=ZXfCHu4a_+boaYHpvarG4x1qQcdtVABm8FJ_mufMD8-A@mail.gmail.com> (raw)
In-Reply-To: <1352612867-32354-3-git-send-email-vapier@gentoo.org>

Hi Mike

On Sat, Nov 10, 2012 at 11:47 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  tools/env/Makefile | 11 ++++++++++-
>  tools/env/fw_env.h | 25 -------------------------
>  2 files changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/tools/env/Makefile b/tools/env/Makefile
> index ab73c8c..62a113a 100644
> --- a/tools/env/Makefile
> +++ b/tools/env/Makefile
> @@ -24,7 +24,7 @@
>  include $(TOPDIR)/config.mk
>
>  HOSTSRCS := $(SRCTREE)/lib/crc32.c  fw_env.c  fw_env_main.c
> -HEADERS        := fw_env.h $(OBJTREE)/include/config.h
> +HEADERS        := fw_env.h

I think this is the wrong approach.  We depend on the config.h being
included and the entire default env being available.  If you want to
get this behavior, I suggest you detect if there is a configured
board, and if so, include the config.h, and if not, bake in the bit
you need to cope with not having one.

>  # Compile for a hosted environment on the target
>  HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
> @@ -33,6 +33,15 @@ HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
>                 -DUSE_HOSTCC \
>                 -DTEXT_BASE=$(TEXT_BASE)
>
> +# Pass CONFIG_xxx settings via the command line so that we can build w/out
> +# a config.h file existing in the first place.  Useful for generic builds.
> +CONFIG_VARS_TO_PASS = \
> +       ENV_OVERWRITE \
> +       OVERWRITE_ETHADDR_ONCE \
> +       ETHADDR

This doesn't look very maintainable, and it doesn't even include the
variables currently used.

> +HOSTCPPFLAGS += \
> +       $(foreach x,$(CONFIG_VARS_TO_PASS),$(if $(CONFIG_$(x)),-DCONFIG_$(x)=$(CONFIG_$(x))))
> +
>  ifeq ($(MTD_VERSION),old)
>  HOSTCPPFLAGS += -DMTD_OLD
>  endif
> diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
> index a1a6807..19703c7 100644
> --- a/tools/env/fw_env.h
> +++ b/tools/env/fw_env.h
> @@ -21,15 +21,6 @@
>   * MA 02111-1307 USA
>   */
>
> -/* Pull in the current config to define the default environment */
> -#ifndef __ASSEMBLY__
> -#define __ASSEMBLY__ /* get only #defines from config.h */
> -#include <config.h>
> -#undef __ASSEMBLY__
> -#else
> -#include <config.h>
> -#endif
> -
>  /*
>   * To build the utility with the static configuration
>   * comment out the next line.
> @@ -52,22 +43,6 @@
>  #define DEVICE2_ENVSECTORS     2
>  #endif
>
> -#ifndef CONFIG_BAUDRATE
> -#define CONFIG_BAUDRATE                115200
> -#endif
> -
> -#ifndef CONFIG_BOOTDELAY
> -#define CONFIG_BOOTDELAY       5       /* autoboot after 5 seconds     */
> -#endif
> -
> -#ifndef CONFIG_BOOTCOMMAND
> -#define CONFIG_BOOTCOMMAND                                                     \
> -       "bootp; "                                                               \
> -       "setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} "        \
> -       "ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; "   \
> -       "bootm"
> -#endif
> -

I agree that some of this should be cleaned up, but not as a result of
removing the config.h.

>  extern int   fw_printenv(int argc, char *argv[]);
>  extern char *fw_getenv  (char *name);
>  extern int fw_setenv  (int argc, char *argv[]);
> --
> 1.7.12.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

NAK

Thanks,
-Joe