On Fri, 15 Apr 2022 12:28:22 -0500
Chris Morgan <macroalph...@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.przyw...@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);
