From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sourav Poddar Date: Sun, 6 Oct 2013 21:21:26 +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> <524F1EF1.2040302@ti.com> <524FACD0.6020007@ti.com> <524FE1FA.3050804@ti.com> <525020F5.2070402@ti.com> <525137ED.6080508@ti.com> Message-ID: <525186FE.2020105@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 Sunday 06 October 2013 09:00 PM, Jagan Teki wrote: > On Sun, Oct 6, 2013 at 3:44 PM, Sourav Poddar wrote: >> On Sunday 06 October 2013 02:14 PM, Jagan Teki wrote: >>> What if this code is placed in cs_active() with BEGIN flag.? >> + /* 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; >> Functionality wise it wont effect. I am open to what you >> suggest here, whether to move it or not. >> >> Though, just one thing you should note here, is that the >> above code just mask the cmd register bits. The actual >> cmd register write happens inside while loop and only when >> that write happens, then the cs gets activated. >> So, above code does not activate the cs, it just prepare the mask >> that will enable cs later. > OK, just park this as of now try to send next level patch-set we will > discuss more. > Ok. I am working on the next version. >>> On Sat, Oct 5, 2013 at 7:53 PM, Sourav Poddar >>> wrote: >>>> On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote: >>>>> On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddar >>>>> wrote: >>>>>> On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote: >>>>>>> On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddar >>>>>>> wrote: >>>>>>>> On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote: >>>>>>>>> On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddar >>>>>>>>> wrote: >>>>>>>>>> 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. >>>>>>>>> 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 >>>>>>>>>>>> +#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. >>>>>>>>> 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. >>>>>>>> I dont think so it make much sense to put it ib cs_activate apis, >>>>>>>> this >>>>>>>> portion >>>>>>>> does not really justify that. This code is more SOC specific control >>>>>>>> module >>>>>>>> parameters which allows you to switch between the configuaration port >>>>>>>> and memory mapped port. >>>>>>>> >>>>>>>>> Where is SPI_XFER_BEGIN are you not using this? >>>>>>>> We dont need SPI_XFER_BEGIN here, for memory mapped we just need >>>>>>>> MEM_MAP flag, do the required memory mapped read and end the >>>>>>>> transfer. >>>>>>>> This is what differentiate a SPI based read and a memory mapped read. >>>>>>> OK. >>>>>>> Understand, let me clear once are you trying to use erase/write from >>>>>>> spi_flash or not? >>>>>>> The reason for asking if yes, we need to use BEGIN on driver and also >>>>>>> these is one more controller already exists on spi >>>>>>> where the controller uses read for mmap and erase/write is common - >>>>>>> are your driver looks same as this or different? >>>>>>> >>>>>> Yes, erase and write commands are also done. >>>>>> Generally, BEGIN flag is used sp that cs_activate can be called. >>>>>> But, in my case cs is activated in a different way, its enabled by >>>>>> writing in a command register. So, there is no need for a BEGIN. >>>>> OK, please do all modifications as we discussed so-far. >>>>> and send the next level- so we can discuss more. >>>>> >>>>> Please follow the driver code model- what I referred in previous mail. >>>>> >>>> Ok. I will send the v5 with all the discussed modification. >>>> >>>>>>>>>>>> + >>>>>>>>>>> --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/ >>>>>>>> >>> > >