[U-Boot] [PATCH v7 01/17] sf: Add extended read commands support

[Jagan Teki]

Hi,

On Mon, Jan 13, 2014 at 4:26 PM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
> Hi Gerhard Sittig,
>
> Thanks for your comments, please see below
>
> On Mon, Jan 13, 2014 at 3:04 PM, Gerhard Sittig <gsi at denx.de> wrote:
>> On Sun, Jan 12, 2014 at 22:29 +0530, Jagannadha Sutradharudu Teki wrote:
>>>
>>> Current sf uses FAST_READ command, this patch adds support to
>>> use the different/extended read command.
>>>
>>> This implementation will determine the fastest command by taking
>>> the supported commands from the flash and the controller, controller
>>> is always been a priority.
>>
>> I don't quite get this.  You have a combination of several
>> components which all have to participate appropriately when you
>> want the total of it to work correctly.
>>
>> How can one specific component "be a priority" there?  Isn't the
>> lowest common denominator required instead?  The strength of a
>> chain is determined by its weakest link after all.
>>
>>
>> The other issue I have here (as well as in Linux kernel changes
>> and discussions) is that people seem to forget that the board is
>> involved as well.
>>
>> Your commit message suggests that "parallel" communication is
>> used as soon as
>> - the SPI controller has such a feature
>> - a flash chip was detected which according to a fixed database
>>   supports this feature
>>
>> This decision is automatic.  Neither is there a "how is the
>> board wired?" condition involved, nor can users or even
>> developers adjust this behaviour (say, by optionally setting a
>> maximum width).
>>
>> I'd suggest to provide a "maximum number of lines to use" option,
>> for the same reasons that we may want to limit transfer rates
>> instead of always using the maximum of what individual chips
>> could do (the reasons: the number of lines connected as well as
>> their electrical and mechanical properties do have an impact).
>>
>> Alternatively one may implement auto-detection, starting at the
>> most capable feature and falling back until a working condition
>> was found.  But this is fragile if the communication used at
>> detection time does not involve all potentially used lines (and
>> in all directions, mind you).  This won't solve potential bugs in
>> chips either which the user may want to override.  Or EMI related
>> restrictions which a developer may want to apply despite the
>> chips' and board's being more capable.  Or pin mux settings which
>> dedicate unused "upper SPI data lines" to other features (as we
>> have seen with MMC cards recently).
>
> Yes the main reason for going maximum or based on controller setting
> transfer is to improve sf performance. I missed your point as didn't
> see much use cases from board to connected flash perceptive.
>
> I thought we have two options to meet this requirement.
> 1. dynamic - auto-detection:
>     Get the max_nof_lines based on the board to connected flash.
>     So-that we can configure the transfer mode based on the lines.
>     no idea yet, how to get the max_nof_lines dynamically - any inputs.
> 2. static: from user level we may give the max_nof_lines
>     ex: sf probe probe [[[bus:]cs]:max_nof_line] [hz] [mode]
>     If user can't input max_nof_line so-that driver will go with single and
>     the rest case transfer modes assigned based on the given input.

As this change involves mostly on the driver parts I a thinking to go-ahead and
try to push this series to master. yes I would mark this as my TODO
list and will
trigger one more thread.

Hope this idea sounds valid, let me know for any issues.

-- 
Thanks,
Jagan.