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

[Magnus Lilja]

Dear Kyungmin Park,

Some more comments.

2008/10/28 Kyungmin Park <kmpark at infradead.org>:
> It supports basic operation such as create, remove, read, and write.
>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---
> diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
> new file mode 100644
> index 0000000..8c46cca
> --- /dev/null
> +++ b/common/cmd_ubi.c
> +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 = malloc(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]);

The printf()'s before and after memcpy() looks like stuff used during
development of this file and they should be removed.

> +
> +               buf += len;
> +               len = size > tbuf_size ? tbuf_size : size;
> +       } while (size);
> +
> +       free(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 init 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("Incorrect type\n");
> +                               return 1;
> +                       }
> +                       argc--;
> +               }
> +               /* E.g., create volume size */
> +               if (argc == 4) {
> +                       err = parse_num(&size, argv[3]);
> +                       if (err) {
> +                               printf("Incorrect type\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("Please see usage\n");
> +                       return 1;
> +               }
> +
> +               addr = simple_strtoul(argv[2], NULL, 16);
> +               err = parse_num(&size, argv[4]);
> +               if (err) {
> +                       printf("Please see usage\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("Please see usage\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);

It seems to me that ubi_volume_read() can return 0 when there is an
error, so it may not be correct to use that return value here. AFAIK
do_ubi()-like functions return 0 upon success and non-zero otherwise.


Regards, Magnus