[PATCH] sunxi: fix initial environment loading without MMC

[Samuel Holland]

Hi Andre,

On 4/20/22 7:34 PM, Andre Przywara wrote:
> Commit e42dad4168fe ("sunxi: use boot source for determining environment
> location") changed our implementation of env_get_location() and enabled
> it for every board, even those without MMC support (like the C.H.I.P.
> boards). However the default fallback location of ENVL_FAT does not cope
> very well without MMC support compiled in, so the board hangs when trying
> to initially load the environment.
> 
> Change the default fallback location to be ENVL_FAT only when the FAT
> environment support is enabled, and use ENVL_NOWHERE and ENVL_UBI as
> alternative fallbacks, when those sources are enabled.
> 
> This fixes U-Boot loading on the C.H.I.P. boards.
> 
> Fixes: e42dad4168fe ("sunxi: use boot source for determining environment location")
> Reported-by: Chris Morgan <macroalpha82 at gmail.com>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
>  board/sunxi/board.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 89324159d55..befb6076ca6 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -132,7 +132,14 @@ void i2c_init_board(void)
>   */
>  enum env_location env_get_location(enum env_operation op, int prio)
>  {
> -	enum env_location boot_loc = ENVL_FAT;
> +	enum env_location boot_loc;
> +
> +	if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE))
> +		boot_loc = ENVL_NOWHERE;
> +	else if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT))
> +		boot_loc = ENVL_FAT;
> +	else if (IS_ENABLED(CONFIG_ENV_IS_IN_UBI))
> +		boot_loc = ENVL_UBI;

This could leave boot_loc uninitialized. And there is still an unconditional use
of ENVL_FAT in the BOOT_DEVICE_MMCx case.

>  	gd->env_load_prio = prio;

I don't think the hook is supposed to change this variable.

I'm still a bit confused on the fallback logic you have in place. Splitting it
up into three blocks doesn't help. If the goal is to load the environment from
the boot device, while preferring filesystems over raw block devices, I propose
the following:

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 427113534b..27508bd306 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -129,26 +129,38 @@
  * 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)
 {
-	enum env_location boot_loc = ENVL_FAT;
+	if (prio > 1)
+		return ENVL_UNKNOWN;

-	gd->env_load_prio = prio;
+	if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE))
+		return ENVL_NOWHERE;

 	switch (sunxi_get_boot_device()) {
 	case BOOT_DEVICE_MMC1:
 	case BOOT_DEVICE_MMC2:
-		boot_loc = ENVL_FAT;
+		if (prio == 0) {
+			if (IS_ENABLED(CONFIG_ENV_IS_IN_EXT4))
+				return ENVL_EXT4;
+			if (IS_ENABLED(CONFIG_ENV_IS_IN_FAT))
+				return ENVL_FAT;
+		}
+		if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC))
+			return ENVL_MMC;
 		break;
 	case BOOT_DEVICE_NAND:
+		if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_UBI))
+			return ENVL_UBI;
 		if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND))
-			boot_loc = ENVL_NAND;
+			return ENVL_NAND;
 		break;
 	case BOOT_DEVICE_SPI:
+		if (prio == 0 && IS_ENABLED(CONFIG_ENV_IS_IN_UBI))
+			return ENVL_UBI;
 		if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH))
-			boot_loc = ENVL_SPI_FLASH;
+			return ENVL_SPI_FLASH;
 		break;
 	case BOOT_DEVICE_BOARD:
 		break;
@@ -156,23 +168,6 @@
 		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;
 }


Regards,
Samuel