[U-Boot] [PATCH 2/6] sf: Optimize flash features code

[Jagan Teki]

Hi Marek,

Please ignore the last sent!

On Tue, Jan 21, 2014 at 1:09 PM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
> Hi Marek,
>
> On Tue, Jan 21, 2014 at 4:31 AM, Marek Vasut <marex at denx.de> wrote:
>> On Monday, January 20, 2014 at 02:32:27 PM, Jagan Teki wrote:
>>> On Mon, Jan 20, 2014 at 6:49 PM, Marek Vasut <marex at denx.de> wrote:
>>> > On Saturday, January 18, 2014 at 09:51:56 PM, Jagan Teki wrote:
>>> >> On Sun, Jan 19, 2014 at 2:09 AM, Marek Vasut <marex at denx.de> wrote:
>>> >> > On Saturday, January 18, 2014 at 09:06:29 PM, Jagannadha Sutradharudu
>>> >> > Teki
>>> >> >
>>> >> > wrote:
>>> >> >> - Shrink spi_slave {}
>>> >> >> - Shrink spi_flash_params {}
>>> >> >> - Documentation for sf features
>>> >> >>
>>> >> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
>>> >> >> Cc: Marek Vasut <marex at denx.de>
>>> >> >> ---
>>> >> >>
>>> >> >>  doc/SPI/README.sf-features    | 122 ++++++++++++++++++++++++++++++
>>> >> >>  drivers/mtd/spi/sf.c          |   4 +-
>>> >> >>  drivers/mtd/spi/sf_internal.h |   1 -
>>> >> >>  drivers/mtd/spi/sf_ops.c      |   8 +-
>>> >> >>  drivers/mtd/spi/sf_params.c   | 172
>>> >> >>
>>> >> >> +++++++++++++++++++++--------------------- drivers/mtd/spi/sf_probe.c
>>> >> >>
>>> >> >>  71 ++++++++---------
>>> >> >>  include/spi.h                 |  42 ++++-------
>>> >> >>  include/spi_flash.h           |  24 +++---
>>> >> >>  8 files changed, 270 insertions(+), 174 deletions(-)
>>> >> >>  create mode 100644 doc/SPI/README.sf-features
>>> >> >>
>>> >> >> diff --git a/doc/SPI/README.sf-features b/doc/SPI/README.sf-features
>>> >> >> new file mode 100644
>>> >> >> index 0000000..d35f56d
>>> >> >> --- /dev/null
>>> >> >> +++ b/doc/SPI/README.sf-features
>>> >> >> @@ -0,0 +1,122 @@
>>> >> >> +SPI FLASH feature enhancements:
>>> >> >> +==============================
>>> >> >> +
>>> >> >> +This document describes how to extend the current data structures in
>>> >> >> spi subsystem +for making use of new flash features/operations w.r.t
>>> >> >> to controller driver support. +
>>> >> >> +1. spi_slave:
>>> >> >> +
>>> >> >> +struct spi_slave {
>>> >> >> +    ..........
>>> >> >> +    u32 mode_bits;
>>> >> >> +    ........
>>> >> >> +};
>>> >> >> +
>>> >> >> + at mode_bits can be used to expose the SPI RX/TX operation modes, bus
>>> >> >> options and +few flags which are used to extended the flash specific
>>> >> >> features/operations +- include/spi.h
>>> >> >> +
>>> >> >> +mode_bits:
>>> >> >> +- SPI_TX_QPP: 4-Wire tx transfer operation quad page program
>>> >> >> +- SPI_RX_SLOW: 1-wire rx transfer operation array slow read
>>> >> >> +- SPI_RX_DUAL: 2-wire rx transfer operation dual fast read
>>> >> >> +- SPI_RX_DUAL_IO: 2-wire rx transfer operation dual io fast read
>>> >> >> +- SPI_RX_QUAD: 4-wire rx transfer operation quad fast read
>>> >> >> +- SPI_RX_QUAD_IO: 4-wire rx transfer operation quad io fast read
>>> >> >> +- SPI_SHARED: dual flash devices are connected in shared bus
>>> >> >> connection +- SPI_SEPARATED: dual flash devices are connected in
>>> >> >> separate bus connection +- SPI_U_PAGE: select the upper flash in
>>> >> >> dual flash shared bus connection [1] +
>>> >> >
>>> >> > A generic SPI controller _does_ _not_ _care_ about any SPI flash crud.
>>> >> > The SPI bus controller (which is what this is for) and SPI-NOR
>>> >> > controller are two different things and must have two different slave
>>> >> > structures.
>>> >>
>>> >> You mean mode_bits need to move in one more structure.
>>> >> Just leave about new SPI-NOR as of now for this release we discuss more
>>> >> soon.
>>> >
>>> > The mode_bits have no place in this structure. The slave can indicate
>>> > whether it can be connected over 1,2,4... lines , but must not indicate
>>> > that it supports some SPI-flash specific properties.
>>>
>>> What do you mean by this - can you elaborate.
>>
>> I mean there do exist SPI controllers which can output data via more than one
>> (MOSI) line . They do support 1, 2 or even 4 lines. This comes useful when
>> driving SPI flashes indeed, _BUT_ (!) this is not tied to driving SPI flashes.
>> On the contrary, you can connect any device which can be driven over such
>> "parallelised" SPI and it will work. And only such property shall also land in
>> the struct spi_slave {} .
>>
>> Properties like "let's assume the SPI slave is a SPI NOR and it supports SPI NOR
>> command FOO" do not go into struct spi_slave.
>>
>>> As of now drivers in drivers/spi need to inform the flash through spi_slave
>>> {} no other alternative ie way remaining flash properties memory_map etc..
>>> handle. Even Linux follow the same w/o new SPI-NOR framework.
>>
>> Linux is now going for a separate SPI-NOR controller framework and this is what
>> we will do as well. The SF layer will be only a unifying layer providing access
>> to either SPI-NOR API (for the SPI-NOR controllers) or a translation layer to
>> communicate SPI-NOR commands over generic SPI API (for the old-school regular
>> way of doing SPI NOR connection to SPI bus).

Just take an example of ti qspi controller which is connected to serial nor

Current design:

cmd_sf.c
------------
spi_flash.h
--------------
sf (_probe, _ops), sf.c
-------------------------------------------
drivers/spi/ti_qspi.c (inform flash properties through "mode_bits" spi_slave {}
-------------------------------------------------------------------------------

Proposed one (may be):

Suppose If we have one more qspi controller(zynq) which is connected
to serial nor

                                                cmd_sf.c
                                                -----------
                                               spi_flash.h
                                              ---------------
                                               SF-NOR
------------------------------------------------------------------------------------------------
drivers/spi/ti_qspi.c
   drivers/mtd/sf_nor/zynq_qspi.c
(inform flash properties
(inform flash properties
through "mode_bits" of
through "mode_bits" of
spi_slave {})
        sf_nor_device {}

Please comment on above proposed design will ask you more
once this set..thanks!

-- 
Jagan.