Re: [PATCH V4 1/8] env_default: Allow CONFIG_EXTRA_ENV_TEXT to override CFG_EXTRA_ENV_SETTINGS

[Nishanth Menon <nm@ti.com>]

On 17:16-20230822, Simon Glass wrote:
> Hi Nishanth,
> 
> On Tue, 22 Aug 2023 at 12:41, Nishanth Menon <nm@ti.com> wrote:
> >
> > CFG_EXTRA_ENV_SETTINGS is set in common board config files, This allows
> > for majority of the settings to be set in a common manner. However, the
> > minor variations between various board can be addressed by the board.env
> > files. The board.env files are converted into CONFIG_EXTRA_ENV_TEXT.
> >
> > However, this creates a minor problem. For example:
> > distro_bootcmd.h and used by ti_armv7_common.h uses it as:
> >  #define BOOT_TARGET_DEVICES(func) \
> >         func(MMC, mmc, 0) \
> >         func(MMC, mmc, 1)
> 
> OK, but we should be using bootstd for new boards.

Agreed, and I think I am using bootstd[1] unless I am missing something
completely.

we still have a problem with EXTRA_ENV_SETTINGS and ENV_TEXT. This patch
helps enforce priority of ENV_TEXT.

But overall, I agree that we need to move off from EXTRA_ENV to text..
probably in stages, I guess.

> >
> > Which in turn generates:
> > boot_targets=mmc0 mmc1
> >
> > And this probably works fine for most boards, However when the
> > boot_targets need to be reversed, the preferred behavior would have been
> > to define it in board.env file as:
> > boot_targets=mmc1 mmc0
> >
> > By changing the order of the inclusion, we allow for the
> > CONFIG_EXTRA_ENV_TEXT to have a higher priority in the definition.
> >
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> > Cc: Simon Glass <sjg@chromium.org>
> >
> > New patch
> >
> >  include/env_default.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/env_default.h b/include/env_default.h
> > index b16c22d5a28c..714dfa9e845e 100644
> > --- a/include/env_default.h
> > +++ b/include/env_default.h
> > @@ -112,12 +112,12 @@ const char default_environment[] = {
> >  #ifdef CONFIG_MTDPARTS_DEFAULT
> >         "mtdparts="     CONFIG_MTDPARTS_DEFAULT         "\0"
> >  #endif
> > +#ifdef CFG_EXTRA_ENV_SETTINGS
> > +       CFG_EXTRA_ENV_SETTINGS
> > +#endif
> >  #ifdef CONFIG_EXTRA_ENV_TEXT
> >         /* This is created in the Makefile */
> >         CONFIG_EXTRA_ENV_TEXT
> > -#endif
> > -#ifdef CFG_EXTRA_ENV_SETTINGS
> > -       CFG_EXTRA_ENV_SETTINGS
> >  #endif
> 
> and text environment

Could you clarify what I am missing?

[1] https://u-boot.readthedocs.io/en/latest/develop/bootstd.html
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D