[U-Boot] [PATCH] tools: env: Add support for direct read/write UBI volumes

Up to now we were able to read/write environment data from/to UBI volumes only indirectly by gluebi driver. This driver creates NAND MTD on top of UBI volumes, which is quite a workaroung for this use case.
Add support for direct read/write UBI volumes in order to not use obsolete gluebi driver.
Forward-ported from this patch: http://patchwork.ozlabs.org/patch/619305/
Original patch: Signed-off-by: Marcin Niestroj m.niestroj@grinn-global.com
Forward port: Signed-off-by: S. Lockwood-Childs sjl@vctlabs.com --- tools/env/fw_env.c | 255 +++++++++++++++++++++++++++++++++++++++++++++++- tools/env/fw_env.config | 8 ++ 2 files changed, 261 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index ab06415..867fba5 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -25,6 +25,7 @@ #include <sys/ioctl.h> #include <sys/stat.h> #include <unistd.h> +#include <dirent.h>
#ifdef MTD_OLD # include <stdint.h> @@ -34,6 +35,8 @@ # include <mtd/mtd-user.h> #endif
+#include <mtd/ubi-user.h> + #include "fw_env_private.h" #include "fw_env.h"
@@ -58,6 +61,7 @@ struct envdev_s { ulong erase_size; /* device erase size */ ulong env_sectors; /* number of environment sectors */ uint8_t mtd_type; /* type of the MTD device */ + int is_ubi; /* set if we use UBI volume */ };
static struct envdev_s envdevices[2] = @@ -76,6 +80,7 @@ static int dev_current; #define DEVESIZE(i) envdevices[(i)].erase_size #define ENVSECTORS(i) envdevices[(i)].env_sectors #define DEVTYPE(i) envdevices[(i)].mtd_type +#define IS_UBI(i) envdevices[(i)].is_ubi
#define CUR_ENVSIZE ENVSIZE(dev_current)
@@ -122,6 +127,228 @@ static unsigned char obsolete_flag = 0; #define DEFAULT_ENV_INSTANCE_STATIC #include <env_default.h>
+#define UBI_DEV_START "/dev/ubi" +#define UBI_SYSFS "/sys/class/ubi" +#define UBI_VOL_NAME_PATT "ubi%d_%d" + +static int is_ubi_devname(const char *devname) +{ + return !strncmp(devname, UBI_DEV_START, sizeof(UBI_DEV_START) - 1); +} + +static int ubi_check_volume_sysfs_name(const char *volume_sysfs_name, + const char *volname) +{ + char path[256]; + FILE *file; + char *name; + int ret; + + strcpy(path, UBI_SYSFS "/"); + strcat(path, volume_sysfs_name); + strcat(path, "/name"); + + file = fopen(path, "r"); + if (!file) + return -1; + + ret = fscanf(file, "%ms", &name); + fclose(file); + if (ret <= 0 || !name) { + fprintf(stderr, + "Failed to read from file %s, ret = %d, name = %s\n", + path, ret, name); + return -1; + } + + if (!strcmp(name, volname)) { + free(name); + return 0; + } + free(name); + + return -1; +} + +static int ubi_get_volnum_by_name(int devnum, const char *volname) +{ + DIR *sysfs_ubi; + struct dirent *dirent; + int ret; + int tmp_devnum; + int volnum; + + sysfs_ubi = opendir(UBI_SYSFS); + if (!sysfs_ubi) + return -1; + +#ifdef DEBUG + fprintf(stderr, "Looking for volume name "%s"\n", volname); +#endif + + while (1) { + dirent = readdir(sysfs_ubi); + if (!dirent) + return -1; + + ret = sscanf(dirent->d_name, UBI_VOL_NAME_PATT, + &tmp_devnum, &volnum); + if (ret == 2 && devnum == tmp_devnum) { + if (ubi_check_volume_sysfs_name(dirent->d_name, + volname) == 0) + return volnum; + } + } + + return -1; +} + +static int ubi_get_devnum_by_devname(const char *devname) +{ + int devnum; + int ret; + + ret = sscanf(devname + sizeof(UBI_DEV_START) - 1, "%d", &devnum); + if (ret != 1) + return -1; + + return devnum; +} + +static const char *ubi_get_volume_devname(const char *devname, + const char *volname) +{ + char *volume_devname; + int volnum; + int devnum; + int ret; + + devnum = ubi_get_devnum_by_devname(devname); + if (devnum < 0) + return NULL; + + volnum = ubi_get_volnum_by_name(devnum, volname); + if (volnum < 0) + return NULL; + + ret = asprintf(&volume_devname, "%s_%d", devname, volnum); + if (ret < 0) + return NULL; + +#ifdef DEBUG + fprintf(stderr, "Found ubi volume "%s:%s" -> %s\n", + devname, volname, volume_devname); +#endif + + return volume_devname; +} + +static void ubi_check_dev(unsigned int dev_id) +{ + char *devname = (char *)DEVNAME(dev_id); + char *pname; + const char *volname = NULL; + const char *volume_devname; + + if (!is_ubi_devname(DEVNAME(dev_id))) + return; + + IS_UBI(dev_id) = 1; + + for (pname = devname; *pname != '\0'; pname++) { + if (*pname == ':') { + *pname = '\0'; + volname = pname + 1; + break; + } + } + + if (volname) { + /* Let's find real volume device name */ + volume_devname = ubi_get_volume_devname(devname, volname); + if (!volume_devname) { + fprintf(stderr, "Didn't found ubi volume "%s"\n", + volname); + return; + } + + free(devname); + DEVNAME(dev_id) = volume_devname; + } +} + +static int ubi_update_start(int fd, int64_t bytes) +{ + if (ioctl(fd, UBI_IOCVOLUP, &bytes)) + return -1; + return 0; +} + +static int ubi_read(int fd, void *buf, size_t count) +{ + ssize_t ret; + + while (count > 0) { + ret = read(fd, buf, count); + if (ret > 0) { + count -= ret; + buf += ret; + + continue; + } + + if (ret == 0) { + /* + * Happens in case of too short volume data size. If we + * return error status we will fail it will be treated + * as UBI device error. + * + * Leave catching this error to CRC check. + */ + fprintf(stderr, "Warning: end of data on ubi volume\n"); + return 0; + } else if (errno == EBADF) { + /* + * Happens in case of corrupted volume. The same as + * above, we cannot return error now, as we will still + * be able to successfully write environment later. + */ + fprintf(stderr, "Warning: corrupted volume?\n"); + return 0; + } else if (errno == EINTR) { + continue; + } + + fprintf(stderr, "Cannot read %u bytes from ubi volume, %s\n", + (unsigned int)count, strerror(errno)); + return -1; + } + + return 0; +} + +static int ubi_write(int fd, const void *buf, size_t count) +{ + ssize_t ret; + + while (count > 0) { + ret = write(fd, buf, count); + if (ret <= 0) { + if (ret < 0 && errno == EINTR) + continue; + + fprintf(stderr, "Cannot write %u bytes to ubi volume\n", + (unsigned int)count); + return -1; + } + + count -= ret; + buf += ret; + } + + return 0; +} + static int flash_io (int mode); static int parse_config(struct env_opts *opts);
@@ -1022,6 +1249,12 @@ static int flash_write (int fd_current, int fd_target, int dev_target) DEVOFFSET (dev_target), DEVNAME (dev_target)); #endif
+ if (IS_UBI(dev_target)) { + if (ubi_update_start(fd_target, CUR_ENVSIZE) < 0) + return 0; + return ubi_write(fd_target, environment.image, CUR_ENVSIZE); + } + rc = flash_write_buf(dev_target, fd_target, environment.image, CUR_ENVSIZE); if (rc < 0) @@ -1046,6 +1279,12 @@ static int flash_read (int fd) { int rc;
+ if (IS_UBI(dev_current)) { + DEVTYPE(dev_current) = MTD_ABSENT; + + return ubi_read(fd, environment.image, CUR_ENVSIZE); + } + rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE, DEVOFFSET(dev_current)); if (rc != CUR_ENVSIZE) @@ -1234,7 +1473,8 @@ int fw_env_open(struct env_opts *opts) DEVTYPE(!dev_current) == MTD_UBIVOLUME) { environment.flag_scheme = FLAG_INCREMENTAL; } else if (DEVTYPE(dev_current) == MTD_ABSENT && - DEVTYPE(!dev_current) == MTD_ABSENT) { + DEVTYPE(!dev_current) == MTD_ABSENT && + IS_UBI(dev_current) == IS_UBI(!dev_current)) { environment.flag_scheme = FLAG_INCREMENTAL; } else { fprintf (stderr, "Incompatible flash types!\n"); @@ -1347,8 +1587,12 @@ int fw_env_close(struct env_opts *opts) static int check_device_config(int dev) { struct stat st; + int32_t lnum = 0; int fd, rc = 0;
+ /* Fills in IS_UBI(), converts DEVNAME() with ubi volume name */ + ubi_check_dev(dev); + fd = open(DEVNAME(dev), O_RDONLY); if (fd < 0) { fprintf(stderr, @@ -1364,7 +1608,14 @@ static int check_device_config(int dev) goto err; }
- if (S_ISCHR(st.st_mode)) { + if (IS_UBI(dev)) { + rc = ioctl(fd, UBI_IOCEBISMAP, &lnum); + if (rc < 0) { + fprintf(stderr, "Cannot get UBI information for %s\n", + DEVNAME(dev)); + goto err; + } + } else if (S_ISCHR(st.st_mode)) { struct mtd_info_user mtdinfo; rc = ioctl(fd, MEMGETINFO, &mtdinfo); if (rc < 0) { diff --git a/tools/env/fw_env.config b/tools/env/fw_env.config index 7916ebd..053895a 100644 --- a/tools/env/fw_env.config +++ b/tools/env/fw_env.config @@ -28,3 +28,11 @@
# VFAT example #/boot/uboot.env 0x0000 0x4000 + +# UBI volume +#/dev/ubi0_0 0x0 0x1f000 0x1f000 +#/dev/ubi0_1 0x0 0x1f000 0x1f000 + +# UBI volume by name +#/dev/ubi0:env 0x0 0x1f000 0x1f000 +#/dev/ubi0:env-redund 0x0 0x1f000 0x1f000

On Tue, Nov 14, 2017 at 11:01:26PM -0800, S. Lockwood-Childs wrote:
Up to now we were able to read/write environment data from/to UBI volumes only indirectly by gluebi driver. This driver creates NAND MTD on top of UBI volumes, which is quite a workaroung for this use case.
Add support for direct read/write UBI volumes in order to not use obsolete gluebi driver.
Forward-ported from this patch: http://patchwork.ozlabs.org/patch/619305/
Original patch: Signed-off-by: Marcin Niestroj m.niestroj@grinn-global.com
Forward port: Signed-off-by: S. Lockwood-Childs sjl@vctlabs.com
Applied to u-boot/master, thanks!

Hi,
On Tue, 14 Nov 2017 23:01:26 -0800 S. Lockwood-Childs wrote:
Up to now we were able to read/write environment data from/to UBI volumes only indirectly by gluebi driver. This driver creates NAND MTD on top of UBI volumes, which is quite a workaroung for this use case.
Add support for direct read/write UBI volumes in order to not use obsolete gluebi driver.
Forward-ported from this patch: http://patchwork.ozlabs.org/patch/619305/
Original patch: Signed-off-by: Marcin Niestroj m.niestroj@grinn-global.com
Forward port: Signed-off-by: S. Lockwood-Childs sjl@vctlabs.com
tools/env/fw_env.c | 255 +++++++++++++++++++++++++++++++++++++++++++++++- tools/env/fw_env.config | 8 ++ 2 files changed, 261 insertions(+), 2 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index ab06415..867fba5 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -25,6 +25,7 @@ #include <sys/ioctl.h> #include <sys/stat.h> #include <unistd.h> +#include <dirent.h>
#ifdef MTD_OLD # include <stdint.h> @@ -34,6 +35,8 @@ # include <mtd/mtd-user.h> #endif
+#include <mtd/ubi-user.h>
#include "fw_env_private.h" #include "fw_env.h"
@@ -58,6 +61,7 @@ struct envdev_s { ulong erase_size; /* device erase size */ ulong env_sectors; /* number of environment sectors */ uint8_t mtd_type; /* type of the MTD device */
- int is_ubi; /* set if we use UBI volume */
};
static struct envdev_s envdevices[2] = @@ -76,6 +80,7 @@ static int dev_current; #define DEVESIZE(i) envdevices[(i)].erase_size #define ENVSECTORS(i) envdevices[(i)].env_sectors #define DEVTYPE(i) envdevices[(i)].mtd_type +#define IS_UBI(i) envdevices[(i)].is_ubi
#define CUR_ENVSIZE ENVSIZE(dev_current)
@@ -122,6 +127,228 @@ static unsigned char obsolete_flag = 0; #define DEFAULT_ENV_INSTANCE_STATIC #include <env_default.h>
+#define UBI_DEV_START "/dev/ubi" +#define UBI_SYSFS "/sys/class/ubi" +#define UBI_VOL_NAME_PATT "ubi%d_%d"
+static int is_ubi_devname(const char *devname) +{
- return !strncmp(devname, UBI_DEV_START, sizeof(UBI_DEV_START) - 1);
+}
+static int ubi_check_volume_sysfs_name(const char *volume_sysfs_name,
const char *volname)
+{
- char path[256];
/usr/include/linux/limits.h specifies the constant 'PATH_MAX' which should be used here.
- FILE *file;
- char *name;
- int ret;
- strcpy(path, UBI_SYSFS "/");
- strcat(path, volume_sysfs_name);
- strcat(path, "/name");
- file = fopen(path, "r");
- if (!file)
return -1;
- ret = fscanf(file, "%ms", &name);
- fclose(file);
- if (ret <= 0 || !name) {
fprintf(stderr,
"Failed to read from file %s, ret = %d, name = %s\n",
path, ret, name);
return -1;
- }
- if (!strcmp(name, volname)) {
free(name);
return 0;
- }
- free(name);
- return -1;
+}
+static int ubi_get_volnum_by_name(int devnum, const char *volname) +{
- DIR *sysfs_ubi;
- struct dirent *dirent;
- int ret;
- int tmp_devnum;
- int volnum;
- sysfs_ubi = opendir(UBI_SYSFS);
- if (!sysfs_ubi)
return -1;
+#ifdef DEBUG
- fprintf(stderr, "Looking for volume name "%s"\n", volname);
+#endif
- while (1) {
dirent = readdir(sysfs_ubi);
if (!dirent)
return -1;
ret = sscanf(dirent->d_name, UBI_VOL_NAME_PATT,
&tmp_devnum, &volnum);
if (ret == 2 && devnum == tmp_devnum) {
if (ubi_check_volume_sysfs_name(dirent->d_name,
volname) == 0)
return volnum;
}
- }
- return -1;
This can never be reached.
+}
+static int ubi_get_devnum_by_devname(const char *devname) +{
- int devnum;
- int ret;
- ret = sscanf(devname + sizeof(UBI_DEV_START) - 1, "%d", &devnum);
- if (ret != 1)
return -1;
- return devnum;
+}
+static const char *ubi_get_volume_devname(const char *devname,
const char *volname)
+{
- char *volume_devname;
- int volnum;
- int devnum;
- int ret;
- devnum = ubi_get_devnum_by_devname(devname);
- if (devnum < 0)
return NULL;
- volnum = ubi_get_volnum_by_name(devnum, volname);
- if (volnum < 0)
return NULL;
- ret = asprintf(&volume_devname, "%s_%d", devname, volnum);
- if (ret < 0)
return NULL;
+#ifdef DEBUG
- fprintf(stderr, "Found ubi volume "%s:%s" -> %s\n",
devname, volname, volume_devname);
+#endif
- return volume_devname;
+}
+static void ubi_check_dev(unsigned int dev_id) +{
- char *devname = (char *)DEVNAME(dev_id);
- char *pname;
- const char *volname = NULL;
- const char *volume_devname;
- if (!is_ubi_devname(DEVNAME(dev_id)))
return;
- IS_UBI(dev_id) = 1;
- for (pname = devname; *pname != '\0'; pname++) {
if (*pname == ':') {
*pname = '\0';
volname = pname + 1;
break;
}
- }
- if (volname) {
/* Let's find real volume device name */
volume_devname = ubi_get_volume_devname(devname, volname);
if (!volume_devname) {
fprintf(stderr, "Didn't found ubi volume \"%s\"\n",
volname);
return;
}
free(devname);
DEVNAME(dev_id) = volume_devname;
- }
+}
+static int ubi_update_start(int fd, int64_t bytes) +{
- if (ioctl(fd, UBI_IOCVOLUP, &bytes))
return -1;
- return 0;
+}
+static int ubi_read(int fd, void *buf, size_t count) +{
- ssize_t ret;
- while (count > 0) {
ret = read(fd, buf, count);
if (ret > 0) {
count -= ret;
buf += ret;
continue;
}
if (ret == 0) {
/*
* Happens in case of too short volume data size. If we
* return error status we will fail it will be treated
* as UBI device error.
This sentence no sense.
*
* Leave catching this error to CRC check.
*/
fprintf(stderr, "Warning: end of data on ubi volume\n");
return 0;
} else if (errno == EBADF) {
/*
* Happens in case of corrupted volume. The same as
* above, we cannot return error now, as we will still
* be able to successfully write environment later.
*/
fprintf(stderr, "Warning: corrupted volume?\n");
return 0;
} else if (errno == EINTR) {
continue;
}
fprintf(stderr, "Cannot read %u bytes from ubi volume, %s\n",
(unsigned int)count, strerror(errno));
return -1;
- }
- return 0;
+}
+static int ubi_write(int fd, const void *buf, size_t count) +{
- ssize_t ret;
- while (count > 0) {
ret = write(fd, buf, count);
if (ret <= 0) {
if (ret < 0 && errno == EINTR)
continue;
fprintf(stderr, "Cannot write %u bytes to ubi volume\n",
(unsigned int)count);
'%zu' is an appropriate format string for size_t values. No need for a type cast.
@@ -1347,8 +1587,12 @@ int fw_env_close(struct env_opts *opts) static int check_device_config(int dev) { struct stat st;
- int32_t lnum = 0;
This could be defined right before the ioctl, which is the only user of this variable.
int fd, rc = 0;
- /* Fills in IS_UBI(), converts DEVNAME() with ubi volume name */
- ubi_check_dev(dev);
- fd = open(DEVNAME(dev), O_RDONLY); if (fd < 0) { fprintf(stderr,
@@ -1364,7 +1608,14 @@ static int check_device_config(int dev) goto err; }
- if (S_ISCHR(st.st_mode)) {
- if (IS_UBI(dev)) {
rc = ioctl(fd, UBI_IOCEBISMAP, &lnum);
if (rc < 0) {
fprintf(stderr, "Cannot get UBI information for %s\n",
DEVNAME(dev));
goto err;
}
- } else if (S_ISCHR(st.st_mode)) { struct mtd_info_user mtdinfo; rc = ioctl(fd, MEMGETINFO, &mtdinfo); if (rc < 0) {
Lothar Waßmann
participants (3)
-
Lothar Waßmann
-
S. Lockwood-Childs
-
Tom Rini