[U-Boot] [PATCH] env: Correct case of no sub-init function

With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com --- env/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/env.c b/env/env.c index 2b8b9611cfff..8671f13f8d9e 100644 --- a/env/env.c +++ b/env/env.c @@ -138,7 +138,7 @@ int env_init(void) ret = drv->init(); if (ret == -ENOENT) { gd->env_addr = (ulong)&default_environment[0]; - gd->env_valid = 0; + gd->env_valid = ENV_VALID;
return 0; } else if (ret) {

On 19 August 2017 at 20:27, Tom Rini trini@konsulko.com wrote:
With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com
env/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
(I sent a similar patch just now after my build completed, but please ignore it)

With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value. Finally, in the case of ENV_IS_NOWHERE we must still say that our envionrment is invalid after init for things to continue to function.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v2: - Also correct env/nowhere.c so that sandbox tests function again. --- env/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/env.c b/env/env.c index 2b8b9611cfff..8671f13f8d9e 100644 --- a/env/env.c +++ b/env/env.c @@ -138,7 +138,7 @@ int env_init(void) ret = drv->init(); if (ret == -ENOENT) { gd->env_addr = (ulong)&default_environment[0]; - gd->env_valid = 0; + gd->env_valid = ENV_VALID;
return 0; } else if (ret) {

On 08/20/2017 04:27 PM, Tom Rini wrote:
With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value. Finally, in the case of ENV_IS_NOWHERE we must still say that our envionrment is invalid after init for things to continue to function.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com
Tested-by: Marek Vasut marek.vasut@gmail.com
Changes in v2:
- Also correct env/nowhere.c so that sandbox tests function again.
env/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/env.c b/env/env.c index 2b8b9611cfff..8671f13f8d9e 100644 --- a/env/env.c +++ b/env/env.c @@ -138,7 +138,7 @@ int env_init(void) ret = drv->init(); if (ret == -ENOENT) { gd->env_addr = (ulong)&default_environment[0];
gd->env_valid = 0;
gd->env_valid = ENV_VALID;
return 0; } else if (ret) {

With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value. Finally, in the case of ENV_IS_NOWHERE we must still say that our envionrment is invalid after init for things to continue to function.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com --- Changes in v3: - Actually include changes for env/nowhere.c --- env/env.c | 2 +- env/nowhere.c | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/env/env.c b/env/env.c index 2b8b9611cfff..8671f13f8d9e 100644 --- a/env/env.c +++ b/env/env.c @@ -138,7 +138,7 @@ int env_init(void) ret = drv->init(); if (ret == -ENOENT) { gd->env_addr = (ulong)&default_environment[0]; - gd->env_valid = 0; + gd->env_valid = ENV_VALID;
return 0; } else if (ret) { diff --git a/env/nowhere.c b/env/nowhere.c index d60de494e6c4..f654883c8acb 100644 --- a/env/nowhere.c +++ b/env/nowhere.c @@ -15,7 +15,20 @@
DECLARE_GLOBAL_DATA_PTR;
+/* + * Because we only ever have the default environment available we must mark + * it as invalid. + */ +static int env_nowhere_init(void) +{ + gd->env_addr = (ulong)&default_environment[0]; + gd->env_valid = ENV_INVALID; + + return 0; +} + U_BOOT_ENV_LOCATION(nowhere) = { .location = ENVL_NOWHERE, + .init = env_nowhere_init, ENV_NAME("nowhere") };

On Sun, Aug 20, 2017 at 11:41:40AM -0400, Tom Rini wrote:
With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value. Finally, in the case of ENV_IS_NOWHERE we must still say that our envionrment is invalid after init for things to continue to function.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Tested-by: Marek Vasut marek.vasut@gmail.com Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!

On Sun, Aug 20, 2017 at 6:41 PM, Tom Rini trini@konsulko.com wrote:
With the change to the environment code to remove the common init stage of pointing to the default environment and setting it as valid, combined with the change to switch gd->env_valid from 0/1/2 to an enum we now must set env_valid to one of the enum values rather than an int. And in this case, not only was setting it to an int wrong, it was now the wrong value. Finally, in the case of ENV_IS_NOWHERE we must still say that our envionrment is invalid after init for things to continue to function.
Fixes: 7938822a6b75 ("env: Drop common init() functions") Reported-by: Marek Vasut marek.vasut@gmail.com Reported-by: Andy Shevchenko andy.shevchenko@gmail.com Signed-off-by: Tom Rini trini@konsulko.com
A bit late, but seems it's fixed (Though I'll continue keeping an eye on behaviour)
Tested-by: Andy Shevchenko andy.shevchenko@gmail.com
Changes in v3:
- Actually include changes for env/nowhere.c
env/env.c | 2 +- env/nowhere.c | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/env/env.c b/env/env.c index 2b8b9611cfff..8671f13f8d9e 100644 --- a/env/env.c +++ b/env/env.c @@ -138,7 +138,7 @@ int env_init(void) ret = drv->init(); if (ret == -ENOENT) { gd->env_addr = (ulong)&default_environment[0];
gd->env_valid = 0;
gd->env_valid = ENV_VALID; return 0; } else if (ret) {
diff --git a/env/nowhere.c b/env/nowhere.c index d60de494e6c4..f654883c8acb 100644 --- a/env/nowhere.c +++ b/env/nowhere.c @@ -15,7 +15,20 @@
DECLARE_GLOBAL_DATA_PTR;
+/*
- Because we only ever have the default environment available we must mark
- it as invalid.
- */
+static int env_nowhere_init(void) +{
gd->env_addr = (ulong)&default_environment[0];
gd->env_valid = ENV_INVALID;
return 0;
+}
U_BOOT_ENV_LOCATION(nowhere) = { .location = ENVL_NOWHERE,
.init = env_nowhere_init, ENV_NAME("nowhere")
};
1.9.1
participants (4)
-
Andy Shevchenko
-
Marek Vasut
-
Simon Glass
-
Tom Rini