
Hi Alban,
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Bedel, Alban Sent: Wednesday, June 30, 2021 7:08 PM To: Priyanka Jain priyanka.jain@nxp.com; Varun Sethi V.Sethi@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 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
Thank you for explaining it. I could reproduce the issue in case we enable SYS_REDUNDAND_ENVIRONMENT. Fixed it using another env variable as you suggested. Below are my test steps on lx2160ardb with xspi and SD boot.
######## XSPI BOOT #######
=> qixis_reset altbank <bootup_logs_snip>
Loading Environment from SPIFlash... SF: Detected mt35xu512aba with page size 256 Bytes, erase size 128 KiB, total 64 MiB *** Warning - bad CRC, using default environment
<bootup_logs_snip>
=> pri bootcmd bootcmd=sf probe 0:0; sf read 0x806c0000 0x6c0000 0x40000; env exists mcinitcmd && env exists secureboot && esbc_validate 0x806c0000; sf read 0x80d00000 0xd00000 0x100000; env exists mcinitcmd && fsl_mc lazyapply dpl 0x80d00000; run distro_bootcmd;run xspi_bootcmd; env exists secureboot && esbc_halt;
=> pri mcinitcmd mcinitcmd=sf probe 0:0 && sf read 0x80640000 0x640000 0x80000 && env exists secureboot && esbc_validate 0x80640000 && esbc_validate 0x80680000; sf read 0x80a00000 0xa00000 0x300000 && sf read 0x80e00000 0xe00000 0x100000; fsl_mc start mc 0x80a00000 0x80e00000
=> pri fsl_bootcmd_mcinitcmd_set fsl_bootcmd_mcinitcmd_set=y
=> setenv bootcmd run xspi_bootcmd => saveenv Saving Environment to SPIFlash... Erasing SPI flash...Writing to SPI flash...done Valid environment: 2 OK => pri bootcmd bootcmd=run xspi_bootcmd => qixis_reset altbank <bootup_logs_snip> => pri bootcmd bootcmd=run xspi_bootcmd => env default -a;saveenv ## Resetting to default environment Saving Environment to SPIFlash... Erasing SPI flash...Writing to SPI flash...done Valid environment: 1 OK
=> qixis_reset altbank <bootup_logs_snip> => pri bootcmd bootcmd=sf probe 0:0; sf read 0x806c0000 0x6c0000 0x40000; env exists mcinitcmd && env exists secureboot && esbc_validate 0x806c0000; sf read 0x80d00000 0xd00000 0x100000; env exists mcinitcmd && fsl_mc lazyapply dpl 0x80d00000; run distro_bootcmd;run xspi_bootcmd; env exists secureboot && esbc_halt; => pri fsl_bootcmd_mcinitcmd_set fsl_bootcmd_mcinitcmd_set=y
####### SD BOOT ########
Loading Environment from MMC... *** Warning - bad CRC, using default environment <bootup_logs_snip> => pri bootcmd bootcmd=env exists mcinitcmd && mmcinfo; mmc read 0x80d00000 0x6800 0x800; env exists mcinitcmd && env exists secureboot && mmc read 0x806C0000 0x3600 0x20 && esbc_validate 0x806C0000;env exists mcinitcmd && fsl_mc lazyapply dpl 0x80d00000;run distro_bootcmd;run sd_bootcmd;env exists secureboot && esbc_halt;
=> pri fsl_bootcmd_mcinitcmd_set fsl_bootcmd_mcinitcmd_set=y
=> setenv bootcmd run sd_bootcmd => saveenv Saving Environment to MMC... Writing to redundant MMC(0)... OK
=> pri bootcmd bootcmd=run sd_bootcmd => qixis_reset sd <bootup_logs_snip> => pri bootcmd bootcmd=run sd_bootcmd => qixis_reset sd <bootup_logs_snip> => env default -a;saveenv ## Resetting to default environment Saving Environment to MMC... Writing to MMC(0)... OK => pri bootcmd bootcmd=env exists mcinitcmd && mmcinfo; mmc read 0x80d00000 0x6800 0x800; env exists mcinitcmd && env exists secureboot && mmc read 0x806C0000 0x3600 0x20 && esbc_validate 0x806C0000;env exists mcinitcmd && fsl_mc lazyapply dpl 0x80d00000;run distro_bootcmd;run sd_bootcmd;env exists secureboot && esbc_halt;
=> pri fsl_bootcmd_mcinitcmd_set fsl_bootcmd_mcinitcmd_set=y