On Wed, 2021-06-30 at 12:44 +0000, Wasim Khan (OSS) wrote:
-----Original Message----- From: Bedel, Alban alban.bedel@aerq.com Sent: Wednesday, June 30, 2021 6:03 PM To: Priyanka Jain priyanka.jain@nxp.com; Varun Sethi < V.Sethi@nxp.com>; Wasim Khan wasim.khan@nxp.com; Wasim Khan (OSS) wasim.khan@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