Hi Heiko,
From: Heiko Schocher hs@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@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@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@denx.de
regards Patrick.