On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddar <sourav.pod...@ti.com> wrote: > On Saturday 05 October 2013 12:27 AM, Jagan Teki wrote: >> >> On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar<sourav.pod...@ti.com> >> wrote: >>> >>> From: Matt Porter<matt.por...@linaro.org> >>> >>> Adds a SPI master driver for the TI QSPI peripheral. >>> >>> Signed-off-by: Matt Porter<matt.por...@linaro.org> >>> Signed-off-by: Sourav Poddar<sourav.pod...@ti.com> >>> [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. May be not required, bcz it will come after i apply below s-o-b
> >>> --- >> >> 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<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. >> > 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. Ok, Please remove this function and assign directly. >>> >>> + >>> +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). OK. can you readjust this code by placing existing spi_flash func like spi_cs_activate() Looks like this cs activate code. Where is SPI_XFER_BEGIN are you not using this? > >>> + >> >> --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. ? Sound good, please keep here. > >>> + >>> + /* 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.t...@gmail.com, P: +91-9676773388 Engineer - System Software Hacker U-boot - SPI Custodian and Zynq APSOC Ln: http://www.linkedin.com/in/jaganteki _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot