Hi Stephen,
> From: Stephen Warren <swar...@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 <swar...@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)
Those could be defined as static
> +{
> + 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)
Those could be defined as static
> +{
> + 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)
Those could be defined as static
> +{
> + 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);
>
I've tested it with trats2. It introduces regression with my tests
setup.
I think that it is the highest time to share my test setup for DFU.
Proper patches would be prepared ASAP.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
