[U-Boot] [PATCH] dfu: fix some issues with reads/uploads

Stephen Warren
2014-05-19T23:59:40Z

From: Stephen Warren <swarren@nvidia.com>

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>
---
drivers/dfu/dfu.c | 9 ++-------
drivers/dfu/dfu_mmc.c | 6 ++++++
drivers/dfu/dfu_nand.c | 7 ++++++-
drivers/dfu/dfu_ram.c | 11 ++++++-----
include/dfu.h | 2 ++
5 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index a93810934ac9..af97234390c7 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -241,7 +241,6 @@ static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size)
dfu->crc = crc32(dfu->crc, buf, chunk);
dfu->i_buf += chunk;
dfu->b_left -= chunk;
- dfu->r_left -= chunk;
size -= chunk;
buf += chunk;
readn += chunk;
@@ -287,11 +286,7 @@ 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);

debug("%s: %s %ld [B]\n", __func__, dfu->name, dfu->r_left);

@@ -300,7 +295,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 63cc876612c9..aa077c052f39 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -196,6 +196,11 @@ int dfu_flush_medium_mmc(struct dfu_entity *dfu)
return ret;
}

+long dfu_get_medium_size_mmc(struct dfu_entity *dfu)
+{
+ return dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size;
+}
+
int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf,
long *len)
{
@@ -316,6 +321,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 ccdbef6b75f2..e1c9a8849246 100644
--- 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 335a8e1f2491..b6c6e60c443c 100644
--- 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 26ffbc8e81d2..0cca601d47af 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -96,6 +96,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);

--
1.8.1.5