[PATCH] armv8: fsl : fix bootcmd and mcinitcmd default value

[Bedel, Alban]

On Wed, 2021-06-30 at 11:12 +0000, Priyanka Jain wrote:
> 
> 
> > -----Original Message-----
> > From: Wasim Khan <wasim.khan at nxp.com>
> > Sent: Friday, June 25, 2021 2:40 PM
> > To: Bedel, Alban <alban.bedel at aerq.com>; Priyanka Jain
> > <priyanka.jain at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; Wasim Khan
> > (OSS)
> > <wasim.khan at oss.nxp.com>
> > Cc: u-boot at lists.denx.de
> > Subject: RE: [PATCH] armv8: fsl : fix bootcmd and mcinitcmd default
> > value
> > 
> > Hi Alban,
> > 
> > > -----Original Message-----
> > > From: Bedel, Alban <alban.bedel at aerq.com>
> > > Sent: Wednesday, June 23, 2021 6:38 PM
> > > To: Priyanka Jain <priyanka.jain at nxp.com>; Varun Sethi
> > > <V.Sethi at nxp.com>; Wasim Khan (OSS) <wasim.khan at oss.nxp.com>
> > > Cc: u-boot at lists.denx.de; Wasim Khan <wasim.khan at nxp.com>
> > > Subject: Re: [PATCH] armv8: fsl : fix bootcmd and mcinitcmd
> > > default
> > > value
> > > 
> > > On Wed, 2021-06-16 at 14:19 +0200, Wasim Khan wrote:
> > > > From: Wasim Khan <wasim.khan at nxp.com>
> > > > 
> > > > NXP platforms expect bootcmd and mcinitcmd to be updated as per
> > > > boot
> > > > source.
> > > > 
> > > > commit cbf77d201870f2d12227e2d95718a416b16ec98b breaks this
> > behaviour.
> > > > Revert commit cbf77d201870f2d12227e2d95718a416b16ec98b
> > > 
> > > As I already explained in the prior exchanges we had, I'm fully
> > > convinced that reverting this patch is not the solution to your
> > > problem. Please see the log of my patch for a full explanation,
> > > but
> > > basically the old code not only rely on the a broken assumption,
> > > it also fails to
> > implement it correctly.
> > > 
> > > The current code set `bootcmd` and `mcinicmd` only if they are
> > > not set
> > > which a simple and sane behaviour.
> > 
> > 
> > As I have explained earlier that the bootcmd is always set with
> > default value as "
> > run distro_bootcmd".
> > So fsl_setenv_bootcmd() never gets executed which is causing the
> > issue.
> > 
> > 
> > > When I submitted my patch these variables were not set in the
> > > default
> > > so I suspect that another patch now set these in the default env.
> > 
> > I hard reset my tree to your commit and I still see the issue.
> > Please refer to below logs. I don’t see any other patch causing
> > this issue. Would
> > let @Priyanka Jain to share her comments.
> > 
> > 
> > U-Boot 2021.01-rc3-00115-gcbf77d2018 (Jun 25 2021 - 10:51:56 +0200)
> > SoC:  LX2160ACE Rev2.0 (0x87360020)
> > ...
> > ...
> > MMC:   FSL_SDHC: 0, FSL_SDHC: 1
> > 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
> > 
> > EEPROM: NXID v1
> > ...
> > ...
> > => pri bootcmd
> > bootcmd=run distro_bootcmd
> > 
> > Regards,
> > Wasim
> > 
> > > 
> > > Alban
> > 
> 
> Alban, Wasim,
> 
> Lets try to fix both issues. One being faced by Alban and the one
> faced by Wasim.
> Alban ,
> Can you please provide summary of the issues faced by you.

The issue I faced are well describe in my original patch, but I'll sum
them up again here:

* After issuing `env default -a ; saveenv' the board didn't boot
anymore because `bootcmd` was then empty.

* If redundant env on eMMC was enabled `bootcmd` was then overwritten
at every boot, preventing me from using a custom `bootcmd` at all.

A typical u-boot build is configured at build time for a specific boot
device, but with TF-A there is a single build for all boot devices and
u-boot then have configure the default boot device on the first boot.
This is where the code we are discussing here come into play.

Back when I submitted my patch the default env didn't define `bootcmd`
in my build, so I took that as the way to detect that `bootcmd` need to
be initialised. We could instead just use another env variable to mark
if `bootcmd` has been initialised or not.

Alban