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

Jagan Teki jagannadh.teki at gmail.com
Sun Oct 6 17:30:12 CEST 2013


On Sun, Oct 6, 2013 at 3:44 PM, Sourav Poddar <sourav.poddar at ti.com> 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.

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



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