[PATCH 4/7] sunxi: use boot source for determining environment location

[Chris Morgan]

Will do. Sorry, I stepped away from things for a while but I will test
it now that I am back.

On Wed, Apr 20, 2022 at 6:34 PM Andre Przywara <andre.przywara at arm.com> wrote:
>
> On Fri, 15 Apr 2022 12:28:22 -0500
> Chris Morgan <macroalpha82 at gmail.com> wrote:
>
> Hi Chris,
>
> > On Tue, Jan 11, 2022 at 12:46:04PM +0000, Andre Przywara wrote:
> > > Currently we only support to load the environment from raw MMC or FAT
> > > locations on Allwinner boards. With the advent of SPI flash we probably
> > > also want to support using the environment there, so we need to become
> > > a bit more flexible.
> > >
> > > Change the environment priority function to take the boot source into
> > > account. When booted from eMMC or SD card, we use FAT or MMC, if
> > > configured, as before.
> > > If we are booted from SPI flash, we try to use the environment from
> > > there, if possible. The same is true for NAND flash booting, although
> > > this is somewhat theoretical right now (as untested).
> > >
> > > This way we can use the same image for SD and SPI flash booting, which
> > > allows us to simply copy a booted image from SD card to the SPI flash,
> > > for instance.
> >
> > Unfortunately after a git-bisect I can confirm this patch breaks
> > booting in FEL mode whenever the environment is set to
> > CONFIG_ENV_IS_NOWHERE. Tested and reproducible on an NTC CHIP or a
> > Source Parts Popcorn (both Allwinner R8 chips).
> >
> > The curious thing though is that I could never see env_get_location
> > called, but whenever I either set the environment to something like
> > FAT or comment out this change (which I believe causes the non-board
> > specific version of env_get_location to be used) it would boot again.
> > So while I can confirm this patch breaks booting FEL when the env is
> > set to nowhere, I was not able to figure out why that is.
>
> Alright, thanks for the report. I see the problem: The default
> fallback for the new env_get_location() is ENVL_FAT, as 157 out of the
> 160 Allwinner boards define CONFIG_ENV_IS_IN_FAT.
> However the C.H.I.P. boards are one of the three exceptions, and
> returning FAT when the FAT environment driver is not compiled in will
> not end well. I don't have any CHIP board, but simulated this by
> configuring a normal board with just defining CONFIG_ENV_IS_NOWHERE.
>
> I made a patch that tries to choose a matching fallback. It's not very
> elegant, but seems to fix the problem for me. Please test this once
> posted.
>
> Cheers,
> Andre
>
> >
> > Thank you.
> >
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > ---
> > >  board/sunxi/board.c | 51 ++++++++++++++++++++++++++++++++++++++-------
> > >  1 file changed, 43 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 2790a0f9e8..2472343d00 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -170,21 +170,56 @@ void i2c_init_board(void)
> > >  #endif
> > >  }
> > >
> > > -#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT)
> > > +/*
> > > + * Try to use the environment from the boot source first.
> > > + * For MMC, this means a FAT partition on the boot device (SD or eMMC).
> > > + * If the raw MMC environment is also enabled, this is tried next.
> > > + * SPI flash falls back to FAT (on SD card).
> > > + */
> > >  enum env_location env_get_location(enum env_operation op, int prio)
> > >  {
> > > -   switch (prio) {
> > > -   case 0:
> > > -           return ENVL_FAT;
> > > +   enum env_location boot_loc = ENVL_FAT;
> > >
> > > -   case 1:
> > > -           return ENVL_MMC;
> > > +   gd->env_load_prio = prio;
> > >
> > > +   switch (sunxi_get_boot_device()) {
> > > +   case BOOT_DEVICE_MMC1:
> > > +   case BOOT_DEVICE_MMC2:
> > > +           boot_loc = ENVL_FAT;
> > > +           break;
> > > +   case BOOT_DEVICE_NAND:
> > > +           if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND))
> > > +                   boot_loc = ENVL_NAND;
> > > +           break;
> > > +   case BOOT_DEVICE_SPI:
> > > +           if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
> > > +                   boot_loc = ENVL_SPI_FLASH;
> > > +           break;
> > > +   case BOOT_DEVICE_BOARD:
> > > +           break;
> > >     default:
> > > -           return ENVL_UNKNOWN;
> > > +           break;
> > > +   }
> > > +
> > > +   /* Always try to access the environment on the boot device first. */
> > > +   if (prio == 0)
> > > +           return boot_loc;
> > > +
> > > +   if (prio == 1) {
> > > +           switch (boot_loc) {
> > > +           case ENVL_SPI_FLASH:
> > > +                   return ENVL_FAT;
> > > +           case ENVL_FAT:
> > > +                   if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC))
> > > +                           return ENVL_MMC;
> > > +                   break;
> > > +           default:
> > > +                   break;
> > > +           }
> > >     }
> > > +
> > > +   return ENVL_UNKNOWN;
> > >  }
> > > -#endif
> > >
> > >  #ifdef CONFIG_DM_MMC
> > >  static void mmc_pinmux_setup(int sdc);
>