[U-Boot] [PATCH] dfu: mmc: Replace calls to u-boot commands with native mmc API

[Pantelis Antoniou]

Hi Lukasz,

On Mar 12, 2014, at 10:06 AM, Lukasz Majewski wrote:

> Hi Tom, Pantelis
> 
>> Hi Lukasz,
>> 
>> On Feb 27, 2014, at 11:21 AM, Lukasz Majewski wrote:
>> 
>>> Hi Pantelis,
>>> 
>>>> Hi Lukasz,
>>>> 
>>>> 
>>>> On Feb 27, 2014, at 10:36 AM, Lukasz Majewski wrote:
>>>> 
>>>>> Hi Pantelis,
>>>>> 
>>>>>> Hi Lukasz,
>>>>>> 
>>>>>> Looks fine to me. Let me run a few tests over the weekend and
>>>>>> I'll apply.
>>>>> 
>>>>> Have the run test gone smoothly?
>>>>> 
>>>> 
>>>> In the middle of reworking some core stuff in mmc, will test when
>>>> that's done.
>>> 
>>> Does your rework intend to change the MMC API?
>>> 
>>> I'm regarding this patch as one suitable for v2014.04, since it
>>> replaces cumbersome sprintf() approach with the native mmc API.
>>> 
>>> Those operations are done at dfu_mmc.c, not any core mmc file.
>>> 
>> 
>> Your point is valid; I'll try to give it a spin today.
> 
> Tom, Pantelis would it be possible to add this patch to the v2014.04?
> This patch was initially posted some time ago.
> 
> http://patchwork.ozlabs.org/patch/322450/
> 
> Replacing at DFU core the sprintf() with call to native eMMC API seems
> appealing :-).
> 

Yes, it is :)

I'm going to include it; just busy with some mmc reorg right now.

Regards

-- Pantelis

>> 
>>>> 
>>>> Regards
>>>> 
>>>> -- Pantelis
>>>> 
>>>>>> 
>>>>>> Thanks
>>>>>> 
>>>>>> -- Pantelis
>>>>>> 
>>>>>> On Feb 21, 2014, at 9:23 AM, Lukasz Majewski wrote:
>>>>>> 
>>>>>>> For some time we have been using the run_command() with properly
>>>>>>> crafted string. Such approach turned to be unreliable and error
>>>>>>> prone.
>>>>>>> 
>>>>>>> Switch to "native" mmc subsystem API would allow better type
>>>>>>> checking and shall improve speed.
>>>>>>> 
>>>>>>> Also, it seems that this API is changing less often than u-boot
>>>>>>> commands. The approach similar to env operations on the eMMC has
>>>>>>> been reused.
>>>>>>> 
>>>>>>> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
>>>>>>> ---
>>>>>>> drivers/dfu/dfu_mmc.c |   31 ++++++++++++++++++++++++-------
>>>>>>> 1 file changed, 24 insertions(+), 7 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
>>>>>>> index 0816f46..651cfff 100644
>>>>>>> --- a/drivers/dfu/dfu_mmc.c
>>>>>>> +++ b/drivers/dfu/dfu_mmc.c
>>>>>>> @@ -12,6 +12,7 @@
>>>>>>> #include <errno.h>
>>>>>>> #include <div64.h>
>>>>>>> #include <dfu.h>
>>>>>>> +#include <mmc.h>
>>>>>>> 
>>>>>>> static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE)
>>>>>>> 				dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE];
>>>>>>> @@ -20,8 +21,8 @@ static long dfu_file_buf_len;
>>>>>>> static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu,
>>>>>>> 			u64 offset, void *buf, long *len)
>>>>>>> {
>>>>>>> -	char cmd_buf[DFU_CMD_BUF_SIZE];
>>>>>>> -	u32 blk_start, blk_count;
>>>>>>> +	struct mmc *mmc = find_mmc_device(dfu->dev_num);
>>>>>>> +	u32 blk_start, blk_count, n = 0;
>>>>>>> 
>>>>>>> 	/*
>>>>>>> 	 * We must ensure that we work in lba_blk_size chunks,
>>>>>>> so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum
>>>>>>> dfu_op op, struct dfu_entity *dfu, return -EINVAL;
>>>>>>> 	}
>>>>>>> 
>>>>>>> -	sprintf(cmd_buf, "mmc %s %p %x %x",
>>>>>>> -		op == DFU_OP_READ ? "read" : "write",
>>>>>>> -		 buf, blk_start, blk_count);
>>>>>>> +	debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n",
>>>>>>> __func__,
>>>>>>> +	      op == DFU_OP_READ ? "MMC READ" : "MMC WRITE",
>>>>>>> dfu->dev_num,
>>>>>>> +	      blk_start, blk_count, buf);
>>>>>>> +	switch (op) {
>>>>>>> +	case DFU_OP_READ:
>>>>>>> +		n = mmc->block_dev.block_read(dfu->dev_num,
>>>>>>> blk_start,
>>>>>>> +					      blk_count, buf);
>>>>>>> +		break;
>>>>>>> +	case DFU_OP_WRITE:
>>>>>>> +		n = mmc->block_dev.block_write(dfu->dev_num,
>>>>>>> blk_start,
>>>>>>> +					       blk_count, buf);
>>>>>>> +		break;
>>>>>>> +	default:
>>>>>>> +		error("Operation not supported\n");
>>>>>>> +	}
>>>>>>> 
>>>>>>> -	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
>>>>>>> -	return run_command(cmd_buf, 0);
>>>>>>> +	if (n != blk_count) {
>>>>>>> +		error("MMC operation failed");
>>>>>>> +		return -EIO;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	return 0;
>>>>>>> }
>>>>>>> 
>>>>>>> static int mmc_file_buffer(struct dfu_entity *dfu, void *buf,
>>>>>>> long *len) -- 
>>>>>>> 1.7.10.4
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> -- 
>>>>> Best regards,
>>>>> 
>>>>> Lukasz Majewski
>>>>> 
>>>>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>>>> 
>>> 
>>> 
>>> 
>>> -- 
>>> Best regards,
>>> 
>>> Lukasz Majewski
>>> 
>>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>> 
> 
> -- 
> Best regards,
> 
> Lukasz Majewski
> 
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group