[U-Boot] [PATCH] dfu:mmc: When doing block operations, operate on the given length

[Michael Cashwell]

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 <trini at 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.majewski at samsung.com>
> Signed-off-by: Tom Rini <trini at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot