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

Sourav Poddar sourav.poddar at ti.com
Sun Oct 6 12:14:05 CEST 2013


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.
> On Sat, Oct 5, 2013 at 7:53 PM, Sourav Poddar<sourav.poddar at ti.com>  wrote:
>> On Saturday 05 October 2013 05:10 PM, Jagan Teki wrote:
>>> On Sat, Oct 5, 2013 at 3:25 PM, Sourav Poddar<sourav.poddar at ti.com>
>>> wrote:
>>>> On Saturday 05 October 2013 03:11 PM, Jagan Teki wrote:
>>>>> On Sat, Oct 5, 2013 at 11:38 AM, Sourav Poddar<sourav.poddar at ti.com>
>>>>> wrote:
>>>>>> On Saturday 05 October 2013 01:43 AM, Jagan Teki wrote:
>>>>>>> On Sat, Oct 5, 2013 at 1:32 AM, Sourav Poddar<sourav.poddar at 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.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?
>>>>>>>> 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.
>>>>>> 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/
>>>>>>
>>>
>
>



More information about the U-Boot mailing list