[U-Boot] [PATCH] spi: sf: add support for throughput mesurement of sf read/write

[Jagan Teki]

Hi Mugunthan,

On 21 December 2015 at 11:30, Stefan Roese <sr at denx.de> wrote:
> On 21.12.2015 06:37, Mugunthan V N wrote:
>>
>> On Friday 18 December 2015 11:37 AM, Stefan Roese wrote:
>>>
>>> On 17.12.2015 17:44, Jagan Teki wrote:
>>>>
>>>> On 17 December 2015 at 13:26, Mugunthan V N <mugunthanvnm at ti.com> wrote:
>>>>>
>>>>> On Thursday 17 December 2015 12:43 PM, Jagan Teki wrote:
>>>>>>
>>>>>> On 17 December 2015 at 12:33, Mugunthan V N <mugunthanvnm at ti.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Jagan
>>>>>>>
>>>>>>> On Tuesday 27 October 2015 07:24 PM, Mugunthan V N wrote:
>>>>>>>>
>>>>>>>> This patch adds time measurement and throughput calculation for
>>>>>>>> sf read/write commands.
>>>>>>>>
>>>>>>>> The output of sf read changes from
>>>>>>>>
>>>>>>>> ---8<---
>>>>>>>> SF: 4096 bytes @ 0x0 Read: OK
>>>>>>>> --->8---
>>>>>>>>
>>>>>>>> to
>>>>>>>>
>>>>>>>> ---8<---
>>>>>>>> SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
>>>>>>>> --->8---
>>>>>>>>
>>>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm at ti.com>
>>>>>>
>>>>>>
>>>>>> Was it similar to 'sf update' ? please check it once.
>>>>>>
>>>>>
>>>>> sf update out similar but also uses progressive output, in read/write
>>>>> case it can't be done. The final throughput measurement is similar on
>>>>> both update and read/write.
>>>>
>>>>
>>>> True, that's what if we need a progressed throughput just use 'sf
>>>> update' else normal 'sf read/write' It's look not good to me to add
>>>> extra code on top of generic commands. What ever we wanted to extend
>>>> features let's added it on 'sf update' than sf read/write, Sorry.
>>>
>>>
>>> If I need to measure the time of commands, I use the "time"
>>> command ("time sf write ...") by enabling it via CONFIG_CMD_TIME.
>>> This provides all the needed information to detect performance
>>> changes.
>>>
>>
>> But similar kind of implementations is present for fatload and tftp. So
>> I thought having similar performance log for sf read/write will be good
>> as well.
>
>
> Yes, I understand. I also find this output helpful. But I'm not sure,
> if we should add this timing code to all these functions. Instead
> of just using this common time command, if needed. To keep the
> code size at a minimum.
>
> Of course its no big code addition, so I don't really have any bigger
> reservations against it.

Why can't just use 'sf update' instead of adding again same code in
'sf read/write' since 'sf update' is doing more than just giving
timing performance. Please try!

thanks!
-- 
Jagan.