[U-Boot] U-Boot: Environment flags broken for U-Boot
Patrick DELAUNAY
patrick.delaunay at st.com
Mon Sep 2 15:35:59 UTC 2019
Hi Heiko,
> From: Heiko Schocher <hs at denx.de>
> Sent: lundi 2 septembre 2019 16:03
>
> Hello Patrick,
>
> I am just testing U-Boot Environment flags and they do not work anymore with
> current mainline U-Boot ... I should have made some tbot test for it, but did not
> found yet time for it ...
>
> Here log with current mainline:
>
>
> => printenv heiko
> heiko=changed
> => env flags
> Available variable type flags (position 0):
> Flag Variable Type Name
> ---- ------------------
> s - string
> d - decimal
> x - hexadecimal
> b - boolean
> i - IP address
> m - MAC address
>
> Available variable access flags (position 1):
> Flag Variable Access Name
> ---- --------------------
> a - any
> r - read-only
> o - write-once
> c - change-default
>
> Static flags:
> Variable Name Variable Type Variable Access
> ------------- ------------- ---------------
> eth\d*addr MAC address any
> ipaddr IP address any
> gatewayip IP address any
> netmask IP address any
> serverip IP address any
> nvlan decimal any
> vlan decimal any
> dnsip IP address any
> heiko string write-once
>
> Active flags:
> Variable Name Variable Type Variable Access
> ------------- ------------- ---------------
> .flags string write-once
> netmask IP address any
> serverip IP address any
> heiko string write-once
> ipaddr IP address any
> => setenv heiko foo
> => print heiko
> heiko=foo
> => setenv heiko bar
> => print heiko
> heiko=bar
>
> I can change Environment variable "heiko" but flag is write-once !
>
> Ok, digging around and I found, that in env/common.c changed_ok is NULL which
> results in not checking U-Boot flags:
>
> 26 struct hsearch_data env_htab = {
> 27 #if CONFIG_IS_ENABLED(ENV_SUPPORT)
> 28 /* defined in flags.c, only compile with ENV_SUPPORT */
> 29 .change_ok = env_flags_validate,
> 30 #endif
> 31 };
>
> reason is your commit:
>
> commit 7d4776545b0f8a8827e5d061206faf61c9ba6ea9
> Author: Patrick Delaunay <patrick.delaunay at st.com>
> Date: Thu Apr 18 17:32:49 2019 +0200
>
> env: solve compilation error in SPL
>
> Solve compilation issue when cli_simple.o is used in SPL
> and CONFIG_SPL_ENV_SUPPORT is not defined.
>
> env/built-in.o:(.data.env_htab+0xc): undefined reference to `env_flags_validate'
> u-boot/scripts/Makefile.spl:384: recipe for target 'spl/u-boot-spl' failed
> make[2]: *** [spl/u-boot-spl] Error 1
> u-boot/Makefile:1649: recipe for target 'spl/u-boot-spl' failed
> make[1]: *** [spl/u-boot-spl] Error 2
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
>
>
> ENV_SUPPORT is only defined for SPL and TPL not for U-Boot, which leads in
> change_ok always NULL in U-Boot ...
>
> :-(
>
> reverting this commit and it works again as expected ...
>
> Urgs ... since april 2019 nobody tested this feature
>
> :-(
>
> Nevertheless, reverting commit and I see:
>
> => print heiko
> heiko=test
> => setenv heiko foo
> ## Error inserting "heiko" variable, errno=1 =>
>
> So we should find a solution for this.
>
> Any ideas?
Hi,
Yes I am responsible of the regression, sorry.
When I see the definition CONFIG_SPL_ENV_SUPPORT and CONFIG_TPL_ENV_SUPPORT, I use the generic macro to check the activation of these TPL/SPL feature in the SPL/TPL builds....
But I don't check the existence of the U-Boot flag CONFIG_ENV_SUPPORT when I propose my patch... so my patch is incorrect.
As flags.o is always compiled for U-Boot :
ifndef CONFIG_SPL_BUILD
obj-y += attr.o
obj-y += callback.o
obj-y += flags.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
I see 2 solutions:
1/ change my patch to check U-boot compilation case with !defined(CONFIG_SPL_BUILD)
26 struct hsearch_data env_htab = {
27 #if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
28 /* defined in flags.c, only compile with ENV_SUPPORT for SPL / TPL */
29 .change_ok = env_flags_validate,
30 #endif
31 };
2/ introduce a new Kconfig to env support in U-Boot
config ENV_SUPPORT
bool "Support an environment features"
default y
help
Enable full environment support in U-Boot.
Including attributes, callbacks and flags.
And the Makefile is more simple :
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-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
extra-$(CONFIG_ENV_IS_EMBEDDED) += embedded.o
obj-$(CONFIG_ENV_IS_IN_EEPROM) += embedded.o
extra-$(CONFIG_ENV_IS_IN_FLASH) += embedded.o
obj-$(CONFIG_ENV_IS_IN_NVRAM) += embedded.o
obj-$(CONFIG_ENV_IS_IN_NVRAM) += nvram.o
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
endif
but we have also other impact with hashtable...
obj-$(CONFIG_$(SPL_TPL_)ENV_SUPPORT) += hashtable.o
.....
and I have others issues in cmd/nvedit.c / cmd/nvedit.c
=> I don't sure of the side effect if I remove all this part in proper U-Boot.
So I prefer the solution 1, but I can go deeper with solution 2 (only remove flags.c) if you prefer.
If you are allign with my porposal 1 I propose this patch asap:
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
};
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
regards
Patrick.
More information about the U-Boot
mailing list