Hi Troy, On 3/9/23 20:34, Troy Kisky wrote:
On Wed, Mar 1, 2023 at 9:39 AM Quentin Schulz <foss+ub...@0leil.net> wrote:From: Quentin Schulz <quentin.sch...@theobroma-systems.com> Instead of letting the compiler error out if CONFIG_ENV_IS_NOWHERE is not selected by the user, let's just enforce it when the user builds for Ringneck PX30 so that no check needs to be performed by the compiler and the configuration is always valid. Suggested-by: Tom Rini <tr...@konsulko.com> Cc: Quentin Schulz <foss+ub...@0leil.net> Signed-off-by: Quentin Schulz <quentin.sch...@theobroma-systems.com> --- board/theobroma-systems/ringneck_px30/Kconfig | 1 + board/theobroma-systems/ringneck_px30/ringneck-px30.c | 4 ---- configs/ringneck-px30_defconfig | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/board/theobroma-systems/ringneck_px30/Kconfig b/board/theobroma-systems/ringneck_px30/Kconfig index 24d94807db8..c33253bdad8 100644 --- a/board/theobroma-systems/ringneck_px30/Kconfig +++ b/board/theobroma-systems/ringneck_px30/Kconfig @@ -11,6 +11,7 @@ config SYS_CONFIG_NAME config BOARD_SPECIFIC_OPTIONS # dummy def_bool y + select ENV_IS_NOWHERE config ENV_SIZE default 0x3000 diff --git a/board/theobroma-systems/ringneck_px30/ringneck-px30.c b/board/theobroma-systems/ringneck_px30/ringneck-px30.c index 47d1a40ef7c..bb1bb4acf5c 100644 --- a/board/theobroma-systems/ringneck_px30/ringneck-px30.c +++ b/board/theobroma-systems/ringneck_px30/ringneck-px30.c @@ -118,10 +118,6 @@ int mmc_get_env_dev(void) return CONFIG_SYS_MMC_ENV_DEV; } -#if !IS_ENABLED(CONFIG_ENV_IS_NOWHERE) -#error Please enable CONFIG_ENV_IS_NOWHERE -#endif - enum env_location arch_env_get_location(enum env_operation op, int prio) { const char *boot_device = diff --git a/configs/ringneck-px30_defconfig b/configs/ringneck-px30_defconfig index 91706d8def2..e9234efc2a0 100644 --- a/configs/ringneck-px30_defconfig +++ b/configs/ringneck-px30_defconfig @@ -68,7 +68,6 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_OF_SPL_REMOVE_PROPS="interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_ENV_OVERWRITE=y -CONFIG_ENV_IS_NOWHERE=y CONFIG_ENV_IS_IN_MMC=yHi Quentin, Should you delete the line CONFIG_ENV_IS_IN_MMC=y as well ?
We want to allow the user to pick either SPI flash or eMMC flash or both storage media (or none) as possible storage location for the U-Boot environment.
Since there's no need to enforce support for both flash storage medium kinds, I don't believe we should do the same as ENV_IS_NOWHERE for ENV_IS_IN_MMC or ENV_IS_IN_SPI_FLASH especially since this is handled in the board C file, c.f.: https://source.denx.de/u-boot/u-boot/-/blob/master/board/theobroma-systems/ringneck_px30/ringneck-px30.c#L141-L144 and https://source.denx.de/u-boot/u-boot/-/blob/master/board/theobroma-systems/puma_rk3399/puma-rk3399.c#L159-L166.
While we aren't too much interested in no storage medium for U-Boot environment, this is unfortunately a requirement in how U-Boot handles missing U-Boot environment in supported storage media for the environment, so here we are :)
Does that answer your question? BR, Quentin
