Merge branch '2021-04-16-env-updates'

- SPI Flash ENV improvements / cleanups
- Redundant support for FAT
- Assorted bugfixes

env/Kconfig

@@ -324,6 +324,20 @@ config ENV_IS_IN_SPI_FLASH
 	  during a "saveenv" operation. CONFIG_ENV_OFFSET_REDUND must be
 	  aligned to an erase sector boundary.
 
+config ENV_SECT_SIZE_AUTO
+	bool "Use automatically detected sector size"
+	depends on ENV_IS_IN_SPI_FLASH
+	help
+	  Some boards exist in multiple variants, with different
+	  flashes having different sector sizes. In such cases, you
+	  can select this option to make U-Boot use the actual sector
+	  size when figuring out how much to erase, which can thus be
+	  more efficient on the flashes with smaller erase size. Since
+	  the environment must always be aligned on a sector boundary,
+	  CONFIG_ENV_OFFSET must be aligned to the largest of the
+	  different sector sizes, and CONFIG_ENV_SECT_SIZE should be
+	  set to that value.
+
 config USE_ENV_SPI_BUS
 	bool "SPI flash bus for environment"
 	depends on ENV_IS_IN_SPI_FLASH
@@ -462,6 +476,14 @@ config ENV_FAT_FILE
 	  It's a string of the FAT file name. This file use to store the
 	  environment.
 
+config ENV_FAT_FILE_REDUND
+	string "Name of the FAT file to use for the environment"
+	depends on ENV_IS_IN_FAT && SYS_REDUNDAND_ENVIRONMENT
+	default "uboot-redund.env"
+	help
+	  It's a string of the FAT file name. This file use to store the
+	  redundant environment.
+
 config ENV_EXT4_INTERFACE
 	string "Name of the block device for the environment"
 	depends on ENV_IS_IN_EXT4

env/common.c

@@ -145,7 +145,7 @@ static unsigned char env_flags;
 int env_check_redund(const char *buf1, int buf1_read_fail,
 		     const char *buf2, int buf2_read_fail)
 {
-	int crc1_ok, crc2_ok;
+	int crc1_ok = 0, crc2_ok = 0;
 	env_t *tmp_env1, *tmp_env2;
 
 	tmp_env1 = (env_t *)buf1;
@@ -153,25 +153,18 @@ int env_check_redund(const char *buf1, int buf1_read_fail,
 
 	if (buf1_read_fail && buf2_read_fail) {
 		puts("*** Error - No Valid Environment Area found\n");
+		return -EIO;
 	} else if (buf1_read_fail || buf2_read_fail) {
 		puts("*** Warning - some problems detected ");
 		puts("reading environment; recovered successfully\n");
 	}
 
-	if (buf1_read_fail && buf2_read_fail) {
-		return -EIO;
-	} else if (!buf1_read_fail && buf2_read_fail) {
-		gd->env_valid = ENV_VALID;
-		return -EINVAL;
-	} else if (buf1_read_fail && !buf2_read_fail) {
-		gd->env_valid = ENV_REDUND;
-		return -ENOENT;
-	}
-
-	crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) ==
-			tmp_env1->crc;
-	crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) ==
-			tmp_env2->crc;
+	if (!buf1_read_fail)
+		crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) ==
+				tmp_env1->crc;
+	if (!buf2_read_fail)
+		crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) ==
+				tmp_env2->crc;
 
 	if (!crc1_ok && !crc2_ok) {
 		return -ENOMSG; /* needed for env_load() */
@@ -208,10 +201,6 @@ int env_import_redund(const char *buf1, int buf1_read_fail,
 	if (ret == -EIO) {
 		env_set_default("bad env area", 0);
 		return -EIO;
-	} else if (ret == -EINVAL) {
-		return env_import((char *)buf1, 1, flags);
-	} else if (ret == -ENOENT) {
-		return env_import((char *)buf2, 1, flags);
 	} else if (ret == -ENOMSG) {
 		env_set_default("bad CRC", 0);
 		return -ENOMSG;

env/env.c

@@ -335,6 +335,9 @@ int env_init(void)
 
 		debug("%s: Environment %s init done (ret=%d)\n", __func__,
 		      drv->name, ret);
+
+		if (gd->env_valid == ENV_INVALID)
+			ret = -ENOENT;
 	}
 
 	if (!prio)

env/ext4.c

@@ -188,6 +188,5 @@ U_BOOT_ENV_LOCATION(ext4) = {
 	ENV_NAME("EXT4")
 	.load		= env_ext4_load,
 	.save		= ENV_SAVE_PTR(env_ext4_save),
-	.erase		= CONFIG_IS_ENABLED(CMD_ERASEENV) ? env_ext4_erase :
-							    NULL,
+	.erase		= ENV_ERASE_PTR(env_ext4_erase),
 };

env/fat.c

@@ -18,6 +18,7 @@
 #include <fat.h>
 #include <mmc.h>
 #include <asm/cache.h>
+#include <asm/global_data.h>
 #include <linux/stddef.h>
 
 #ifdef CONFIG_SPL_BUILD
@@ -29,6 +30,8 @@
 # define LOADENV
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static char *env_fat_device_and_part(void)
 {
 #ifdef CONFIG_MMC
@@ -53,6 +56,7 @@ static int env_fat_save(void)
 	env_t __aligned(ARCH_DMA_MINALIGN) env_new;
 	struct blk_desc *dev_desc = NULL;
 	struct disk_partition info;
+	const char *file = CONFIG_ENV_FAT_FILE;
 	int dev, part;
 	int err;
 	loff_t size;
@@ -78,29 +82,41 @@ static int env_fat_save(void)
 		return 1;
 	}
 
-	err = file_fat_write(CONFIG_ENV_FAT_FILE, (void *)&env_new, 0, sizeof(env_t),
-			     &size);
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+	if (gd->env_valid == ENV_VALID)
+		file = CONFIG_ENV_FAT_FILE_REDUND;
+#endif
+
+	err = file_fat_write(file, (void *)&env_new, 0, sizeof(env_t), &size);
 	if (err == -1) {
 		/*
 		 * This printf is embedded in the messages from env_save that
 		 * will calling it. The missing \n is intentional.
 		 */
 		printf("Unable to write \"%s\" from %s%d:%d... ",
-			CONFIG_ENV_FAT_FILE, CONFIG_ENV_FAT_INTERFACE, dev, part);
+			file, CONFIG_ENV_FAT_INTERFACE, dev, part);
 		return 1;
 	}
 
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+	gd->env_valid = (gd->env_valid == ENV_REDUND) ? ENV_VALID : ENV_REDUND;
+#endif
+
 	return 0;
 }
 
 #ifdef LOADENV
 static int env_fat_load(void)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf1, CONFIG_ENV_SIZE);
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+	ALLOC_CACHE_ALIGN_BUFFER(char, buf2, CONFIG_ENV_SIZE);
+	int err2;
+#endif
 	struct blk_desc *dev_desc = NULL;
 	struct disk_partition info;
 	int dev, part;
-	int err;
+	int err1;
 
 #ifdef CONFIG_MMC
 	if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc"))
@@ -124,8 +140,15 @@ static int env_fat_load(void)
 		goto err_env_relocate;
 	}
 
-	err = file_fat_read(CONFIG_ENV_FAT_FILE, buf, CONFIG_ENV_SIZE);
-	if (err == -1) {
+	err1 = file_fat_read(CONFIG_ENV_FAT_FILE, buf1, CONFIG_ENV_SIZE);
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
+	err2 = file_fat_read(CONFIG_ENV_FAT_FILE_REDUND, buf2, CONFIG_ENV_SIZE);
+
+	err1 = (err1 >= 0) ? 0 : -1;
+	err2 = (err2 >= 0) ? 0 : -1;
+	return env_import_redund(buf1, err1, buf2, err2, H_EXTERNAL);
+#else
+	if (err1 < 0) {
 		/*
 		 * This printf is embedded in the messages from env_save that
 		 * will calling it. The missing \n is intentional.
@@ -135,7 +158,8 @@ static int env_fat_load(void)
 		goto err_env_relocate;
 	}
 
-	return env_import(buf, 1, H_EXTERNAL);
+	return env_import(buf1, 1, H_EXTERNAL);
+#endif
 
 err_env_relocate:
 	env_set_default(NULL, 0);

env/flags.c

@@ -563,12 +563,13 @@ int env_flags_validate(const struct env_entry *item, const char *newval,
 		return 1;
 #endif
 
-#ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
 	if (flag & H_FORCE) {
+#ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
 		printf("## Error: Can't force access to \"%s\"\n", name);
+#else
 		return 0;
-	}
 #endif
+	}
 	switch (op) {
 	case env_op_delete:
 		if (item->flags & ENV_FLAGS_VARACCESS_PREVENT_DELETE) {

env/mmc.c

@@ -233,7 +233,6 @@ fini:
 	return ret;
 }
 
-#if defined(CONFIG_CMD_ERASEENV)
 static inline int erase_env(struct mmc *mmc, unsigned long size,
 			    unsigned long offset)
 {
@@ -279,7 +278,6 @@ static int env_mmc_erase(void)
 
 	return ret;
 }
-#endif /* CONFIG_CMD_ERASEENV */
 #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */
 
 static inline int read_env(struct mmc *mmc, unsigned long size,
@@ -394,8 +392,6 @@ U_BOOT_ENV_LOCATION(mmc) = {
 	.load		= env_mmc_load,
 #ifndef CONFIG_SPL_BUILD
 	.save		= env_save_ptr(env_mmc_save),
-#if defined(CONFIG_CMD_ERASEENV)
-	.erase		= env_mmc_erase,
-#endif
+	.erase		= ENV_ERASE_PTR(env_mmc_erase)
 #endif
 };

env/sf.c

@@ -28,16 +28,23 @@
 #define INITENV
 #endif
 
+#define	OFFSET_INVALID		(~(u32)0)
+
 #ifdef CONFIG_ENV_OFFSET_REDUND
+#define ENV_OFFSET_REDUND	CONFIG_ENV_OFFSET_REDUND
+
 static ulong env_offset		= CONFIG_ENV_OFFSET;
 static ulong env_new_offset	= CONFIG_ENV_OFFSET_REDUND;
+
+#else
+
+#define ENV_OFFSET_REDUND	OFFSET_INVALID
+
 #endif /* CONFIG_ENV_OFFSET_REDUND */
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static struct spi_flash *env_flash;
-
-static int setup_flash_device(void)
+static int setup_flash_device(struct spi_flash **env_flash)
 {
 #if CONFIG_IS_ENABLED(DM_SPI_FLASH)
 	struct udevice *new;
@@ -52,14 +59,11 @@ static int setup_flash_device(void)
 		return ret;
 	}
 
-	env_flash = dev_get_uclass_priv(new);
+	*env_flash = dev_get_uclass_priv(new);
 #else
-	if (env_flash)
-		spi_flash_free(env_flash);
-
-	env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-				    CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
-	if (!env_flash) {
+	*env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
+				     CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
+	if (!*env_flash) {
 		env_set_default("spi_flash_probe() failed", 0);
 		return -EIO;
 	}
@@ -72,13 +76,18 @@ static int env_sf_save(void)
 {
 	env_t	env_new;
 	char	*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
-	u32	saved_size, saved_offset, sector;
+	u32	saved_size = 0, saved_offset = 0, sector;
+	u32	sect_size = CONFIG_ENV_SECT_SIZE;
 	int	ret;
+	struct spi_flash *env_flash;
 
-	ret = setup_flash_device();
+	ret = setup_flash_device(&env_flash);
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO))
+		sect_size = env_flash->mtd.erasesize;
+
 	ret = env_export(&env_new);
 	if (ret)
 		return -EIO;
@@ -93,8 +102,8 @@ static int env_sf_save(void)
 	}
 
 	/* Is the sector larger than the env (i.e. embedded) */
-	if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
-		saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+	if (sect_size > CONFIG_ENV_SIZE) {
+		saved_size = sect_size - CONFIG_ENV_SIZE;
 		saved_offset = env_new_offset + CONFIG_ENV_SIZE;
 		saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
 		if (!saved_buffer) {
@@ -107,11 +116,11 @@ static int env_sf_save(void)
 			goto done;
 	}
 
-	sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+	sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, sect_size);
 
 	puts("Erasing SPI flash...");
 	ret = spi_flash_erase(env_flash, env_new_offset,
-				sector * CONFIG_ENV_SECT_SIZE);
+				sector * sect_size);
 	if (ret)
 		goto done;
 
@@ -122,7 +131,7 @@ static int env_sf_save(void)
 	if (ret)
 		goto done;
 
-	if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+	if (sect_size > CONFIG_ENV_SIZE) {
 		ret = spi_flash_write(env_flash, saved_offset,
 					saved_size, saved_buffer);
 		if (ret)
@@ -141,6 +150,8 @@ static int env_sf_save(void)
 	printf("Valid environment: %d\n", (int)gd->env_valid);
 
 done:
+	spi_flash_free(env_flash);
+
 	if (saved_buffer)
 		free(saved_buffer);
 
@@ -152,6 +163,7 @@ static int env_sf_load(void)
 	int ret;
 	int read1_fail, read2_fail;
 	env_t *tmp_env1, *tmp_env2;
+	struct spi_flash *env_flash;
 
 	tmp_env1 = (env_t *)memalign(ARCH_DMA_MINALIGN,
 			CONFIG_ENV_SIZE);
@@ -163,7 +175,7 @@ static int env_sf_load(void)
 		goto out;
 	}
 
-	ret = setup_flash_device();
+	ret = setup_flash_device(&env_flash);
 	if (ret)
 		goto out;
 
@@ -176,7 +188,6 @@ static int env_sf_load(void)
 				read2_fail, H_EXTERNAL);
 
 	spi_flash_free(env_flash);
-	env_flash = NULL;
 out:
 	free(tmp_env1);
 	free(tmp_env2);
@@ -186,18 +197,23 @@ out:
 #else
 static int env_sf_save(void)
 {
-	u32	saved_size, saved_offset, sector;
+	u32	saved_size = 0, saved_offset = 0, sector;
+	u32	sect_size = CONFIG_ENV_SECT_SIZE;
 	char	*saved_buffer = NULL;
 	int	ret = 1;
 	env_t	env_new;
+	struct spi_flash *env_flash;
 
-	ret = setup_flash_device();
+	ret = setup_flash_device(&env_flash);
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO))
+		sect_size = env_flash->mtd.erasesize;
+
 	/* Is the sector larger than the env (i.e. embedded) */
-	if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
-		saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+	if (sect_size > CONFIG_ENV_SIZE) {
+		saved_size = sect_size - CONFIG_ENV_SIZE;
 		saved_offset = CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE;
 		saved_buffer = malloc(saved_size);
 		if (!saved_buffer)
@@ -213,11 +229,11 @@ static int env_sf_save(void)
 	if (ret)
 		goto done;
 
-	sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+	sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, sect_size);
 
 	puts("Erasing SPI flash...");
 	ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
-		sector * CONFIG_ENV_SECT_SIZE);
+		sector * sect_size);
 	if (ret)
 		goto done;
 
@@ -227,7 +243,7 @@ static int env_sf_save(void)
 	if (ret)
 		goto done;
 
-	if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+	if (sect_size > CONFIG_ENV_SIZE) {
 		ret = spi_flash_write(env_flash, saved_offset,
 			saved_size, saved_buffer);
 		if (ret)
@@ -238,6 +254,8 @@ static int env_sf_save(void)
 	puts("done\n");
 
 done:
+	spi_flash_free(env_flash);
+
 	if (saved_buffer)
 		free(saved_buffer);
 
@@ -248,6 +266,7 @@ static int env_sf_load(void)
 {
 	int ret;
 	char *buf = NULL;
+	struct spi_flash *env_flash;
 
 	buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE);
 	if (!buf) {
@@ -255,7 +274,7 @@ static int env_sf_load(void)
 		return -EIO;
 	}
 
-	ret = setup_flash_device();
+	ret = setup_flash_device(&env_flash);
 	if (ret)
 		goto out;
 
@@ -272,7 +291,6 @@ static int env_sf_load(void)
 
 err_read:
 	spi_flash_free(env_flash);
-	env_flash = NULL;
 out:
 	free(buf);
 
@@ -280,6 +298,30 @@ out:
 }
 #endif
 
+static int env_sf_erase(void)
+{
+	int ret;
+	env_t env;
+	struct spi_flash *env_flash;
+
+	ret = setup_flash_device(&env_flash);
+	if (ret)
+		return ret;
+
+	memset(&env, 0, sizeof(env_t));
+	ret = spi_flash_write(env_flash, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, &env);
+	if (ret)
+		goto done;
+
+	if (ENV_OFFSET_REDUND != OFFSET_INVALID)
+		ret = spi_flash_write(env_flash, ENV_OFFSET_REDUND, CONFIG_ENV_SIZE, &env);
+
+done:
+	spi_flash_free(env_flash);
+
+	return ret;
+}
+
 #if CONFIG_ENV_ADDR != 0x0
 __weak void *env_sf_get_env_addr(void)
 {
@@ -320,6 +362,7 @@ static int env_sf_init_early(void)
 	int crc1_ok;
 	env_t *tmp_env2 = NULL;
 	env_t *tmp_env1;
+	struct spi_flash *env_flash;
 
 	/*
 	 * if malloc is not ready yet, we cannot use
@@ -337,7 +380,7 @@ static int env_sf_init_early(void)
 	if (!tmp_env1 || !tmp_env2)
 		goto out;
 
-	ret = setup_flash_device();
+	ret = setup_flash_device(&env_flash);
 	if (ret)
 		goto out;
 
@@ -351,7 +394,7 @@ static int env_sf_init_early(void)
 		ret = env_check_redund((char *)tmp_env1, read1_fail,
 				       (char *)tmp_env2, read2_fail);
 
-		if (ret == -EIO || ret == -ENOMSG)
+		if (ret < 0)
 			goto err_read;
 
 		if (gd->env_valid == ENV_VALID)
@@ -372,10 +415,12 @@ static int env_sf_init_early(void)
 		gd->env_addr = (unsigned long)&tmp_env1->data;
 	}
 
+	spi_flash_free(env_flash);
+
 	return 0;
 err_read:
 	spi_flash_free(env_flash);
-	env_flash = NULL;
+
 	free(tmp_env1);
 	if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
 		free(tmp_env2);
@@ -406,6 +451,7 @@ U_BOOT_ENV_LOCATION(sf) = {
 	.location	= ENVL_SPI_FLASH,
 	ENV_NAME("SPIFlash")
 	.load		= env_sf_load,
-	.save		= CONFIG_IS_ENABLED(SAVEENV) ? ENV_SAVE_PTR(env_sf_save) : NULL,
+	.save		= ENV_SAVE_PTR(env_sf_save),
+	.erase		= ENV_ERASE_PTR(env_sf_erase),
 	.init		= env_sf_init,
 };

include/env.h

@@ -328,8 +328,6 @@ int env_export(struct environment_s *env_out);
  * @buf2_read_fail: 0 if buf2 is valid, non-zero if invalid
  * @return 0 if OK,
  *	-EIO if no environment is valid,
- *	-EINVAL if read of second entry is good
- *	-ENOENT if read of first entry is good
  *	-ENOMSG if the CRC was bad
  */
 

include/env_internal.h

@@ -211,6 +211,7 @@ struct env_driver {
 #endif
 
 #define ENV_SAVE_PTR(x) (CONFIG_IS_ENABLED(SAVEENV) ? (x) : NULL)
+#define ENV_ERASE_PTR(x) (CONFIG_IS_ENABLED(CMD_ERASEENV) ? (x) : NULL)
 
 extern struct hsearch_data env_htab;