[openwrt/openwrt] uboot-envtools: support NVMEM based access
LEDE Commits
lede-commits at lists.infradead.org
Mon Jul 11 02:15:30 PDT 2022
rmilecki pushed a commit to openwrt/openwrt.git, branch master:
https://git.openwrt.org/cb27179e62ed25f2dd98ffc53c4aaefcd861b6c0
commit cb27179e62ed25f2dd98ffc53c4aaefcd861b6c0
Author: Rafał Miłecki <rafal at milecki.pl>
AuthorDate: Mon Jul 11 11:10:52 2022 +0200
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 <rafal at milecki.pl>
---
...-make-flash_io-take-buffer-as-an-argument.patch | 144 +++++++++++++++++
...plify-logic-code-paths-in-the-fw_env_open.patch | 173 +++++++++++++++++++++
...dd-fallback-to-Linux-s-NVMEM-based-access.patch | 110 +++++++++++++
3 files changed, 427 insertions(+)
diff --git a/package/boot/uboot-envtools/patches/100-fw_env-make-flash_io-take-buffer-as-an-argument.patch b/package/boot/uboot-envtools/patches/100-fw_env-make-flash_io-take-buffer-as-an-argument.patch
new file mode 100644
index 0000000000..c3b20edbdd
--- /dev/null
+++ b/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?= <rafal at milecki.pl>
+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 <rafal at milecki.pl>
+---
+ 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;
+ }
diff --git a/package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch b/package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch
new file mode 100644
index 0000000000..dbb71bd433
--- /dev/null
+++ b/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?= <rafal at milecki.pl>
+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 <rafal at milecki.pl>
+---
+ 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
diff --git a/package/boot/uboot-envtools/patches/102-fw_env-add-fallback-to-Linux-s-NVMEM-based-access.patch b/package/boot/uboot-envtools/patches/102-fw_env-add-fallback-to-Linux-s-NVMEM-based-access.patch
new file mode 100644
index 0000000000..da17350b40
--- /dev/null
+++ b/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?= <rafal at milecki.pl>
+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 <rafal at milecki.pl>
+---
+ 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;
More information about the lede-commits
mailing list