diff mbox

[U-Boot,v3] common/Makefile: Add new symbol CONFIG_SPL_ENV_SUPPORT for environment in SPL

Message ID 1368584128-2274-1-git-send-email-ying.zhang@freescale.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

ying.zhang@freescale.com May 15, 2013, 2:15 a.m. UTC 
From: Ying Zhang <b40530@freescale.com>

There will need the environment in SPL for reasons other than network
support (in particular, hwconfig contains info for how to set up DDR).

Add a new symbol CONFIG_SPL_ENV_SUPPORT to replace CONFIG_SPL_NET_SUPPORT
for environment in common/Makefile.

Signed-off-by: Ying Zhang <b40530@freescale.com>
---
Compared with the previous version, add symbol CONFIG_SPL_ENV_SUPPORT
in include/configs/a3m071.h.

 README                       |    3 +++
 common/Makefile              |   19 +++++++++----------
 include/configs/a3m071.h     |    1 +
 include/configs/am335x_evm.h |    1 +
 include/configs/pcm051.h     |    1 +
 5 files changed, 15 insertions(+), 10 deletions(-)

Comments

Tom Rini May 15, 2013, 2:07 p.m. UTC | #1 
On Wed, May 15, 2013 at 10:15:28AM +0800, ying.zhang@freescale.com wrote:

> From: Ying Zhang <b40530@freescale.com>
> 
> There will need the environment in SPL for reasons other than network
> support (in particular, hwconfig contains info for how to set up DDR).
> 
> Add a new symbol CONFIG_SPL_ENV_SUPPORT to replace CONFIG_SPL_NET_SUPPORT
> for environment in common/Makefile.
> 
> Signed-off-by: Ying Zhang <b40530@freescale.com>
[snip]
>  # environment
> @@ -67,7 +66,6 @@ COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
>  COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
>  COBJS-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
>  COBJS-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
> -COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o

You need to move all of these options down so that adding them to SPL
later, as needed, doesn't have further deltas, and to keep things
organized still.

>  # command
>  COBJS-$(CONFIG_CMD_AMBAPP) += cmd_ambapp.o
> @@ -215,18 +213,16 @@ COBJS-$(CONFIG_CMD_GPT) += cmd_gpt.o
>  endif
>  
>  ifdef CONFIG_SPL_BUILD
> -COBJS-y += cmd_nvedit.o
> -COBJS-y += env_common.o
>  COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o

This CONFIG_ENV_IS_IN_FLASH is now duplicated and needs to be removed,
when you move all of the other CONFIG_ENV_IS_... down to the always part
of the Makefile.

[snip]
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index ef00306..f47d3d1 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -325,6 +325,7 @@
>  #define CONFIG_SPL_GPIO_SUPPORT
>  #define CONFIG_SPL_YMODEM_SUPPORT
>  #define CONFIG_SPL_NET_SUPPORT
> +#define CONFIG_SPL_ENV_SUPPORT
>  #define CONFIG_SPL_NET_VCI_STRING	"AM335x U-Boot SPL"
>  #define CONFIG_SPL_ETH_SUPPORT
>  #define CONFIG_SPL_SPI_SUPPORT
> diff --git a/include/configs/pcm051.h b/include/configs/pcm051.h
> index d0ea74e..926842f 100644
> --- a/include/configs/pcm051.h
> +++ b/include/configs/pcm051.h
> @@ -224,6 +224,7 @@
>  #define CONFIG_SPL_GPIO_SUPPORT
>  #define CONFIG_SPL_YMODEM_SUPPORT
>  #define CONFIG_SPL_NET_SUPPORT
> +#define CONFIG_SPL_ENV_SUPPORT
>  #define CONFIG_SPL_NET_VCI_STRING	"pcm051 U-Boot SPL"
>  #define CONFIG_SPL_ETH_SUPPORT
>  #define CONFIG_SPL_SPI_SUPPORT

Have you made sure these two boards still compile?  I bet they don't as
they aren't setting CONFIG_ENV_IS_NOWHERE for SPL and
CONFIG_ENV_IS_...somewhere-else for non-SPL.
Zhang Ying-B40530 May 16, 2013, 7:17 a.m. UTC | #2 
-----Original Message-----
From: Tom Rini [mailto:tom.rini@gmail.com] On Behalf Of Tom Rini
Sent: Wednesday, May 15, 2013 10:07 PM
To: Zhang Ying-B40530
Cc: u-boot@lists.denx.de; Wood Scott-B07421; afleming@gmail.com; Xie Xiaobo-R63061; Zhang Ying-B40530
Subject: Re: [U-Boot] [PATCH v3] common/Makefile: Add new symbol CONFIG_SPL_ENV_SUPPORT for environment in SPL

On Wed, May 15, 2013 at 10:15:28AM +0800, ying.zhang@freescale.com wrote:

> From: Ying Zhang <b40530@freescale.com>
> 
> There will need the environment in SPL for reasons other than network
> support (in particular, hwconfig contains info for how to set up DDR).
> 
> Add a new symbol CONFIG_SPL_ENV_SUPPORT to replace CONFIG_SPL_NET_SUPPORT
> for environment in common/Makefile.
> 
> Signed-off-by: Ying Zhang <b40530@freescale.com>
[snip]
>  # environment
> @@ -67,7 +66,6 @@ COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
>  COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
>  COBJS-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
>  COBJS-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
> -COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o

You need to move all of these options down so that adding them to SPL
later, as needed, doesn't have further deltas, and to keep things
organized still.

>  # command
>  COBJS-$(CONFIG_CMD_AMBAPP) += cmd_ambapp.o
> @@ -215,18 +213,16 @@ COBJS-$(CONFIG_CMD_GPT) += cmd_gpt.o
>  endif
>  
>  ifdef CONFIG_SPL_BUILD
> -COBJS-y += cmd_nvedit.o
> -COBJS-y += env_common.o
>  COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o

This CONFIG_ENV_IS_IN_FLASH is now duplicated and needs to be removed,
when you move all of the other CONFIG_ENV_IS_... down to the always part
of the Makefile.

[snip]
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index ef00306..f47d3d1 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -325,6 +325,7 @@
>  #define CONFIG_SPL_GPIO_SUPPORT
>  #define CONFIG_SPL_YMODEM_SUPPORT
>  #define CONFIG_SPL_NET_SUPPORT
> +#define CONFIG_SPL_ENV_SUPPORT
>  #define CONFIG_SPL_NET_VCI_STRING	"AM335x U-Boot SPL"
>  #define CONFIG_SPL_ETH_SUPPORT
>  #define CONFIG_SPL_SPI_SUPPORT
> diff --git a/include/configs/pcm051.h b/include/configs/pcm051.h
> index d0ea74e..926842f 100644
> --- a/include/configs/pcm051.h
> +++ b/include/configs/pcm051.h
> @@ -224,6 +224,7 @@
>  #define CONFIG_SPL_GPIO_SUPPORT
>  #define CONFIG_SPL_YMODEM_SUPPORT
>  #define CONFIG_SPL_NET_SUPPORT
> +#define CONFIG_SPL_ENV_SUPPORT
>  #define CONFIG_SPL_NET_VCI_STRING	"pcm051 U-Boot SPL"
>  #define CONFIG_SPL_ETH_SUPPORT
>  #define CONFIG_SPL_SPI_SUPPORT

Have you made sure these two boards still compile?  I bet they don't as
they aren't setting CONFIG_ENV_IS_NOWHERE for SPL and
CONFIG_ENV_IS_...somewhere-else for non-SPL.
[Zhang Ying] 
Oh, there are some confusion. I thought CONFIG_ENV_IS_xxx defined is
consistent for both SPL and non-SPL.
If you want to set CONFIG_ENV_IS_NOWHERE for SPL and CONFIG_ENV_IS_xxx
somewhere-else for non-SPL, so all of lines CONFIG_ENV_IS_... cann't 
be moved to the public area, because CONFIG_ENV_IS_NOWHERE has always
been not effective and CONFIG_ENV_IS_... has been effective in 
common/Makefile.

Unless we define some new symbols that for SPL?
Tom Rini May 16, 2013, 1:14 p.m. UTC | #3 
On Thu, May 16, 2013 at 07:17:50AM +0000, Zhang Ying-B40530 wrote:

[snip]
> Oh, there are some confusion. I thought CONFIG_ENV_IS_xxx defined is
> consistent for both SPL and non-SPL.
> If you want to set CONFIG_ENV_IS_NOWHERE for SPL and CONFIG_ENV_IS_xxx
> somewhere-else for non-SPL, so all of lines CONFIG_ENV_IS_... cann't 
> be moved to the public area, because CONFIG_ENV_IS_NOWHERE has always
> been not effective and CONFIG_ENV_IS_... has been effective in 
> common/Makefile.
> 
> Unless we define some new symbols that for SPL?

OK, you're right, this is a bit more complex than it looks.  Today,
a3m071 relies on SPL always building cmd_nvedit.o and env_common.o and
duplicated CONFIG_ENV_IS_IN_FLASH in the SPL section.
CONFIG_SPL_NET_SUPPORT relies on the same always-built ins and adds
env_nowhere.o which works because the regular CONFIG_ENV_IS_IN_...
section is in the non-SPL-only area.

Part of the answer / problem is, wow, OK, we need to re-sort common/ a
bit:
$ ls common/*.c | wc -l
160
$ ls common/cmd*.c | wc -l
106
$ ls common/env*.c | wc -l
17

Now, your end-goal is to have env from, I assume, NAND, also exist on
SPL?  I guess for now, lets go ahead and duplicate a few lines of
ENV_IS.. inside the SPL area and when you add NAND env, add ifneq
($(CONFIG_SPL_NET_SUPPORT),y)...endif around it and add to the README
that CONFIG_SPL_NET_SUPPORT conflicts with SPL env from nand.
diff mbox

Patch

diff --git a/README b/README
index 5bf4afa..daf1fa1 100644
--- a/README
+++ b/README
@@ -2985,6 +2985,9 @@  FIT uImage format:
 		CONFIG_SPL_LIBGENERIC_SUPPORT
 		Support for lib/libgeneric.o in SPL binary
 
+		CONFIG_SPL_ENV_SUPPORT
+		Support for the environment operating in SPL binary
+
 		CONFIG_SPL_PAD_TO
 		Image offset to which the SPL should be padded before appending
 		the SPL payload. By default, this is defined as
diff --git a/common/Makefile b/common/Makefile
index 0429a3c..43daaa8 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -44,7 +44,6 @@  COBJS-$(CONFIG_SYS_GENERIC_BOARD) += board_r.o
 COBJS-y += cmd_boot.o
 COBJS-$(CONFIG_CMD_BOOTM) += cmd_bootm.o
 COBJS-y += cmd_help.o
-COBJS-y += cmd_nvedit.o
 COBJS-y += cmd_version.o
 
 # environment
@@ -67,7 +66,6 @@  COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
 COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
 COBJS-$(CONFIG_ENV_IS_IN_REMOTE) += env_remote.o
 COBJS-$(CONFIG_ENV_IS_IN_UBI) += env_ubi.o
-COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
 
 # command
 COBJS-$(CONFIG_CMD_AMBAPP) += cmd_ambapp.o
@@ -215,18 +213,16 @@  COBJS-$(CONFIG_CMD_GPT) += cmd_gpt.o
 endif
 
 ifdef CONFIG_SPL_BUILD
-COBJS-y += cmd_nvedit.o
-COBJS-y += env_common.o
 COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
 COBJS-$(CONFIG_SPL_YMODEM_SUPPORT) += xyzModem.o
-COBJS-$(CONFIG_SPL_NET_SUPPORT) += cmd_nvedit.o
-COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_attr.o
-COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_callback.o
-COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_common.o
-COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_flags.o
-COBJS-$(CONFIG_SPL_NET_SUPPORT) += env_nowhere.o
 COBJS-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o
+# environment
+COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_common.o
+COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_attr.o
+COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_flags.o
+COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_callback.o
 endif
+# core command
+COBJS-y += cmd_nvedit.o
+# environment
+COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
 COBJS-$(CONFIG_BOUNCE_BUFFER) += bouncebuf.o
 COBJS-y += console.o
 COBJS-y += dlmalloc.o
diff --git a/include/configs/a3m071.h b/include/configs/a3m071.h
index e9af825..8f29229 100644
--- a/include/configs/a3m071.h
+++ b/include/configs/a3m071.h
@@ -426,6 +426,7 @@ 
 #define CONFIG_SPL_BSS_MAX_SIZE		(64 << 10)
 
 #define CONFIG_SPL_OS_BOOT
+#define CONFIG_SPL_ENV_SUPPORT
 /* Place patched DT blob (fdt) at this address */
 #define CONFIG_SYS_SPL_ARGS_ADDR	0x01800000
 
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index ef00306..f47d3d1 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -325,6 +325,7 @@ 
 #define CONFIG_SPL_GPIO_SUPPORT
 #define CONFIG_SPL_YMODEM_SUPPORT
 #define CONFIG_SPL_NET_SUPPORT
+#define CONFIG_SPL_ENV_SUPPORT
 #define CONFIG_SPL_NET_VCI_STRING	"AM335x U-Boot SPL"
 #define CONFIG_SPL_ETH_SUPPORT
 #define CONFIG_SPL_SPI_SUPPORT
diff --git a/include/configs/pcm051.h b/include/configs/pcm051.h
index d0ea74e..926842f 100644
--- a/include/configs/pcm051.h
+++ b/include/configs/pcm051.h
@@ -224,6 +224,7 @@ 
 #define CONFIG_SPL_GPIO_SUPPORT
 #define CONFIG_SPL_YMODEM_SUPPORT
 #define CONFIG_SPL_NET_SUPPORT
+#define CONFIG_SPL_ENV_SUPPORT
 #define CONFIG_SPL_NET_VCI_STRING	"pcm051 U-Boot SPL"
 #define CONFIG_SPL_ETH_SUPPORT
 #define CONFIG_SPL_SPI_SUPPORT