Hi Nishanth, On Tue, 22 Aug 2023 at 17:34, Nishanth Menon <n...@ti.com> wrote: > > On 17:16-20230822, Simon Glass wrote: > > Hi Nishanth, > > > > On Tue, 22 Aug 2023 at 12:41, Nishanth Menon <n...@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.
Then why do you have BOOT_TARGET_DEVICES() ? > > 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. OK good > > > > > > > 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 <n...@ti.com> > > > --- > > > Cc: Simon Glass <s...@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? I'm not sure about that. I have no objection to this patch. > > [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 Regards, Simon