[PATCH v4 00/10] Add support for ubi environment to create volumes (v4)

[PATCH v4 00/10] Add support for ubi environment to create volumes (v4)

[Weijie Gao]

This patch series expose more ubi helpers to allow the ubi environment to
create ubi env volumes before loading if they're not exist.

Changes:
v4: Adjust command message. Error messages will be printed in both command
     and api call.
    Add parameter check for "ubi rename" subcommand
    Fix duplicated call to ubi_require_volume() in command.
    Squash api function comments.
    Change all return value to negative.
    Add another kconfig option to control whether to create static volume.
    Read environment volume only when it exists or has been created.

v3: Fix command messages
    Fix some incorrect code flow and error handling
    Correct dynamic parameter type
    Add comments for exported functions

v2: Add const qualifier for char */void * in function parameters
    Adjust normal command message

Weijie Gao (10):
  ubi: remove unnecessary extern directive from function prototypes
  cmd: ubi: mark read-only function parameters with const
  cmd: ubi: use void * for buf parameter in ubi_volume_read
  cmd: ubifs: mark string parameters with const
  cmd: ubi: change the type of parameter dynamic to bool
  cmd: ubi: change all positive error return value to negative
  cmd: ubi: reorganize command messages
  cmd: ubi: export more APIs to public
  cmd: ubi: allow creating volume with all free spaces in ubi_create_vol
  env: ubi: add support to create environment volume if it does not
    exist

 cmd/ubi.c             | 276 +++++++++++++++++++++++++++---------------
 cmd/ubifs.c           |   2 +-
 env/Kconfig           |  20 +++
 env/ubi.c             |  65 ++++++++--
 fs/ubifs/super.c      |   2 +-
 fs/ubifs/ubifs.c      |   2 +-
 include/ubi_uboot.h   | 106 ++++++++++++++--
 include/ubifs_uboot.h |   4 +-
 8 files changed, 355 insertions(+), 122 deletions(-)

-- 
2.45.2

[PATCH v4 01/10] ubi: remove unnecessary extern directive from function prototypes

[Weijie Gao]

The extern directive is unnecessary for function declaration and should be
removed.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v4: not changed
v3: not changed
v2: new
---
 include/ubi_uboot.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index ea0db69c72a..05fb9634d81 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -44,12 +44,12 @@
 #endif
 
 /* functions */
-extern int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
-extern int ubi_init(void);
-extern void ubi_exit(void);
-extern int ubi_part(char *part_name, const char *vid_header_offset);
-extern int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t size);
-extern int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t size);
+int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
+int ubi_init(void);
+void ubi_exit(void);
+int ubi_part(char *part_name, const char *vid_header_offset);
+int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t size);
+int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t size);
 
 extern struct ubi_device *ubi_devices[];
 int cmd_ubifs_mount(char *vol_name);
-- 
2.45.2

[PATCH v4 02/10] cmd: ubi: mark read-only function parameters with const

[Weijie Gao]

Parameters like part/volume name and buffer for writing are not being
modified by the callee functions and should be marked const.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v4: not changed
v3: not changed
v2: new
---
 cmd/ubi.c           | 41 ++++++++++++++++++++++-------------------
 include/ubi_uboot.h |  7 ++++---
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 93de6f3aea2..f75ceff11ee 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -39,7 +39,7 @@ static struct ubi_device *ubi;
 #include <ubifs_uboot.h>
 #endif
 
-static void display_volume_info(struct ubi_device *ubi)
+static void display_volume_info(const struct ubi_device *ubi)
 {
 	int i;
 
@@ -50,7 +50,7 @@ static void display_volume_info(struct ubi_device *ubi)
 	}
 }
 
-static void display_ubi_info(struct ubi_device *ubi)
+static void display_ubi_info(const struct ubi_device *ubi)
 {
 	ubi_msg("MTD device name:            \"%s\"", ubi->mtd->name);
 	ubi_msg("MTD device size:            %llu MiB", ubi->flash_size >> 20);
@@ -149,12 +149,12 @@ static int ubi_list(const char *var, int numeric)
 	return 0;
 }
 
-static int ubi_check_volumename(const struct ubi_volume *vol, char *name)
+static int ubi_check_volumename(const struct ubi_volume *vol, const char *name)
 {
 	return strcmp(vol->name, name);
 }
 
-static int ubi_check(char *name)
+static int ubi_check(const char *name)
 {
 	int i;
 
@@ -213,8 +213,8 @@ bad:
 	return err;
 }
 
-static int ubi_create_vol(char *volume, int64_t size, int dynamic, int vol_id,
-			  bool skipcheck)
+static int ubi_create_vol(const char *volume, int64_t size, int dynamic,
+			  int vol_id, bool skipcheck)
 {
 	struct ubi_mkvol_req req;
 	int err;
@@ -247,7 +247,7 @@ static int ubi_create_vol(char *volume, int64_t size, int dynamic, int vol_id,
 	return ubi_create_volume(ubi, &req);
 }
 
-static struct ubi_volume *ubi_find_volume(char *volume)
+static struct ubi_volume *ubi_find_volume(const char *volume)
 {
 	struct ubi_volume *vol;
 	int i;
@@ -262,7 +262,7 @@ static struct ubi_volume *ubi_find_volume(char *volume)
 	return NULL;
 }
 
-static int ubi_remove_vol(char *volume)
+static int ubi_remove_vol(const char *volume)
 {
 	int err, reserved_pebs, i;
 	struct ubi_volume *vol;
@@ -316,7 +316,7 @@ out_err:
 	return err;
 }
 
-static int ubi_rename_vol(char *oldname, char *newname)
+static int ubi_rename_vol(const char *oldname, const char *newname)
 {
 	struct ubi_volume *vol;
 	struct ubi_rename_entry rename;
@@ -354,7 +354,8 @@ static int ubi_rename_vol(char *oldname, char *newname)
 	return ubi_rename_volumes(ubi, &list);
 }
 
-static int ubi_volume_continue_write(char *volume, void *buf, size_t size)
+static int ubi_volume_continue_write(const char *volume, const void *buf,
+				     size_t size)
 {
 	int err;
 	struct ubi_volume *vol;
@@ -394,8 +395,8 @@ static int ubi_volume_continue_write(char *volume, void *buf, size_t size)
 	return 0;
 }
 
-int ubi_volume_begin_write(char *volume, void *buf, size_t size,
-	size_t full_size)
+int ubi_volume_begin_write(const char *volume, const void *buf, size_t size,
+			   size_t full_size)
 {
 	int err;
 	int rsvd_bytes;
@@ -424,8 +425,8 @@ int ubi_volume_begin_write(char *volume, void *buf, size_t size,
 	return ubi_volume_continue_write(volume, buf, size);
 }
 
-static int ubi_volume_offset_write(char *volume, void *buf, loff_t offset,
-				   size_t size)
+static int ubi_volume_offset_write(const char *volume, const void *buf,
+				   loff_t offset, size_t size)
 {
 	int len, tbuf_size, ret;
 	u64 lnum;
@@ -487,7 +488,8 @@ exit:
 	return ret;
 }
 
-int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t size)
+int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
+		     size_t size)
 {
 	int ret;
 
@@ -503,7 +505,7 @@ int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t size)
 	return ret;
 }
 
-int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t size)
+int ubi_volume_read(const char *volume, char *buf, loff_t offset, size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
@@ -587,7 +589,8 @@ int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t size)
 	return err;
 }
 
-static int ubi_dev_scan(struct mtd_info *info, const char *vid_header_offset)
+static int ubi_dev_scan(const struct mtd_info *info,
+			const char *vid_header_offset)
 {
 	char ubi_mtd_param_buffer[80];
 	int err;
@@ -611,7 +614,7 @@ static int ubi_dev_scan(struct mtd_info *info, const char *vid_header_offset)
 	return 0;
 }
 
-static int ubi_set_skip_check(char *volume, bool skip_check)
+static int ubi_set_skip_check(const char *volume, bool skip_check)
 {
 	struct ubi_vtbl_record vtbl_rec;
 	struct ubi_volume *vol;
@@ -658,7 +661,7 @@ static int ubi_detach(void)
 	return 0;
 }
 
-int ubi_part(char *part_name, const char *vid_header_offset)
+int ubi_part(const char *part_name, const char *vid_header_offset)
 {
 	struct mtd_info *mtd;
 	int err;
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 05fb9634d81..a949d1b80dd 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -47,9 +47,10 @@
 int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
 int ubi_init(void);
 void ubi_exit(void);
-int ubi_part(char *part_name, const char *vid_header_offset);
-int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t size);
-int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t size);
+int ubi_part(const char *part_name, const char *vid_header_offset);
+int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
+		     size_t size);
+int ubi_volume_read(const char *volume, char *buf, loff_t offset, size_t size);
 
 extern struct ubi_device *ubi_devices[];
 int cmd_ubifs_mount(char *vol_name);
-- 
2.45.2

[PATCH v4 03/10] cmd: ubi: use void * for buf parameter in ubi_volume_read

[Weijie Gao]

Use void * to avoid explicit type casting as what ubi_volume_write has done
already.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v4: not changed
v3: not changed
v2: new
---
 cmd/ubi.c           | 4 ++--
 env/ubi.c           | 4 ++--
 include/ubi_uboot.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index f75ceff11ee..8e3cfaaddbb 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -505,7 +505,7 @@ int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
 	return ret;
 }
 
-int ubi_volume_read(const char *volume, char *buf, loff_t offset, size_t size)
+int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
@@ -877,7 +877,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		}
 
 		if (argc == 3) {
-			return ubi_volume_read(argv[3], (char *)addr, 0, size);
+			return ubi_volume_read(argv[3], (void *)addr, 0, size);
 		}
 	}
 
diff --git a/env/ubi.c b/env/ubi.c
index e9865b45ebc..46970ba754f 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -132,14 +132,14 @@ static int env_ubi_load(void)
 		return -EIO;
 	}
 
-	read1_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)tmp_env1, 0,
+	read1_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME, tmp_env1, 0,
 				     CONFIG_ENV_SIZE);
 	if (read1_fail)
 		printf("\n** Unable to read env from %s:%s **\n",
 		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
 
 	read2_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND,
-				     (void *)tmp_env2, 0, CONFIG_ENV_SIZE);
+				     tmp_env2, 0, CONFIG_ENV_SIZE);
 	if (read2_fail)
 		printf("\n** Unable to read redundant env from %s:%s **\n",
 		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index a949d1b80dd..dd22ec7537a 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -50,7 +50,7 @@ void ubi_exit(void);
 int ubi_part(const char *part_name, const char *vid_header_offset);
 int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
 		     size_t size);
-int ubi_volume_read(const char *volume, char *buf, loff_t offset, size_t size);
+int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size);
 
 extern struct ubi_device *ubi_devices[];
 int cmd_ubifs_mount(char *vol_name);
-- 
2.45.2

[PATCH v4 04/10] cmd: ubifs: mark string parameters with const

[Weijie Gao]

File name and volume name should be const as they will not be modified in
these functions.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v4: not changed
v3: not changed
v2: new
---
 cmd/ubifs.c           | 2 +-
 fs/ubifs/super.c      | 2 +-
 fs/ubifs/ubifs.c      | 2 +-
 include/ubi_uboot.h   | 2 +-
 include/ubifs_uboot.h | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/cmd/ubifs.c b/cmd/ubifs.c
index 22e95db8ca5..81f2d37fc7b 100644
--- a/cmd/ubifs.c
+++ b/cmd/ubifs.c
@@ -19,7 +19,7 @@
 static int ubifs_initialized;
 static int ubifs_mounted;
 
-int cmd_ubifs_mount(char *vol_name)
+int cmd_ubifs_mount(const char *vol_name)
 {
 	int ret;
 
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index b6004b88f4e..9c8974a97a5 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2694,7 +2694,7 @@ MODULE_VERSION(__stringify(UBIFS_VERSION));
 MODULE_AUTHOR("Artem Bityutskiy, Adrian Hunter");
 MODULE_DESCRIPTION("UBIFS - UBI File System");
 #else
-int uboot_ubifs_mount(char *vol_name)
+int uboot_ubifs_mount(const char *vol_name)
 {
 	struct dentry *ret;
 	int flags;
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
index b0cc0d2e1b2..f6f21e6af22 100644
--- a/fs/ubifs/ubifs.c
+++ b/fs/ubifs/ubifs.c
@@ -974,7 +974,7 @@ void ubifs_close(void)
 }
 
 /* Compat wrappers for common/cmd_ubifs.c */
-int ubifs_load(char *filename, unsigned long addr, u32 size)
+int ubifs_load(const char *filename, unsigned long addr, u32 size)
 {
 	loff_t actread;
 	int err;
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index dd22ec7537a..6ebd8a3b613 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -53,7 +53,7 @@ int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
 int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size);
 
 extern struct ubi_device *ubi_devices[];
-int cmd_ubifs_mount(char *vol_name);
+int cmd_ubifs_mount(const char *vol_name);
 int cmd_ubifs_umount(void);
 
 #if IS_ENABLED(CONFIG_UBI_BLOCK)
diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h
index db8a29e9bbd..0877dd84f99 100644
--- a/include/ubifs_uboot.h
+++ b/include/ubifs_uboot.h
@@ -18,10 +18,10 @@ struct blk_desc;
 struct disk_partition;
 
 int ubifs_init(void);
-int uboot_ubifs_mount(char *vol_name);
+int uboot_ubifs_mount(const char *vol_name);
 void uboot_ubifs_umount(void);
 int ubifs_is_mounted(void);
-int ubifs_load(char *filename, unsigned long addr, u32 size);
+int ubifs_load(const char *filename, unsigned long addr, u32 size);
 
 int ubifs_set_blk_dev(struct blk_desc *rbdd, struct disk_partition *info);
 int ubifs_ls(const char *dir_name);
-- 
2.45.2

[PATCH v4 05/10] cmd: ubi: change the type of parameter dynamic to bool

[Weijie Gao]

This patch changes the type of the 'dynamic' parameter of ubi_create_vol()
to bool as it's used as a boolean.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
v4: not changed
v3: new
---
 cmd/ubi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 8e3cfaaddbb..8d04ff61fb6 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -213,7 +213,7 @@ bad:
 	return err;
 }
 
-static int ubi_create_vol(const char *volume, int64_t size, int dynamic,
+static int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
 			  int vol_id, bool skipcheck)
 {
 	struct ubi_mkvol_req req;
@@ -765,7 +765,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	}
 
 	if (strncmp(argv[1], "create", 6) == 0) {
-		int dynamic = 1;	/* default: dynamic volume */
+		bool dynamic = true;	/* default: dynamic volume */
 		int id = UBI_VOL_NUM_AUTO;
 
 		/* Use maximum available size */
@@ -786,7 +786,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		/* E.g., create volume size type */
 		if (argc == 5) {
 			if (strncmp(argv[4], "s", 1) == 0)
-				dynamic = 0;
+				dynamic = false;
 			else if (strncmp(argv[4], "d", 1) != 0) {
 				printf("Incorrect type\n");
 				return 1;
-- 
2.45.2

[PATCH v4 06/10] cmd: ubi: change all positive error return value to negative

[Weijie Gao]

Change all return value using errno codes to negative. This makes it
consistent with the linux ubi layer.

Also, to follow the standard definition of U-Boot command, in the do_ubi()
command handler, the return value is converted to CMD_RET_FAILURE for error
returning, and CMD_RET_USAGE for incorrect usage.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
v4: new
---
 cmd/ubi.c | 106 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 62 insertions(+), 44 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 8d04ff61fb6..23b846f22ce 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -172,7 +172,7 @@ static int ubi_check(const char *name)
 static int verify_mkvol_req(const struct ubi_device *ubi,
 			    const struct ubi_mkvol_req *req)
 {
-	int n, err = EINVAL;
+	int n, err = -EINVAL;
 
 	if (req->bytes < 0 || req->alignment < 0 || req->vol_type < 0 ||
 	    req->name_len < 0)
@@ -187,7 +187,7 @@ static int verify_mkvol_req(const struct ubi_device *ubi,
 
 	if (req->bytes == 0) {
 		printf("No space left in UBI device!\n");
-		err = ENOMEM;
+		err = -ENOMEM;
 		goto bad;
 	}
 
@@ -204,7 +204,7 @@ static int verify_mkvol_req(const struct ubi_device *ubi,
 
 	if (req->name_len > UBI_VOL_NAME_MAX) {
 		printf("Name too long!\n");
-		err = ENAMETOOLONG;
+		err = -ENAMETOOLONG;
 		goto bad;
 	}
 
@@ -269,13 +269,13 @@ static int ubi_remove_vol(const char *volume)
 
 	vol = ubi_find_volume(volume);
 	if (vol == NULL)
-		return ENODEV;
+		return -ENODEV;
 
 	printf("Remove UBI volume %s (id %d)\n", vol->name, vol->vol_id);
 
 	if (ubi->ro_mode) {
 		printf("It's read-only mode\n");
-		err = EROFS;
+		err = -EROFS;
 		goto out_err;
 	}
 
@@ -311,8 +311,6 @@ static int ubi_remove_vol(const char *volume)
 	return 0;
 out_err:
 	ubi_err(ubi, "cannot remove volume %s, error %d", volume, err);
-	if (err < 0)
-		err = -err;
 	return err;
 }
 
@@ -326,19 +324,19 @@ static int ubi_rename_vol(const char *oldname, const char *newname)
 	vol = ubi_find_volume(oldname);
 	if (!vol) {
 		printf("%s: volume %s doesn't exist\n", __func__, oldname);
-		return ENODEV;
+		return -ENODEV;
 	}
 
 	if (!ubi_check(newname)) {
 		printf("%s: volume %s already exist\n", __func__, newname);
-		return EINVAL;
+		return -EINVAL;
 	}
 
 	printf("Rename UBI volume %s to %s\n", oldname, newname);
 
 	if (ubi->ro_mode) {
 		printf("%s: ubi device is in read-only mode\n", __func__);
-		return EROFS;
+		return -EROFS;
 	}
 
 	rename.new_name_len = strlen(newname);
@@ -362,17 +360,17 @@ static int ubi_volume_continue_write(const char *volume, const void *buf,
 
 	vol = ubi_find_volume(volume);
 	if (vol == NULL)
-		return ENODEV;
+		return -ENODEV;
 
 	if (!vol->updating) {
 		printf("UBI volume update was not initiated\n");
-		return EINVAL;
+		return -EINVAL;
 	}
 
 	err = ubi_more_update_data(ubi, vol, buf, size);
 	if (err < 0) {
 		printf("Couldnt or partially wrote data\n");
-		return -err;
+		return err;
 	}
 
 	if (err) {
@@ -380,7 +378,7 @@ static int ubi_volume_continue_write(const char *volume, const void *buf,
 
 		err = ubi_check_volume(ubi, vol->vol_id);
 		if (err < 0)
-			return -err;
+			return err;
 
 		if (err) {
 			ubi_warn(ubi, "volume %d on UBI device %d is corrupt",
@@ -404,18 +402,18 @@ int ubi_volume_begin_write(const char *volume, const void *buf, size_t size,
 
 	vol = ubi_find_volume(volume);
 	if (vol == NULL)
-		return ENODEV;
+		return -ENODEV;
 
 	rsvd_bytes = vol->reserved_pebs * (ubi->leb_size - vol->data_pad);
 	if (size > rsvd_bytes) {
 		printf("size > volume size! Aborting!\n");
-		return EINVAL;
+		return -EINVAL;
 	}
 
 	err = ubi_start_update(ubi, vol, full_size);
 	if (err < 0) {
 		printf("Cannot start volume update\n");
-		return -err;
+		return err;
 	}
 
 	/* The volume is just wiped out */
@@ -516,15 +514,15 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 
 	vol = ubi_find_volume(volume);
 	if (vol == NULL)
-		return ENODEV;
+		return -ENODEV;
 
 	if (vol->updating) {
 		printf("updating");
-		return EBUSY;
+		return -EBUSY;
 	}
 	if (vol->upd_marker) {
 		printf("damaged volume, update marker is set");
-		return EBADF;
+		return -EBADF;
 	}
 	if (offp == vol->used_bytes)
 		return 0;
@@ -547,7 +545,7 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 	tbuf = malloc_cache_aligned(tbuf_size);
 	if (!tbuf) {
 		printf("NO MEM\n");
-		return ENOMEM;
+		return -ENOMEM;
 	}
 	len = size > tbuf_size ? tbuf_size : size;
 
@@ -563,7 +561,6 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 		err = ubi_eba_read_leb(ubi, vol, lnum, tbuf, off, len, 0);
 		if (err) {
 			printf("read err %x\n", err);
-			err = -err;
 			break;
 		}
 		off += len;
@@ -603,13 +600,13 @@ static int ubi_dev_scan(const struct mtd_info *info,
 
 	err = ubi_mtd_param_parse(ubi_mtd_param_buffer, NULL);
 	if (err)
-		return -err;
+		return err;
 
 	led_activity_blink();
 	err = ubi_init();
 	led_activity_off();
 	if (err)
-		return -err;
+		return err;
 
 	return 0;
 }
@@ -621,7 +618,7 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
 
 	vol = ubi_find_volume(volume);
 	if (!vol)
-		return ENODEV;
+		return -ENODEV;
 
 	printf("%sing skip_check on volume %s\n",
 	       skip_check ? "Sett" : "Clear", volume);
@@ -677,7 +674,7 @@ int ubi_part(const char *part_name, const char *vid_header_offset)
 	mtd = get_mtd_device_nm(part_name);
 	if (IS_ERR(mtd)) {
 		printf("Partition %s not found!\n", part_name);
-		return 1;
+		return PTR_ERR(mtd);
 	}
 	put_mtd_device(mtd);
 
@@ -698,6 +695,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	int64_t size;
 	ulong addr = 0;
 	bool skipcheck = false;
+	int ret;
 
 	if (argc < 2)
 		return CMD_RET_USAGE;
@@ -712,7 +710,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		if (argc == 2) {
 			if (!ubi) {
 				printf("Error, no UBI device selected!\n");
-				return 1;
+				return CMD_RET_FAILURE;
 			}
 
 			printf("Device %d: %s, MTD partition %s\n",
@@ -726,12 +724,15 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		if (argc > 3)
 			vid_header_offset = argv[3];
 
-		return ubi_part(argv[2], vid_header_offset);
+		ret = ubi_part(argv[2], vid_header_offset);
+		if (ret)
+			return CMD_RET_FAILURE;
+		return 0;
 	}
 
 	if ((strcmp(argv[1], "part") != 0) && !ubi) {
 		printf("Error, no UBI device selected!\n");
-		return 1;
+		return CMD_RET_FAILURE;
 	}
 
 	if (strcmp(argv[1], "info") == 0) {
@@ -761,7 +762,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 			return ubi_check(argv[2]);
 
 		printf("Error, no volume name passed\n");
-		return 1;
+		return CMD_RET_FAILURE;
 	}
 
 	if (strncmp(argv[1], "create", 6) == 0) {
@@ -789,7 +790,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 				dynamic = false;
 			else if (strncmp(argv[4], "d", 1) != 0) {
 				printf("Incorrect type\n");
-				return 1;
+				return CMD_RET_FAILURE;
 			}
 			argc--;
 		}
@@ -806,34 +807,46 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		}
 		/* E.g., create volume */
 		if (argc == 3) {
-			return ubi_create_vol(argv[2], size, dynamic, id,
-					      skipcheck);
+			ret = ubi_create_vol(argv[2], size, dynamic, id,
+					     skipcheck);
+			if (ret)
+				return CMD_RET_FAILURE;
+			return 0;
 		}
 	}
 
 	if (strncmp(argv[1], "remove", 6) == 0) {
 		/* E.g., remove volume */
-		if (argc == 3)
-			return ubi_remove_vol(argv[2]);
+		if (argc == 3) {
+			ret = ubi_remove_vol(argv[2]);
+			if (ret)
+				return CMD_RET_FAILURE;
+			return 0;
+		}
 	}
 
-	if (IS_ENABLED(CONFIG_CMD_UBI_RENAME) && !strncmp(argv[1], "rename", 6))
-		return ubi_rename_vol(argv[2], argv[3]);
+	if (IS_ENABLED(CONFIG_CMD_UBI_RENAME) && !strncmp(argv[1], "rename", 6)) {
+		ret = ubi_rename_vol(argv[2], argv[3]);
+		if (ret)
+			return CMD_RET_FAILURE;
+		return 0;
+	}
 
 	if (strncmp(argv[1], "skipcheck", 9) == 0) {
 		/* E.g., change skip_check flag */
 		if (argc == 4) {
 			skipcheck = strncmp(argv[3], "on", 2) == 0;
-			return ubi_set_skip_check(argv[2], skipcheck);
+			ret = ubi_set_skip_check(argv[2], skipcheck);
+			if (ret)
+				return CMD_RET_FAILURE;
+			return 0;
 		}
 	}
 
 	if (strncmp(argv[1], "write", 5) == 0) {
-		int ret;
-
 		if (argc < 5) {
 			printf("Please see usage\n");
-			return 1;
+			return CMD_RET_USAGE;
 		}
 
 		addr = hextoul(argv[2], NULL);
@@ -858,7 +871,9 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 			       argv[3]);
 		}
 
-		return ret;
+		if (ret)
+			return CMD_RET_FAILURE;
+		return 0;
 	}
 
 	if (strncmp(argv[1], "read", 4) == 0) {
@@ -877,12 +892,15 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		}
 
 		if (argc == 3) {
-			return ubi_volume_read(argv[3], (void *)addr, 0, size);
+			ret = ubi_volume_read(argv[3], (void *)addr, 0, size);
+			if (ret)
+				return CMD_RET_FAILURE;
+			return 0;
 		}
 	}
 
 	printf("Please see usage\n");
-	return 1;
+	return CMD_RET_USAGE;
 }
 
 U_BOOT_CMD(
-- 
2.45.2

[PATCH v4 07/10] cmd: ubi: reorganize command messages

[Weijie Gao]

This patch moves normal subcommand messages into the main command function.
This will allow current and potential api functions being called with clean
output on success.

A new function ubi_require_volume() is added for finding and printing error
message if volume not found. The original ubi_find_volume() will be silent
for being an api function.

To avoid ubi_require_volume() being called twice for volume read/remove,
some changes are required:
- The parameter of ubi_remove_vol() is changed to accept
'struct ubi_volume *' directly.
- The original ubi_volume_read() is renamed to __ubi_volume_read, with its
  first parameter changed to accept also 'struct ubi_volume *' directly.
- A new ubi_volume_read() is added to wraps __ubi_volume_read() to accept
  volume name as its first parameter.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
v4: Make error message always be printed in both command and api calls.
    Avoid duplicated call to ubi_require_volume().
    Add parameter check for "ubi rename" subcommand.
v3: Fix use-after-free and "ubi read" command message displaying error
v2: new
---
 cmd/ubi.c | 118 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 80 insertions(+), 38 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 23b846f22ce..b0b513cfe53 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -241,8 +241,7 @@ static int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
 		printf("verify_mkvol_req failed %d\n", err);
 		return err;
 	}
-	printf("Creating %s volume %s of size %lld\n",
-		dynamic ? "dynamic" : "static", volume, size);
+
 	/* Call real ubi create volume */
 	return ubi_create_volume(ubi, &req);
 }
@@ -258,20 +257,22 @@ static struct ubi_volume *ubi_find_volume(const char *volume)
 			return vol;
 	}
 
-	printf("Volume %s not found!\n", volume);
 	return NULL;
 }
 
-static int ubi_remove_vol(const char *volume)
+static struct ubi_volume *ubi_require_volume(const char *volume)
 {
-	int err, reserved_pebs, i;
-	struct ubi_volume *vol;
+	struct ubi_volume *vol = ubi_find_volume(volume);
 
-	vol = ubi_find_volume(volume);
-	if (vol == NULL)
-		return -ENODEV;
+	if (!vol)
+		printf("Volume %s not found!\n", volume);
+
+	return vol;
+}
 
-	printf("Remove UBI volume %s (id %d)\n", vol->name, vol->vol_id);
+static int ubi_remove_vol(struct ubi_volume *vol)
+{
+	int err, reserved_pebs, i;
 
 	if (ubi->ro_mode) {
 		printf("It's read-only mode\n");
@@ -310,7 +311,7 @@ static int ubi_remove_vol(const char *volume)
 
 	return 0;
 out_err:
-	ubi_err(ubi, "cannot remove volume %s, error %d", volume, err);
+	ubi_err(ubi, "cannot remove volume %s, error %d", vol->name, err);
 	return err;
 }
 
@@ -321,19 +322,15 @@ static int ubi_rename_vol(const char *oldname, const char *newname)
 	struct ubi_volume_desc desc;
 	struct list_head list;
 
-	vol = ubi_find_volume(oldname);
-	if (!vol) {
-		printf("%s: volume %s doesn't exist\n", __func__, oldname);
+	vol = ubi_require_volume(oldname);
+	if (!vol)
 		return -ENODEV;
-	}
 
 	if (!ubi_check(newname)) {
 		printf("%s: volume %s already exist\n", __func__, newname);
 		return -EINVAL;
 	}
 
-	printf("Rename UBI volume %s to %s\n", oldname, newname);
-
 	if (ubi->ro_mode) {
 		printf("%s: ubi device is in read-only mode\n", __func__);
 		return -EROFS;
@@ -358,7 +355,7 @@ static int ubi_volume_continue_write(const char *volume, const void *buf,
 	int err;
 	struct ubi_volume *vol;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (vol == NULL)
 		return -ENODEV;
 
@@ -400,7 +397,7 @@ int ubi_volume_begin_write(const char *volume, const void *buf, size_t size,
 	int rsvd_bytes;
 	struct ubi_volume *vol;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (vol == NULL)
 		return -ENODEV;
 
@@ -432,7 +429,7 @@ static int ubi_volume_offset_write(const char *volume, const void *buf,
 	loff_t off = offset;
 	void *tbuf;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (!vol)
 		return -ENODEV;
 
@@ -503,19 +500,15 @@ int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
 	return ret;
 }
 
-int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
+static int __ubi_volume_read(struct ubi_volume *vol, void *buf, loff_t offset,
+			     size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
 	unsigned long long tmp;
-	struct ubi_volume *vol;
 	loff_t offp = offset;
 	size_t len_read;
 
-	vol = ubi_find_volume(volume);
-	if (vol == NULL)
-		return -ENODEV;
-
 	if (vol->updating) {
 		printf("updating");
 		return -EBUSY;
@@ -527,12 +520,8 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 	if (offp == vol->used_bytes)
 		return 0;
 
-	if (size == 0) {
-		printf("No size specified -> Using max size (%lld)\n", vol->used_bytes);
+	if (size == 0)
 		size = vol->used_bytes;
-	}
-
-	printf("Read %zu bytes from volume %s to %p\n", size, volume, buf);
 
 	if (vol->corrupted)
 		printf("read from corrupted volume %d", vol->vol_id);
@@ -586,6 +575,17 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 	return err;
 }
 
+int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
+{
+	struct ubi_volume *vol;
+
+	vol = ubi_require_volume(volume);
+	if (!vol)
+		return -ENODEV;
+
+	return __ubi_volume_read(vol, buf, offset, size);
+}
+
 static int ubi_dev_scan(const struct mtd_info *info,
 			const char *vid_header_offset)
 {
@@ -616,13 +616,10 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
 	struct ubi_vtbl_record vtbl_rec;
 	struct ubi_volume *vol;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (!vol)
 		return -ENODEV;
 
-	printf("%sing skip_check on volume %s\n",
-	       skip_check ? "Sett" : "Clear", volume);
-
 	vtbl_rec = ubi->vtbl[vol->vol_id];
 	if (skip_check) {
 		vtbl_rec.flags |= UBI_VTBL_SKIP_CRC_CHECK_FLG;
@@ -695,6 +692,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	int64_t size;
 	ulong addr = 0;
 	bool skipcheck = false;
+	struct ubi_volume *vol;
 	int ret;
 
 	if (argc < 2)
@@ -803,7 +801,9 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		/* Use maximum available size */
 		if (!size) {
 			size = (int64_t)ubi->avail_pebs * ubi->leb_size;
-			printf("No size specified -> Using max size (%lld)\n", size);
+			if (size)
+				printf("No size specified -> Using max size (%lld)\n",
+				       size);
 		}
 		/* E.g., create volume */
 		if (argc == 3) {
@@ -811,6 +811,10 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 					     skipcheck);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("Created %s volume %s of size %lld\n",
+			       dynamic ? "dynamic" : "static", argv[2], size);
+
 			return 0;
 		}
 	}
@@ -818,17 +822,37 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	if (strncmp(argv[1], "remove", 6) == 0) {
 		/* E.g., remove volume */
 		if (argc == 3) {
-			ret = ubi_remove_vol(argv[2]);
+			int vol_id;
+
+			vol = ubi_require_volume(argv[2]);
+			if (!vol)
+				return CMD_RET_FAILURE;
+
+			vol_id = vol->vol_id;
+
+			ret = ubi_remove_vol(vol);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("Removed UBI volume %s (id %d)\n", argv[2],
+			       vol_id);
+
 			return 0;
 		}
 	}
 
 	if (IS_ENABLED(CONFIG_CMD_UBI_RENAME) && !strncmp(argv[1], "rename", 6)) {
+		if (argc < 4) {
+			printf("Please see usage\n");
+			return CMD_RET_USAGE;
+		}
+
 		ret = ubi_rename_vol(argv[2], argv[3]);
 		if (ret)
 			return CMD_RET_FAILURE;
+
+		printf("UBI volume %s renamed to %s\n", argv[2], argv[3]);
+
 		return 0;
 	}
 
@@ -839,6 +863,10 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 			ret = ubi_set_skip_check(argv[2], skipcheck);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("%s skip_check on volume %s\n",
+			       skipcheck ? "Set" : "Cleared", argv[2]);
+
 			return 0;
 		}
 	}
@@ -892,9 +920,23 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		}
 
 		if (argc == 3) {
-			ret = ubi_volume_read(argv[3], (void *)addr, 0, size);
+			vol = ubi_require_volume(argv[3]);
+			if (!vol)
+				return CMD_RET_FAILURE;
+
+			if (!size) {
+				printf("No size specified -> Using max size (%lld)\n",
+				       vol->used_bytes);
+				size = vol->used_bytes;
+			}
+
+			ret = __ubi_volume_read(vol, (void *)addr, 0, size);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("%lld bytes read from volume %s to 0x%lx\n",
+			       size, argv[3], addr);
+
 			return 0;
 		}
 	}
-- 
2.45.2

[PATCH v4 08/10] cmd: ubi: export more APIs to public

[Weijie Gao]

Export the following functions to public:

- ubi_detach(): this is paired with ubi_part(). One may call this function
  to completely clean up the ubi subsystem after using ubi_part().

- ubi_{create,find,remove}_vol: this is a set of functions for volume
  management.

The original ubi_create_vol is renamed to __ubi_create_vol to allow the new
ubi_create_vol() being used as a wrapper for __ubi_create_vol() with volume
name.

Also, comments are added for all exported functions.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
v4: squashed api function comments into this patch.
    make ubi_remove_vol() as a wrapper of __ubi_remove_vol() due to changes
     of previous patch.
v3: updated commit message
v2: not changed
---
 cmd/ubi.c           | 23 +++++++++---
 include/ubi_uboot.h | 91 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 6 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index b0b513cfe53..9b2b66b5a99 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -213,8 +213,8 @@ bad:
 	return err;
 }
 
-static int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
-			  int vol_id, bool skipcheck)
+int ubi_create_vol(const char *volume, int64_t size, bool dynamic, int vol_id,
+		   bool skipcheck)
 {
 	struct ubi_mkvol_req req;
 	int err;
@@ -246,7 +246,7 @@ static int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
 	return ubi_create_volume(ubi, &req);
 }
 
-static struct ubi_volume *ubi_find_volume(const char *volume)
+struct ubi_volume *ubi_find_volume(const char *volume)
 {
 	struct ubi_volume *vol;
 	int i;
@@ -270,7 +270,7 @@ static struct ubi_volume *ubi_require_volume(const char *volume)
 	return vol;
 }
 
-static int ubi_remove_vol(struct ubi_volume *vol)
+static int __ubi_remove_vol(struct ubi_volume *vol)
 {
 	int err, reserved_pebs, i;
 
@@ -315,6 +315,17 @@ out_err:
 	return err;
 }
 
+int ubi_remove_vol(const char *volume)
+{
+	struct ubi_volume *vol;
+
+	vol = ubi_require_volume(volume);
+	if (!vol)
+		return -ENODEV;
+
+	return __ubi_remove_vol(vol);
+}
+
 static int ubi_rename_vol(const char *oldname, const char *newname)
 {
 	struct ubi_volume *vol;
@@ -632,7 +643,7 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
 	return ubi_change_vtbl_record(ubi, vol->vol_id, &vtbl_rec);
 }
 
-static int ubi_detach(void)
+int ubi_detach(void)
 {
 #ifdef CONFIG_CMD_UBIFS
 	/*
@@ -830,7 +841,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 
 			vol_id = vol->vol_id;
 
-			ret = ubi_remove_vol(vol);
+			ret = __ubi_remove_vol(vol);
 			if (ret)
 				return CMD_RET_FAILURE;
 
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 6ebd8a3b613..c337b760e96 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -47,11 +47,102 @@
 int ubi_mtd_param_parse(const char *val, struct kernel_param *kp);
 int ubi_init(void);
 void ubi_exit(void);
+
+/**
+ * ubi_detach() - detach UBI from MTD partition
+ *
+ * This function performs the cleanup of the UBI subsystem to make sure the
+ * MTD partition can be safely used for another purpose, or be attached again
+ * with ubi_part().
+ *
+ * Any mounted UBIFS will be unmounted automatically.
+ *
+ * Return: 0
+ */
+int ubi_detach(void);
+
+/**
+ * ubi_part() - attach UBI to MTD partition
+ * @part_name: name of the MTD partition to attach
+ * @vid_header_offset: VID header offset (string)
+ *
+ * This function detaches any existing UBI device, then probes for the
+ * specified MTD partition, and then scans it to initialize UBI.
+ *
+ * @vid_header_offset is optional and is usually set to NULL.
+ *
+ * Return: 0 on success, 1 if partition not found, or -ve on error.
+ */
 int ubi_part(const char *part_name, const char *vid_header_offset);
+
+/**
+ * ubi_volume_write() - write data to UBI volume
+ * @volume: name of the volume to write to
+ * @buf: data buffer to be written
+ * @offset: start offset for writing
+ * @size: number of bytes to write
+ *
+ * This function writes data to the specific UBI volume. If the offset is zero,
+ * it initiates a full volume update. Otherwise, it performs an offset-based
+ * write using LEB changes.
+ *
+ * Return: 0 on success, or -ve on error.
+ */
 int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
 		     size_t size);
+
+/**
+ * ubi_volume_read() - read data from UBI volume
+ * @volume: name of the volume to read from
+ * @buf: buffer to hold the read data
+ * @offset: start offset for reading
+ * @size: number of bytes to read
+ *
+ * This function reads data from the specified UBI volume. If @size is zero,
+ * the function reads the entire volume content starting from @offset.
+ *
+ * Return: 0 on success, or -ve on error.
+ */
 int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size);
 
+/**
+ * ubi_create_vol() - create UBI volume
+ * @volume: name of the volume to create
+ * @size: size of the volume in bytes
+ * @dynamic: create dynamic volume if set to true
+ * @vol_id: volume ID
+ * @skipcheck: skip CRC check on this volume if set to true
+ *
+ * This function creates a new UBI volume with the specified parameters.
+ * If @size is negative, all available space will be used.
+ * For volume ID auto assignment, pass %UBI_VOL_NUM_AUTO to @vol_id.
+ *
+ * Return: 0 on success, or -ve on error.
+ */
+int ubi_create_vol(const char *volume, int64_t size, bool dynamic, int vol_id,
+		   bool skipcheck);
+
+/**
+ * ubi_find_volume() - find UBI volume by name
+ * @volume: name of the volume to find
+ *
+ * This function searches for a UBI volume with the specified name in the
+ * current UBI device.
+ *
+ * Return: pointer to the UBI volume structure, or %NULL if not found.
+ */
+struct ubi_volume *ubi_find_volume(const char *volume);
+
+/**
+ * ubi_remove_vol() - remove UBI volume
+ * @volume: name of the volume to remove
+ *
+ * This function removes an existing UBI volume from the current UBI device.
+ *
+ * Return: 0 on success, or -ve on error.
+ */
+int ubi_remove_vol(const char *volume);
+
 extern struct ubi_device *ubi_devices[];
 int cmd_ubifs_mount(const char *vol_name);
 int cmd_ubifs_umount(void);
-- 
2.45.2

[PATCH v4 09/10] cmd: ubi: allow creating volume with all free spaces in ubi_create_vol

[Weijie Gao]

Although the ubi command itself supports creating volume with all
free spaces, the api ubi_create_vol() does not.

Since negative size is invalid, this patch replaces negative size
with all free space size in ubi_create_vol().

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v4: not changed
v3: not changed
v2: not changed
---
 cmd/ubi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 9b2b66b5a99..b2287c0cf0b 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -226,7 +226,11 @@ int ubi_create_vol(const char *volume, int64_t size, bool dynamic, int vol_id,
 
 	req.vol_id = vol_id;
 	req.alignment = 1;
-	req.bytes = size;
+
+	if (size < 0)
+		req.bytes = ubi->avail_pebs * ubi->leb_size;
+	else
+		req.bytes = size;
 
 	strcpy(req.name, volume);
 	req.name_len = strlen(volume);
-- 
2.45.2

[PATCH v4 10/10] env: ubi: add support to create environment volume if it does not exist

[Weijie Gao]

When U-Boot is booting from a fresh device, the environment volume may not
exist in the factory UBI image. This is a common case where factory UBI
image contains only volumes with valid data.

With the current design, even if the volume is created manually, the
environment will still be unusable (e.g., saveenv) before a reboot.

This patch adds support to automatically create missing volumes before
loading environment. This will make environment available at first boot.

There are two options:
CONFIG_ENV_UBI_VOLUME_CREATE: whether to enable volume creation
CONFIG_ENV_UBI_VOLUME_STATIC: create static volume (default is dynamic)

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
v4: Add option to control whether to create static volume
    Read environment volume only when it exists or has been created.
v3: fix error handling and default env setting
v2: updated kconfig help text
    added error output when failed to create volume
---
 env/Kconfig | 20 +++++++++++++++++
 env/ubi.c   | 65 ++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index 7abd82ab6f3..9ec3e090481 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -716,6 +716,26 @@ config ENV_UBI_VOLUME_REDUND
 	help
 	  Name of the redundant volume that you want to store the environment in.
 
+config ENV_UBI_VOLUME_CREATE
+	bool "Create UBI volume if it does not exist"
+	depends on ENV_IS_IN_UBI
+	help
+	  This option is useful if U-Boot will be booted from a fresh device
+	  where the environment volume has not been created.
+	  This is a common case where factory UBI image contains only volumes
+	  with valid data.
+	  By enabling this option, any missing environment volumes will be
+	  created before loading.
+	  If CONFIG_ENV_UBI_VOLUME_REDUND is also enabled, both volumes will be
+	  created,
+
+config ENV_UBI_VOLUME_STATIC
+	bool "Create static UBI volume"
+	depends on ENV_UBI_VOLUME_CREATE
+	help
+	  By default environment volume will be dynamic.
+	  Enable this option to create static volume.
+
 config ENV_UBI_VID_OFFSET
 	int "ubi environment VID offset"
 	depends on ENV_IS_IN_UBI
diff --git a/env/ubi.c b/env/ubi.c
index 46970ba754f..810faaf7744 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -103,12 +103,30 @@ static int env_ubi_save(void)
 }
 #endif /* CONFIG_ENV_REDUNDANT */
 
+static int env_ubi_volume_create(const char *volume)
+{
+	bool dynamic = !IS_ENABLED(CONFIG_ENV_UBI_VOLUME_STATIC);
+	struct ubi_volume *vol;
+	int ret;
+
+	vol = ubi_find_volume(volume);
+	if (vol)
+		return 0;
+
+	ret = ubi_create_vol(volume, CONFIG_ENV_SIZE, dynamic, UBI_VOL_NUM_AUTO,
+			     false);
+	if (ret)
+		printf("Failed to create environment volume '%s'\n", volume);
+
+	return ret;
+}
+
 #ifdef CONFIG_ENV_REDUNDANT
 static int env_ubi_load(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
 	ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
-	int read1_fail, read2_fail;
+	int read1_fail, read2_fail, create1_fail = 0, create2_fail = 0;
 	env_t *tmp_env1, *tmp_env2;
 
 	/*
@@ -132,17 +150,35 @@ static int env_ubi_load(void)
 		return -EIO;
 	}
 
-	read1_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME, tmp_env1, 0,
-				     CONFIG_ENV_SIZE);
-	if (read1_fail)
-		printf("\n** Unable to read env from %s:%s **\n",
-		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+	if (IS_ENABLED(CONFIG_ENV_UBI_VOLUME_CREATE)) {
+		create1_fail = env_ubi_volume_create(CONFIG_ENV_UBI_VOLUME);
+		create2_fail = env_ubi_volume_create(CONFIG_ENV_UBI_VOLUME_REDUND);
+		if (create1_fail && create2_fail) {
+			env_set_default(NULL, 0);
+			return -ENODEV;
+		}
+	}
 
-	read2_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND,
-				     tmp_env2, 0, CONFIG_ENV_SIZE);
-	if (read2_fail)
-		printf("\n** Unable to read redundant env from %s:%s **\n",
-		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME_REDUND);
+	if (!create1_fail) {
+		read1_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME, tmp_env1, 0,
+					     CONFIG_ENV_SIZE);
+		if (read1_fail)
+			printf("\n** Unable to read env from %s:%s **\n",
+			       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
+	} else {
+		read1_fail = create1_fail;
+	}
+
+	if (!create2_fail) {
+		read2_fail = ubi_volume_read(CONFIG_ENV_UBI_VOLUME_REDUND,
+					     tmp_env2, 0, CONFIG_ENV_SIZE);
+		if (read2_fail)
+			printf("\n** Unable to read redundant env from %s:%s **\n",
+			       CONFIG_ENV_UBI_PART,
+			       CONFIG_ENV_UBI_VOLUME_REDUND);
+	} else {
+		read2_fail = create2_fail;
+	}
 
 	return env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2,
 				 read2_fail, H_EXTERNAL);
@@ -169,6 +205,13 @@ static int env_ubi_load(void)
 		return -EIO;
 	}
 
+	if (IS_ENABLED(CONFIG_ENV_UBI_VOLUME_CREATE)) {
+		if (env_ubi_volume_create(CONFIG_ENV_UBI_VOLUME)) {
+			env_set_default(NULL, 0);
+			return -ENODEV;
+		}
+	}
+
 	if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, buf, 0, CONFIG_ENV_SIZE)) {
 		printf("\n** Unable to read env from %s:%s **\n",
 		       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
-- 
2.45.2

Re: [PATCH v4 05/10] cmd: ubi: change the type of parameter dynamic to bool

[Simon Glass]

On 2026-05-13T08:02:45, Weijie Gao <weijie.gao@mediatek.com> wrote:
> cmd: ubi: change the type of parameter dynamic to bool
>
> This patch changes the type of the 'dynamic' parameter of ubi_create_vol()
> to bool as it's used as a boolean.
>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>
> cmd/ubi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v4 06/10] cmd: ubi: change all positive error return value to negative

[Simon Glass]

Hi Weijie,

On 2026-05-13T08:02:45, Weijie Gao <weijie.gao@mediatek.com> wrote:
> cmd: ubi: change all positive error return value to negative
>
> Change all return value using errno codes to negative. This makes it
> consistent with the linux ubi layer.
>
> Also, to follow the standard definition of U-Boot command, in the do_ubi()
> command handler, the return value is converted to CMD_RET_FAILURE for error
> returning, and CMD_RET_USAGE for incorrect usage.
>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>
> cmd/ubi.c | 106 ++++++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 62 insertions(+), 44 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -761,7 +762,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>                       return ubi_check(argv[2]);
>
>               printf("Error, no volume name passed\n");
> -             return 1;
> +             return CMD_RET_FAILURE;

A missing argument is a usage error, not a runtime failure. Please use
CMD_RET_USAGE here, and likewise for the 'Incorrect type' and 'no
volume name passed' paths in the create handler, so the user sees the
help text.

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -677,7 +674,7 @@ int ubi_part(const char *part_name, const char *vid_header_offset)
>       mtd = get_mtd_device_nm(part_name);
>       if (IS_ERR(mtd)) {
>               printf("Partition %s not found!\n", part_name);
> -             return 1;
> +             return PTR_ERR(mtd);
>       }

PTR_ERR() returns long but ubi_part() returns int. Please cast, or
just return a specific errno like -ENODEV

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -806,34 +807,46 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>               }
>               /* E.g., create volume */
>               if (argc == 3) {
> -                     return ubi_create_vol(argv[2], size, dynamic, id,
> -                                           skipcheck);
> +                     ret = ubi_create_vol(argv[2], size, dynamic, id,
> +                                          skipcheck);
> +                     if (ret)
> +                             return CMD_RET_FAILURE;
> +                     return 0;
>               }

This three-line conversion is repeated about seven times in do_ubi().
Since the called functions already print their own errors, a helper or
just 'return ret ? CMD_RET_FAILURE : 0;' would cut the duplication.
What do you think?

Regards,
Simon

Re: [PATCH v4 07/10] cmd: ubi: reorganize command messages

[Simon Glass]

On 2026-05-13T08:02:45, Weijie Gao <weijie.gao@mediatek.com> wrote:
> cmd: ubi: reorganize command messages
>
> This patch moves normal subcommand messages into the main command function.
> This will allow current and potential api functions being called with clean
> output on success.
>
> A new function ubi_require_volume() is added for finding and printing error
> message if volume not found. The original ubi_find_volume() will be silent
> for being an api function.
>
> To avoid ubi_require_volume() being called twice for volume read/remove,
> some changes are required:
> - The parameter of ubi_remove_vol() is changed to accept
> 'struct ubi_volume *' directly.
> - The original ubi_volume_read() is renamed to __ubi_volume_read, with its
>   first parameter changed to accept also 'struct ubi_volume *' directly.
> - A new ubi_volume_read() is added to wraps __ubi_volume_read() to accept
>   volume name as its first parameter.
>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>
> cmd/ubi.c | 118 ++++++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 80 insertions(+), 38 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v4 08/10] cmd: ubi: export more APIs to public

[Simon Glass]

Hi Weijie,

On 2026-05-13T08:02:45, Weijie Gao <weijie.gao@mediatek.com> wrote:
> cmd: ubi: export more APIs to public
>
> Export the following functions to public:
>
> - ubi_detach(): this is paired with ubi_part(). One may call this function
>   to completely clean up the ubi subsystem after using ubi_part().
>
> - ubi_{create,find,remove}_vol: this is a set of functions for volume
>   management.
>
> The original ubi_create_vol is renamed to __ubi_create_vol to allow the new
> ubi_create_vol() being used as a wrapper for __ubi_create_vol() with volume
> name.
>
> Also, comments are added for all exported functions.
>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>
> cmd/ubi.c           | 23 ++++++++++----
>  include/ubi_uboot.h | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 108 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

> diff --git a/cmd/ubi.c b/cmd/ubi.c
> @@ -213,8 +213,8 @@ bad:
>       return err;
>  }
>
> -static int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
> -                       int vol_id, bool skipcheck)
> +int ubi_create_vol(const char *volume, int64_t size, bool dynamic, int vol_id,
> +                bool skipcheck)

The commit message is now out of date - it says ubi_create_vol() is
renamed to __ubi_create_vol() and wrapped.

> diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
> @@ -47,11 +47,102 @@
> + * ubi_part() - attach UBI to MTD partition
> + * @part_name: name of the MTD partition to attach
> + * @vid_header_offset: VID header offset (string)
> + *
> + * This function detaches any existing UBI device, then probes for the
> + * specified MTD partition, and then scans it to initialize UBI.
> + *
> + * @vid_header_offset is optional and is usually set to NULL.
> + *
> + * Return: 0 on success, 1 if partition not found, or -ve on error.
> + */
>  int ubi_part(const char *part_name, const char *vid_header_offset);

Just to check, where does the '1 if partition not found' come from? My
reading of ubi_dev_scan() / do_ubi() is that a missing partition
surfaces as a negative errno. If there is no path that returns +1,
please drop that clause.

Regards,
Simon

Re: [PATCH v4 10/10] env: ubi: add support to create environment volume if it does not exist

[Simon Glass]

Hi Weijie,

On 2026-05-13T08:02:45, Weijie Gao <weijie.gao@mediatek.com> wrote:
> env: ubi: add support to create environment volume if it does not exist
>
> When U-Boot is booting from a fresh device, the environment volume may not
> exist in the factory UBI image. This is a common case where factory UBI
> image contains only volumes with valid data.
>
> With the current design, even if the volume is created manually, the
> environment will still be unusable (e.g., saveenv) before a reboot.
>
> This patch adds support to automatically create missing volumes before
> loading environment. This will make environment available at first boot.
>
> There are two options:
> CONFIG_ENV_UBI_VOLUME_CREATE: whether to enable volume creation
> CONFIG_ENV_UBI_VOLUME_STATIC: create static volume (default is dynamic)
>
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
>
> env/Kconfig | 20 +++++++++++++++++++
>  env/ubi.c   | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  2 files changed, 74 insertions(+), 11 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

> diff --git a/env/Kconfig b/env/Kconfig
> @@ -716,6 +716,26 @@ config ENV_UBI_VOLUME_REDUND
> +config ENV_UBI_VOLUME_CREATE
> +     bool "Create UBI volume if it does not exist"
> +     depends on ENV_IS_IN_UBI
> +     help
> +       This option is useful if U-Boot will be booted from a fresh device
> +       where the environment volume has not been created.
> +       This is a common case where factory UBI image contains only volumes
> +       with valid data.
> +       By enabling this option, any missing environment volumes will be
> +       created before loading.
> +       If CONFIG_ENV_UBI_VOLUME_REDUND is also enabled, both volumes will be
> +       created,

Trailing comma should be a full stop. Also drop the CONFIG_ prefix -
the Kconfig convention is to refer to the symbol as
ENV_UBI_VOLUME_REDUND.

Regards,
Simon