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

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