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

login
register
mail settings
Submitter Mike Frysinger
Date Nov. 11, 2012, 5:47 a.m.
Message ID <1352612867-32354-3-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/198242/
State Rejected
Delegated to: Joe Hershberger
Headers show

Comments

Mike Frysinger - Nov. 11, 2012, 5:47 a.m.
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(-)
Peter Korsgaard - Nov. 12, 2012, 12:14 p.m.
>>>>> "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>
Joe Hershberger - Dec. 15, 2012, 6:04 p.m.
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
Stefan Roese - May 21, 2013, 11:19 a.m.
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

Patch

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
 
 # 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
+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
-
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);