Hi Alban,
> -----Original Message-----
> From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Bedel, Alban
> Sent: Wednesday, June 30, 2021 7:08 PM
> To: Priyanka Jain <priyanka.j...@nxp.com>; Varun Sethi <v.se...@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 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
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
