I was just bitten in this area today but in a different way.

Larger than DDR? Any update larger than the default 4MB dfu_buf[] fails too. 
(As a hack I redefined dfu_buf[] to be a cast of I think 
CONFIG_SYS_SDRAM_BASE.) But I'd love to hear more about being able to handle 
updates larger than DDR.

But on the code below, (both before and after the patch) the amount written is 
the size of the mmc partition. Why write more data than was received from the 
host? Why isn't the incoming len value used?

Lastly, I encountered a zero dfu->data.mmc.lba_blk_size. This gets initialized 
in the mmc_init() path from the card meta data. But if you just do "dfu mmc 0" 
right after booting that won't have been done. The MMC controller is ready but 
the card structures have not been read.

I have a hack there to do do the a "mmc 0 rescan" command subordinate to the 
dfu command but that feels gross. There has to be a better way.

Thoughts? Have you not seen dfu->data.mmc.lba_blk_size be zero?

-Mike

On Mar 7, 2013, at 9:25 AM, Tom Rini <tr...@ti.com> wrote:

> When working on RAW partitions, it's possible that the whole area
> is larger than DDR.  So what we need to do is make sure that the length
> we are given is aligned with the LBA block size, then pass that length
> in as our count of LBA blocks to operate on.  In doing this, we no
> longer need to modify *len on read operations.
> 
> Cc: Lukasz Majewski <l.majew...@samsung.com>
> Signed-off-by: Tom Rini <tr...@ti.com>
> ---
> drivers/dfu/dfu_mmc.c |   21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 083d745..0bed405 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -34,14 +34,21 @@ static int mmc_block_op(enum dfu_mmc_op op, struct 
> dfu_entity *dfu,
> {
>       char cmd_buf[DFU_CMD_BUF_SIZE];
> 
> -     sprintf(cmd_buf, "mmc %s 0x%x %x %x",
> -             op == DFU_OP_READ ? "read" : "write",
> -             (unsigned int) buf,
> -             dfu->data.mmc.lba_start,
> -             dfu->data.mmc.lba_size);
> +     /*
> +      * We must ensure that we read in lba_blk_size chunks, so ALIGN
> +      * this value.
> +      */
> +     *len = ALIGN(*len, dfu->data.mmc.lba_blk_size);
> +
> +     if (*len > (dfu->data.mmc.lba_size * dfu->data.mmc.lba_blk_size)) {
> +             puts("Request would exceed designated area!\n");
> +             return -EINVAL;
> +     }
> 
> -     if (op == DFU_OP_READ)
> -             *len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size;
> +     sprintf(cmd_buf, "mmc %s 0x%x %x %lx",
> +             op == DFU_OP_READ ? "read" : "write",
> +             (unsigned int) buf, dfu->data.mmc.lba_start,
> +             *len / dfu->data.mmc.lba_blk_size);
> 
>       debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
>       return run_command(cmd_buf, 0);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to