Message ID | 20230428134535.26275-1-stefan.herbrechtsmeier-oss@weidmueller.com |
---|---|
State | Rejected |
Delegated to: | Heiko Schocher |
Headers | show |
Series | env: Remove misuse of env is nowhere driver | expand |
Hello Stefan, On 28.04.23 15:45, Stefan Herbrechtsmeier wrote: > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > When using a list of writeable variables, the initial values come from > the built-in default environment since commit 5ab81058364b > ("env: Complete generic support for writable list"). Remove unnecessary > misuse of the env is nowhere driver as default environment. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > --- > > board/aristainetos/aristainetos.c | 19 ------------------- > configs/aristainetos2c_defconfig | 1 - > configs/aristainetos2ccslb_defconfig | 1 - > env/env.c | 2 -- > 4 files changed, 23 deletions(-) NACK. You completely break the protected environment functionality for the aristainetos board when you remove env_get_location() Does it build anymore? Also you mix up a change in common code and in board code. And grepping in code for ENVL_NOWHERE has much more finds... bye, Heiko
Hello Stefan, On 29.04.23 06:30, Heiko Schocher wrote: > Hello Stefan, > > On 28.04.23 15:45, Stefan Herbrechtsmeier wrote: >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> >> When using a list of writeable variables, the initial values come from >> the built-in default environment since commit 5ab81058364b >> ("env: Complete generic support for writable list"). Remove unnecessary >> misuse of the env is nowhere driver as default environment. >> >> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> --- >> >> board/aristainetos/aristainetos.c | 19 ------------------- >> configs/aristainetos2c_defconfig | 1 - >> configs/aristainetos2ccslb_defconfig | 1 - >> env/env.c | 2 -- >> 4 files changed, 23 deletions(-) > > NACK. Sorry for being to fast with my NACK (I was on thw wrong code base ...)! Tested-by: Heiko Schocher <hs@denx.de> Thanks! bye, Heiko
Hi Heiko, Am 29.04.2023 um 06:30 schrieb Heiko Schocher: > Hello Stefan, > > On 28.04.23 15:45, Stefan Herbrechtsmeier wrote: >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> >> When using a list of writeable variables, the initial values come from >> the built-in default environment since commit 5ab81058364b >> ("env: Complete generic support for writable list"). Remove unnecessary >> misuse of the env is nowhere driver as default environment. >> >> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> --- >> >> board/aristainetos/aristainetos.c | 19 ------------------- >> configs/aristainetos2c_defconfig | 1 - >> configs/aristainetos2ccslb_defconfig | 1 - >> env/env.c | 2 -- >> 4 files changed, 23 deletions(-) > Also you mix up a change in common code and in board code. I use one commit to avoid a breakage in between the commits but it looks like all affected boards use one environment source only so that a separation should be okay. > And grepping in code for ENVL_NOWHERE has much more finds... Thanks for the hint. I have recheck the code and add two additional boards. Regards Stefan
diff --git a/board/aristainetos/aristainetos.c b/board/aristainetos/aristainetos.c index 770f3d7d0d..35beda5cf5 100644 --- a/board/aristainetos/aristainetos.c +++ b/board/aristainetos/aristainetos.c @@ -529,22 +529,3 @@ int embedded_dtb_select(void) return 0; } #endif - -enum env_location env_get_location(enum env_operation op, int prio) -{ - if (op == ENVOP_SAVE || op == ENVOP_ERASE) - return ENVL_SPI_FLASH; - - switch (prio) { - case 0: - return ENVL_NOWHERE; - - case 1: - return ENVL_SPI_FLASH; - - default: - return ENVL_UNKNOWN; - } - - return ENVL_UNKNOWN; -} diff --git a/configs/aristainetos2c_defconfig b/configs/aristainetos2c_defconfig index bd7947b46b..ec93284c20 100644 --- a/configs/aristainetos2c_defconfig +++ b/configs/aristainetos2c_defconfig @@ -58,7 +58,6 @@ CONFIG_OF_CONTROL=y CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_OVERWRITE=y -CONFIG_ENV_IS_NOWHERE=y CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_ENV_SPI_EARLY=y CONFIG_SYS_REDUNDAND_ENVIRONMENT=y diff --git a/configs/aristainetos2ccslb_defconfig b/configs/aristainetos2ccslb_defconfig index 3fb6e71c67..01d9169950 100644 --- a/configs/aristainetos2ccslb_defconfig +++ b/configs/aristainetos2ccslb_defconfig @@ -58,7 +58,6 @@ CONFIG_OF_CONTROL=y CONFIG_DTB_RESELECT=y CONFIG_MULTI_DTB_FIT=y CONFIG_ENV_OVERWRITE=y -CONFIG_ENV_IS_NOWHERE=y CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_ENV_SPI_EARLY=y CONFIG_SYS_REDUNDAND_ENVIRONMENT=y diff --git a/env/env.c b/env/env.c index ad774f4117..2aa52c98f8 100644 --- a/env/env.c +++ b/env/env.c @@ -217,9 +217,7 @@ int env_load(void) printf("OK\n"); gd->env_load_prio = prio; -#if !CONFIG_IS_ENABLED(ENV_APPEND) return 0; -#endif } else if (ret == -ENOMSG) { /* Handle "bad CRC" case */ if (best_prio == -1)