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

Chris Morgan
2022-04-15T17:28:36Z

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.

Thank you.

>
> Signed-off-by: Andre Przywara <andre.przywara@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);