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

[Pantelis Antoniou]

On Mar 12, 2014, at 1:12 PM, Lukasz Majewski wrote:

> Hi Pantelis, Tom,
> 
>> 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.
> 
> I think that, it would be better to ask Tom to pull it directly, since
> it is changing the code in the DFU not mmc.
> 

FWIW your patch applies cleanly even after the changes.

It will be part of the next mmc pull request, but I have no objection to Tom picking it up directly.

Regards

-- Pantelis

> 
>> 
>> 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
>> 
> 
> 
> 
> -- 
> Best regards,
> 
> Lukasz Majewski
> 
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group