diff mbox series

env: Remove misuse of env is nowhere driver

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

Commit Message

Stefan Herbrechtsmeier April 28, 2023, 1:45 p.m. UTC
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(-)

Comments

Heiko Schocher April 29, 2023, 4:30 a.m. UTC | #1
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
Heiko Schocher May 2, 2023, 4:39 a.m. UTC | #2
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
Stefan Herbrechtsmeier May 2, 2023, 10:13 a.m. UTC | #3
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 mbox series

Patch

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)