[PATCH V3] env: Fix invalid env handling in env_init()

[PATCH V3] env: Fix invalid env handling in env_init()

[Marek Vasut]

This fixes the case where there are multiple environment drivers, one of
them is the default environment one, and it is followed by an environment
driver which does not implement .init() callback. The default environment
driver sets gd->env_valid to ENV_INVALID and returns 0 from its .init()
callback implementation, which is valid behavior for default environment.

Since the subsequent environment driver does not implement .init(), it
also does not modify the $ret variable in the loop. Therefore, the loop
is exited with gd->env_valid=ENV_INVALID and ret=0, which means that the
code further down in env_init() will not reset the environment to the
default one, which is incorrect.

This patch sets the $ret variable back to -ENOENT in case the env_valid
is set to ENV_INVALID by an environment driver, so that the environment
would be correctly reset back to default one, unless a subsequent driver
loads a valid environment.

Signed-off-by: Marek Vasut <marex@denx.de>
---
V2: Reword commit message
V3: Rebase
---
 env/env.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/env/env.c b/env/env.c
index 37b4b54cb75..7faa7ab661b 100644
--- a/env/env.c
+++ b/env/env.c
@@ -334,6 +334,9 @@ int env_init(void)
 
 		debug("%s: Environment %s init done (ret=%d)\n", __func__,
 		      drv->name, ret);
+
+		if (gd->env_valid == ENV_INVALID)
+			ret = -ENOENT;
 	}
 
 	if (!prio)
-- 
2.29.2

[PATCH V3] env: Fix invalid env handling in env_init()

[Tim Harvey]

On Wed, Jan 20, 2021 at 6:45 AM Marek Vasut <marex@denx.de> wrote:
>
> This fixes the case where there are multiple environment drivers, one of
> them is the default environment one, and it is followed by an environment
> driver which does not implement .init() callback. The default environment
> driver sets gd->env_valid to ENV_INVALID and returns 0 from its .init()
> callback implementation, which is valid behavior for default environment.
>
> Since the subsequent environment driver does not implement .init(), it
> also does not modify the $ret variable in the loop. Therefore, the loop
> is exited with gd->env_valid=ENV_INVALID and ret=0, which means that the
> code further down in env_init() will not reset the environment to the
> default one, which is incorrect.
>
> This patch sets the $ret variable back to -ENOENT in case the env_valid
> is set to ENV_INVALID by an environment driver, so that the environment
> would be correctly reset back to default one, unless a subsequent driver
> loads a valid environment.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> V2: Reword commit message
> V3: Rebase
> ---
>  env/env.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/env/env.c b/env/env.c
> index 37b4b54cb75..7faa7ab661b 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -334,6 +334,9 @@ int env_init(void)
>
>                 debug("%s: Environment %s init done (ret=%d)\n", __func__,
>                       drv->name, ret);
> +
> +               if (gd->env_valid == ENV_INVALID)
> +                       ret = -ENOENT;
>         }
>
>         if (!prio)
> --
> 2.29.2
>

Without this patch, multiple environments with MMC being one of them
fail to work as described in the commit message (becuase MMC does not
have an init callback). I verified this patch resolves the issue using
an IMX8M mini with CONFIG_ENV_IS_NOWHERE=y and CONFIG_ENV_IS_IN_MMC=y.

Tested-By: Tim Harvey <tharvey@gateworks.com>

[PATCH V3] env: Fix invalid env handling in env_init()

[Marek Vasut]

On 4/5/21 6:26 PM, Tim Harvey wrote:
> On Wed, Jan 20, 2021 at 6:45 AM Marek Vasut <marex@denx.de> wrote:
>>
>> This fixes the case where there are multiple environment drivers, one of
>> them is the default environment one, and it is followed by an environment
>> driver which does not implement .init() callback. The default environment
>> driver sets gd->env_valid to ENV_INVALID and returns 0 from its .init()
>> callback implementation, which is valid behavior for default environment.
>>
>> Since the subsequent environment driver does not implement .init(), it
>> also does not modify the $ret variable in the loop. Therefore, the loop
>> is exited with gd->env_valid=ENV_INVALID and ret=0, which means that the
>> code further down in env_init() will not reset the environment to the
>> default one, which is incorrect.
>>
>> This patch sets the $ret variable back to -ENOENT in case the env_valid
>> is set to ENV_INVALID by an environment driver, so that the environment
>> would be correctly reset back to default one, unless a subsequent driver
>> loads a valid environment.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> V2: Reword commit message
>> V3: Rebase
>> ---
>>   env/env.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/env/env.c b/env/env.c
>> index 37b4b54cb75..7faa7ab661b 100644
>> --- a/env/env.c
>> +++ b/env/env.c
>> @@ -334,6 +334,9 @@ int env_init(void)
>>
>>                  debug("%s: Environment %s init done (ret=%d)\n", __func__,
>>                        drv->name, ret);
>> +
>> +               if (gd->env_valid == ENV_INVALID)
>> +                       ret = -ENOENT;
>>          }
>>
>>          if (!prio)
>> --
>> 2.29.2
>>
> 
> Without this patch, multiple environments with MMC being one of them
> fail to work as described in the commit message (becuase MMC does not
> have an init callback). I verified this patch resolves the issue using
> an IMX8M mini with CONFIG_ENV_IS_NOWHERE=y and CONFIG_ENV_IS_IN_MMC=y.
> 
> Tested-By: Tim Harvey <tharvey@gateworks.com>

Thank you. This patch is still missing upstream.

[PATCH V3] env: Fix invalid env handling in env_init()

[Tom Rini]

On Wed, Jan 20, 2021 at 03:45:16PM +0100, Marek Vasut wrote:

> This fixes the case where there are multiple environment drivers, one of
> them is the default environment one, and it is followed by an environment
> driver which does not implement .init() callback. The default environment
> driver sets gd->env_valid to ENV_INVALID and returns 0 from its .init()
> callback implementation, which is valid behavior for default environment.
> 
> Since the subsequent environment driver does not implement .init(), it
> also does not modify the $ret variable in the loop. Therefore, the loop
> is exited with gd->env_valid=ENV_INVALID and ret=0, which means that the
> code further down in env_init() will not reset the environment to the
> default one, which is incorrect.
> 
> This patch sets the $ret variable back to -ENOENT in case the env_valid
> is set to ENV_INVALID by an environment driver, so that the environment
> would be correctly reset back to default one, unless a subsequent driver
> loads a valid environment.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Tested-By: Tim Harvey <tharvey@gateworks.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210418/4f2b0816/attachment.sig>