From cb27179e62ed25f2dd98ffc53c4aaefcd861b6c0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Mon, 11 Jul 2022 11:10:52 +0200 Subject: [PATCH] uboot-envtools: support NVMEM based access MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- ...-flash_io-take-buffer-as-an-argument.patch | 144 +++++++++++++++ ...-logic-code-paths-in-the-fw_env_open.patch | 173 ++++++++++++++++++ ...llback-to-Linux-s-NVMEM-based-access.patch | 110 +++++++++++ 3 files changed, 427 insertions(+) create mode 100644 package/boot/uboot-envtools/patches/100-fw_env-make-flash_io-take-buffer-as-an-argument.patch create mode 100644 package/boot/uboot-envtools/patches/101-fw_env-simplify-logic-code-paths-in-the-fw_env_open.patch create mode 100644 package/boot/uboot-envtools/patches/102-fw_env-add-fallback-to-Linux-s-NVMEM-based-access.patch 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 00000000000..c3b20edbdd6 --- /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?= +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 +--- + 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 00000000000..dbb71bd4337 --- /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?= +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 +--- + 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 00000000000..da17350b40a --- /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?= +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 +--- + 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; -- 2.30.2