| Message ID | 20200326230200.12617-2-rasmus.villemoes@prevas.dk |
|---|---|
| State | Accepted |
| Commit | 1d0adee45cc2d3257c926d6022bf1922805b928e |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | allow CONFIG_SPL_SAVEENV to work with ENV_IS_IN_SPI_FLASH | expand |
On Fri, Mar 27, 2020 at 12:01:59AM +0100, Rasmus Villemoes wrote: > Currently, quite a few storage drivers currently do not honour > SPL_SAVEENV. That is, whether or not one enables CONFIG_SPL_SAVEENV, > the backend drivers do not provide the .save method. Witness > > env/fat.c:#ifdef CONFIG_SPL_BUILD > ... > env/fat.c-#else > env/fat.c-# define LOADENV > env/fat.c:# if defined(CONFIG_CMD_SAVEENV) > env/fat.c:# define CMD_SAVEENV > env/fat.c-# endif > env/fat.c-#endif > env/fat.c- > env/fat.c:#ifdef CMD_SAVEENV > env/fat.c-static int env_fat_save(void) > > env/flash.c:#ifndef CONFIG_SPL_BUILD > env/flash.c:# if defined(CONFIG_CMD_SAVEENV) && defined(CONFIG_CMD_FLASH) > env/flash.c:# define CMD_SAVEENV > ... > env/flash.c:#ifdef CMD_SAVEENV > env/flash.c-static int env_flash_save(void) > > env/mmc.c:#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD) > env/mmc.c-static inline int write_env(struct mmc *mmc, unsigned long size, > > env/nand.c:#if defined(CONFIG_CMD_SAVEENV) && defined(CONFIG_CMD_NAND) && \ > env/nand.c: !defined(CONFIG_SPL_BUILD) > env/nand.c:#define CMD_SAVEENV > ... > env/nand.c:#ifdef CMD_SAVEENV > env/nand.c-/* > env/nand.c- * The legacy NAND code saved the environment in the first NAND device i.e., > env/nand.c- * nand_dev_desc + 0. This is also the behaviour using the new NAND code. > env/nand.c- */ > env/nand.c-static int writeenv(size_t offset, u_char *buf) > > env/sf.c:#ifndef CONFIG_SPL_BUILD > env/sf.c:#define CMD_SAVEENV > env/sf.c-#define INITENV > env/sf.c-#endif > ... > env/sf.c:#ifdef CMD_SAVEENV > env/sf.c-static int env_sf_save(void) > > In all these cases, the mere presence of CONFIG_SPL_BUILD means the > save method does not get built. > > Now, it is currently a bit awkward to write a proper test for whether > saving the environment is enabled in the current context; something > like > > #if (!defined(CONFIG_SPL_BUILD) && defined(CONFIG_CMD_SAVEENV)) || \ > (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_SAVEENV)) > > But we already have a rather elegant mechanism that implicitly does > the CONFIG_SPL_BUILD tests, namely CONFIG_IS_ENABLED(). Using that > requires that the controlling config symbols follow a strict pattern: > FOO for U-Boot proper, SPL_FOO for SPL. > > This patch introduces CONFIG_SAVEENV as an alias for > CONFIG_CMD_SAVEENV. That way, the above can simply be written > > #if CONFIG_IS_ENABLED(SAVEENV) > > and moreover, CONFIG_IS_ENABLED(SAVEENV) can also be used in C code, > avoiding ifdeffery and providing more compile testing. > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> Applied to u-boot/master, thanks!
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 && \
Currently, quite a few storage drivers currently do not honour SPL_SAVEENV. That is, whether or not one enables CONFIG_SPL_SAVEENV, the backend drivers do not provide the .save method. Witness env/fat.c:#ifdef CONFIG_SPL_BUILD ... env/fat.c-#else env/fat.c-# define LOADENV env/fat.c:# if defined(CONFIG_CMD_SAVEENV) env/fat.c:# define CMD_SAVEENV env/fat.c-# endif env/fat.c-#endif env/fat.c- env/fat.c:#ifdef CMD_SAVEENV env/fat.c-static int env_fat_save(void) env/flash.c:#ifndef CONFIG_SPL_BUILD env/flash.c:# if defined(CONFIG_CMD_SAVEENV) && defined(CONFIG_CMD_FLASH) env/flash.c:# define CMD_SAVEENV ... env/flash.c:#ifdef CMD_SAVEENV env/flash.c-static int env_flash_save(void) env/mmc.c:#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD) env/mmc.c-static inline int write_env(struct mmc *mmc, unsigned long size, env/nand.c:#if defined(CONFIG_CMD_SAVEENV) && defined(CONFIG_CMD_NAND) && \ env/nand.c: !defined(CONFIG_SPL_BUILD) env/nand.c:#define CMD_SAVEENV ... env/nand.c:#ifdef CMD_SAVEENV env/nand.c-/* env/nand.c- * The legacy NAND code saved the environment in the first NAND device i.e., env/nand.c- * nand_dev_desc + 0. This is also the behaviour using the new NAND code. env/nand.c- */ env/nand.c-static int writeenv(size_t offset, u_char *buf) env/sf.c:#ifndef CONFIG_SPL_BUILD env/sf.c:#define CMD_SAVEENV env/sf.c-#define INITENV env/sf.c-#endif ... env/sf.c:#ifdef CMD_SAVEENV env/sf.c-static int env_sf_save(void) In all these cases, the mere presence of CONFIG_SPL_BUILD means the save method does not get built. Now, it is currently a bit awkward to write a proper test for whether saving the environment is enabled in the current context; something like #if (!defined(CONFIG_SPL_BUILD) && defined(CONFIG_CMD_SAVEENV)) || \ (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_SAVEENV)) But we already have a rather elegant mechanism that implicitly does the CONFIG_SPL_BUILD tests, namely CONFIG_IS_ENABLED(). Using that requires that the controlling config symbols follow a strict pattern: FOO for U-Boot proper, SPL_FOO for SPL. This patch introduces CONFIG_SAVEENV as an alias for CONFIG_CMD_SAVEENV. That way, the above can simply be written #if CONFIG_IS_ENABLED(SAVEENV) and moreover, CONFIG_IS_ENABLED(SAVEENV) can also be used in C code, avoiding ifdeffery and providing more compile testing. Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- v2: Expand commit message, explain why one needs a new config symbol in order to use CONFIG_IS_ENABLED, and demonstrate how many of the storage drivers don't compile their .save method when CONFIG_SPL_BUILD. The patch itself is the same. env/Kconfig | 3 +++ 1 file changed, 3 insertions(+)