From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sourav Poddar Date: Sat, 5 Oct 2013 01:32:57 +0530 Subject: [U-Boot] [UBOOT][PATCHv4 4/6] spi: add TI QSPI driver In-Reply-To: References: <1380898293-13755-1-git-send-email-sourav.poddar@ti.com> <1380898293-13755-5-git-send-email-sourav.poddar@ti.com> Message-ID: <524F1EF1.2040302@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: > On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar wrote: >> From: Matt Porter >> >> Adds a SPI master driver for the TI QSPI peripheral. >> >> Signed-off-by: Matt Porter >> Signed-off-by: Sourav Poddar >> [Added quad read support and memory mapped support). > What is this comment, any specific? This simply tell the portion which i did in the patch. >> --- > 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. > Ok. >> 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+ > ahh..I copied it from a patch on some list. May be something went wrong, I will check. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +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. > ok >> +}; >> + >> +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 Ok. >> + >> +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? Actually one. >> + >> +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() > Ok. >> + >> + 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? Yes, memory map does not care about setting the command register and doing the transfer using the normal spi framework. Memory ma has a different set of registers, set up register(configured above). Here, we just switch the controller to memory mapped port and use flash->memory_map to read the data from the memory mapped port(0x5c000000). >> + > --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(); > I understand this point, but it was done purposefully. I wanted to hit this point only if memory map is not invoked. If you see sf_ops.c, I am invoking the memory mapped part like this " spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP)" If I place TAG+..TAG- before memory map stuff above, it will always goto out. So, either I keep it here or pass some dummy non zero value for bitlen in above mentioned spi_xfer. ? >> + >> + /* 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/ >