[U-Boot] [PATCH 1/2] qspi:fsl implement AHB read

[Jagan Teki]

On 8 January 2015 at 17:10, Peng Fan <Peng.Fan at freescale.com> wrote:
>
> On 1/8/2015 7:21 PM, Jagan Teki wrote:
>>
>> On 8 January 2015 at 15:48, Peng Fan <Peng.Fan at freescale.com> wrote:
>>>
>>> Hi, Jagan
>>>
>>> On 1/8/2015 2:20 PM, Jagan Teki wrote:
>>>>
>>>> On 8 January 2015 at 08:10, Peng Fan <Peng.Fan at freescale.com> wrote:
>>>>>
>>>>> The QSPI controller in i.MX 6SoloX and Vybrid supports reading data
>>>>> using
>>>>> IP register and AHB bus.
>>>>>
>>>>> The original driver only supports reading data from IP interface. The
>>>>> IC
>>>>> team suggests to use AHB read which is faster then IP read. Using AHB
>>>>> read,
>>>>> we can directly memcpy, a "missed" access to the buffer will cause the
>>>>> controller to clear the buffer and use the SEQID stored in bfgencr
>>>>> register
>>>>> to initiate a read from flash device.
>>>>
>>>> So for the read connections - controller is connected through IP and
>>>> AHB interface
>>>> is it? and I'm thinking these two are bus protocols which can use any
>>>> peripheral unlike
>>>> qspi.
>>>
>>>
>>> Yeah. fsl qspi controller is connected through IP and AHB interface.
>>> register access through IP interface, Data access can use IP interface or
>>> AHB interface. There is a memory mapped region for AHB data read.  If
>>> want
>>> to use AHB data read  for QSPI,  the controller should be configured by
>>> IP
>>> interface, and then can use memcpy to read data. This patch adds the
>>> configuration for using QSPI AHB read, this configuration is dedicated
>>> for
>>> qspi.
>>
>> Yes - I understand.
>>
>> I'm thinking this ahb is a generic but not only specific to qspi the
>> read of this
>> interface should be generic somewhere in arch code so-that fsl_qspi will
>> call
>> those routines on based on the need. - does it make sense?
>
>
> The AHB read function is just memcpy.  This original code use "qspi_op_read"
> to read data from IP interface. In this patch, if configured
> CONFIG_FSL_QSPI_AHB_READ, use qsp_ahb_read. This function is very simple, as
> following:
>
> +mcr_reg = qspi_read32(&regs->mcr);
>
> +
>
> +qspi_write32(&regs->mcr, QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
>
> +QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>
> +
>
> +/* Read out the data directly from the AHB buffer. */
>
> +memcpy(rxbuf, (u8 *)(q->amba_base + q->sf_addr), len);
>
> +
>
> +qspi_write32(&regs->mcr, mcr_reg);
>
>
> It's just a wrapper function to call memcpy and save/restore qspi mcr
> register, nothing else. I think it's not needed to wrap memcpy and move it
> to arch code, since it is simple and contains qspi register save and
> restore.
>
>
>>>>> Since AHB bus is 64 bit width, we can not set MCR register using 32bit.
>>>>> In
>>>>> order to minimize code change, redefine QSPI_MCR_END_CFD_LE to 64bit
>>>>> Little
>>>>> endian but not 32bit Little endia.
>>>>>
>>>>> Introduce a new configuration option CONFIG_SYS_FSL_QSPI_AHB. If want
>>>>> to
>>>>> use AHB read, just define CONFIG_SYS_FSL_QSPI_AHB. If not, just ignore
>>>>> it.
>>>>> Actually if Vybrid is migrated to use AHB read, this option can be
>>>>> removed and
>>>>> IP read function can be discared. The reason to introduce this option
>>>>> is that only i.MX SOC is tested in my side, no Vybrid platform for me.
>>>>
>>>> So, where did you define?
>>>
>>>
>>> The configuration is defined in board header file in mx6sxsabresd.h,
>>> using
>>> #define CONFIG_SYS_FSL_QSPI_AHB.
>>
>> Yes I saw in 2/2
>>
>>>
>>>>> In spi_setup_slave, the original piece code to set AHB is deleted,
>>>>> since
>>>>> Vybrid platform does not use this to intiate AHB read. Instead, add
>>>>> qspi_init_ahb_read function if defined CONFIG_SYS_FSL_QSPI_AHB.
>>>>>
>>>>> Signed-off-by: Peng Fan <Peng.Fan at freescale.com>
>>>>> ---
>>>>>    drivers/spi/fsl_qspi.c | 137
>>>>> +++++++++++++++++++++++++++++++++++++++++++++----
>>>>>    drivers/spi/fsl_qspi.h |  15 ++++++
>>>>>    2 files changed, 142 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
>>>>> index ad4f4ce..5e0b069 100644
>>>>> --- a/drivers/spi/fsl_qspi.c
>>>>> +++ b/drivers/spi/fsl_qspi.c
>>>>> @@ -263,6 +263,110 @@ static void qspi_set_lut(struct fsl_qspi *qspi)
>>>>>           qspi_write32(&regs->lckcr, QSPI_LCKCR_LOCK);
>>>>>    }
>>>>>
>>>>> +#if defined(CONFIG_SYS_FSL_QSPI_AHB)
>>>>> +/*
>>>>> + * If we have changed the content of the flash by writing or erasing,
>>>>> + * we need to invalidate the AHB buffer. If we do not do so, we may
>>>>> read
>>>>> out
>>>>> + * the wrong data. The spec tells us reset the AHB domain and Serial
>>>>> Flash
>>>>> + * domain at the same time.
>>>>> + */
>>>>> +static inline void qspi_ahb_invalid(struct fsl_qspi *q)
>>>>> +{
>>>>> +       struct fsl_qspi_regs *regs = (struct fsl_qspi_regs
>>>>> *)q->reg_base;
>>>>> +       u32 reg;
>>>>> +
>>>>> +       reg = qspi_read32(&regs->mcr);
>>>>> +       reg |= QSPI_MCR_SWRSTHD_MASK | QSPI_MCR_SWRSTSD_MASK;
>>>>> +       qspi_write32(&regs->mcr, reg);
>>>>> +
>>>>> +       /*
>>>>> +        * The minimum delay : 1 AHB + 2 SFCK clocks.
>>>>> +        * Delay 1 us is enough.
>>>>> +        */
>>>>> +       udelay(1);
>>>>> +
>>>>> +       reg &= ~(QSPI_MCR_SWRSTHD_MASK | QSPI_MCR_SWRSTSD_MASK);
>>>>> +       qspi_write32(&regs->mcr, reg);
>>>>> +}
>>>>> +
>>>>> +/* Read out the data from the AHB buffer. */
>>>>> +static inline void qspi_ahb_read(struct fsl_qspi *q, u8 *rxbuf, int
>>>>> len)
>>>>> +{
>>>>> +       struct fsl_qspi_regs *regs = (struct fsl_qspi_regs
>>>>> *)q->reg_base;
>>>>> +       u32 mcr_reg;
>>>>> +
>>>>> +       mcr_reg = qspi_read32(&regs->mcr);
>>>>> +
>>>>> +       qspi_write32(&regs->mcr, QSPI_MCR_CLR_RXF_MASK |
>>>>> QSPI_MCR_CLR_TXF_MASK |
>>>>> +                    QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
>>>>> +
>>>>> +       /* Read out the data directly from the AHB buffer. */
>>>>> +       memcpy(rxbuf, (u8 *)(q->amba_base + q->sf_addr), len);
>>>>> +
>>>>> +       qspi_write32(&regs->mcr, mcr_reg);
>>>>> +}
>>>>> +
>>>>> +static void qspi_enable_ddr_mode(struct fsl_qspi_regs *regs)
>>>>> +{
>>>>> +       u32 reg, reg2;
>>>>> +
>>>>> +       reg = qspi_read32(&regs->mcr);
>>>>> +       /* Disable the module */
>>>>> +       qspi_write32(&regs->mcr, reg | QSPI_MCR_MDIS_MASK);
>>>>> +
>>>>> +       /* Set the Sampling Register for DDR */
>>>>> +       reg2 = qspi_read32(&regs->smpr);
>>>>> +       reg2 &= ~QSPI_SMPR_DDRSMP_MASK;
>>>>> +       reg2 |= (2 << QSPI_SMPR_DDRSMP_SHIFT);
>>>>> +       qspi_write32(&regs->smpr, reg2);
>>>>> +
>>>>> +       /* Enable the module again (enable the DDR too) */
>>>>> +       reg |= QSPI_MCR_DDR_EN_MASK;
>>>>> +       /* Enable bit 29 for imx6sx */
>>>>> +       reg |= (1 << 29);
>>>>> +
>>>>> +       qspi_write32(&regs->mcr, reg);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * There are two different ways to read out the data from the flash:
>>>>> + *  the "IP Command Read" and the "AHB Command Read".
>>>>> + *
>>>>> + * The IC guy suggests we use the "AHB Command Read" which is faster
>>>>> + * then the "IP Command Read". (What's more is that there is a bug in
>>>>> + * the "IP Command Read" in the Vybrid.)
>>>>> + *
>>>>> + * After we set up the registers for the "AHB Command Read", we can
>>>>> use
>>>>> + * the memcpy to read the data directly. A "missed" access to the
>>>>> buffer
>>>>> + * causes the controller to clear the buffer, and use the sequence
>>>>> pointed
>>>>> + * by the QUADSPI_BFGENCR[SEQID] to initiate a read from the flash.
>>>>> + */
>>>>> +static void qspi_init_ahb_read(struct fsl_qspi_regs *regs)
>>>>> +{
>>>>> +       /* AHB configuration for access buffer 0/1/2 .*/
>>>>> +       qspi_write32(&regs->buf0cr, QSPI_BUFXCR_INVALID_MSTRID);
>>>>> +       qspi_write32(&regs->buf1cr, QSPI_BUFXCR_INVALID_MSTRID);
>>>>> +       qspi_write32(&regs->buf2cr, QSPI_BUFXCR_INVALID_MSTRID);
>>>>> +       qspi_write32(&regs->buf3cr, QSPI_BUF3CR_ALLMST_MASK |
>>>>> +                    (0x80 << QSPI_BUF3CR_ADATSZ_SHIFT));
>>>>> +
>>>>> +       /* We only use the buffer3 */
>>>>> +       qspi_write32(&regs->buf0ind, 0);
>>>>> +       qspi_write32(&regs->buf1ind, 0);
>>>>> +       qspi_write32(&regs->buf2ind, 0);
>>>>> +
>>>>> +       /*
>>>>> +        * Set the default lut sequence for AHB Read.
>>>>> +        * Parallel mode is disabled.
>>>>> +        */
>>>>> +       qspi_write32(&regs->bfgencr,
>>>>> +                    SEQID_FAST_READ << QSPI_BFGENCR_SEQID_SHIFT);
>>>>> +
>>>>> +       /*Enable DDR Mode*/
>>>>> +       qspi_enable_ddr_mode(regs);
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>>    void spi_init()
>>>>>    {
>>>>>           /* do nothing */
>>>>> @@ -273,8 +377,8 @@ struct spi_slave *spi_setup_slave(unsigned int bus,
>>>>> unsigned int cs,
>>>>>    {
>>>>>           struct fsl_qspi *qspi;
>>>>>           struct fsl_qspi_regs *regs;
>>>>> -       u32 reg_val, smpr_val;
>>>>> -       u32 total_size, seq_id;
>>>>> +       u32 smpr_val;
>>>>> +       u32 total_size;
>>>>>
>>>>>           if (bus >= ARRAY_SIZE(spi_bases))
>>>>>                   return NULL;
>>>>> @@ -329,13 +433,9 @@ struct spi_slave *spi_setup_slave(unsigned int
>>>>> bus,
>>>>> unsigned int cs,
>>>>>           qspi_write32(&regs->smpr, smpr_val);
>>>>>           qspi_write32(&regs->mcr, QSPI_MCR_RESERVED_MASK);
>>>>>
>>>>> -       seq_id = 0;
>>>>> -       reg_val = qspi_read32(&regs->bfgencr);
>>>>> -       reg_val &= ~QSPI_BFGENCR_SEQID_MASK;
>>>>> -       reg_val |= (seq_id << QSPI_BFGENCR_SEQID_SHIFT);
>>>>> -       reg_val &= ~QSPI_BFGENCR_PAR_EN_MASK;
>>>>> -       qspi_write32(&regs->bfgencr, reg_val);
>>>>> -
>>>>> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
>>>>> +       qspi_init_ahb_read(regs);
>>>>> +#endif
>>>>>           return &qspi->slave;
>>>>>    }
>>>>>
>>>>> @@ -426,6 +526,8 @@ static void qspi_op_rdid(struct fsl_qspi *qspi, u32
>>>>> *rxbuf, u32 len)
>>>>>           qspi_write32(&regs->mcr, mcr_reg);
>>>>>    }
>>>>>
>>>>> +#ifndef CONFIG_SYS_FSL_QSPI_AHB
>>>>> +/* If not use AHB read, read data from ip interface */
>>>>>    static void qspi_op_read(struct fsl_qspi *qspi, u32 *rxbuf, u32 len)
>>>>>    {
>>>>>           struct fsl_qspi_regs *regs = (struct fsl_qspi_regs
>>>>> *)qspi->reg_base;
>>>>> @@ -469,6 +571,7 @@ static void qspi_op_read(struct fsl_qspi *qspi, u32
>>>>> *rxbuf, u32 len)
>>>>>
>>>>>           qspi_write32(&regs->mcr, mcr_reg);
>>>>>    }
>>>>> +#endif
>>>>>
>>>>>    static void qspi_op_write(struct fsl_qspi *qspi, u8 *txbuf, u32 len)
>>>>>    {
>>>>> @@ -643,8 +746,13 @@ int spi_xfer(struct spi_slave *slave, unsigned int
>>>>> bitlen,
>>>>>           }
>>>>>
>>>>>           if (din) {
>>>>> -               if (qspi->cur_seqid == QSPI_CMD_FAST_READ)
>>>>> +               if (qspi->cur_seqid == QSPI_CMD_FAST_READ) {
>>>>> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
>>>>> +                       qspi_ahb_read(qspi, din, bytes);
>>>>> +#else
>>>>>                           qspi_op_read(qspi, din, bytes);
>>>>> +#endif
>>>>> +               }
>>>>>                   else if (qspi->cur_seqid == QSPI_CMD_RDID)
>>>>>                           qspi_op_rdid(qspi, din, bytes);
>>>>>                   else if (qspi->cur_seqid == QSPI_CMD_RDSR)
>>>>> @@ -658,6 +766,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int
>>>>> bitlen,
>>>>>    #endif
>>>>>           }
>>>>>
>>>>> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
>>>>> +       if ((qspi->cur_seqid == QSPI_CMD_SE) ||
>>>>> +           (qspi->cur_seqid == QSPI_CMD_PP) ||
>>>>> +           (qspi->cur_seqid == QSPI_CMD_BE_4K) ||
>>>>> +           (qspi->cur_seqid == QSPI_CMD_WREAR) ||
>>>>> +           (qspi->cur_seqid == QSPI_CMD_BRWR))
>>>>> +               qspi_ahb_invalid(qspi);
>>>>> +#endif
>>>>> +
>>>>>           return 0;
>>>>>    }
>>>>>
>>>>> diff --git a/drivers/spi/fsl_qspi.h b/drivers/spi/fsl_qspi.h
>>>>> index db400e6..6cb3610 100644
>>>>> --- a/drivers/spi/fsl_qspi.h
>>>>> +++ b/drivers/spi/fsl_qspi.h
>>>>> @@ -58,7 +58,12 @@ struct fsl_qspi_regs {
>>>>>
>>>>>    #define QSPI_MCR_END_CFD_SHIFT         2
>>>>>    #define QSPI_MCR_END_CFD_MASK          (3 << QSPI_MCR_END_CFD_SHIFT)
>>>>> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
>>>>> +/* AHB needs 64bit operation */
>>>>> +#define QSPI_MCR_END_CFD_LE            (3 << QSPI_MCR_END_CFD_SHIFT)
>>>>> +#else
>>>>>    #define QSPI_MCR_END_CFD_LE            (1 << QSPI_MCR_END_CFD_SHIFT)
>>>>> +#endif
>>>>>    #define QSPI_MCR_DDR_EN_SHIFT          7
>>>>>    #define QSPI_MCR_DDR_EN_MASK           (1 << QSPI_MCR_DDR_EN_SHIFT)
>>>>>    #define QSPI_MCR_CLR_RXF_SHIFT         10
>>>>> @@ -69,6 +74,10 @@ struct fsl_qspi_regs {
>>>>>    #define QSPI_MCR_MDIS_MASK             (1 << QSPI_MCR_MDIS_SHIFT)
>>>>>    #define QSPI_MCR_RESERVED_SHIFT                16
>>>>>    #define QSPI_MCR_RESERVED_MASK         (0xf <<
>>>>> QSPI_MCR_RESERVED_SHIFT)
>>>>> +#define QSPI_MCR_SWRSTHD_SHIFT         1
>>>>> +#define QSPI_MCR_SWRSTHD_MASK          (1 << QSPI_MCR_SWRSTHD_SHIFT)
>>>>> +#define QSPI_MCR_SWRSTSD_SHIFT         0
>>>>> +#define QSPI_MCR_SWRSTSD_MASK          (1 << QSPI_MCR_SWRSTSD_SHIFT)
>>>>>
>>>>>    #define QSPI_SMPR_HSENA_SHIFT          0
>>>>>    #define QSPI_SMPR_HSENA_MASK           (1 << QSPI_SMPR_HSENA_SHIFT)
>>>>> @@ -79,6 +88,12 @@ struct fsl_qspi_regs {
>>>>>    #define QSPI_SMPR_DDRSMP_SHIFT         16
>>>>>    #define QSPI_SMPR_DDRSMP_MASK          (7 << QSPI_SMPR_DDRSMP_SHIFT)
>>>>>
>>>>> +#define QSPI_BUFXCR_INVALID_MSTRID     0xe
>>>>> +#define QSPI_BUF3CR_ALLMST_SHIFT       31
>>>>> +#define QSPI_BUF3CR_ALLMST_MASK                (1 <<
>>>>> QSPI_BUF3CR_ALLMST_SHIFT)
>>>>> +#define QSPI_BUF3CR_ADATSZ_SHIFT       8
>>>>> +#define QSPI_BUF3CR_ADATSZ_MASK                (0xFF <<
>>>>> QSPI_BUF3CR_ADATSZ_SHIFT)
>>>>> +
>>>>>    #define QSPI_BFGENCR_SEQID_SHIFT       12
>>>>>    #define QSPI_BFGENCR_SEQID_MASK                (0xf <<
>>>>> QSPI_BFGENCR_SEQID_SHIFT)
>>>>>    #define QSPI_BFGENCR_PAR_EN_SHIFT      16
>>>>> --
>>>>> 1.8.4
>>
>> thanks!
>
>

Applied to u-boot-spi/master

thanks!
-- 
Jagan.