dfu: fix some issues with reads/uploads

DFU read support appears to rely upon dfu->read_medium() updating the
passed-by-reference len parameter to indicate the remaining size
available for reading.

dfu_read_medium_mmc() never does this, and the implementation of
dfu_read_medium_nand() will only work if called just once; it hard-codes
the value to the total size of the NAND device irrespective of read
offset.

I believe that overloading dfu->read_medium() is confusing. As such,
this patch introduces a new function dfu->get_medium_size() which can
be used to explicitly find out the medium size, and nothing else.
dfu_read() is modified to use this function to set the initial value for
dfu->r_left, rather than attempting to use the side-effects of
dfu->read_medium() for this purpose.

Due to this change, dfu_read() must initially set dfu->b_left to 0, since
no data has been read.

dfu_read_buffer_fill() must also be modified not to adjust dfu->r_left
when simply copying data from dfu->i_buf_start to the upload request
buffer. r_left represents the amount of data left to be read from HW.
That value is not affected by the memcpy(), but only by calls to
dfu->read_medium().

After this change, I can read from either a 4MB or 1.5MB chunk of a 4MB
eMMC boot partion with CONFIG_SYS_DFU_DATA_BUF_SIZE==1MB. Without this
change, attempting to do that would result in DFU read returning no data
at all due to r_left never being set.

Signed-off-by: Stephen Warren <swarren@nvidia.com>

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index dc09ff6466e6f284bbc4c4764097fa612406d8a2..dab5e7048ed53adc3b4c8fd4246ecd9f6f90b0d2 100644 (file)
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -267,7 +267,6 @@ static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size)
 
                        dfu->i_buf += chunk;
                        dfu->b_left -= chunk;
-                       dfu->r_left -= chunk;
                        size -= chunk;
                        buf += chunk;
                        readn += chunk;
@@ -313,10 +312,19 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
                if (dfu->i_buf_start == NULL)
                        return -ENOMEM;
 
-               ret = dfu->read_medium(dfu, 0, dfu->i_buf_start, &dfu->r_left);
-               if (ret != 0) {
-                       debug("%s: failed to get r_left\n", __func__);
-                       return ret;
+               dfu->r_left = dfu->get_medium_size(dfu);
+               if (dfu->r_left < 0)
+                       return dfu->r_left;
+               switch (dfu->layout) {
+               case DFU_RAW_ADDR:
+               case DFU_RAM_ADDR:
+                       break;
+               default:
+                       if (dfu->r_left >= dfu_buf_size) {
+                               printf("%s: File too big for buffer\n",
+                                      __func__);
+                               return -EOVERFLOW;
+                       }
                }
 
                debug("%s: %s %ld [B]\n", __func__, dfu->name, dfu->r_left);
@@ -326,7 +334,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
                dfu->offset = 0;
                dfu->i_buf_end = dfu_get_buf() + dfu_buf_size;
                dfu->i_buf = dfu->i_buf_start;
-               dfu->b_left = min(dfu_buf_size, dfu->r_left);
+               dfu->b_left = 0;
 
                dfu->bad_skip = 0;
 

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 63cc876612c9aa44df4539e5e6c1a3b1d0af0b14..322bd8c5d2de2e0e0797665ca49f1a7fa5aea052 100644 (file)
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -12,6 +12,8 @@
 #include <errno.h>
 #include <div64.h>
 #include <dfu.h>
+#include <ext4fs.h>
+#include <fat.h>
 #include <mmc.h>
 
 static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
@@ -113,22 +115,17 @@ static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len)
 static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
                        void *buf, long *len)
 {
+       const char *fsname, *opname;
        char cmd_buf[DFU_CMD_BUF_SIZE];
        char *str_env;
        int ret;
 
        switch (dfu->layout) {
        case DFU_FS_FAT:
-               sprintf(cmd_buf, "fat%s mmc %d:%d 0x%x %s",
-                       op == DFU_OP_READ ? "load" : "write",
-                       dfu->data.mmc.dev, dfu->data.mmc.part,
-                       (unsigned int) buf, dfu->name);
+               fsname = "fat";
                break;
        case DFU_FS_EXT4:
-               sprintf(cmd_buf, "ext4%s mmc %d:%d 0x%x /%s",
-                       op == DFU_OP_READ ? "load" : "write",
-                       dfu->data.mmc.dev, dfu->data.mmc.part,
-                       (unsigned int) buf, dfu->name);
+               fsname = "ext4";
                break;
        default:
                printf("%s: Layout (%s) not (yet) supported!\n", __func__,
@@ -136,6 +133,28 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
                return -1;
        }
 
+       switch (op) {
+       case DFU_OP_READ:
+               opname = "load";
+               break;
+       case DFU_OP_WRITE:
+               opname = "write";
+               break;
+       case DFU_OP_SIZE:
+               opname = "size";
+               break;
+       default:
+               return -1;
+       }
+
+       sprintf(cmd_buf, "%s%s mmc %d:%d", fsname, opname,
+               dfu->data.mmc.dev, dfu->data.mmc.part);
+
+       if (op != DFU_OP_SIZE)
+               sprintf(cmd_buf + strlen(cmd_buf), " 0x%x", (unsigned int)buf);
+
+       sprintf(cmd_buf + strlen(cmd_buf), " %s", dfu->name);
+
        if (op == DFU_OP_WRITE)
                sprintf(cmd_buf + strlen(cmd_buf), " %lx", *len);
 
@@ -147,7 +166,7 @@ static int mmc_file_op(enum dfu_op op, struct dfu_entity *dfu,
                return ret;
        }
 
-       if (dfu->layout != DFU_RAW_ADDR && op == DFU_OP_READ) {
+       if (op != DFU_OP_WRITE) {
                str_env = getenv("filesize");
                if (str_env == NULL) {
                        puts("dfu: Wrong file size!\n");
@@ -196,6 +215,27 @@ int dfu_flush_medium_mmc(struct dfu_entity *dfu)
        return ret;
 }
 
+long dfu_get_medium_size_mmc(struct dfu_entity *dfu)
+{
+       int ret;
+       long len;
+
+       switch (dfu->layout) {
+       case DFU_RAW_ADDR:
+               return dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size;
+       case DFU_FS_FAT:
+       case DFU_FS_EXT4:
+               ret = mmc_file_op(DFU_OP_SIZE, dfu, NULL, &len);
+               if (ret < 0)
+                       return ret;
+               return len;
+       default:
+               printf("%s: Layout (%s) not (yet) supported!\n", __func__,
+                      dfu_get_layout(dfu->layout));
+               return -1;
+       }
+}
+
 int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
                long *len)
 {
@@ -316,6 +356,7 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
        }
 
        dfu->dev_type = DFU_DEV_MMC;
+       dfu->get_medium_size = dfu_get_medium_size_mmc;
        dfu->read_medium = dfu_read_medium_mmc;
        dfu->write_medium = dfu_write_medium_mmc;
        dfu->flush_medium = dfu_flush_medium_mmc;

diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
index ccdbef6b75f26fe78fe5fc52438d797bad5b3b46..e1c9a88492468f7b50fe29c09b2337ed0c27641d 100644 (file)
--- a/drivers/dfu/dfu_nand.c
+++ b/drivers/dfu/dfu_nand.c
@@ -114,6 +114,11 @@ static int dfu_write_medium_nand(struct dfu_entity *dfu,
        return ret;
 }
 
+long dfu_get_medium_size_nand(struct dfu_entity *dfu)
+{
+       return dfu->data.nand.size;
+}
+
 static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset, void *buf,
                long *len)
 {
@@ -121,7 +126,6 @@ static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset, void *buf,
 
        switch (dfu->layout) {
        case DFU_RAW_ADDR:
-               *len = dfu->data.nand.size;
                ret = nand_block_read(dfu, offset, buf, len);
                break;
        default:
@@ -220,6 +224,7 @@ int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
                return -1;
        }
 
+       dfu->get_medium_size = dfu_get_medium_size_nand;
        dfu->read_medium = dfu_read_medium_nand;
        dfu->write_medium = dfu_write_medium_nand;
        dfu->flush_medium = dfu_flush_medium_nand;

diff --git a/drivers/dfu/dfu_ram.c b/drivers/dfu/dfu_ram.c
index 335a8e1f2491cd9dc477885f83a76677a1725407..b6c6e60c443c279ca150e8b23e0a222a6fb754a1 100644 (file)
--- a/drivers/dfu/dfu_ram.c
+++ b/drivers/dfu/dfu_ram.c
@@ -41,14 +41,14 @@ static int dfu_write_medium_ram(struct dfu_entity *dfu, u64 offset,
        return dfu_transfer_medium_ram(DFU_OP_WRITE, dfu, offset, buf, len);
 }
 
+long dfu_get_medium_size_ram(struct dfu_entity *dfu)
+{
+       return dfu->data.ram.size;
+}
+
 static int dfu_read_medium_ram(struct dfu_entity *dfu, u64 offset,
                               void *buf, long *len)
 {
-       if (!*len) {
-               *len = dfu->data.ram.size;
-               return 0;
-       }
-
        return dfu_transfer_medium_ram(DFU_OP_READ, dfu, offset, buf, len);
 }
 
@@ -69,6 +69,7 @@ int dfu_fill_entity_ram(struct dfu_entity *dfu, char *s)
        dfu->data.ram.size = simple_strtoul(s, &s, 16);
 
        dfu->write_medium = dfu_write_medium_ram;
+       dfu->get_medium_size = dfu_get_medium_size_ram;
        dfu->read_medium = dfu_read_medium_ram;
 
        dfu->inited = 0;

diff --git a/include/dfu.h b/include/dfu.h
index 26ffbc8e81d2d4ac82620b179a84f5077cf01b7b..df720310f2cc86ec21a1cf7148db0d5dfe840def 100644 (file)
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -35,6 +35,7 @@ enum dfu_layout {
 enum dfu_op {
        DFU_OP_READ = 1,
        DFU_OP_WRITE,
+       DFU_OP_SIZE,
 };
 
 struct mmc_internal_data {
@@ -96,6 +97,8 @@ struct dfu_entity {
                struct ram_internal_data ram;
        } data;
 
+       long (*get_medium_size)(struct dfu_entity *dfu);
+
        int (*read_medium)(struct dfu_entity *dfu,
                        u64 offset, void *buf, long *len);