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

[Marek Vasut]

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).

> If your question, like need a separate structure for flash specific
> properties please wait will wound-up all these in new framework.
> 
> I'm planning to push in today release.

I doubt anything is going in today's release ;-)

Actually, you need to really start discerning which _must_ go into a release and 
which simply does not go into a release and waits for -next. This is important 
role of a maintainer and plays important role in keeping the codebase in good 
shape.

Code which is untested, code which changes core stuff and isn't an obvious 
bugfix etc. simply can wait. Code which is obvious bugfix goes in even in the -
rc period. Of course, there are all kinds of exceptions, that depends on the 
rationale and negotiation.