[U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions

[Jagan Teki]

Hi Thomas,

On Thu, Jan 10, 2013 at 8:03 PM, Langer Thomas (LQDE RD ST PON SW)
<thomas.langer at lantiq.com> wrote:
> Hello Jagan,
>
>> All these patches are added a support for read and write instruction
>> for programming/reading SPI flash.
>
> I have written some weeks ago that I would really appreciate the support of
> dual and quad IO accesses for serial flashes. I just  think, this is not an acceptable
> way to do this.
>

First of all thank you very much for your comments.

> It is important to know for all, who do not know the details on the hardware
> requirements of this feature, that these new transfers require a special SPI
> controller! A standard SPI hardware, which is being used on most (maybe all)
> existing boards, cannot use the dual or quad IO features at all!

I am some how unclear about your  concern.

Basically I have added this code that is purely specific to mtd flash
layer, means
the entire code is alters the functionalists of flash parts(stmicro,
spansion, winbond..etc).

whether the respective SOC SPI/QSPI controller driver can support
these transfer or not, may be
we couldn't take care at this point of time.

Please comment if I am wrong with my analyses.

>
> And in addition, I still haven't seen any change, which indicates the required
> transfer mode to the SPI layer! How do you tell your SPI driver, which part
> of the transfer should be done in dual or quad mode?

I don't think so, but the SPI/QSPI controller  driver should aware
respective read/write/erase instructions
transfer changes those are trying to pass from mtd flash. .

Suppose if the flash used QUAD PP , then the respective h/w SPI/QSPI
controller must
have a support to do the QUAD PP transfers (by adding QUAD_PP command transfer).

I think this job is specific to respective h/w driver, they need to
add these transfers support.
currently I have used a zynq qspi controller for testing these
instruction by adding respective
instructions are this driver.

http://git.xilinx.com/?p=u-boot-xlnx.git;a=blob;f=drivers/spi/zynq_qspi.c;h=642241d0fc25d04c06ecbeb5d01ba06108307c62;hb=master

>
>>
>> Read and Write instruction are implemented as a command line
>> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>>
>> Currently I have added below instructions those are commonly available
>> on all flash types.
>> pp - Page Program (existing one)
>> qpp - Quad-input Page Program
>> afr - Array Fast Read (existing one)
>> asr - Array Slow Read
>> dofr - Dual Output Fast Read
>> qofr - Quad Output Fast Read
>> diofr - Dual IO Fast Read
>> qiofr - Quad IO Fast Read
>>
>> I have tested mostly of the instruction on real h/w.
>>
>> This entire implementation will change the current sf framework little bit but
>> I thought these changes are worth to add.
>
> This means, all your patches adding new code, which has no benefit for existing
> boards, but changing the commands in an incompatible way, which forces all users
> to adapt their definitions!
> I don't think this will be accepted, as I would not.
>
> The biggest question is: Do somebody really need the flexibility to select the
> used instruction at this level?
> If you have a platform, which contains an extended SPI controller and has also a
> supported flash, I expect it would be okay to always use the dual or quad instructions!
> And this can be completely handled by the lower level functions, no need to expose
> this to the command line!

Yes, basically if some don't want/they don't support the respective
instruction at their h/w or
QSPI/SPI controller level certainly they couldn't use.
instead they may use the respective supported instructions at command
level, I suspect this could be
a selective features from users point of view.

>
> I think, first of all, you should add some config options (e.g. CONFIG_SYS_SF_DUAL /
> CONFIG_SYS_SF_QUAD) to indicate, that the platform / board will support these.
> Then, during "sf probe", there should be a detection, if the flash will support this
> (depending on the type) and is enabled (checking the config bit for quad IO).
> Depending on this, either specific functions for read/write could be assigned or
> some other data, which indicate the usable instructions, can be set.

May be your taking about run-time detection of specific instruction
supported by respective flash.
Actually i don't have any idea how to detect whether the flash
supports  pp, qpp, fr, sr, dor, qor?

Please pass any information if there is a flexible way to detect this feature.

But currently these(pp, qpp, afr, asr, dor, qor, dior, qior)
instructions are commonly supported once from
spansion, numonyx and winbond flashes.this is the reason for me to
give these instructions support at this point of time.

Thanks,
Jagan.

>
> As it depends on the flash type and manufacturer, how the lower sequence must
> look like (for the command/address/data phases, it might be 1-1-4, 1-4-4 or even 4-4-4),
> this should be flexible to be assigned from the detection code.
> For example, please check the differences between Spansion and Macronix!
>
>>
>> Request for all your comment, so-that I can move forward.
>> Please let me know for any issue regarding this new implementation.
>>
>> Thanks,
>> Jagan.
>>
>
> Please think about my comments. I just want to avoid to bloat the code with something,
> which is currently not usable for existing boards (as far as I know) and not flexible enough
> to support different, manufacturer specific command sets.
>
> Best Regards,
> Thomas
>