[U-Boot] [UBOOT][PATCHv4 4/6] spi: add TI QSPI driver

Jagan Teki jagannadh.teki at gmail.com
Fri Oct 4 20:57:05 CEST 2013


On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar <sourav.poddar at ti.com> wrote:
> From: Matt Porter <matt.porter at linaro.org>
>
> Adds a SPI master driver for the TI QSPI peripheral.
>
> Signed-off-by: Matt Porter <matt.porter at linaro.org>
> Signed-off-by: Sourav Poddar <sourav.poddar at ti.com>
> [Added quad read support and memory mapped support).
What is this comment, any specific?

> ---
You missed change log for all patches, i think you have summarized in 0/6.
I feel it's better write it on individual patches.

>  drivers/spi/Makefile  |    1 +
>  drivers/spi/ti_qspi.c |  328 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 329 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/ti_qspi.c
>
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 91d24ce..e5941b0 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_FDT_SPI) += fdt_spi.o
>  COBJS-$(CONFIG_TEGRA20_SFLASH) += tegra20_sflash.o
>  COBJS-$(CONFIG_TEGRA20_SLINK) += tegra20_slink.o
>  COBJS-$(CONFIG_TEGRA114_SPI) += tegra114_spi.o
> +COBJS-$(CONFIG_TI_QSPI) += ti_qspi.o
>  COBJS-$(CONFIG_XILINX_SPI) += xilinx_spi.o
>  COBJS-$(CONFIG_ZYNQ_SPI) += zynq_spi.o
>
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> new file mode 100644
> index 0000000..d8a03a8
> --- /dev/null
> +++ b/drivers/spi/ti_qspi.c
> @@ -0,0 +1,328 @@
> +/*
> + * TI QSPI driver
> + *
> + * Copyright (C) 2013, Texas Instruments, Incorporated
> + *
> + * SPDX-License-Identifier:     GPL-2.0+

Got below format after apply this patch - please check
 *Â SPDX-License-Identifier:Â Â Â Â Â GPL-2.0+

> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/omap.h>
> +#include <malloc.h>
> +#include <spi.h>
> +
> +struct qspi_regs {
> +u32 pid;
> +u32 pad0[3];
> +u32 sysconfig;
> +u32 pad1[3];
> +u32 intr_status_raw_set;
> +u32 intr_status_enabled_clear;
> +u32 intr_enable_set;
> +u32 intr_enable_clear;
> +u32 intc_eoi;
> +u32 pad2[3];
> +u32 spi_clock_cntrl;
> +u32 spi_dc;
> +u32 spi_cmd;
> +u32 spi_status;
> +u32 spi_data;
> +u32 spi_setup0;
> +u32 spi_setup1;
> +u32 spi_setup2;
> +u32 spi_setup3;
> +u32 spi_switch;
> +u32 spi_data1;
> +u32 spi_data2;
> +u32 spi_data3;
Please add tab space.

> +};
> +
> +struct qspi_slave {
> +       struct spi_slave slave;
> +       struct qspi_regs *base;
> +       unsigned int mode;
> +       u32 cmd;
> +       u32 dc;
> +};
> +

-- TAG+
> +#define QSPI_TIMEOUT                   2000000
> +
> +#define QSPI_FCLK                      192000000
> +
> +/* Clock Control */
> +#define QSPI_CLK_EN                    (1 << 31)
> +#define QSPI_CLK_DIV_MAX               0xffff
> +
> +/* Command */
> +#define QSPI_EN_CS(n)                  (n << 28)
> +#define QSPI_WLEN(n)                   ((n-1) << 19)
> +#define QSPI_3_PIN                     (1 << 18)
> +#define QSPI_RD_SNGL                   (1 << 16)
> +#define QSPI_WR_SNGL                   (2 << 16)
> +#define QSPI_INVAL                     (4 << 16)
> +#define QSPI_RD_QUAD                   (7 << 16)
> +
> +/* Device Control */
> +#define QSPI_DD(m, n)                  (m << (3 + n*8))
> +#define QSPI_CKPHA(n)                  (1 << (2 + n*8))
> +#define QSPI_CSPOL(n)                  (1 << (1 + n*8))
> +#define QSPI_CKPOL(n)                  (1 << (n*8))
> +
> +/* Status */
> +#define QSPI_WC                                (1 << 1)
> +#define QSPI_BUSY                      (1 << 0)
> +#define QSPI_WC_BUSY                   (QSPI_WC | QSPI_BUSY)
> +#define QSPI_XFER_DONE                 QSPI_WC
> +
> +#define MM_SWITCH                      0x01
> +#define MEM_CS                         0x100
> +#define MEM_CS_UNSELECT                        0xfffff0ff
> +#define MMAP_START_ADDR                        0x5c000000
> +#define CORE_CTRL_IO                   0x4a002558
> +
> +#define QSPI_CMD_READ                  (0x3 << 0)
> +#define QSPI_CMD_READ_QUAD             (0x6b << 0)
> +#define QSPI_CMD_READ_FAST             (0x0b << 0)
> +#define QSPI_SETUP0_NUM_A_BYTES                (0x2 << 8)
> +#define QSPI_SETUP0_NUM_D_BYTES_NO_BITS        (0x0 << 10)
> +#define QSPI_SETUP0_NUM_D_BYTES_8_BITS (0x1 << 10)
> +#define QSPI_SETUP0_READ_NORMAL                (0x0 << 12)
> +#define QSPI_SETUP0_READ_QUAD          (0x3 << 12)
> +#define QSPI_CMD_WRITE                 (0x2 << 16)
> +#define QSPI_NUM_DUMMY_BITS            (0x0 << 24)
--TAG-

TAG+ ... TAG- please move these macro definitions in below headers
> +
> +static inline struct qspi_slave *to_qspi_slave(struct spi_slave *slave)
> +{
> +       return container_of(slave, struct qspi_slave, slave);
> +}
> +static inline struct qspi_regs *get_qspi_bus(int dev)
> +{
> +       if (!dev)
> +               return (struct qspi_regs *)QSPI_BASE;
> +       else
> +               return NULL;
> +}
Is this function really required, how many bus controller you have?

> +
> +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> +{
> +       return 1;
> +}
> +
> +void spi_cs_activate(struct spi_slave *slave)
> +{
> +       /* CS handled in xfer */
> +       return;
> +}
> +
> +void spi_cs_deactivate(struct spi_slave *slave)
> +{
> +       /* CS handled in xfer */
> +       return;
> +}
> +
> +void spi_init(void)
> +{
> +       /* nothing to do */
> +}
> +
> +void spi_set_up_spi_register(struct qspi_slave *qslave)
> +{
> +       u32 memval = 0;
> +       struct spi_slave *slave = &qslave->slave;
> +
> +       slave->memory_map = (void *)MMAP_START_ADDR;
> +
> +       memval |= (QSPI_CMD_READ | QSPI_SETUP0_NUM_A_BYTES |
> +               QSPI_SETUP0_NUM_D_BYTES_NO_BITS | QSPI_SETUP0_READ_NORMAL |
> +                       QSPI_CMD_WRITE | QSPI_NUM_DUMMY_BITS);
> +
> +       writel(memval, &qslave->base->spi_setup0);
> +}
> +
> +void spi_set_speed(struct spi_slave *slave, uint hz)
> +{
> +       struct qspi_slave *qslave = to_qspi_slave(slave);
> +
> +       uint clk_div;
> +
> +       if (!hz)
> +               clk_div = 0;
> +       else
> +               clk_div = (QSPI_FCLK / hz) - 1;
> +
> +       debug("%s: hz: %d, clock divider %d\n", __func__, hz, clk_div);
> +
> +       /* disable SCLK */
> +       writel(readl(&qslave->base->spi_clock_cntrl) & ~QSPI_CLK_EN,
> +                       &qslave->base->spi_clock_cntrl);
> +
> +       if (clk_div < 0) {
> +               debug("%s: clock divider < 0, using /1 divider\n", __func__);
> +               clk_div = 0;
> +       }
> +
> +       if (clk_div > QSPI_CLK_DIV_MAX) {
> +               debug("%s: clock divider >%d , using /%d divider\n",
> +                       __func__, QSPI_CLK_DIV_MAX, QSPI_CLK_DIV_MAX + 1);
> +               clk_div = QSPI_CLK_DIV_MAX;
> +       }
> +
> +       /* enable SCLK */
> +       writel(QSPI_CLK_EN | clk_div, &qslave->base->spi_clock_cntrl);
> +       debug("%s: spi_clock_cntrl %08x\n", __func__,
> +               readl(&qslave->base->spi_clock_cntrl));
> +}
> +
> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> +                                 unsigned int max_hz, unsigned int mode)
> +{
> +       struct qspi_slave *qslave;
> +
> +       qslave = spi_alloc_slave(struct qspi_slave, bus, cs);
> +       if (!qslave)
> +               return NULL;
> +
> +       qslave->base = get_qspi_bus(bus);
> +       if (qslave->base)
> +               debug("No Qspi device found\n");
> +       spi_set_speed(&qslave->slave, max_hz);
> +       qslave->mode = mode;
> +
> +#ifdef CONFIG_MMAP
> +       spi_set_up_spi_register(qslave);
> +#endif
> +
> +       debug("%s: bus:%i cs:%i mode:%i\n", __func__, bus, cs, mode);
> +
> +       return &qslave->slave;
> +}
> +
> +void spi_free_slave(struct spi_slave *slave)
> +{
> +       struct qspi_slave *qslave = to_qspi_slave(slave);
> +       free(qslave);
> +}
> +
> +int spi_claim_bus(struct spi_slave *slave)
> +{
> +       struct qspi_slave *qslave = to_qspi_slave(slave);
> +
> +       debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
> +
> +       writel(0, &qslave->base->spi_dc);
> +       writel(0, &qslave->base->spi_cmd);
> +       writel(0, &qslave->base->spi_data);
> +
> +       return 0;
> +}
> +
> +void spi_release_bus(struct spi_slave *slave)
> +{
> +       struct qspi_slave *qslave = to_qspi_slave(slave);
> +
> +       debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
> +
> +       writel(0, &qslave->base->spi_dc);
> +       writel(0, &qslave->base->spi_cmd);
> +       writel(0, &qslave->base->spi_data);
> +}
> +
> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
> +            void *din, unsigned long flags)
> +{
> +       struct qspi_slave *qslave = to_qspi_slave(slave);
> +       uint words = bitlen >> 3; /* fixed 8-bit word length */
> +       const uchar *txp = dout;
> +       uchar *rxp = din;
> +       uint status;
> +       int timeout, val;
> +
> +       debug("%s: bus:%i cs:%i bitlen:%i words:%i flags:%lx\n", __func__,
> +               slave->bus, slave->cs, bitlen, words, flags);
> +
> +       qslave->dc = 0;
> +       if (qslave->mode & SPI_CPHA)
> +               qslave->dc |= QSPI_CKPHA(slave->cs);
> +       if (qslave->mode & SPI_CPOL)
> +               qslave->dc |= QSPI_CKPOL(slave->cs);
> +       if (qslave->mode & SPI_CS_HIGH)
> +               qslave->dc |= QSPI_CSPOL(slave->cs);
> +
> +       writel(qslave->dc, &qslave->base->spi_dc);
Adjust this code in spi_claim_bus()

> +
> +       if (flags & SPI_XFER_MEM_MAP) {
> +               writel(MM_SWITCH, &qslave->base->spi_switch);
> +               val = readl(CORE_CTRL_IO);
> +               val |= MEM_CS;
> +               writel(val, CORE_CTRL_IO);
> +               return 0;
> +       } else if (flags & SPI_XFER_MEM_MAP_END) {
> +               writel(~MM_SWITCH, &qslave->base->spi_switch);
> +               val = readl(CORE_CTRL_IO);
> +               val &= MEM_CS_UNSELECT;
> +               writel(val, CORE_CTRL_IO);
> +               return 0;
> +       }
What is this your are returning from here directly for memory_map? is it?

> +

--TAG+
> +       if (bitlen == 0)
> +               goto out;
> +
> +       if (bitlen % 8) {
> +               flags |= SPI_XFER_END;
> +               goto out;
> +       }
--TAG-

TAG+ .. TAG- please move this code on start of the xfer..ie below debug();

> +
> +       /* setup command reg */
> +       qslave->cmd = 0;
> +       qslave->cmd |= QSPI_WLEN(8);
> +       qslave->cmd |= QSPI_EN_CS(slave->cs);
> +       if (flags & SPI_3WIRE)
> +               qslave->cmd |= QSPI_3_PIN;
> +       qslave->cmd |= 0xfff;
> +
> +       while (words--) {
> +               if (txp) {
> +                       debug("tx cmd %08x dc %08x data %02x\n",
> +                             qslave->cmd | QSPI_WR_SNGL, qslave->dc, *txp);
> +                       writel(*txp++, &qslave->base->spi_data);
> +                       writel(qslave->cmd | QSPI_WR_SNGL,
> +                               &qslave->base->spi_cmd);
> +                       status = readl(&qslave->base->spi_status);
> +                       timeout = QSPI_TIMEOUT;
> +                       while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) {
> +                               if (--timeout < 0) {
> +                                       printf("QSPI tx timed out\n");
> +                                       return -1;
> +                               }
> +                               status = readl(&qslave->base->spi_status);
> +                       }
> +                       debug("tx done, status %08x\n", status);
> +               }
> +               if (rxp) {
> +                       qslave->cmd |= QSPI_RD_SNGL;
> +                       debug("rx cmd %08x dc %08x\n",
> +                               qslave->cmd, qslave->dc);
> +                       writel(qslave->cmd, &qslave->base->spi_cmd);
> +                       status = readl(&qslave->base->spi_status);
> +                       timeout = QSPI_TIMEOUT;
> +                       while ((status & QSPI_WC_BUSY) != QSPI_XFER_DONE) {
> +                               if (--timeout < 0) {
> +                                       printf("QSPI rx timed out\n");
> +                                       return -1;
> +                               }
> +                               status = readl(&qslave->base->spi_status);
> +                       }
> +                       *rxp++ = readl(&qslave->base->spi_data);
> +                       debug("rx done, status %08x, read %02x\n",
> +                               status, *(rxp-1));
> +               }
> +       }
> +
> +out:
> +       /* Terminate frame */
> +       if (flags & SPI_XFER_END)
> +               writel(qslave->cmd | QSPI_INVAL, &qslave->base->spi_cmd);
Please palce this code in spi_cs_deactivate()

I request you please follow the code structure in below thread.
I feel better if you use same prints that used in below example driver.
http://patchwork.ozlabs.org/patch/265683/

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


More information about the U-Boot mailing list