[U-Boot] [PATCH v2] spi/cadence: Adding Cadence SPI driver support for SOCFPGA

[Jagan Teki]

HI Chin Liang,

On Thu, Jan 9, 2014 at 8:06 PM, Chin Liang See <clsee at altera.com> wrote:
> Hi Jagan,
>
> On Wed, 2014-01-08 at 17:43 +0530, Jagan Teki wrote:
>> Hi Chin Liang See,
>>
>> On Thu, Jan 2, 2014 at 8:13 AM, Chin Liang See <clsee at altera.com> wrote:
>> > To add the Cadence SPI driver support for Altera SOCFPGA. It
>> > required information such as clocks and timing from platform's
>> > configuration header file within include/configs folder
>> >
>> > Signed-off-by: Chin Liang See <clsee at altera.com>
>> > Cc: Jagan Teki <jagannadh.teki at gmail.com>
>> > Cc: Gerhard Sittig <gsi at denx.de>
>> > ---
>> > Changes for v2
>> > - Combine driver into single C file instead of 2
>> > - Added documentation on the macro used
>> > - Using structure for registers instead of macro
>> > ---
>> >  doc/README.socfpga         |   47 ++
>> >  drivers/spi/Makefile       |    1 +
>> >  drivers/spi/cadence_qspi.c | 1018 ++++++++++++++++++++++++++++++++++++++++++++
>> >  drivers/spi/cadence_qspi.h |  170 ++++++++
>> >  4 files changed, 1236 insertions(+)
>> >  create mode 100644 drivers/spi/cadence_qspi.c
>> >  create mode 100644 drivers/spi/cadence_qspi.h
>> >
>> > diff --git a/doc/README.socfpga b/doc/README.socfpga
>> > index cfcbbfe..242af97 100644
>> > --- a/doc/README.socfpga
>> > +++ b/doc/README.socfpga
>> > @@ -51,3 +51,50 @@ the card
>> >  #define CONFIG_SOCFPGA_DWMMC_BUS_HZ    50000000
>> >  -> The clock rate to controller. Do note the controller have a wrapper which
>> >  divide the clock from PLL by 4.
>> > +
>> > +--------------------------------------------
>> > +cadence_qspi
>> > +--------------------------------------------
>> > +Here are macro and detailed configuration required to enable Cadence QSPI
>> > +controller support within SOCFPGA
>> > +
>> > +#define CONFIG_SPI_FLASH
>> > +-> To enable the SPI flash framework support
>> > +
>> > +#define CONFIG_CMD_SF
>> > +-> To enable the console support for SPI flash
>> > +
>> > +#define CONFIG_SF_DEFAULT_SPEED                (50000000)
>> > +-> To set the target SPI clock frequency in Hz
>> > +
>> > +#define CONFIG_SF_DEFAULT_MODE         SPI_MODE_3
>> > +-> To set the SPI mode (CPOL & CPHA). Normally use mode 3 for serial NOR flash
>> > +
>> > +#define CONFIG_SPI_FLASH_QUAD          (1)
>> > +-> To enable the Quad IO mode for performance boost
>> > +
>> > +#define CONFIG_SPI_FLASH_STMICRO
>> > +#define CONFIG_SPI_FLASH_SPANSION
>> > +-> To enable the SPI flash support for vendor Micron and Spansion
>> > +
>> > +#define CONFIG_CQSPI_BASE              (SOCFPGA_QSPIREGS_ADDRESS)
>> > +#define CONFIG_CQSPI_AHB_BASE          (SOCFPGA_QSPIDATA_ADDRESS)
>> > +-> To specify the base address for controller CSR base and AHB data base addr
>> > +
>> > +#define CONFIG_CQSPI_REF_CLK           (400000000)
>> > +-> The clock frequency supplied from PLL to the QSPI controller
>> > +
>> > +#define CONFIG_CQSPI_PAGE_SIZE         (256)
>> > +-> To define the page size of serial flash in bytes
>> > +
>> > +#define CONFIG_CQSPI_BLOCK_SIZE                (16)
>> > +-> To define the block size of serial flash in pages
>> > +
>> > +#define CONFIG_CQSPI_DECODER           (0)
>> > +-> To enable the 4-to-16 decoder which enable up to 16 serial flash devices
>> > +
>> > +#define CONFIG_CQSPI_TSHSL_NS          (200)
>> > +#define CONFIG_CQSPI_TSD2D_NS          (255)
>> > +#define CONFIG_CQSPI_TCHSH_NS          (20)
>> > +#define CONFIG_CQSPI_TSLCH_NS          (20)
>> > +-> Configure the controller based on serial flash device timing characteristic
>> Do we really require this, because most of the known macros definitions.
>> Better to not write too many duplicates - Yes there are few macro's
>> which are specific to cadence
>> but I don't think those were required.
>
> Oh actually this is to address Gerhard's comment. This document will
> guide user the required macro (and its details) in order to use this
> Cadence QSPI controller.

Yes - but I don't see the actual usecase here better to comment
cadence macros in
driver file itself because it could be new style for each spi configs,
this sound may not valid i guess.
and also send the config patch where this got enable.

>
>> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> > index ed4ecd7..b8d56ea 100644
>> > --- a/drivers/spi/Makefile
>> > +++ b/drivers/spi/Makefile
>> > @@ -15,6 +15,7 @@ obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o
>> >  obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o
>> >  obj-$(CONFIG_BFIN_SPI) += bfin_spi.o
>> >  obj-$(CONFIG_BFIN_SPI6XX) += bfin_spi6xx.o
>> > +obj-$(CONFIG_CADENCE_QSPI) += cadence_qspi.o
>> >  obj-$(CONFIG_CF_SPI) += cf_spi.o
>> >  obj-$(CONFIG_CF_QSPI) += cf_qspi.o
>> >  obj-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
>> > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>> > new file mode 100644
>> > index 0000000..4712b45
>> > --- /dev/null
>> > +++ b/drivers/spi/cadence_qspi.c
>> > @@ -0,0 +1,1018 @@
>> > +/*
>> > + * (C) Copyright 2014 Altera Corporation <www.altera.com>
>> > + *
>> > + * SPDX-License-Identifier:    GPL-2.0+
>> > + */
>> > +
>> > +#include <common.h>
>> > +#include <asm/io.h>
>> > +#include <asm/errno.h>
>> > +#include <malloc.h>
>> > +#include <spi.h>
>> > +#include "cadence_qspi.h"
>>
>> Try to move the stuff from header here.!
>
> Actually I was doing this initially but it bloated this c file a lot.
> Hopefully its ok that we separate out to a header file. But if you think
> this is required, I can make the v3.

My big concern here was about the driver code, I am not satisfied the
way that the code
is organized(means no.of functions) w.r.t different modes/commands.
As this is a bootloader code I want to see the functionality
spi_xfer() as minimum/optimize as possible.

I am open to discuss more with next level patches as well if you have
more questions.

-- 
Thanks,
Jagan.