[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Patrick Delaunay]

Add a new flag CONFIG_ENV_SUPPORT to compile all
the environment features in U-Boot (attributes, callbacks
and flags). It is the equivalent of the 2 existing flags
- CONFIG_SPL_ENV_SUPPORT for SPL
- CONFIG_TPL_ENV_SUPPORT for TPL

This new configuration allows to use the macro
CONFIG_IS_ENABLED(ENV_SUPPORT) in the code without
issue and solves the regression introduced by
commit 7d4776545b0f ("env: solve compilation error
in SPL"); change_ok was always NULL in U-Boot.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 cmd/Kconfig            |  2 ++
 env/Kconfig            |  7 +++++++
 env/Makefile           | 11 ++++-------
 include/env_callback.h |  4 ++++
 include/env_flags.h    |  4 ++++
 5 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 05872fa..f7a1b1f 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -465,6 +465,7 @@ config CMD_ENV_EXISTS
 
 config CMD_ENV_CALLBACK
 	bool "env callbacks - print callbacks and their associated variables"
+	depends on ENV_SUPPORT
 	help
 	  Some environment variable have callbacks defined by
 	  U_BOOT_ENV_CALLBACK. These are called when the variable changes.
@@ -473,6 +474,7 @@ config CMD_ENV_CALLBACK
 
 config CMD_ENV_FLAGS
 	bool "env flags -print variables that have non-default flags"
+	depends on ENV_SUPPORT
 	help
 	  Some environment variables have special flags that control their
 	  behaviour. For example, serial# can only be written once and cannot
diff --git a/env/Kconfig b/env/Kconfig
index 74db2f3..f0c5a7a 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -1,5 +1,12 @@
 menu "Environment"
 
+config ENV_SUPPORT
+	bool "Support all environment features"
+	default y
+	help
+	  Enable full environment support in U-Boot,
+	  including attributes, callbacks and flags.
+
 config ENV_IS_NOWHERE
 	bool "Environment is not stored"
 	default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \
diff --git a/env/Makefile b/env/Makefile
index 90144d6..2a468ac 100644
--- a/env/Makefile
+++ b/env/Makefile
@@ -5,10 +5,11 @@
 
 obj-y += common.o env.o
 
+obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += attr.o
+obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += flags.o
+obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += callback.o
+
 ifndef CONFIG_SPL_BUILD
-obj-y += attr.o
-obj-y += callback.o
-obj-y += flags.o
 obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
 extra-$(CONFIG_ENV_IS_EMBEDDED) += embedded.o
 obj-$(CONFIG_ENV_IS_IN_EEPROM) += embedded.o
@@ -19,10 +20,6 @@ obj-$(CONFIG_ENV_IS_IN_ONENAND) += onenand.o
 obj-$(CONFIG_ENV_IS_IN_SATA) += sata.o
 obj-$(CONFIG_ENV_IS_IN_REMOTE) += remote.o
 obj-$(CONFIG_ENV_IS_IN_UBI) += ubi.o
-else
-obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += attr.o
-obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += flags.o
-obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += callback.o
 endif
 
 obj-$(CONFIG_$(SPL_TPL_)ENV_IS_NOWHERE) += nowhere.o
diff --git a/include/env_callback.h b/include/env_callback.h
index 982c078..a757fe6 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -72,6 +72,10 @@
 	"serial#:serialno," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
+#if CONFIG_IS_ENABLED(ENV_SUPPORT)
 void env_callback_init(struct env_entry *var_entry);
+#else
+static inline void env_callback_init(struct env_entry *var_entry) { }
+#endif
 
 #endif /* __ENV_CALLBACK_H__ */
diff --git a/include/env_flags.h b/include/env_flags.h
index e5380f2..ec480e3 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -153,7 +153,11 @@ int env_flags_validate_env_set_params(char *name, char *const val[], int count);
  * When adding a variable to the environment, initialize the flags for that
  * variable.
  */
+#if CONFIG_IS_ENABLED(ENV_SUPPORT)
 void env_flags_init(struct env_entry *var_entry);
+#else
+static inline void env_flags_init(struct env_entry *var_entry) { }
+#endif
 
 /*
  * Validate the newval for to conform with the requirements defined by its flags
-- 
2.7.4

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Lukasz Majewski]

Hi Patrick,

> Add a new flag CONFIG_ENV_SUPPORT to compile all
> the environment features in U-Boot (attributes, callbacks
> and flags). It is the equivalent of the 2 existing flags
> - CONFIG_SPL_ENV_SUPPORT for SPL
> - CONFIG_TPL_ENV_SUPPORT for TPL

Shouldn't it be the "supplement" ?

I guess that it is possible to define 

CONFIG_SPL_ENV_SUPPORT to have ENV support in SPL

CONFIG_TPL_ENV_SUPPORT to have ENV support in TPL

and there is a third flag - CONFIG_ENV_SUPPORT to enable envs support
in U-Boot proper?

In that way one can enable ENV support only in SPL (via SPL_ENV_SUPPORT)
and disable envs in U-Boot proper (by NOT defining ENV_SUPPORT) and use
build in envs (in the binary).

> 
> This new configuration allows to use the macro
> CONFIG_IS_ENABLED(ENV_SUPPORT) in the code without
> issue and solves the regression introduced by
> commit 7d4776545b0f ("env: solve compilation error
> in SPL"); change_ok was always NULL in U-Boot.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> 
>  cmd/Kconfig            |  2 ++
>  env/Kconfig            |  7 +++++++
>  env/Makefile           | 11 ++++-------
>  include/env_callback.h |  4 ++++
>  include/env_flags.h    |  4 ++++
>  5 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 05872fa..f7a1b1f 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -465,6 +465,7 @@ config CMD_ENV_EXISTS
>  
>  config CMD_ENV_CALLBACK
>  	bool "env callbacks - print callbacks and their associated
> variables"
> +	depends on ENV_SUPPORT
>  	help
>  	  Some environment variable have callbacks defined by
>  	  U_BOOT_ENV_CALLBACK. These are called when the variable
> changes. @@ -473,6 +474,7 @@ config CMD_ENV_CALLBACK
>  
>  config CMD_ENV_FLAGS
>  	bool "env flags -print variables that have non-default flags"
> +	depends on ENV_SUPPORT
>  	help
>  	  Some environment variables have special flags that control
> their behaviour. For example, serial# can only be written once and
> cannot diff --git a/env/Kconfig b/env/Kconfig
> index 74db2f3..f0c5a7a 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -1,5 +1,12 @@
>  menu "Environment"
>  
> +config ENV_SUPPORT
> +	bool "Support all environment features"
> +	default y
> +	help
> +	  Enable full environment support in U-Boot,
> +	  including attributes, callbacks and flags.
> +
>  config ENV_IS_NOWHERE
>  	bool "Environment is not stored"
>  	default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \
> diff --git a/env/Makefile b/env/Makefile
> index 90144d6..2a468ac 100644
> --- a/env/Makefile
> +++ b/env/Makefile
> @@ -5,10 +5,11 @@
>  
>  obj-y += common.o env.o
>  
> +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += attr.o
> +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += flags.o
> +obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += callback.o
> +
>  ifndef CONFIG_SPL_BUILD
> -obj-y += attr.o
> -obj-y += callback.o
> -obj-y += flags.o
>  obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
>  extra-$(CONFIG_ENV_IS_EMBEDDED) += embedded.o
>  obj-$(CONFIG_ENV_IS_IN_EEPROM) += embedded.o
> @@ -19,10 +20,6 @@ obj-$(CONFIG_ENV_IS_IN_ONENAND) += onenand.o
>  obj-$(CONFIG_ENV_IS_IN_SATA) += sata.o
>  obj-$(CONFIG_ENV_IS_IN_REMOTE) += remote.o
>  obj-$(CONFIG_ENV_IS_IN_UBI) += ubi.o
> -else
> -obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += attr.o
> -obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += flags.o
> -obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += callback.o
>  endif
>  
>  obj-$(CONFIG_$(SPL_TPL_)ENV_IS_NOWHERE) += nowhere.o
> diff --git a/include/env_callback.h b/include/env_callback.h
> index 982c078..a757fe6 100644
> --- a/include/env_callback.h
> +++ b/include/env_callback.h
> @@ -72,6 +72,10 @@
>  	"serial#:serialno," \
>  	CONFIG_ENV_CALLBACK_LIST_STATIC
>  
> +#if CONFIG_IS_ENABLED(ENV_SUPPORT)
>  void env_callback_init(struct env_entry *var_entry);
> +#else
> +static inline void env_callback_init(struct env_entry *var_entry) { }
> +#endif
>  
>  #endif /* __ENV_CALLBACK_H__ */
> diff --git a/include/env_flags.h b/include/env_flags.h
> index e5380f2..ec480e3 100644
> --- a/include/env_flags.h
> +++ b/include/env_flags.h
> @@ -153,7 +153,11 @@ int env_flags_validate_env_set_params(char
> *name, char *const val[], int count);
>   * When adding a variable to the environment, initialize the flags
> for that
>   * variable.
>   */
> +#if CONFIG_IS_ENABLED(ENV_SUPPORT)
>  void env_flags_init(struct env_entry *var_entry);
> +#else
> +static inline void env_flags_init(struct env_entry *var_entry) { }
> +#endif
>  
>  /*
>   * Validate the newval for to conform with the requirements defined
> by its flags



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190904/aad48e6f/attachment.sig>

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Patrick DELAUNAY]

Hi Lukasz,

> From: Lukasz Majewski <lukma@denx.de>
> Sent: mercredi 4 septembre 2019 10:52
> 
> Hi Patrick,
> 
> > Add a new flag CONFIG_ENV_SUPPORT to compile all the environment
> > features in U-Boot (attributes, callbacks and flags). It is the
> > equivalent of the 2 existing flags
> > - CONFIG_SPL_ENV_SUPPORT for SPL
> > - CONFIG_TPL_ENV_SUPPORT for TPL
> 
> Shouldn't it be the "supplement" ?
> 
> I guess that it is possible to define
> 
> CONFIG_SPL_ENV_SUPPORT to have ENV support in SPL
> 
> CONFIG_TPL_ENV_SUPPORT to have ENV support in TPL
> 
> and there is a third flag - CONFIG_ENV_SUPPORT to enable envs support in U-
> Boot proper?
> 
> In that way one can enable ENV support only in SPL (via SPL_ENV_SUPPORT)
> and disable envs in U-Boot proper (by NOT defining ENV_SUPPORT) and use
> build in envs (in the binary).

Yes exactly the support for U-Boot, SPL or TPL are now independent,
I will update the commit message in V2, "supplement" is more clear.

> 
> >
> > This new configuration allows to use the macro
> > CONFIG_IS_ENABLED(ENV_SUPPORT) in the code without issue and solves
> > the regression introduced by commit 7d4776545b0f ("env: solve
> > compilation error in SPL"); change_ok was always NULL in U-Boot.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  cmd/Kconfig            |  2 ++
> >  env/Kconfig            |  7 +++++++
> >  env/Makefile           | 11 ++++-------
> >  include/env_callback.h |  4 ++++
> >  include/env_flags.h    |  4 ++++
> >  5 files changed, 21 insertions(+), 7 deletions(-)

> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 

Best regards

Patrick Delaunay

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Wolfgang Denk]

Dear Patrick,

In message <1567530547-14331-1-git-send-email-patrick.delaunay@st.com> you wrote:
> Add a new flag CONFIG_ENV_SUPPORT to compile all
> the environment features in U-Boot (attributes, callbacks
> and flags). It is the equivalent of the 2 existing flags

I think this is a bda name, as it is misleading.  It sounds as if it
is used to enable the support of environment vaiables at all, which
it does not - instead it only enables / disables a few specific
extended features.  This must be reflected in the name.

> - CONFIG_SPL_ENV_SUPPORT for SPL
> - CONFIG_TPL_ENV_SUPPORT for TPL

This scares me.  Why are there different settings for SPL, TPL and
U-Boot proper?  This looks conceptually broken to me - if a system
is configured to use a specific set of environment features and
extensions, then the exact same settings must be use in all of SPL,
TPL and U-Boot proper.

I understand that it may be desirable for example to reduce the size
of the SPL by omitting some environment extensions, but provide
these in U-Boot proper, but this is broken and dangerous.  For
example, U-Boot flags are often used to enforce a certain level of
security (say, by making environment variables read-only or such).
The same level of handling and protection must also be maintained in
SPL and TPL.

So please reconsider this whole implementation, and make sure that
only a single macro ise used everywhere to enable these features.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Never ascribe to malice that which can  adequately  be  explained  by
stupidity.

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Patrick DELAUNAY]

Hi,

> From: Wolfgang Denk <wd@denx.de>
> Sent: samedi 7 septembre 2019 13:52
> 
> Dear Patrick,
> 
> In message <1567530547-14331-1-git-send-email-patrick.delaunay@st.com> you
> wrote:
> > Add a new flag CONFIG_ENV_SUPPORT to compile all the environment
> > features in U-Boot (attributes, callbacks and flags). It is the
> > equivalent of the 2 existing flags
> 
> I think this is a bda name, as it is misleading.  It sounds as if it is used to enable
> the support of environment vaiables at all, which it does not - instead it only
> enables / disables a few specific extended features.  This must be reflected in the
> name.

Yes, I use the name than SPL/TPL to use the macro CONFIG_IS_ENABLED(...)

And I agree the name seens not perfect.

> > - CONFIG_SPL_ENV_SUPPORT for SPL
> > - CONFIG_TPL_ENV_SUPPORT for TPL

These pre-existing name are defined in common/spl/Kconfig

With the same issue (env/common.o env/env.o are always compiled for SPL/TPL
so it is alo bad names)

> This scares me.  Why are there different settings for SPL, TPL and U-Boot
> proper?  This looks conceptually broken to me - if a system is configured to use a
> specific set of environment features and extensions, then the exact same settings
> must be use in all of SPL, TPL and U-Boot proper.
> 
> I understand that it may be desirable for example to reduce the size of the SPL by
> omitting some environment extensions, but provide these in U-Boot proper, but
> this is broken and dangerous.  For example, U-Boot flags are often used to
> enforce a certain level of security (say, by making environment variables read-
> only or such).

I agree, that scare me also.
For me also ENV_SUPPORT should be always enable for U-Boot.

My preferred proposal was only to solve the regression introduced by my initial patch and 
always set change_ok for U-Boot proper (when CONFIG_SPL_BUILD is not defined):

struct hsearch_data env_htab = {
- #if CONFIG_IS_ENABLED(ENV_SUPPORT)
-        /* defined in flags.c, only compile with ENV_SUPPORT */
+#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
+        /* defined in flags.c, only compile with ENV_SUPPORT for SPL / TPL */
         .change_ok = env_flags_validate,
 #endif
 };

http://u-boot.10912.n7.nabble.com/U-Boot-Environment-flags-broken-for-U-Boot-tt382673.html#a382688

The same test is already done in:

drivers/reset/reset-socfpga.c:47:#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
drivers/input/input.c:656:#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)

> The same level of handling and protection must also be maintained in SPL and
> TPL.

if I understood correctly the proposed dependency is :
	TPL ENV_SUPPORT select SPL_ENV_SUPPORT
	SPL ENV_SUPPORT select ENV_SUPPORT

> So please reconsider this whole implementation, and make sure that only a single
> macro ise used everywhere to enable these features.

But, if I use the same CONFIG for the 3 binary SPL,TPL and U-Boot,
l increase the size of TPL/SPL for all the platforms when these additional features are not needed.

And I am not the sure of the correct dependency for ENV between binary.

Heiko what is you feedback on Wolfgang remarks....

Do you think that I need to come back to the first/simple proposal ?

> 
> Best regards,
> 
> Wolfgang Denk
> 
> --

Best regards

Patrick Delaunay

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Tom Rini]

On Tue, Sep 10, 2019 at 11:01:18AM +0000, Patrick DELAUNAY wrote:
> Hi,
> 
> > From: Wolfgang Denk <wd@denx.de>
> > Sent: samedi 7 septembre 2019 13:52
> > 
> > Dear Patrick,
> > 
> > In message <1567530547-14331-1-git-send-email-patrick.delaunay@st.com> you
> > wrote:
> > > Add a new flag CONFIG_ENV_SUPPORT to compile all the environment
> > > features in U-Boot (attributes, callbacks and flags). It is the
> > > equivalent of the 2 existing flags
> > 
> > I think this is a bda name, as it is misleading.  It sounds as if it is used to enable
> > the support of environment vaiables at all, which it does not - instead it only
> > enables / disables a few specific extended features.  This must be reflected in the
> > name.
> 
> Yes, I use the name than SPL/TPL to use the macro CONFIG_IS_ENABLED(...)
> 
> And I agree the name seens not perfect.
> 
> > > - CONFIG_SPL_ENV_SUPPORT for SPL
> > > - CONFIG_TPL_ENV_SUPPORT for TPL
> 
> These pre-existing name are defined in common/spl/Kconfig
> 
> With the same issue (env/common.o env/env.o are always compiled for SPL/TPL
> so it is alo bad names)
> 
> > This scares me.  Why are there different settings for SPL, TPL and U-Boot
> > proper?  This looks conceptually broken to me - if a system is configured to use a
> > specific set of environment features and extensions, then the exact same settings
> > must be use in all of SPL, TPL and U-Boot proper.
> > 
> > I understand that it may be desirable for example to reduce the size of the SPL by
> > omitting some environment extensions, but provide these in U-Boot proper, but
> > this is broken and dangerous.  For example, U-Boot flags are often used to
> > enforce a certain level of security (say, by making environment variables read-
> > only or such).
> 
> I agree, that scare me also.
> For me also ENV_SUPPORT should be always enable for U-Boot.
> 
> My preferred proposal was only to solve the regression introduced by my initial patch and 
> always set change_ok for U-Boot proper (when CONFIG_SPL_BUILD is not defined):
> 
> struct hsearch_data env_htab = {
> - #if CONFIG_IS_ENABLED(ENV_SUPPORT)
> -        /* defined in flags.c, only compile with ENV_SUPPORT */
> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> +        /* defined in flags.c, only compile with ENV_SUPPORT for SPL / TPL */
>          .change_ok = env_flags_validate,
>  #endif
>  };
> 
> http://u-boot.10912.n7.nabble.com/U-Boot-Environment-flags-broken-for-U-Boot-tt382673.html#a382688
> 
> The same test is already done in:
> 
> drivers/reset/reset-socfpga.c:47:#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> drivers/input/input.c:656:#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> 
> > The same level of handling and protection must also be maintained in SPL and
> > TPL.
> 
> if I understood correctly the proposed dependency is :
> 	TPL ENV_SUPPORT select SPL_ENV_SUPPORT
> 	SPL ENV_SUPPORT select ENV_SUPPORT
> 
> > So please reconsider this whole implementation, and make sure that only a single
> > macro ise used everywhere to enable these features.
> 
> But, if I use the same CONFIG for the 3 binary SPL,TPL and U-Boot,
> l increase the size of TPL/SPL for all the platforms when these additional features are not needed.
> 
> And I am not the sure of the correct dependency for ENV between binary.
> 
> Heiko what is you feedback on Wolfgang remarks....
> 
> Do you think that I need to come back to the first/simple proposal ?

My two cents is that I would like to see a regression fix "soon" and for
this release, and some corrections / updates to names, what is/isn't
possible to enable (keeping in mind what is desirable to allow) for the
next release.  Thanks all!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190910/6e39ac19/attachment.sig>

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Wolfgang Denk]

Dear Patrick,

In message <9c7801afb8c94c638933cf33746ae300@SFHDAG6NODE3.st.com> you wrote:
> 
> And I agree the name seens not perfect.
> 
> > > - CONFIG_SPL_ENV_SUPPORT for SPL
> > > - CONFIG_TPL_ENV_SUPPORT for TPL
> 
> These pre-existing name are defined in common/spl/Kconfig
> 
> With the same issue (env/common.o env/env.o are always compiled for SPL/TPL
> so it is alo bad names)

Correct.

> > So please reconsider this whole implementation, and make sure that only a single
> > macro ise used everywhere to enable these features.
> 
> But, if I use the same CONFIG for the 3 binary SPL,TPL and U-Boot,
> l increase the size of TPL/SPL for all the platforms when these
> additional features are not needed.

Either the U-Boot environment makes use of these features, then they
have to be enabled, and exactly the same way in SPL, TPL and U-Boot
proper.  Or you don't need them, then they can be disabled, but
again in a consistent way in SPL, TPL, and U-Boot proper.

It is not acceptable to have for example .flags support in U-Boot,
but not is SPL.  If you cannot affort the size in SPL (and need
environment there at all), then you cannot have it in U-Boot either.
Yes, this is sad, but anything else would break the implementation of
these features, and given that they are often used to implement some
level of protection or security, introduce massive security issues.


So if SPL size is critical, you can try do not access the
environment at all and omit _all_ of the environment code there; or
you can try to arrange for a read-only implementation (omitting at
least the code needed for "env save" including write routines to
storage).  But you CANNOT omit the extensions if these are present
in U-Boot proper.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Die Scheu vor Verantwortung ist die Krankheit unserer Zeit.
                                                 -- Otto von Bismarck

[U-Boot] [PATCH] env: Add CONFIG_ENV_SUPPORT

[Patrick DELAUNAY]

Hi Wolfgang an Tom,

> 
> Dear Patrick,
> 
> In message <9c7801afb8c94c638933cf33746ae300@SFHDAG6NODE3.st.com>
> you wrote:
> >
> > And I agree the name seens not perfect.
> >
> > > > - CONFIG_SPL_ENV_SUPPORT for SPL
> > > > - CONFIG_TPL_ENV_SUPPORT for TPL
> >
> > These pre-existing name are defined in common/spl/Kconfig
> >
> > With the same issue (env/common.o env/env.o are always compiled for
> > SPL/TPL so it is alo bad names)
> 
> Correct.
> 
> > > So please reconsider this whole implementation, and make sure that
> > > only a single macro ise used everywhere to enable these features.
> >
> > But, if I use the same CONFIG for the 3 binary SPL,TPL and U-Boot, l
> > increase the size of TPL/SPL for all the platforms when these
> > additional features are not needed.
> 
> Either the U-Boot environment makes use of these features, then they have to be
> enabled, and exactly the same way in SPL, TPL and U-Boot proper.  Or you don't
> need them, then they can be disabled, but again in a consistent way in SPL, TPL,
> and U-Boot proper.
> 
> It is not acceptable to have for example .flags support in U-Boot, but not is SPL.
> If you cannot affort the size in SPL (and need environment there at all), then you
> cannot have it in U-Boot either.
> Yes, this is sad, but anything else would break the implementation of these
> features, and given that they are often used to implement some level of protection
> or security, introduce massive security issues.
> 
> 
> So if SPL size is critical, you can try do not access the environment at all and omit
> _all_ of the environment code there; or you can try to arrange for a read-only
> implementation (omitting at least the code needed for "env save" including write
> routines to storage).  But you CANNOT omit the extensions if these are present in
> U-Boot proper.

I am working on a update of the first proposal and I will delivered it in 2 step:

1- a  simple patch to solve the regression
    (to have short term solution  / integration in master as requested by Tom)

2- introduction of CONFGI_ENV_FULL_SUPPORT
	=> compilation of attr / flags / callback for TPL/SPL/U-boot
	=> feature can't be remove in SPL/TPL independently 
	As proposed by Wolfgang


> Best regards,
> 
> Wolfgang Denk
> 

Regards

Patrick