[U-Boot] [PATCH v4 00/36] sf: Add common probe and extended/quad read/write cmds support

[Jagan Teki]

Thanks for your comments, see below

On Wed, Sep 25, 2013 at 1:02 AM,  <thomas.langer at lantiq.com> wrote:
> Hello Jagan,
>
> Am 24.09.2013 20:38, schrieb Jagannadha Sutradharudu Teki:
>> This patch series is a combination of
>> "sf: Add common probe support"
>> "sf: Add support for extended/quad read and write commands"
>> http://www.mail-archive.com/u-boot@lists.denx.de/msg121668.html
>> http://u-boot.10912.n7.nabble.com/PATCH-v2-00-10-sf-Add-support-for-extended-quad-read-and-write-commands-td160964.html
>>
>> This patch series adds common probe support for all flash vendors except ramtron.
>>
>> spi_flash_probe is a new addition where all flash driver
>> probing is combined into a common file, this means spi_flash_probe.c
>> adds a new probing style common to all flashes.
>>
>>
>> Apart from common probing, this series also adds support for
>> extended read commands and quad read/write commands.
>>
>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/150148
>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/167026
>>
>> There is a bit discussion going on for supporting this:
>> http://u-boot.10912.n7.nabble.com/PATCH-00-12-cmd-sf-Add-support-for-read-and-write-instructions-td143749.html
>>
>> Concept:
>> Initially I tried to add individual sf write/read commands to respective
>> flash read/write commands, but later some discussion to mainline about this and
>> changed the implementation.
>>
>> As Michal and me were trying to change this as like the
>> "implementation will discover the fastest command by taking the supported
>> commands from flash and a controller. Controller supported commands will always been a priority."
>>
>> Means I have added rd_cmd and wr_cmd params on spi_flash_id_params table.
>> So the flash user may fill these with flash supported commands, and also spi controller
>> use is also have rd_cmd and wr_cmd from spi.h, so the spi user will fill these with
>> controller supported commands. and the resultent command is calculated based fastest
>> commands by taking inputs from spi and flash, but spi(controller) has always a priority.
>>
>> Supported commands:
>> - CMD_READ_ARRAY_SLOW
>> - CMD_READ_ARRAY_FAST
>> - CMD_READ_DUAL_OUTPUT_FAST
>> - CMD_READ_DUAL_IO_FAST
>> - CMD_READ_QUAD_OUTPUT_FAST
>> - CMD_PAGE_PROGRAM
>> - CMD_QUAD_PAGE_PROGRAM
> I miss an important detail in this series, regarding dual/quad-io support:
> How is the spi controller is informed about the transfer details?
> I see only setting the cmds according the features of flash and controller,
> but no additional indication to the spi controller, that this is then a
> dual-
> or quad-io transfer.How the spi controller should know about this???

This implementation will discover the fastest command by taking the supported
commands from flash and a controller. Controller supported commands
will always been a priority.

Means controller driver should have a code to support whether I am
supporting this
new extended read/write or quad command.

And this support will discover the fastest command by comparing the
commands from flash and spi.
flash anyway we added in params list and controller the respective
controller will initialize spi slave in the driver itself .

qspi->slave.rd_cmd = READ_CMD_FULL;
qspi->slave.wr_cmd = PAGE_PROGRAM | QUAD_PAGE_PROGRAM;

Please refer https://github.com/Xilinx/u-boot-xlnx/blob/master-next/drivers/spi/zynq_qspi.c

spi_flash layer should send any commands based on the respective
supported, but it is
supported by respective spi controller that should controller driver must aware.

This is more generic solution as we discussed so-many months ago.
http://u-boot.10912.n7.nabble.com/PATCH-00-12-cmd-sf-Add-support-for-read-and-write-instructions-td143749.html

We tried to discover the supported commands runtime from respective
flashes, but ie. going to be a huge
code to decode all these. Instead filling params, like sectors and
idcodes should be an code driven task as per as
u-boot code is concern.

> By decoding the cmd itself again? But the data should be a transparent byte
> stream to the controller!
> Otherwise you need a table of commands for decoding also inside the
> controller
> driver, which I consider a bad idea, as you need to update them (in each
> driver)
> for new cmds added to the serial-flash driver!
>
> In the linux kernel, the solution in the spi layer was to add new
> elements to
> the transfer structure (struct spi_transfer -> bitwidth), which can be set
> for each part of a message.
> In u-boot we have the spi_xfer function, which has a "flags" parameter
> we could
> use for this.
>
> As long as the details for this (including a spi controller driver,
> which can
> handle dual/quad-io transfers) are not fixed, I would suggest to leave the
> patches regarding the extended cmds out of the series of sf-cleanup (which
> really looks good!) and fix the issues until the next merge window!

I am planning to push this series, as we started this since long months back.
We discussed many threads, and tested this approach on read hw as well.

Please let me know for any thoughts.

>
>> Testing repo branch:
>> -------------------
>> $ git clone git://git.denx.de/u-boot-spi.git
>> $ cd u-boot-spi
>> $ git checkout -b master-next-test origin/master-next-test
>>
>> REQUEST FOR ALL SPI CODE CONTRIBUTORS/USERS, PLEASE TEST THESE CHANGES
>> W.R.T YOUR HW IF POSSIBLE.
>>
>> Please let me know for any issues/concerns/questions.
>>
>> --
>> Thanks,
>> Jagan.
> Best Regards,
> Thomas
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

-- 
Thanks,
Jagan.
--------
Jagannadha Sutradharudu Teki,
E: jagannadh.teki at gmail.com, P: +91-9676773388
Engineer - System Software Hacker
U-boot - SPI Custodian and Zynq APSOC
Ln: http://www.linkedin.com/in/jaganteki