| Message ID | 20231112000923.73568-3-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | bootm: Refactoring to reduce reliance on CMDLINE (part A) | expand |
On 11/12/23 01:08, Simon Glass wrote: > The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef > condition from the code it calls. Use the same condition to avoid a > build warning if CONFIG_CMD_SAVEENV is disabled. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > env/mmc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/env/mmc.c b/env/mmc.c > index cb14bbb58f13..da84cddd74f0 100644 > --- a/env/mmc.c > +++ b/env/mmc.c > @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = { > .location = ENVL_MMC, > ENV_NAME("MMC") > .load = env_mmc_load, > -#ifndef CONFIG_SPL_BUILD > +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD) According to README CONFIG_SPL_BUILD is not defined for TPL builds. I assume that we don't want to have below fields in TPL either. Please, use #if CONFIG_IS_ENABLED(CMD_SAVEENV) Best regards Heinrich > .save = env_save_ptr(env_mmc_save), > .erase = ENV_ERASE_PTR(env_mmc_erase) > #endif
Hi Heinrich, On Wed, 15 Nov 2023 at 03:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 11/12/23 01:08, Simon Glass wrote: > > The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef > > condition from the code it calls. Use the same condition to avoid a > > build warning if CONFIG_CMD_SAVEENV is disabled. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > env/mmc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/env/mmc.c b/env/mmc.c > > index cb14bbb58f13..da84cddd74f0 100644 > > --- a/env/mmc.c > > +++ b/env/mmc.c > > @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = { > > .location = ENVL_MMC, > > ENV_NAME("MMC") > > .load = env_mmc_load, > > -#ifndef CONFIG_SPL_BUILD > > +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD) > > According to README CONFIG_SPL_BUILD is not defined for TPL builds. > > I assume that we don't want to have below fields in TPL either. Please, use > > #if CONFIG_IS_ENABLED(CMD_SAVEENV) I missed this comment in the new version. But note that CONFIG_SPL_BUILD covers TPL (and others) as well. It is a bit confusing. Perhaps we should introduce CONFIG_XPL_BUILD to mean anything other than U-Boot proper? > > Best regards > > Heinrich > > > .save = env_save_ptr(env_mmc_save), > > .erase = ENV_ERASE_PTR(env_mmc_erase) > > #endif > Regards, Simon
On Sun, Nov 19, 2023 at 07:49:32AM -0700, Simon Glass wrote: > Hi Heinrich, > > On Wed, 15 Nov 2023 at 03:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 11/12/23 01:08, Simon Glass wrote: > > > The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef > > > condition from the code it calls. Use the same condition to avoid a > > > build warning if CONFIG_CMD_SAVEENV is disabled. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > env/mmc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/env/mmc.c b/env/mmc.c > > > index cb14bbb58f13..da84cddd74f0 100644 > > > --- a/env/mmc.c > > > +++ b/env/mmc.c > > > @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = { > > > .location = ENVL_MMC, > > > ENV_NAME("MMC") > > > .load = env_mmc_load, > > > -#ifndef CONFIG_SPL_BUILD > > > +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD) > > > > According to README CONFIG_SPL_BUILD is not defined for TPL builds. > > > > I assume that we don't want to have below fields in TPL either. Please, use > > > > #if CONFIG_IS_ENABLED(CMD_SAVEENV) > > I missed this comment in the new version. But note that > CONFIG_SPL_BUILD covers TPL (and others) as well. > > It is a bit confusing. Perhaps we should introduce CONFIG_XPL_BUILD to > mean anything other than U-Boot proper? We can see if something like that makes more sense as we further separate out "library" functionality from "command" functionality. We might swing back to needing to save environment changes from the non-cmdline use cases all the same (assorted canary type environment variables, etc) and be back to needing to tweak this differently. So what's here is fine with me for today. Reviewed-by: Tom Rini <trini@konsulko.com> ... and just put that on the next iteration of the series.
diff --git a/env/mmc.c b/env/mmc.c index cb14bbb58f13..da84cddd74f0 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -495,7 +495,7 @@ U_BOOT_ENV_LOCATION(mmc) = { .location = ENVL_MMC, ENV_NAME("MMC") .load = env_mmc_load, -#ifndef CONFIG_SPL_BUILD +#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD) .save = env_save_ptr(env_mmc_save), .erase = ENV_ERASE_PTR(env_mmc_erase) #endif
The declaration of U_BOOT_ENV_LOCATION() uses a different #ifdef condition from the code it calls. Use the same condition to avoid a build warning if CONFIG_CMD_SAVEENV is disabled. Signed-off-by: Simon Glass <sjg@chromium.org> --- env/mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)