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

[Jagan Teki]

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.

Please comment?

-- 
Thanks,
Jagan.