From patchwork Wed Feb 19 09:47:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236551 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Wed, 19 Feb 2020 09:47:39 +0000 Subject: [PATCH 1/5] env: add SAVEENV as an alias of the CMD_SAVEENV symbol In-Reply-To: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> References: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> Message-ID: <20200219094726.26798-2-rasmus.villemoes@prevas.dk> Currently, testing whether to compile in support for saving the environment is a bit awkward when one needs to take SPL_SAVEENV into account, and quite a few storage drivers currently do not honour SPL_SAVEENV. To make it a bit easier to decide whether environment saving should be enabled, introduce SAVEENV as an alias for the CMD_SAVEENV symbol. Then one can simply use CONFIG_IS_ENABLED(SAVEENV) Signed-off-by: Rasmus Villemoes --- env/Kconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index 0d6f559b39..969308fe6c 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -3,6 +3,9 @@ menu "Environment" config ENV_SUPPORT def_bool y +config SAVEENV + def_bool y if CMD_SAVEENV + config ENV_IS_NOWHERE bool "Environment is not stored" default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \ From patchwork Wed Feb 19 09:47:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236554 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Wed, 19 Feb 2020 09:47:40 +0000 Subject: [PATCH 2/5] env_internal.h: add alternative ENV_SAVE_PTR macro In-Reply-To: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> References: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> Message-ID: <20200219094726.26798-3-rasmus.villemoes@prevas.dk> The current definition of the env_save_ptr does not take SPL_SAVEENV into account. Moreover, the way it is implemented means that drivers need to guard the definitions of their _save methods with ifdefs to avoid "defined but unused" warnings in case CMD_SAVEENV=n. The ifdeffery can be avoided by using a "something ? x : NULL" construction instead and still have the compiler elide the _save method when it is not referenced. Unfortunately we can't just switch the existing env_save_ptr macro, since that would give a lot of build errors unless all the ifdeffery is removed at the same time. Conversely, removing that ifdeffery first would merely lead to the "defined but unused" warnings temporarily, but for some storage drivers it requires a bit more work than just removing their private CMD_SAVEENV logic. So introduce an alternative to env_save_ptr, which for lack of a better name is simply uppercased, allowing one to update storage drivers piecemeal to both reduce their ifdeffery and honour CONFIG_SPL_SAVEENV. Signed-off-by: Rasmus Villemoes --- include/env_internal.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/env_internal.h b/include/env_internal.h index 90a4df8a72..e89fbdb1b7 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -207,6 +207,8 @@ struct env_driver { #define env_save_ptr(x) NULL #endif +#define ENV_SAVE_PTR(x) (CONFIG_IS_ENABLED(SAVEENV) ? (x) : NULL) + extern struct hsearch_data env_htab; #endif /* DO_DEPS_ONLY */ From patchwork Wed Feb 19 09:47:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236552 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Wed, 19 Feb 2020 09:47:41 +0000 Subject: [PATCH 3/5] env/fat.c: remove private CMD_SAVEENV logic In-Reply-To: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> References: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> Message-ID: <20200219094726.26798-4-rasmus.villemoes@prevas.dk> Always compile the env_fat_save() function, and let CONFIG_IS_ENABLED(SAVEENV) (via the ENV_SAVE_PTR macro) decide whether it actually ends up being compiled in. Signed-off-by: Rasmus Villemoes --- env/fat.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/env/fat.c b/env/fat.c index 1836556f36..cf2e5e2b26 100644 --- a/env/fat.c +++ b/env/fat.c @@ -26,12 +26,8 @@ # endif #else # define LOADENV -# if defined(CONFIG_CMD_SAVEENV) -# define CMD_SAVEENV -# endif #endif -#ifdef CMD_SAVEENV static int env_fat_save(void) { env_t __aligned(ARCH_DMA_MINALIGN) env_new; @@ -76,7 +72,6 @@ static int env_fat_save(void) return 0; } -#endif /* CMD_SAVEENV */ #ifdef LOADENV static int env_fat_load(void) @@ -135,7 +130,5 @@ U_BOOT_ENV_LOCATION(fat) = { #ifdef LOADENV .load = env_fat_load, #endif -#ifdef CMD_SAVEENV - .save = env_save_ptr(env_fat_save), -#endif + .save = ENV_SAVE_PTR(env_fat_save), }; From patchwork Wed Feb 19 09:47:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236555 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Wed, 19 Feb 2020 09:47:42 +0000 Subject: [PATCH 4/5] env/ext4.c: remove CONFIG_CMD_SAVEENV ifdef In-Reply-To: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> References: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> Message-ID: <20200219094726.26798-5-rasmus.villemoes@prevas.dk> Removing this ifdef/endif pair yields a "defined but unused warning" for CONFIG_CMD_SAVEENV=n, but that vanishes if we use the ENV_SAVE_PTR macro instead. This gives slightly better compile testing, and moreover, it's possible to have CONFIG_CMD_SAVEENV=n CONFIG_SPL_SAVEENV=y SPL_ENV_IS_IN_EXT4=y in which case env_ext4_save would erroneously not be compiled in. Signed-off-by: Rasmus Villemoes --- env/ext4.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/env/ext4.c b/env/ext4.c index 1f6b1b5bd8..911e19c6d3 100644 --- a/env/ext4.c +++ b/env/ext4.c @@ -41,7 +41,6 @@ __weak const char *env_ext4_get_dev_part(void) return (const char *)CONFIG_ENV_EXT4_DEVICE_AND_PART; } -#ifdef CONFIG_CMD_SAVEENV static int env_ext4_save(void) { env_t env_new; @@ -83,7 +82,6 @@ static int env_ext4_save(void) puts("done\n"); return 0; } -#endif /* CONFIG_CMD_SAVEENV */ static int env_ext4_load(void) { @@ -137,5 +135,5 @@ U_BOOT_ENV_LOCATION(ext4) = { .location = ENVL_EXT4, ENV_NAME("EXT4") .load = env_ext4_load, - .save = env_save_ptr(env_ext4_save), + .save = ENV_SAVE_PTR(env_ext4_save), }; From patchwork Wed Feb 19 09:47:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236553 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Wed, 19 Feb 2020 09:47:43 +0000 Subject: [PATCH 5/5] env/sf.c: drop private CMD_SAVEENV logic In-Reply-To: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> References: <20200219094726.26798-1-rasmus.villemoes@prevas.dk> Message-ID: <20200219094726.26798-6-rasmus.villemoes@prevas.dk> Deciding whether to compile the env_sf_save() function based solely on CONFIG_SPL_BUILD is wrong: For U-Boot proper, it leads to a build warning in case CONFIG_CMD_SAVEENV=n (because the env_save_ptr() macro causes the function to indeed not be referenced anywhere). And for SPL, when one selects CONFIG_SPL_SAVEENV, one obviously expects to actually be able to save the environment. Signed-off-by: Rasmus Villemoes --- env/sf.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/env/sf.c b/env/sf.c index 5ef4055219..22b70ad319 100644 --- a/env/sf.c +++ b/env/sf.c @@ -21,16 +21,12 @@ #include #ifndef CONFIG_SPL_BUILD -#define CMD_SAVEENV #define INITENV #endif #ifdef CONFIG_ENV_OFFSET_REDUND -#ifdef CMD_SAVEENV static ulong env_offset = CONFIG_ENV_OFFSET; static ulong env_new_offset = CONFIG_ENV_OFFSET_REDUND; -#endif - #endif /* CONFIG_ENV_OFFSET_REDUND */ DECLARE_GLOBAL_DATA_PTR; @@ -69,7 +65,6 @@ static int setup_flash_device(void) } #if defined(CONFIG_ENV_OFFSET_REDUND) -#ifdef CMD_SAVEENV static int env_sf_save(void) { env_t env_new; @@ -148,7 +143,6 @@ static int env_sf_save(void) return ret; } -#endif /* CMD_SAVEENV */ static int env_sf_load(void) { @@ -187,7 +181,6 @@ out: return ret; } #else -#ifdef CMD_SAVEENV static int env_sf_save(void) { u32 saved_size, saved_offset, sector; @@ -247,7 +240,6 @@ static int env_sf_save(void) return ret; } -#endif /* CMD_SAVEENV */ static int env_sf_load(void) { @@ -313,9 +305,7 @@ U_BOOT_ENV_LOCATION(sf) = { .location = ENVL_SPI_FLASH, ENV_NAME("SPI Flash") .load = env_sf_load, -#ifdef CMD_SAVEENV - .save = env_save_ptr(env_sf_save), -#endif + .save = ENV_SAVE_PTR(env_sf_save), #if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0) .init = env_sf_init, #endif