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

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(-)

>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

 Mike> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

I haven't tested this, but we have been carrying a similar patch in
buildroot for a while.

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

NAK

Thanks,
-Joe

On 12/15/2012 07:04 PM, Joe Hershberger wrote:

<big snip>

>> -#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@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
> 
> NAK

Mike, any chance that you might find some time to rework this patch for
mainline acceptance?

BTW: I'm using this patch for Yocto builds of the Linux environment
tools. Without it, building fails.

Thanks,
Stefan