Hi, On Thu, Jan 10, 2013 at 6:33 AM, Langer Thomas (LQDE RD ST PON SW) <[email protected]> wrote: > Hello Jagan, > >> All these patches are added a support for read and write instruction >> for programming/reading SPI flash.
Hmmm just saw this comment. > > 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. I suspect the SPI device will need to provide some flags to indicate that it supports it, and then we will probably need some new SPI_... flags in spi.h to select the required behaviour? > > 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! Well maybe we could deal with that later? I think it would be good to get some basic framework in there, and people can then adapt for their chips, etc. > >> >> 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. I'm sure some boards can use it. I'm interested in knowing which board this was tested on. I sent a few patches to make it easier to extend SPI and SPI flash interfaces without breaking old drivers. http://patchwork.ozlabs.org/patch/208226/ http://patchwork.ozlabs.org/patch/208228/ Regards, Simon > > Best Regards, > Thomas > > _______________________________________________ > U-Boot mailing list > [email protected] > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
