Patchwork [U-Boot,v4] tools/env: add posibility to inject configuration

login
register
mail settings
Submitter Andreas Bießmann
Date Jan. 24, 2012, 9:10 a.m.
Message ID <1327396229-17892-1-git-send-email-andreas.devel@googlemail.com>
Download mbox | patch
Permalink /patch/137529/
State New
Delegated to: Joe Hershberger
Headers show

Comments

Andreas Bießmann - Jan. 24, 2012, 9:10 a.m.
From: Andreas Bießmann <biessmann@corscience.de>

If one want to use fw_printenv/fw_setenv in special variants (eg compiled in
MTD parameters without configuration file) he need to change the sources.
This patch add the posibillity to change the behaviour of fw_printenv by
defining a specific configuration header at compile time.
Therefore no need to patch the sources for special environment which fits
better into automated build environments.

Signed-off-by: Andreas Bießmann <biessmann@corscience.de>
---
total: 0 errors, 0 warnings, 164 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE

0001-tools-env-add-posibility-to-inject-configuration.patch has no obvious style problems and is ready for submission.

changes since v1:
 - use ?= style in Makefile as suggested by Mike
 - remove c++ style comments in header

changes since v2:
 - place copied/generated fw_env_config.h in include/generated
 - adopt tools/env/Makefile to new placement of fw_env_config.h

changes since v3:
 - add (C) header
 - generate empty config.h for unconfigured U-Boot tree
 - rebase

 tools/env/Makefile           |   29 +++++++++++++++---
 tools/env/fw_env.h           |   32 +++-----------------
 tools/env/fw_env_config.h.in |   66 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 32 deletions(-)
 create mode 100644 tools/env/fw_env_config.h.in
Andreas Bießmann - Feb. 20, 2012, 4:37 p.m.
On 24.01.2012 10:10, Andreas Bießmann wrote:
> From: Andreas Bießmann <biessmann@corscience.de>
> 
> If one want to use fw_printenv/fw_setenv in special variants (eg compiled in
> MTD parameters without configuration file) he need to change the sources.
> This patch add the posibillity to change the behaviour of fw_printenv by
> defining a specific configuration header at compile time.
> Therefore no need to patch the sources for special environment which fits
> better into automated build environments.
> 
> Signed-off-by: Andreas Bießmann <biessmann@corscience.de>
> ---
> total: 0 errors, 0 warnings, 164 lines checked
> 
> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
> 
> 0001-tools-env-add-posibility-to-inject-configuration.patch has no obvious style problems and is ready for submission.
> 
> changes since v1:
>  - use ?= style in Makefile as suggested by Mike
>  - remove c++ style comments in header
> 
> changes since v2:
>  - place copied/generated fw_env_config.h in include/generated
>  - adopt tools/env/Makefile to new placement of fw_env_config.h
> 
> changes since v3:
>  - add (C) header
>  - generate empty config.h for unconfigured U-Boot tree
>  - rebase
> 
>  tools/env/Makefile           |   29 +++++++++++++++---
>  tools/env/fw_env.h           |   32 +++-----------------
>  tools/env/fw_env_config.h.in |   66 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+), 32 deletions(-)
>  create mode 100644 tools/env/fw_env_config.h.in

ping?
Joe Hershberger - Oct. 16, 2012, 12:40 a.m.
Hi Andreas,

On Tue, Jan 24, 2012 at 3:10 AM, Andreas Bießmann
<andreas.devel@googlemail.com> wrote:
> From: Andreas Bießmann <biessmann@corscience.de>
>
> If one want to use fw_printenv/fw_setenv in special variants (eg compiled in
> MTD parameters without configuration file) he need to change the sources.
> This patch add the posibillity to change the behaviour of fw_printenv by
> defining a specific configuration header at compile time.
> Therefore no need to patch the sources for special environment which fits
> better into automated build environments.
>
> Signed-off-by: Andreas Bießmann <biessmann@corscience.de>
> ---
> total: 0 errors, 0 warnings, 164 lines checked
>
> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
>
> 0001-tools-env-add-posibility-to-inject-configuration.patch has no obvious style problems and is ready for submission.
>
> changes since v1:
>  - use ?= style in Makefile as suggested by Mike
>  - remove c++ style comments in header
>
> changes since v2:
>  - place copied/generated fw_env_config.h in include/generated
>  - adopt tools/env/Makefile to new placement of fw_env_config.h
>
> changes since v3:
>  - add (C) header
>  - generate empty config.h for unconfigured U-Boot tree
>  - rebase
>
>  tools/env/Makefile           |   29 +++++++++++++++---
>  tools/env/fw_env.h           |   32 +++-----------------
>  tools/env/fw_env_config.h.in |   66 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+), 32 deletions(-)
>  create mode 100644 tools/env/fw_env_config.h.in
>
> diff --git a/tools/env/Makefile b/tools/env/Makefile
> index 28b73da..4a0b2b0 100644
> --- a/tools/env/Makefile
> +++ b/tools/env/Makefile
> @@ -2,6 +2,9 @@
>  # (C) Copyright 2002-2006
>  # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
>  #
> +# (C) Copyright 2012
> +# Andreas Bießmann, Corscience GmbH&Co.KG, biessmann@corscience.de
> +#
>  # See file CREDITS for list of people who contributed to this
>  # project.
>  #
> @@ -24,26 +27,42 @@
>  include $(TOPDIR)/config.mk
>
>  HOSTSRCS := $(SRCTREE)/lib/crc32.c  fw_env.c  fw_env_main.c
> -HEADERS        := fw_env.h
> +HEADERS  := fw_env.h $(OBJTREE)/include/generated/fw_env_config.h
> +FW_ENV_CONFIG ?= fw_env_config.h.in
>
>  # Compile for a hosted environment on the target
>  HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
> -               -idirafter $(OBJTREE)/include2 \
>                 -idirafter $(OBJTREE)/include \
> +               -idirafter $(OBJTREE)/include/generated \
>                 -DUSE_HOSTCC
>
>  ifeq ($(MTD_VERSION),old)
>  HOSTCPPFLAGS += -DMTD_OLD
>  endif
>
> -all:   $(obj)fw_printenv
> +all: $(obj)fw_printenv
>
>  # Some files complain if compiled with -pedantic, use HOSTCFLAGS_NOPED
> -$(obj)fw_printenv:     $(HOSTSRCS) $(HEADERS)
> +$(obj)fw_printenv: $(HOSTSRCS) $(HEADERS)
>         $(HOSTCC) $(HOSTCFLAGS_NOPED) $(HOSTLDFLAGS) -o $@ $(HOSTSRCS)
>
> +$(OBJTREE)/include/generated/fw_env_config.h: $(FW_ENV_CONFIG)
> +       @cp -f $< $@
> +
> +# add additional dependency for .depend
> +$(obj).depend: $(OBJTREE)/include/generated/fw_env_config.h
> +
> +ifneq ($(OBJTREE)/include/config.mk,$(wildcard $(OBJTREE)/include/config.mk))
> +# our U-Boot tree is not configured, generate fake config.h
> +$(OBJTREE)/include/config.h:
> +       @cat /dev/null > $@
> +
> +# add additional dependency for .depend
> +$(obj).depend: $(OBJTREE)/include/config.h
> +endif
> +
>  clean:
> -       rm -f $(obj)fw_printenv
> +       rm -f $(obj)fw_printenv $(obj)fw_env_config.h
>
>  #########################################################################
>
> diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
> index 2dcb373..c237154 100644
> --- a/tools/env/fw_env.h
> +++ b/tools/env/fw_env.h
> @@ -20,34 +20,10 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>   * MA 02111-1307 USA
>   */
> +#ifndef _FW_ENV_H_
> +#define _FW_ENV_H_
>
> -/*
> - * To build the utility with the run-time configuration
> - * uncomment the next line.
> - * See included "fw_env.config" sample file
> - * for notes on configuration.
> - */
> -#define CONFIG_FILE     "/etc/fw_env.config"
> -
> -#define HAVE_REDUND /* For systems with 2 env sectors */
> -#define DEVICE1_NAME      "/dev/mtd1"
> -#define DEVICE2_NAME      "/dev/mtd2"
> -#define DEVICE1_OFFSET    0x0000
> -#define ENV1_SIZE         0x4000
> -#define DEVICE1_ESIZE     0x4000
> -#define DEVICE1_ENVSECTORS     2
> -#define DEVICE2_OFFSET    0x0000
> -#define ENV2_SIZE         0x4000
> -#define DEVICE2_ESIZE     0x4000
> -#define DEVICE2_ENVSECTORS     2

Why not just have these settings directly be #define'd in the board
config file?  You can still leave the one default here that is
actually used (CONFIG_FILE "/etc/fw_env.config") and make it
overridable.  The rest just need to be documented in the README.

This new scheme seems overly complicated and I don't know what it buys
you over the normal place for configuration.

[...]

-Joe
Andreas Bießmann - Oct. 16, 2012, 9:43 a.m.
Hi Joe,

On 16.10.2012 02:40, Joe Hershberger wrote:
> Hi Andreas,
> 
> On Tue, Jan 24, 2012 at 3:10 AM, Andreas Bießmann
> <andreas.devel@googlemail.com> wrote:
>> From: Andreas Bießmann <biessmann@corscience.de>
>>
>> If one want to use fw_printenv/fw_setenv in special variants (eg compiled in
>> MTD parameters without configuration file) he need to change the sources.
>> This patch add the posibillity to change the behaviour of fw_printenv by
>> defining a specific configuration header at compile time.
>> Therefore no need to patch the sources for special environment which fits
>> better into automated build environments.
>>
>> Signed-off-by: Andreas Bießmann <biessmann@corscience.de>
>> ---
>> total: 0 errors, 0 warnings, 164 lines checked
>>
>> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
>>
>> 0001-tools-env-add-posibility-to-inject-configuration.patch has no obvious style problems and is ready for submission.
>>
>> changes since v1:
>>  - use ?= style in Makefile as suggested by Mike
>>  - remove c++ style comments in header
>>
>> changes since v2:
>>  - place copied/generated fw_env_config.h in include/generated
>>  - adopt tools/env/Makefile to new placement of fw_env_config.h
>>
>> changes since v3:
>>  - add (C) header
>>  - generate empty config.h for unconfigured U-Boot tree
>>  - rebase
>>
>>  tools/env/Makefile           |   29 +++++++++++++++---
>>  tools/env/fw_env.h           |   32 +++-----------------
>>  tools/env/fw_env_config.h.in |   66 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 95 insertions(+), 32 deletions(-)
>>  create mode 100644 tools/env/fw_env_config.h.in

<snip>

> Why not just have these settings directly be #define'd in the board
> config file?  You can still leave the one default here that is
> actually used (CONFIG_FILE "/etc/fw_env.config") and make it
> overridable.  The rest just need to be documented in the README.

This is correct but not the whole truth. It is not possible to setup a
default environment in the config file, but would be when someone
provides a proper header. However, if he can provide a header he could
also just use the header provided by his board config.

You see the whole story is not only about setup for mtd, but for
customizing this tool in a distributed environment. In the end I think
the proposed header is also not the best solution.

> This new scheme seems overly complicated and I don't know what it buys
> you over the normal place for configuration.

You are right, it is complicated. I think we should name this patch a
RFC and forget about it. Nevertheless there is a need to customize the
tool at build time (especially regarding default environment) and we
should think about how to change this tool.
AFAIR the discussion about environment at all is ongoing, hopefully this
can give some input. I must confess that I didn't work on that problem
for a long time, therefore I did not remember this patch.

Best regards

Andreas Bießmann

Patch

diff --git a/tools/env/Makefile b/tools/env/Makefile
index 28b73da..4a0b2b0 100644
--- a/tools/env/Makefile
+++ b/tools/env/Makefile
@@ -2,6 +2,9 @@ 
 # (C) Copyright 2002-2006
 # Wolfgang Denk, DENX Software Engineering, wd@denx.de.
 #
+# (C) Copyright 2012
+# Andreas Bießmann, Corscience GmbH&Co.KG, biessmann@corscience.de
+#
 # See file CREDITS for list of people who contributed to this
 # project.
 #
@@ -24,26 +27,42 @@ 
 include $(TOPDIR)/config.mk
 
 HOSTSRCS := $(SRCTREE)/lib/crc32.c  fw_env.c  fw_env_main.c
-HEADERS	:= fw_env.h
+HEADERS  := fw_env.h $(OBJTREE)/include/generated/fw_env_config.h
+FW_ENV_CONFIG ?= fw_env_config.h.in
 
 # Compile for a hosted environment on the target
 HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
-		-idirafter $(OBJTREE)/include2 \
 		-idirafter $(OBJTREE)/include \
+		-idirafter $(OBJTREE)/include/generated \
 		-DUSE_HOSTCC
 
 ifeq ($(MTD_VERSION),old)
 HOSTCPPFLAGS += -DMTD_OLD
 endif
 
-all:	$(obj)fw_printenv
+all: $(obj)fw_printenv
 
 # Some files complain if compiled with -pedantic, use HOSTCFLAGS_NOPED
-$(obj)fw_printenv:	$(HOSTSRCS) $(HEADERS)
+$(obj)fw_printenv: $(HOSTSRCS) $(HEADERS)
 	$(HOSTCC) $(HOSTCFLAGS_NOPED) $(HOSTLDFLAGS) -o $@ $(HOSTSRCS)
 
+$(OBJTREE)/include/generated/fw_env_config.h: $(FW_ENV_CONFIG)
+	@cp -f $< $@
+
+# add additional dependency for .depend
+$(obj).depend: $(OBJTREE)/include/generated/fw_env_config.h
+
+ifneq ($(OBJTREE)/include/config.mk,$(wildcard $(OBJTREE)/include/config.mk))
+# our U-Boot tree is not configured, generate fake config.h
+$(OBJTREE)/include/config.h:
+	@cat /dev/null > $@
+
+# add additional dependency for .depend
+$(obj).depend: $(OBJTREE)/include/config.h
+endif
+
 clean:
-	rm -f $(obj)fw_printenv
+	rm -f $(obj)fw_printenv $(obj)fw_env_config.h
 
 #########################################################################
 
diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 2dcb373..c237154 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -20,34 +20,10 @@ 
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
  * MA 02111-1307 USA
  */
+#ifndef _FW_ENV_H_
+#define _FW_ENV_H_
 
-/*
- * To build the utility with the run-time configuration
- * uncomment the next line.
- * See included "fw_env.config" sample file
- * for notes on configuration.
- */
-#define CONFIG_FILE     "/etc/fw_env.config"
-
-#define HAVE_REDUND /* For systems with 2 env sectors */
-#define DEVICE1_NAME      "/dev/mtd1"
-#define DEVICE2_NAME      "/dev/mtd2"
-#define DEVICE1_OFFSET    0x0000
-#define ENV1_SIZE         0x4000
-#define DEVICE1_ESIZE     0x4000
-#define DEVICE1_ENVSECTORS     2
-#define DEVICE2_OFFSET    0x0000
-#define ENV2_SIZE         0x4000
-#define DEVICE2_ESIZE     0x4000
-#define DEVICE2_ENVSECTORS     2
-
-#define CONFIG_BAUDRATE		115200
-#define CONFIG_BOOTDELAY	5	/* autoboot after 5 seconds	*/
-#define CONFIG_BOOTCOMMAND							\
-	"bootp; "								\
-	"setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} "	\
-	"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; "	\
-	"bootm"
+#include "fw_env_config.h"
 
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
@@ -58,3 +34,5 @@  extern int fw_env_write(char *name, char *value);
 extern int fw_env_close(void);
 
 extern unsigned	long  crc32	 (unsigned long, const unsigned char *, unsigned);
+
+#endif
diff --git a/tools/env/fw_env_config.h.in b/tools/env/fw_env_config.h.in
new file mode 100644
index 0000000..1f69c28
--- /dev/null
+++ b/tools/env/fw_env_config.h.in
@@ -0,0 +1,66 @@ 
+/*
+ * (C) Copyright 2011-2012
+ * Andreas Bießmann, Corscience GmbH&Co.KG, biessmann@corscience.de
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/*
+ * This is an example configuration for fw_printenv/fw_setenv
+ *
+ * If you like to specialize your build of fw_printenv you may copy this file,
+ * modify it and add FW_ENV_CONFIG define to the make call:
+ *
+ * make HOSTCC=${CROSS_COMPILE}gcc FW_ENV_CONFIG=/my/special/header env
+ *
+ * If the FW_ENV_CONFIG is not defined, this file will be used
+ */
+
+#ifndef _FW_ENV_CONFIG_H_
+#define _FW_ENV_CONFIG_H_
+
+/*
+ * To build the utility with the run-time configuration
+ * uncomment the next line.
+ * See included "fw_env.config" sample file
+ * for notes on configuration.
+ */
+#define CONFIG_FILE     "/etc/fw_env.config"
+
+#define HAVE_REDUND /* For systems with 2 env sectors */
+#define DEVICE1_NAME      "/dev/mtd1"
+#define DEVICE2_NAME      "/dev/mtd2"
+#define DEVICE1_OFFSET    0x0000
+#define ENV1_SIZE         0x4000
+#define DEVICE1_ESIZE     0x4000
+#define DEVICE1_ENVSECTORS     2
+#define DEVICE2_OFFSET    0x0000
+#define ENV2_SIZE         0x4000
+#define DEVICE2_ESIZE     0x4000
+#define DEVICE2_ENVSECTORS     2
+
+#define CONFIG_BAUDRATE		115200
+#define CONFIG_BOOTDELAY	5	/* autoboot after 5 seconds	*/
+#define CONFIG_BOOTCOMMAND							\
+	"bootp; "								\
+	"setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} "	\
+	"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; "	\
+	"bootm"
+
+#endif