[U-Boot] [PATCH] [UBI] UBI command support

[U-Boot] [PATCH] [UBI] UBI command support

[Kyungmin Park]

UBI command support

Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 common/Makefile  |    1 +
 common/cmd_ubi.c |  535 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 536 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_ubi.c

diff --git a/common/Makefile b/common/Makefile
index f00cbd9..b02a541 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -139,6 +139,7 @@ COBJS-$(CONFIG_CMD_SETEXPR) += cmd_setexpr.o
 COBJS-$(CONFIG_CMD_SPI) += cmd_spi.o
 COBJS-$(CONFIG_CMD_STRINGS) += cmd_strings.o
 COBJS-$(CONFIG_CMD_TERMINAL) += cmd_terminal.o
+COBJS-$(CONFIG_CMD_UBI) += cmd_ubi.o
 COBJS-$(CONFIG_CMD_UNIVERSE) += cmd_universe.o
 ifdef CONFIG_CMD_USB
 COBJS-y += cmd_usb.o
diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
new file mode 100644
index 0000000..795805e
--- /dev/null
+++ b/common/cmd_ubi.c
@@ -0,0 +1,535 @@
+/*
+ * Unsorted Block Image commands
+ */
+
+#include <common.h>
+#include <command.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <ubi_uboot.h>
+#include <asm/errno.h>
+
+/* board/\*\/ubi.c */
+extern int ubi_board_scan(void);
+
+/* drivers/mtd/ubi/build.c */
+extern struct ubi_device *ubi_devices[];
+
+/* Private own data */
+static struct ubi_device *ubi;
+static int ubi_initialized;
+
+static void ubi_dump_vol_info(const struct ubi_volume *vol)
+{
+	dbg_msg("volume information dump:");
+	dbg_msg("vol_id          %d", vol->vol_id);
+	dbg_msg("reserved_pebs   %d", vol->reserved_pebs);
+	dbg_msg("alignment       %d", vol->alignment);
+	dbg_msg("data_pad        %d", vol->data_pad);
+	dbg_msg("vol_type        %d", vol->vol_type);
+	dbg_msg("name_len        %d", vol->name_len);
+	dbg_msg("usable_leb_size %d", vol->usable_leb_size);
+	dbg_msg("used_ebs        %d", vol->used_ebs);
+	dbg_msg("used_bytes      %lld", vol->used_bytes);
+	dbg_msg("last_eb_bytes   %d", vol->last_eb_bytes);
+	dbg_msg("corrupted       %d", vol->corrupted);
+	dbg_msg("upd_marker      %d", vol->upd_marker);
+
+	if (vol->name_len <= UBI_VOL_NAME_MAX &&
+		strnlen(vol->name, vol->name_len + 1) == vol->name_len) {
+		dbg_msg("name            %s", vol->name);
+	} else {
+		dbg_msg("the 1st 5 characters of the name: %c%c%c%c%c",
+				vol->name[0], vol->name[1], vol->name[2],
+				vol->name[3], vol->name[4]);
+	}
+	printf("\n");
+}
+
+static void display_volume_info(struct ubi_device *ubi)
+{
+	int i;
+
+	for (i = 0; i < (ubi->vtbl_slots + 1); i++) {
+		if (!ubi->volumes[i])
+			continue;	/* Empty record */
+		ubi_dump_vol_info(ubi->volumes[i]);
+	}
+}
+
+static void display_ubi_info(struct ubi_device *ubi)
+{
+	ubi_msg("MTD device name:            \"%s\"", ubi->mtd->name);
+	ubi_msg("MTD device size:            %llu MiB", ubi->flash_size >> 20);
+	ubi_msg("physical eraseblock size:   %d bytes (%d KiB)",
+			ubi->peb_size, ubi->peb_size >> 10);
+	ubi_msg("logical eraseblock size:    %d bytes", ubi->leb_size);
+	ubi_msg("number of good PEBs:        %d", ubi->good_peb_count);
+	ubi_msg("number of bad PEBs:         %d", ubi->bad_peb_count);
+	ubi_msg("smallest flash I/O unit:    %d", ubi->min_io_size);
+	ubi_msg("VID header offset:          %d (aligned %d)",
+			ubi->vid_hdr_offset, ubi->vid_hdr_aloffset);
+	ubi_msg("data offset:                %d", ubi->leb_start);
+	ubi_msg("max. allowed volumes:       %d", ubi->vtbl_slots);
+	ubi_msg("wear-leveling threshold:    %d", CONFIG_MTD_UBI_WL_THRESHOLD);
+	ubi_msg("number of internal volumes: %d", UBI_INT_VOL_COUNT);
+	ubi_msg("number of user volumes:     %d",
+			ubi->vol_count - UBI_INT_VOL_COUNT);
+	ubi_msg("available PEBs:             %d", ubi->avail_pebs);
+	ubi_msg("total number of reserved PEBs: %d", ubi->rsvd_pebs);
+	ubi_msg("number of PEBs reserved for bad PEB handling: %d",
+			ubi->beb_rsvd_pebs);
+	ubi_msg("max/mean erase counter: %d/%d", ubi->max_ec, ubi->mean_ec);
+}
+
+static int ubi_info(int layout)
+{
+	if (layout)
+		display_volume_info(ubi);
+	else
+		display_ubi_info(ubi);
+
+	return 0;
+}
+
+static int ustrtoul(const char *cp, char **endp, unsigned int base)
+{
+        unsigned long result = simple_strtoul(cp, endp, base);
+        switch (**endp) {
+        case 'G' :
+                result *= 1024;
+        case 'M':
+                result *= 1024;
+        case 'K':
+        case 'k':
+                result *= 1024;
+                if ((*endp)[1] == 'i') {
+                        if ((*endp)[2] == 'B')
+                                (*endp) += 3;
+                        else
+                                (*endp) += 2;
+                }
+        }
+        return result;
+}
+
+static int parse_num(size_t *num, const char *token)
+{
+        char *endp;
+        size_t n;
+
+        n = (size_t) ustrtoul(token, &endp, 0);
+        if (*endp)
+                return -EINVAL;
+
+        *num = n;
+        return 0;
+}
+
+static int verify_mkvol_req(const struct ubi_device *ubi,
+			    const struct ubi_mkvol_req *req)
+{
+	int n, err = -EINVAL;
+
+	if (req->bytes < 0 || req->alignment < 0 || req->vol_type < 0 ||
+	    req->name_len < 0)
+		goto bad;
+
+	if ((req->vol_id < 0 || req->vol_id >= ubi->vtbl_slots) &&
+	    req->vol_id != UBI_VOL_NUM_AUTO)
+		goto bad;
+
+	if (req->alignment == 0)
+		goto bad;
+
+	if (req->bytes == 0)
+		goto bad;
+
+	if (req->vol_type != UBI_DYNAMIC_VOLUME &&
+	    req->vol_type != UBI_STATIC_VOLUME)
+		goto bad;
+
+	if (req->alignment > ubi->leb_size)
+		goto bad;
+
+	n = req->alignment % ubi->min_io_size;
+	if (req->alignment != 1 && n)
+		goto bad;
+
+	if (req->name_len > UBI_VOL_NAME_MAX) {
+		err = -ENAMETOOLONG;
+		goto bad;
+	}
+
+	return 0;
+bad:
+	printf("bad volume creation request");
+//      ubi_dbg_dump_mkvol_req(req);
+	return err;
+}
+
+static int ubi_create_vol(char *volume, int size, int dynamic)
+{
+	struct ubi_mkvol_req req;
+	int err;
+
+	if (dynamic)
+		req.vol_type = UBI_DYNAMIC_VOLUME;
+	else
+		req.vol_type = UBI_STATIC_VOLUME;
+
+	req.vol_id = UBI_VOL_NUM_AUTO;
+	req.alignment = 1;
+	req.bytes = size;
+
+	strcpy(req.name, volume);
+	req.name_len = strlen(volume);
+	req.name[req.name_len] = '\0';
+	req.padding1 = 0;
+	/* It's duplicated at drivers/mtd/ubi/cdev.c */
+	err = verify_mkvol_req(ubi, &req);
+	if (err) {
+		printf("verify_mkvol_req failed %d\n", err);
+		return err;
+	}
+	printf("Creating %s volume %s of size %d\n",
+		dynamic ? "dynamic" : "static", volume, size);
+	/* Call real ubi create volume */
+	return ubi_create_volume(ubi, &req);
+}
+
+static int ubi_remove_vol(char *volume)
+{
+	int i, err, reserved_pebs;
+	int found = 0, vol_id = 0;
+	struct ubi_volume *vol;
+
+	for (i = 0; i < ubi->vtbl_slots; i++) {
+		vol = ubi->volumes[i];
+		if (vol && !strcmp(vol->name, volume)) {
+			printf("Volume %s found at valid %d\n", volume, i);
+			vol_id = i;
+			found = 1;
+			break;
+		}
+	}
+	if (!found) {
+		printf("%s volume not found\n", volume);
+		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;
+		goto out_err;
+	}
+
+	err = ubi_change_vtbl_record(ubi, vol_id, NULL);
+	if (err) {
+		printf("Error changing Vol tabel record err=%x\n", err);
+		goto out_err;
+	}
+	reserved_pebs = vol->reserved_pebs;
+	for (i = 0; i < vol->reserved_pebs; i++) {
+		err = ubi_eba_unmap_leb(ubi, vol, i);
+		if (err)
+			goto out_err;
+	}
+
+	kfree(vol->eba_tbl);
+	ubi->volumes[vol_id]->eba_tbl = NULL;
+	ubi->volumes[vol_id] = NULL;
+
+	ubi->rsvd_pebs -= reserved_pebs;
+	ubi->avail_pebs += reserved_pebs;
+	i = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs;
+	if (i > 0) {
+		i = ubi->avail_pebs >= i ? i : ubi->avail_pebs;
+		ubi->avail_pebs -= i;
+		ubi->rsvd_pebs += i;
+		ubi->beb_rsvd_pebs += i;
+		if (i > 0)
+			ubi_msg("reserve more %d PEBs", i);
+	}
+	ubi->vol_count -= 1;
+
+	return 0;
+out_err:
+	ubi_err("cannot remove volume %d, error %d", vol_id, err);
+	return err;
+}
+
+static int ubi_volume_write(char *volume, void *buf, size_t size)
+{
+	int i = 0, err = -1;
+	int rsvd_bytes = 0;
+	int found = 0;
+	struct ubi_volume *vol;
+
+	for (i = 0; i < ubi->vtbl_slots; i++) {
+		vol = ubi->volumes[i];
+		if (vol && !strcmp(vol->name, volume)) {
+			printf("Volume \"%s\" found at volume id %d\n", volume, i);
+			found = 1;
+			break;
+		}
+	}
+	if (!found) {
+		printf("%s voume not found\n", volume);
+		return 1;
+	}
+	rsvd_bytes = vol->reserved_pebs * (ubi->leb_size - vol->data_pad);
+	if (size < 0 || size > rsvd_bytes) {
+		printf("rsvd_bytes=%d vol->reserved_pebs=%d ubi->leb_size=%d\n",
+		     rsvd_bytes, vol->reserved_pebs, ubi->leb_size);
+		printf("vol->data_pad=%d\n", vol->data_pad);
+		printf("Size > volume size !!\n");
+		return 1;
+	}
+	
+	err = ubi_start_update(ubi, vol, size);
+	if (err < 0) {
+		printf("Cannot start volume update\n");
+		return err;
+	}
+	
+	err = ubi_more_update_data(ubi, vol, buf, size);
+	if (err < 0) {
+		printf("Couldnt or partially wrote data \n");
+		return err;
+	}
+
+	if (err) {
+		size = err;
+
+		err = ubi_check_volume(ubi, vol->vol_id);
+		if ( err < 0 )
+			return err;
+
+		if (err) {
+			ubi_warn("volume %d on UBI device %d is corrupted",
+					vol->vol_id, ubi->ubi_num);
+			vol->corrupted = 1;
+		}
+
+		vol->checked = 1;
+		ubi_gluebi_updated(vol);
+	}
+
+	return 0;
+}
+
+static int ubi_volume_read(char *volume, char *buf, size_t size)
+{
+	int err, lnum, off, len, tbuf_size, i = 0;
+	size_t count_save = size;
+	void *tbuf;
+	unsigned long long tmp;
+	struct ubi_volume *vol = NULL;
+	loff_t offp = 0;
+
+	for (i = 0; i < ubi->vtbl_slots; i++) {
+		vol = ubi->volumes[i];
+		if (vol && !strcmp(vol->name, volume)) {
+			printf("Volume %s found at volume id %d\n",
+				volume, vol->vol_id);
+			break;
+		}
+	}
+	if (i == ubi->vtbl_slots) {
+		printf("%s voume not found\n", volume);
+		return 0;
+	}
+
+	printf("read %i bytes from volume %d to %x(buf address)\n",
+	       (int) size, vol->vol_id, (unsigned)buf);
+
+	if (vol->updating) {
+		printf("updating");
+		return -EBUSY;
+	}
+	if (vol->upd_marker) {
+		printf("damaged volume, update marker is set");
+		return -EBADF;
+	}
+	if (offp == vol->used_bytes)
+		return 0;
+
+	if (size == 0) {
+		printf("Read [%lu] bytes\n", (unsigned long) vol->used_bytes);
+		size = vol->used_bytes;
+	}
+
+	if (vol->corrupted)
+		printf("read from corrupted volume %d", vol->vol_id);
+	if (offp + size > vol->used_bytes)
+		count_save = size = vol->used_bytes - offp;
+
+	tbuf_size = vol->usable_leb_size;
+	if (size < tbuf_size)
+		tbuf_size = ALIGN(size, ubi->min_io_size);
+	tbuf = vmalloc(tbuf_size);
+	if (!tbuf) {
+		printf("NO MEM\n");
+		return -ENOMEM;
+	}
+	len = size > tbuf_size ? tbuf_size : size;
+
+	tmp = offp;
+	off = do_div(tmp, vol->usable_leb_size);
+	lnum = tmp;
+	printf("off=%d lnum=%d\n", off, lnum);
+	do {
+		if (off + len >= vol->usable_leb_size)
+			len = vol->usable_leb_size - off;
+
+		err = ubi_eba_read_leb(ubi, vol, lnum, tbuf, off, len, 0);
+		if (err) {
+			printf("read err %x\n", err);
+			break;
+		}
+		off += len;
+		if (off == vol->usable_leb_size) {
+			lnum += 1;
+			off -= vol->usable_leb_size;
+		}
+
+		size -= len;
+		offp += len;
+
+		printf("buf = %x\n", (unsigned)buf);
+		memcpy(buf, tbuf, len);
+		printf("buf[0] = %x\n", buf[0]);
+
+		buf += len;
+		len = size > tbuf_size ? tbuf_size : size;
+	} while (size);
+
+	vfree(tbuf);
+	return err ? err : count_save - size;
+}
+
+static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	size_t size = 0;
+	ulong addr = 0;
+	int err = 0;
+
+	if (!ubi_initialized) {
+		err = ubi_board_scan();
+		if (err) {
+			printf("ubi initialize error %d\n", err);
+			return err;
+		}
+		ubi = ubi_devices[0];
+		ubi_initialized = 1;
+	}
+
+	if (argc < 2) {
+		printf("Usage:\n%s\n", cmdtp->usage);
+		return 1;
+	}
+	if (strcmp(argv[1], "info") == 0) {
+		int layout = 0;
+		if (argc > 2 && !strncmp(argv[2], "l", 1))
+			layout = 1;
+		return ubi_info(layout);
+	}
+	if (strncmp(argv[1], "create", 6) == 0) {
+		int dynamic = 1;	/* default: dynamic volume */
+
+		/* Use maximum available size */
+		size = 0;
+
+		/* E.g., create volume size type */
+		if (argc == 5) {
+			if (strncmp(argv[4], "s", 1) == 0)
+				dynamic = 0;
+			else if (strncmp(argv[4], "d", 1) != 0) {
+				printf("You give wrong type\n");
+				return 1;
+			}
+			argc--;
+		}
+		/* E.g., create volume size */
+		if (argc == 4) {
+			err = parse_num(&size, argv[3]);
+			if (err) {
+				printf("You give correct size\n");
+				return err;
+			}
+			argc--;
+		}
+		/* Use maximum available size */
+		if (!size)
+			size = ubi->avail_pebs * ubi->leb_size;
+		/* E.g., create volume */
+		if (argc == 3)
+			return ubi_create_vol(argv[2], size, dynamic);
+	}
+	if (strncmp(argv[1], "remove", 6) == 0) {
+		/* E.g., remove volume */
+		if (argc == 3)
+			return ubi_remove_vol(argv[2]);
+	}
+	if (strncmp(argv[1], "write", 5) == 0) {
+		if (argc < 5) {
+			printf("You give wrong parameters\n");
+			return 1;
+		}
+
+		addr = simple_strtoul(argv[2], NULL, 16);
+		err = parse_num(&size, argv[4]);
+		if (err) {
+			printf("You give wrong size\n");
+			return err;
+		}
+
+		return ubi_volume_write(argv[3], (void *)addr, size);
+	}
+	if (strncmp(argv[1], "read", 4) == 0) {
+		size = 0;
+
+		/* E.g., read volume size */
+		if (argc == 5) {
+			err = parse_num(&size, argv[4]);
+			if (err) {
+				printf("You give wrong size\n");
+				return err;
+			}
+			argc--;
+		}
+
+		/* E.g., read volume */
+		if (argc == 4) {
+			addr = simple_strtoul(argv[2], NULL, 16);
+			argc--;
+		}
+
+		if (argc == 3)
+			return ubi_volume_read(argv[3], (char *)addr, size);
+	}
+
+	printf("Unknown UBI command or invalid number of arguments\n");
+	return 1;
+}
+
+U_BOOT_CMD(ubi, 6, 1, do_ubi,
+	"ubi      - ubi commands\n",
+	"info [l[ayout]]"
+		" - Display volume and ubi layout information\n"
+	"ubi create[vol] volume [size] [type]"
+		" - create volume name with size\n"
+	"ubi write[vol] address volume size"
+		" - Write volume from address with size\n"
+	"ubi read[vol] address volume [size]"
+		" - Read volume to address with size\n"
+	"ubi remove[vol] volume"
+		" - Remove volume\n"
+	"[Legends]\n"
+	" volume: charater name\n"
+	" size: KiB, MiB, GiB, and bytes\n"
+	" type: s[tatic] or d[ynamic] (default=dynamic)\n"
+);
-- 
1.5.3.3

[U-Boot] [PATCH] [UBI] UBI command support

[Wolfgang Denk]

Dear Kyungmin Park,

In message <20081021091859.GA29306@july> you wrote:
> UBI command support
> 
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
...
> +++ b/common/cmd_ubi.c
> @@ -0,0 +1,535 @@
> +/*
> + * Unsorted Block Image commands
> + */
> +
> +#include <common.h>
> +#include <command.h>

GPL header missing.

> +/* board/\*\/ubi.c */

What's the '\'s there?

> +extern int ubi_board_scan(void);

Please move the prototypes to some header file.

> +/* drivers/mtd/ubi/build.c */
> +extern struct ubi_device *ubi_devices[];
> +
> +/* Private own data */
> +static struct ubi_device *ubi;
> +static int ubi_initialized;
> +
> +static void ubi_dump_vol_info(const struct ubi_volume *vol)
> +{
> +	dbg_msg("volume information dump:");
> +	dbg_msg("vol_id          %d", vol->vol_id);
> +	dbg_msg("reserved_pebs   %d", vol->reserved_pebs);
> +	dbg_msg("alignment       %d", vol->alignment);
> +	dbg_msg("data_pad        %d", vol->data_pad);
> +	dbg_msg("vol_type        %d", vol->vol_type);
> +	dbg_msg("name_len        %d", vol->name_len);
> +	dbg_msg("usable_leb_size %d", vol->usable_leb_size);
> +	dbg_msg("used_ebs        %d", vol->used_ebs);
> +	dbg_msg("used_bytes      %lld", vol->used_bytes);
> +	dbg_msg("last_eb_bytes   %d", vol->last_eb_bytes);
> +	dbg_msg("corrupted       %d", vol->corrupted);
> +	dbg_msg("upd_marker      %d", vol->upd_marker);

Please use the standard  debug()  macro.

> +static int ustrtoul(const char *cp, char **endp, unsigned int base)

Maybe we should add this to some global place, like lib_generic ?

> +        unsigned long result = simple_strtoul(cp, endp, base);
> +        switch (**endp) {
> +        case 'G' :
> +                result *= 1024;

Please add a
		/* fall through */
comment to make clkear that it is intentional not to have a "break;"
here.

> +        case 'M':
> +                result *= 1024;

Ditto.

> +static int verify_mkvol_req(const struct ubi_device *ubi,
> +			    const struct ubi_mkvol_req *req)
> +{
...
> +bad:
> +	printf("bad volume creation request");
> +//      ubi_dbg_dump_mkvol_req(req);
> +	return err;

No C++ comments, please. And please don;t add dead code either.

> +	tbuf = vmalloc(tbuf_size);

Why do we need new alloc() stuff?

> +
> +	vfree(tbuf);

Ditto?

> +static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
> +{
> +	size_t size = 0;
> +	ulong addr = 0;
> +	int err = 0;
> +
> +	if (!ubi_initialized) {
> +		err = ubi_board_scan();
> +		if (err) {
> +			printf("ubi initialize error %d\n", err);

"UBI init error" -- maybe we can decode the error number into some
string?

> +		/* E.g., create volume size type */
> +		if (argc == 5) {
> +			if (strncmp(argv[4], "s", 1) == 0)
> +				dynamic = 0;
> +			else if (strncmp(argv[4], "d", 1) != 0) {
> +				printf("You give wrong type\n");

"Incorrect type". It would also be helpful if the incorrect parameter
gets printed, so the user sees what he passed.

> +				return 1;
> +			}
> +			argc--;
> +		}
> +		/* E.g., create volume size */
> +		if (argc == 4) {
> +			err = parse_num(&size, argv[3]);
> +			if (err) {
> +				printf("You give correct size\n");

"Incorrect size". Please also print incorrect argument value.

> +	if (strncmp(argv[1], "write", 5) == 0) {
> +		if (argc < 5) {
> +			printf("You give wrong parameters\n");

Print usage message instead.

> +		}
> +
> +		addr = simple_strtoul(argv[2], NULL, 16);
> +		err = parse_num(&size, argv[4]);
> +		if (err) {
> +			printf("You give wrong size\n");

See above.

> +			if (err) {
> +				printf("You give wrong size\n");

Ditto.

> +	printf("Unknown UBI command or invalid number of arguments\n");

Print usage message instead.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A stone was placed at a ford in a river with the inscription:
"When this stone is covered it is dangerous to ford here."

[U-Boot] [PATCH] [UBI] UBI command support

[Kyungmin Park]

Dear Wolfgang,

On Tue, Oct 21, 2008 at 7:24 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Kyungmin Park,
>
> In message <20081021091859.GA29306@july> you wrote:
>> UBI command support
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ...
>> +++ b/common/cmd_ubi.c
>> @@ -0,0 +1,535 @@
>> +/*
>> + * Unsorted Block Image commands
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>
> GPL header missing.

Okay I added.

>
>> +/* board/\*\/ubi.c */
>
> What's the '\'s there?
>
>> +extern int ubi_board_scan(void);
>
> Please move the prototypes to some header file.
>
>> +/* drivers/mtd/ubi/build.c */
>> +extern struct ubi_device *ubi_devices[];

Moved to proper header files.

>> +
>> +/* Private own data */
>> +static struct ubi_device *ubi;
>> +static int ubi_initialized;
>> +
>> +static void ubi_dump_vol_info(const struct ubi_volume *vol)
>> +{
>> +     dbg_msg("volume information dump:");
>> +     dbg_msg("vol_id          %d", vol->vol_id);
>> +     dbg_msg("reserved_pebs   %d", vol->reserved_pebs);
>> +     dbg_msg("alignment       %d", vol->alignment);
>> +     dbg_msg("data_pad        %d", vol->data_pad);
>> +     dbg_msg("vol_type        %d", vol->vol_type);
>> +     dbg_msg("name_len        %d", vol->name_len);
>> +     dbg_msg("usable_leb_size %d", vol->usable_leb_size);
>> +     dbg_msg("used_ebs        %d", vol->used_ebs);
>> +     dbg_msg("used_bytes      %lld", vol->used_bytes);
>> +     dbg_msg("last_eb_bytes   %d", vol->last_eb_bytes);
>> +     dbg_msg("corrupted       %d", vol->corrupted);
>> +     dbg_msg("upd_marker      %d", vol->upd_marker);
>
> Please use the standard  debug()  macro.

It's not debug message, it's ubi msg. I fixed.

>
>> +static int ustrtoul(const char *cp, char **endp, unsigned int base)
>
> Maybe we should add this to some global place, like lib_generic ?

Move to lib_generic/vsprintf.c

>
>> +        unsigned long result = simple_strtoul(cp, endp, base);
>> +        switch (**endp) {
>> +        case 'G' :
>> +                result *= 1024;
>
> Please add a
>                /* fall through */
> comment to make clkear that it is intentional not to have a "break;"
> here.
>
>> +        case 'M':
>> +                result *= 1024;
>
> Ditto.

Did

>
>> +static int verify_mkvol_req(const struct ubi_device *ubi,
>> +                         const struct ubi_mkvol_req *req)
>> +{
> ...
>> +bad:
>> +     printf("bad volume creation request");
>> +//      ubi_dbg_dump_mkvol_req(req);
>> +     return err;
>
> No C++ comments, please. And please don;t add dead code either.

Yes I remove it

>
>> +     tbuf = vmalloc(tbuf_size);
>
> Why do we need new alloc() stuff?
>
>> +
>> +     vfree(tbuf);
>
> Ditto?

Use malloc & free.

>
>> +static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>> +{
>> +     size_t size = 0;
>> +     ulong addr = 0;
>> +     int err = 0;
>> +
>> +     if (!ubi_initialized) {
>> +             err = ubi_board_scan();
>> +             if (err) {
>> +                     printf("ubi initialize error %d\n", err);
>
> "UBI init error" -- maybe we can decode the error number into some
> string?
>
>> +             /* E.g., create volume size type */
>> +             if (argc == 5) {
>> +                     if (strncmp(argv[4], "s", 1) == 0)
>> +                             dynamic = 0;
>> +                     else if (strncmp(argv[4], "d", 1) != 0) {
>> +                             printf("You give wrong type\n");
>
> "Incorrect type". It would also be helpful if the incorrect parameter
> gets printed, so the user sees what he passed.
>
>> +                             return 1;
>> +                     }
>> +                     argc--;
>> +             }
>> +             /* E.g., create volume size */
>> +             if (argc == 4) {
>> +                     err = parse_num(&size, argv[3]);
>> +                     if (err) {
>> +                             printf("You give correct size\n");
>
> "Incorrect size". Please also print incorrect argument value.
>
>> +     if (strncmp(argv[1], "write", 5) == 0) {
>> +             if (argc < 5) {
>> +                     printf("You give wrong parameters\n");
>
> Print usage message instead.
>
>> +             }
>> +
>> +             addr = simple_strtoul(argv[2], NULL, 16);
>> +             err = parse_num(&size, argv[4]);
>> +             if (err) {
>> +                     printf("You give wrong size\n");
>
> See above.
>
>> +                     if (err) {
>> +                             printf("You give wrong size\n");
>
> Ditto.
>
>> +     printf("Unknown UBI command or invalid number of arguments\n");
>
> Print usage message instead.
>

How to display usage?

Thank you,
Kyungmin Park

[U-Boot] [PATCH] [UBI] UBI command support

[Wolfgang Denk]

Dear Kyungmin Park,

In message <9c9fda240810211756w189ae2a3rb26389ce61dbe772@mail.gmail.com> you wrote:
...
> >> +     printf("Unknown UBI command or invalid number of arguments\n");
> >
> > Print usage message instead.
> 
> How to display usage?

	printf ("Usage:\n%s\n", cmdtp->usage);

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Misquotation is, in fact, the pride and privilege of the  learned.  A
widely-read  man  never  quotes  accurately,  for  the rather obvious
reason that he has read too widely.
                - Hesketh Pearson _Common Misquotations_ introduction