Re: [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
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
Hi,
On Thu, Jan 10, 2013 at 6:33 AM, Langer Thomas (LQDE RD ST PON SW) thomas.langer@lantiq.com 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 U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Thomas,
On Thu, Jan 10, 2013 at 8:03 PM, Langer Thomas (LQDE RD ST PON SW) thomas.langer@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=...
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
Hello Jagan,
Jagan Teki wrote on 2013-01-16:
Hi Thomas,
On Thu, Jan 10, 2013 at 8:03 PM, Langer Thomas (LQDE RD ST PON SW) thomas.langer@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.
You are right, the MTD/SF layer does not know this. But in your board config you are able to set a compile option to enable this feature.
It is an important rule for U-Boot, that new code must be added in a way that the code size is not increased as long as the feature is disabled!
See here (Note 4): http://www.denx.de/wiki/view/U-Boot/Patches#Notes In the current form, your patches increase the code size for all boards with serial flash!
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=642241d0fc25d04c06ecbeb5d01ba 0 6108307c62;hb=master
I don't think, this is a good idea! You are mixing the SPI driver with details of the flashes! What happens, if you connect something, which is not a flash, to this SPI controller? And then try to send data, which is detected as "qpp" or any other quad/dual io instruction? I assume, this will not work!
I would expect the SPI as transparent interface. The SPI driver should provide an interface to specify the use of 1/2/4 signals for a transfer or part of it. And the flash driver then should use this extended interface to specify the required parts, as it is his job!
For example, with your driver, it would be difficult to support the Macronix MX25L12835F, which allows to enable their "QPI" mode, which then expects some command, with the same opcode as in "None-QPI" mode, have to be send in parallel. And Macronix is also known to use different opcodes than others for some instructions. Does this mean, for supporting this flash, you would have to extend your SPI driver first?
I don't expect something like this to be accepted, either in u-boot nor in the kernel. Your driver has some complex handling inside for things, which are only valid for serial flashes! Please don't do this!
Please define and use a clear interface, where the flash driver can tell the spi driver, what kind of transfer should be used (1/2/4) for which phase (command/address/data). In u-boot, I would use the "flags" argument of the "spi_xfer" function for this.
I would assume, you will get more flexibility, as your spi driver will get smaller and less complex and it does not need to be changed, if some other flash type should be supported. This will then be part of the specific flash driver! (you see??)
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.
I expect, that these flashes either have a specific ID and some flags can be added to the tables, which are used to identify them already. Or some newer flashes might support the JEDEC Standard JESD216: "Serial Flash Discoverable Parameters (SFDP)"?
For the quad-io mode, the corresponding bit from the configuration register should be used!
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.
But it needs a spi driver with specific adaption to these commands, instead of a generic interface, which can be adapted for other spi drivers also.
Thanks, Jagan.
Best Regards, Thomas
participants (3)
-
Jagan Teki -
Langer Thomas (LQDE RD ST PON SW) -
Simon Glass