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

[Langer Thomas (LQDE RD ST PON SW)]

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.

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! 

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?

> 
> 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!

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.

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