On Wed, 2021-06-30 at 12:44 +0000, Wasim Khan (OSS) wrote: > > > > -----Original Message----- > > From: Bedel, Alban <alban.be...@aerq.com> > > Sent: Wednesday, June 30, 2021 6:03 PM > > To: Priyanka Jain <priyanka.j...@nxp.com>; Varun Sethi < > > v.se...@nxp.com>; > > Wasim Khan <wasim.k...@nxp.com>; Wasim Khan (OSS) > > <wasim.k...@oss.nxp.com> > > Cc: u-boot@lists.denx.de > > Subject: Re: [PATCH] armv8: fsl : fix bootcmd and mcinitcmd default > > value > > > > On Wed, 2021-06-30 at 11:12 +0000, Priyanka Jain wrote: > > > > > > snip > > > > > * After issuing `env default -a ; saveenv' the board didn't boot > > anymore because `bootcmd` was then empty. > > This is not the case with latest u-boot code. 'env default -a' set > bootcmd to default one (run distro_bootcmd). > So, we are safe here. > > > > > > * If redundant env on eMMC was enabled `bootcmd` was then overwritten > > at every boot, preventing me from using a custom `bootcmd` at all. > > > > Priyanka, Alban > Can you help me to understand what does enabling 'redundant env' on > eMMC mean and how to enable it ?
See env/Kconfig: config SYS_REDUNDAND_ENVIRONMENT bool "Enable redundant environment support" depends on ENV_IS_IN_EEPROM || ENV_IS_IN_FLASH || ENV_IS_IN_MMC || \ ENV_IS_IN_NAND || ENV_IS_IN_SPI_FLASH || ENV_IS_IN_UBI help Normally, the environemt is stored in a single location. By selecting this option, you can then define where to hold a redundant copy of the environment data, so that there is a valid backup copy in case there is a power failure during a "saveenv" operation. When this option is enabled the internals of the env change significantly and the old code then always detected the env as being the default, erasing any previously user set value at every boot. But beside the fact that it didn't work properly with all configurations, the old code didn't really detect if the env was the default. When it worked, it detected the case where no valid env was stored and u-boot was using its internal in-memory defaults. That's why resetting the env to default would then break the boot. In my patch I replaced this logic by looking if `bootcmd` has the default value, which worked well as long as the default value was "unset". But as we see this is not a viable solution in the long run. My suggestion would be to have something like this: if (env_get_yesno("fsl_bootcmd_set") <= 0) { // Set the default bootcmd according to the boot device ... env_set("fsl_bootcmd_set", "y"); } That way it doesn't matter what the default value of `bootcmd` is and boards also have the possibility to disable this code by setting `fsl_bootcmd_set` to `y` in their default env. I think it would also make sense to have some code that set the TF-A boot device in the env, that might allow handling this in the boot scripts directly instead of all this hard coded logic. Alban