uboot-envtools: support NVMEM based access

This will allow using fw_printenv without /etc/fw_env.config. Once there
is Linux NVMEM driver available for U-Boot env data.

Signed-off-by: Rafał Miłecki <[email protected]>

package/boot/uboot-envtools/patches/100-fw_env-make-flash_io-take-buffer-as-an-argument.patch

@@ -0,0 +1,144 @@
+From f178f7c9550c4fd9c644f79a1eb2dafa5bcdce25 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <[email protected]>
+Date: Wed, 12 Jan 2022 12:47:05 +0100
+Subject: [PATCH] fw_env: make flash_io() take buffer as an argument
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+It's usually easier to understand code & follow it if all arguments are
+passed explicitly. Many coding styles also discourage using global
+variables.
+
+Behaviour of flash_io() was a bit unintuitive as it was writing to a
+buffer referenced in a global struct. That required developers to
+remember how it works and sometimes required hacking "environment"
+global struct variable to read data into a proper buffer.
+
+Signed-off-by: Rafał Miłecki <[email protected]>
+---
+ tools/env/fw_env.c | 32 ++++++++++++++++----------------
+ 1 file changed, 16 insertions(+), 16 deletions(-)
+
+--- a/tools/env/fw_env.c
++++ b/tools/env/fw_env.c
+@@ -346,7 +346,7 @@ static int ubi_write(int fd, const void
+ 	return 0;
+ }
+ 
+-static int flash_io(int mode);
++static int flash_io(int mode, void *buf, size_t count);
+ static int parse_config(struct env_opts *opts);
+ 
+ #if defined(CONFIG_FILE)
+@@ -516,7 +516,7 @@ int fw_env_flush(struct env_opts *opts)
+ 	*environment.crc = crc32(0, (uint8_t *) environment.data, ENV_SIZE);
+ 
+ 	/* write environment back to flash */
+-	if (flash_io(O_RDWR)) {
++	if (flash_io(O_RDWR, environment.image, CUR_ENVSIZE)) {
+ 		fprintf(stderr, "Error: can't write fw_env to flash\n");
+ 		return -1;
+ 	}
+@@ -1185,7 +1185,8 @@ static int flash_flag_obsolete(int dev,
+ 	return rc;
+ }
+ 
+-static int flash_write(int fd_current, int fd_target, int dev_target)
++static int flash_write(int fd_current, int fd_target, int dev_target, void *buf,
++		       size_t count)
+ {
+ 	int rc;
+ 
+@@ -1212,11 +1213,10 @@ static int flash_write(int fd_current, i
+ 	if (IS_UBI(dev_target)) {
+ 		if (ubi_update_start(fd_target, CUR_ENVSIZE) < 0)
+ 			return -1;
+-		return ubi_write(fd_target, environment.image, CUR_ENVSIZE);
++		return ubi_write(fd_target, buf, count);
+ 	}
+ 
+-	rc = flash_write_buf(dev_target, fd_target, environment.image,
+-			     CUR_ENVSIZE);
++	rc = flash_write_buf(dev_target, fd_target, buf, count);
+ 	if (rc < 0)
+ 		return rc;
+ 
+@@ -1235,17 +1235,17 @@ static int flash_write(int fd_current, i
+ 	return 0;
+ }
+ 
+-static int flash_read(int fd)
++static int flash_read(int fd, void *buf, size_t count)
+ {
+ 	int rc;
+ 
+ 	if (IS_UBI(dev_current)) {
+ 		DEVTYPE(dev_current) = MTD_ABSENT;
+ 
+-		return ubi_read(fd, environment.image, CUR_ENVSIZE);
++		return ubi_read(fd, buf, count);
+ 	}
+ 
+-	rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE,
++	rc = flash_read_buf(dev_current, fd, buf, count,
+ 			    DEVOFFSET(dev_current));
+ 	if (rc != CUR_ENVSIZE)
+ 		return -1;
+@@ -1291,7 +1291,7 @@ err:
+ 	return rc;
+ }
+ 
+-static int flash_io_write(int fd_current)
++static int flash_io_write(int fd_current, void *buf, size_t count)
+ {
+ 	int fd_target = -1, rc, dev_target;
+ 	const char *dname, *target_temp = NULL;
+@@ -1322,7 +1322,7 @@ static int flash_io_write(int fd_current
+ 			fd_target = fd_current;
+ 	}
+ 
+-	rc = flash_write(fd_current, fd_target, dev_target);
++	rc = flash_write(fd_current, fd_target, dev_target, buf, count);
+ 
+ 	if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
+ 		fprintf(stderr,
+@@ -1377,7 +1377,7 @@ static int flash_io_write(int fd_current
+ 	return rc;
+ }
+ 
+-static int flash_io(int mode)
++static int flash_io(int mode, void *buf, size_t count)
+ {
+ 	int fd_current, rc;
+ 
+@@ -1391,9 +1391,9 @@ static int flash_io(int mode)
+ 	}
+ 
+ 	if (mode == O_RDWR) {
+-		rc = flash_io_write(fd_current);
++		rc = flash_io_write(fd_current, buf, count);
+ 	} else {
+-		rc = flash_read(fd_current);
++		rc = flash_read(fd_current, buf, count);
+ 	}
+ 
+ 	if (close(fd_current)) {
+@@ -1455,7 +1455,7 @@ int fw_env_open(struct env_opts *opts)
+ 	}
+ 
+ 	dev_current = 0;
+-	if (flash_io(O_RDONLY)) {
++	if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) {
+ 		ret = -EIO;
+ 		goto open_cleanup;
+ 	}
+@@ -1490,7 +1490,7 @@ int fw_env_open(struct env_opts *opts)
+ 		 * other pointers in environment still point inside addr0
+ 		 */
+ 		environment.image = addr1;
+-		if (flash_io(O_RDONLY)) {
++		if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) {
+ 			ret = -EIO;
+ 			goto open_cleanup;
+ 		}

package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch

@@ -0,0 +1,173 @@
+From 07c79dd5fdeaefb39c9e7a97f3b66de63109a18d Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <[email protected]>
+Date: Wed, 12 Jan 2022 12:47:06 +0100
+Subject: [PATCH] fw_env: simplify logic & code paths in the fw_env_open()
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Environment variables can be stored in two formats:
+1. Single entry with header containing CRC32
+2. Two entries with extra flags field in each entry header
+
+For that reason fw_env_open() has two main code paths and there are
+pointers for CRC32/flags/data.
+
+Previous implementation was a bit hard to follow:
+1. It was checking for used format twice (in reversed order each time)
+2. It was setting "environment" global struct fields to some temporary
+   values that required extra comments explaining it
+
+This change simplifies that code:
+1. It introduces two clear code paths
+2. It sets "environment" global struct fields values only once it really
+   knows them
+
+To be fair there are *two* crc32() calls now and an extra pointer
+variable but that should be cheap enough and worth it.
+
+Signed-off-by: Rafał Miłecki <[email protected]>
+---
+ tools/env/fw_env.c | 77 +++++++++++++++++++---------------------------
+ 1 file changed, 31 insertions(+), 46 deletions(-)
+
+--- a/tools/env/fw_env.c
++++ b/tools/env/fw_env.c
+@@ -1421,9 +1421,6 @@ int fw_env_open(struct env_opts *opts)
+ 
+ 	int ret;
+ 
+-	struct env_image_single *single;
+-	struct env_image_redundant *redundant;
+-
+ 	if (!opts)
+ 		opts = &default_opts;
+ 
+@@ -1439,40 +1436,37 @@ int fw_env_open(struct env_opts *opts)
+ 		goto open_cleanup;
+ 	}
+ 
+-	/* read environment from FLASH to local buffer */
+-	environment.image = addr0;
+-
+-	if (have_redund_env) {
+-		redundant = addr0;
+-		environment.crc = &redundant->crc;
+-		environment.flags = &redundant->flags;
+-		environment.data = redundant->data;
+-	} else {
+-		single = addr0;
+-		environment.crc = &single->crc;
+-		environment.flags = NULL;
+-		environment.data = single->data;
+-	}
+-
+ 	dev_current = 0;
+-	if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) {
++	if (flash_io(O_RDONLY, addr0, CUR_ENVSIZE)) {
+ 		ret = -EIO;
+ 		goto open_cleanup;
+ 	}
+ 
+-	crc0 = crc32(0, (uint8_t *)environment.data, ENV_SIZE);
+-
+-	crc0_ok = (crc0 == *environment.crc);
+ 	if (!have_redund_env) {
++		struct env_image_single *single = addr0;
++
++		crc0 = crc32(0, (uint8_t *)single->data, ENV_SIZE);
++		crc0_ok = (crc0 == single->crc);
+ 		if (!crc0_ok) {
+ 			fprintf(stderr,
+ 				"Warning: Bad CRC, using default environment\n");
+-			memcpy(environment.data, default_environment,
++			memcpy(single->data, default_environment,
+ 			       sizeof(default_environment));
+ 			environment.dirty = 1;
+ 		}
++
++		environment.image = addr0;
++		environment.crc = &single->crc;
++		environment.flags = NULL;
++		environment.data = single->data;
+ 	} else {
+-		flag0 = *environment.flags;
++		struct env_image_redundant *redundant0 = addr0;
++		struct env_image_redundant *redundant1;
++
++		crc0 = crc32(0, (uint8_t *)redundant0->data, ENV_SIZE);
++		crc0_ok = (crc0 == redundant0->crc);
++
++		flag0 = redundant0->flags;
+ 
+ 		dev_current = 1;
+ 		addr1 = calloc(1, CUR_ENVSIZE);
+@@ -1483,14 +1477,9 @@ int fw_env_open(struct env_opts *opts)
+ 			ret = -ENOMEM;
+ 			goto open_cleanup;
+ 		}
+-		redundant = addr1;
++		redundant1 = addr1;
+ 
+-		/*
+-		 * have to set environment.image for flash_read(), careful -
+-		 * other pointers in environment still point inside addr0
+-		 */
+-		environment.image = addr1;
+-		if (flash_io(O_RDONLY, environment.image, CUR_ENVSIZE)) {
++		if (flash_io(O_RDONLY, addr1, CUR_ENVSIZE)) {
+ 			ret = -EIO;
+ 			goto open_cleanup;
+ 		}
+@@ -1518,18 +1507,12 @@ int fw_env_open(struct env_opts *opts)
+ 			goto open_cleanup;
+ 		}
+ 
+-		crc1 = crc32(0, (uint8_t *)redundant->data, ENV_SIZE);
++		crc1 = crc32(0, (uint8_t *)redundant1->data, ENV_SIZE);
+ 
+-		crc1_ok = (crc1 == redundant->crc);
+-		flag1 = redundant->flags;
++		crc1_ok = (crc1 == redundant1->crc);
++		flag1 = redundant1->flags;
+ 
+-		/*
+-		 * environment.data still points to ((struct
+-		 * env_image_redundant *)addr0)->data. If the two
+-		 * environments differ, or one has bad crc, force a
+-		 * write-out by marking the environment dirty.
+-		 */
+-		if (memcmp(environment.data, redundant->data, ENV_SIZE) ||
++		if (memcmp(redundant0->data, redundant1->data, ENV_SIZE) ||
+ 		    !crc0_ok || !crc1_ok)
+ 			environment.dirty = 1;
+ 
+@@ -1540,7 +1523,7 @@ int fw_env_open(struct env_opts *opts)
+ 		} else if (!crc0_ok && !crc1_ok) {
+ 			fprintf(stderr,
+ 				"Warning: Bad CRC, using default environment\n");
+-			memcpy(environment.data, default_environment,
++			memcpy(redundant0->data, default_environment,
+ 			       sizeof(default_environment));
+ 			environment.dirty = 1;
+ 			dev_current = 0;
+@@ -1586,13 +1569,15 @@ int fw_env_open(struct env_opts *opts)
+ 		 */
+ 		if (dev_current) {
+ 			environment.image = addr1;
+-			environment.crc = &redundant->crc;
+-			environment.flags = &redundant->flags;
+-			environment.data = redundant->data;
++			environment.crc = &redundant1->crc;
++			environment.flags = &redundant1->flags;
++			environment.data = redundant1->data;
+ 			free(addr0);
+ 		} else {
+ 			environment.image = addr0;
+-			/* Other pointers are already set */
++			environment.crc = &redundant0->crc;
++			environment.flags = &redundant0->flags;
++			environment.data = redundant0->data;
+ 			free(addr1);
+ 		}
+ #ifdef DEBUG

package/boot/uboot-envtools/patches/102-fw_env-add-fallback-to-Linux-s-NVMEM-based-access.patch

@@ -0,0 +1,110 @@
+From 8142c4554ffaa927529f24427a35f7ee2861793a Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <[email protected]>
+Date: Thu, 16 Jun 2022 20:59:03 +0200
+Subject: [PATCH] fw_env: add fallback to Linux's NVMEM based access
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+A new DT binding for describing environment data block has been added in
+Linux's commit 5db1c2dbc04c ("dt-bindings: nvmem: add U-Boot environment
+variables binding"). Once we get a proper Linux NVMEM driver it'll be
+possible to use Linux's binary interface for user-space as documented
+in the:
+https://www.kernel.org/doc/html/latest/driver-api/nvmem.html
+
+This commits makes fw_env fallback to looking for a compatible NVMEM
+device in case config file isn't present. In a long term this may make
+config files redundant and avoid code (info) duplication.
+
+Signed-off-by: Rafał Miłecki <[email protected]>
+---
+ tools/env/fw_env.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
+ 1 file changed, 67 insertions(+), 3 deletions(-)
+
+--- a/tools/env/fw_env.c
++++ b/tools/env/fw_env.c
+@@ -1713,6 +1713,67 @@ static int check_device_config(int dev)
+ 	return rc;
+ }
+ 
++static int find_nvmem_device(void)
++{
++	const char *path = "/sys/bus/nvmem/devices";
++	struct dirent *dent;
++	char *nvmem = NULL;
++	char comp[256];
++	char buf[32];
++	int bytes;
++	DIR *dir;
++
++	dir = opendir(path);
++	if (!dir) {
++		return -EIO;
++	}
++
++	while (!nvmem && (dent = readdir(dir))) {
++		FILE *fp;
++
++		if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, "..")) {
++			continue;
++		}
++
++		bytes = snprintf(comp, sizeof(comp), "%s/%s/of_node/compatible", path, dent->d_name);
++		if (bytes < 0 || bytes == sizeof(comp)) {
++			continue;
++		}
++
++		fp = fopen(comp, "r");
++		if (!fp) {
++			continue;
++		}
++
++		fread(buf, sizeof(buf), 1, fp);
++
++		if (!strcmp(buf, "u-boot,env")) {
++			bytes = asprintf(&nvmem, "%s/%s/nvmem", path, dent->d_name);
++			if (bytes < 0) {
++				nvmem = NULL;
++			}
++		}
++
++		fclose(fp);
++	}
++
++	closedir(dir);
++
++	if (nvmem) {
++		struct stat s;
++
++		stat(nvmem, &s);
++
++		DEVNAME(0) = nvmem;
++		DEVOFFSET(0) = 0;
++		ENVSIZE(0) = s.st_size;
++
++		return 0;
++	}
++
++	return -ENOENT;
++}
++
+ static int parse_config(struct env_opts *opts)
+ {
+ 	int rc;
+@@ -1723,9 +1784,12 @@ static int parse_config(struct env_opts
+ #if defined(CONFIG_FILE)
+ 	/* Fills in DEVNAME(), ENVSIZE(), DEVESIZE(). Or don't. */
+ 	if (get_config(opts->config_file)) {
+-		fprintf(stderr, "Cannot parse config file '%s': %m\n",
+-			opts->config_file);
+-		return -1;
++		if (find_nvmem_device()) {
++			fprintf(stderr, "Cannot parse config file '%s': %m\n",
++				opts->config_file);
++			fprintf(stderr, "Failed to find NVMEM device\n");
++			return -1;
++		}
+ 	}
+ #else
+ 	DEVNAME(0) = DEVICE1_NAME;