[U-Boot] [RFC PATCH] dm:spi:fsl_qspi add DM support

Simon Glass sjg at chromium.org
Thu Jan 22 23:29:05 CET 2015


Hi Peng,

On 16 January 2015 at 22:59, Peng Fan <Peng.Fan at freescale.com> wrote:
> Hi Simon ,Jagan
>
> This patch is based on git://git.denx.de/u-boot-spi.git master branch,
> since some fsl_qspi's new feature is still in this git repo and have
> not been merged to mainline.
> I saw Simon sent out a new patch that remove the per_child_auto_alloc_size
> from the platforms' driver code and move it to spi uclass driver. In
> this patch I do not remove it, since this is a RFC version, and Jagan's
> spi git repo still has it. I can remove it in formal version if needed.
> Please take your time to review and comment this patch.
>
> Thanks.
>
> This patch adds DM support for fsl_qspi driver, the DM part needs
> device tree support.
>
> Partial of the original driver code is reused, such as the AHB part,
> the LUT initialization and etc. The driver now supports new DM and original
> driver by define "CONFIG_DM_SPI". Until device tree is integrated, the
> original part can be discarded.
>
> The driver code file is now as following:
>  "
>
>  Common code that needs both by DM or original driver code.
>
>  #if defined(CONFIG_DM_SPI)
>
>  DM part
>
>  #else
>
>  Original driver code
>
>  #endif
> "
>
> In DM part, some reconstruction is done. A fsl_qspi_runcmd is included to
> simplify the code, but not the original qspi_op_xxxxs. fsl_qspi_get_seqid
> is included to get seqid, but not hardcoded in original qspi_op_xxxxs.

Not sure where this driver is going - I'm happy to pick it up if you
work things out with Jagan. For now I'll just make a few comments, in
case they are useful.

>
> Signed-off-by: Peng Fan <Peng.Fan at freescale.com>
> ---
>  drivers/spi/fsl_qspi.c | 420 +++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/spi/fsl_qspi.h |   1 +
>  2 files changed, 405 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
> index 5e0b069..ee151b3 100644
> --- a/drivers/spi/fsl_qspi.c
> +++ b/drivers/spi/fsl_qspi.c
> @@ -13,6 +13,13 @@
>  #include <linux/sizes.h>
>  #include "fsl_qspi.h"
>
> +#ifdef CONFIG_DM_SPI

Shouldn't need these #ifdefs around header files.

> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <asm/errno.h>
> +#include <spi_flash.h>
> +#endif
> +
>  #define RX_BUFFER_SIZE         0x80
>  #ifdef CONFIG_MX6SX
>  #define TX_BUFFER_SIZE         0x200
> @@ -71,27 +78,41 @@
>  #define qspi_write32           out_be32
>  #endif
>
> -static unsigned long spi_bases[] = {
> -       QSPI0_BASE_ADDR,
> -#ifdef CONFIG_MX6SX
> -       QSPI1_BASE_ADDR,
> -#endif
> -};
> +#ifdef CONFIG_DM_SPI
> +DECLARE_GLOBAL_DATA_PTR;
> +#define QUADSPI_AHBMAP_BANK_MAXSIZE    SZ_64M
>
> -static unsigned long amba_bases[] = {
> -       QSPI0_AMBA_BASE,
> -#ifdef CONFIG_MX6SX
> -       QSPI1_AMBA_BASE,
> -#endif
> +struct fsl_qspi_platdata {
> +       u32             max_hz;
> +       u32             reg_base;
> +       u32             amba_base;
> +       u32             flash_num;
>  };
>
>  struct fsl_qspi {
> +       u32             reg_base;
> +       u32             amba_base;

Should these be structure pointers?

> +       size_t          cmd_len;
> +       u8              cmd_buf[32];
> +       size_t          data_len;
> +       int             qspi_is_init;
> +       size_t          flash_size;
> +       u32             bank_memmap_phy[4];
> +       int             cs;

Do you need cs in here? The per-child platform data has it if you base
it on u-boot-dm/i2c-working (which is what I suggest).

> +       u32             sf_addr;
> +       int             flash_num;
> +       u8              cur_seqid;
> +       u32             freq;
> +};
> +#else

Will there still be non-driver-model users of this driver? Can we
convert them over?

> +struct fsl_qspi {
>         struct spi_slave slave;
>         unsigned long reg_base;
>         unsigned long amba_base;
>         u32 sf_addr;
>         u8 cur_seqid;
>  };
> +#endif
>
>  /* QSPI support swapping the flash read/write data
>   * in hardware for LS102xA, but not for VF610 */
> @@ -104,11 +125,6 @@ static inline u32 qspi_endian_xchg(u32 data)
>  #endif
>  }
>
> -static inline struct fsl_qspi *to_qspi_spi(struct spi_slave *slave)
> -{
> -       return container_of(slave, struct fsl_qspi, slave);
> -}
> -
>  static void qspi_set_lut(struct fsl_qspi *qspi)
>  {
>         struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)qspi->reg_base;
> @@ -367,6 +383,377 @@ static void qspi_init_ahb_read(struct fsl_qspi_regs *regs)
>  }
>  #endif
>
> +#ifdef CONFIG_DM_SPI
> +/* Get the SEQID for the command */
> +static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)

s/q/priv/ or something a bit longer than one character.

> +{
> +       switch (cmd) {
> +       case QSPI_CMD_FAST_READ:
> +       case QSPI_CMD_FAST_READ_4B:
> +               return SEQID_FAST_READ;
> +       case QSPI_CMD_WREN:
> +               return SEQID_WREN;
> +       case QSPI_CMD_RDSR:
> +               return SEQID_RDSR;
> +       case QSPI_CMD_SE:
> +               return SEQID_SE;
> +       case QSPI_CMD_PP:
> +       case QSPI_CMD_PP_4B:
> +               return SEQID_PP;
> +       case QSPI_CMD_RDID:
> +               return SEQID_RDID;
> +       case QSPI_CMD_BE_4K:
> +               return SEQID_BE_4K;
> +#ifdef CONFIG_SPI_FLASH_BAR
> +       case QSPI_CMD_BRRD:
> +               return SEQID_BRRD;
> +       case QSPI_CMD_BRWR:
> +               return SEQID_BRWR;
> +       case QSPI_CMD_RDEAR:
> +               return SEQID_RDEAR;
> +       case QSPI_CMD_WREAR:
> +               return SEQID_WREAR;
> +#endif
> +       default:
> +               break;
> +       }
> +       return -1;
> +}
> +
> +/* Read out the data from the QUADSPI_RBDR buffer registers. */
> +static void fsl_qspi_ip_read(struct fsl_qspi *q, int len, u8 *rxbuf)
> +{
> +       struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)q->reg_base;
> +       u32 tmp;
> +       int i = 0;
> +
> +       while (len > 0) {
> +               tmp = qspi_read32(&regs->rbdr[i]);
> +               tmp = qspi_endian_xchg(tmp);
> +
> +               if (len >= 4) {
> +                       memcpy(rxbuf, &tmp, 4);
> +                       rxbuf += 4;
> +               } else {
> +                       memcpy(rxbuf, &tmp, len);
> +                       break;
> +               }
> +
> +               len -= 4;
> +               i++;
> +       }
> +}
> +
> +/* Write data to the QUADSPI_TBDR buffer registers. */
> +static void fsl_qspi_write_data(struct fsl_qspi *q, int len, u8 *txbuf)
> +{
> +       struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)q->reg_base;
> +       u32 tmp;
> +       u32 t1, t2;
> +       int j;
> +
> +       tmp = qspi_read32(&regs->mcr);
> +       qspi_write32(&regs->mcr, tmp | QSPI_MCR_CLR_RXF_MASK |
> +                    QSPI_MCR_CLR_TXF_MASK);
> +
> +       /* fill the TX data to the FIFO */
> +       t2 = len % 4;
> +       t1 = len >> 2; /* 4 Bytes aligned */
> +
> +       for (j = 0; j < t1; j++) {

What is 11?

> +               memcpy(&tmp, txbuf, 4);
> +               tmp = qspi_endian_xchg(tmp);
> +               qspi_write32(&regs->tbdr, tmp);
> +               txbuf += 4;
> +       }
> +
> +       if (t2) {
> +               tmp = 0;
> +               memcpy(&tmp, txbuf, t2);
> +               tmp = qspi_endian_xchg(tmp);
> +               qspi_write32(&regs->tbdr, tmp);
> +       }
> +}
> +
> +static int fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, int len)
> +{
> +       struct fsl_qspi_regs *regs = (struct fsl_qspi_regs *)q->reg_base;
> +       int seqid;
> +       u32 reg;
> +
> +       /* Wait previous cmd completed */
> +       while (qspi_read32(&regs->sr) & QSPI_SR_BUSY_MASK)
> +               ;

Should check for timeout here?
> +
> +       /* save the reg */
> +       reg = qspi_read32(&regs->mcr);
> +
> +       qspi_write32(&regs->sfar, q->amba_base + q->sf_addr);
> +       qspi_write32(&regs->rbct, QSPI_RBCT_WMRK_MASK | QSPI_RBCT_RXBRD_USEIPS);
> +       qspi_write32(&regs->mcr, reg | QSPI_MCR_CLR_RXF_MASK);
> +
> +       /* trigger the LUT now */
> +       seqid = fsl_qspi_get_seqid(q, cmd);
> +       qspi_write32(&regs->ipcr, (seqid << QSPI_IPCR_SEQID_SHIFT) | len);
> +
> +       /* Wait until completed */
> +       while (qspi_read32(&regs->sr) & QSPI_SR_BUSY_MASK)
> +               ;
> +
> +       /* restore the MCR */
> +       qspi_write32(&regs->mcr, reg);
> +
> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
> +       /* After switch BANK, AHB buffer should also be invalid. */
> +       if ((QSPI_CMD_SE == cmd) || (QSPI_CMD_PP == cmd) ||
> +           (QSPI_CMD_BE_4K == cmd) || (QSPI_CMD_WREAR == cmd) ||
> +           (QSPI_CMD_BRWR == cmd))
> +               qspi_ahb_invalid(q);
> +#endif
> +       return 0;
> +}
> +
> +int fsl_qspi_init(struct fsl_qspi *qspi)
> +{
> +       struct fsl_qspi_regs *regs;
> +       u32 smpr_val;
> +
> +       regs = (struct fsl_qspi_regs *)qspi->reg_base;
> +
> +       qspi_write32(&regs->mcr, QSPI_MCR_RESERVED_MASK | QSPI_MCR_MDIS_MASK);
> +
> +       smpr_val = qspi_read32(&regs->smpr);
> +       qspi_write32(&regs->smpr, smpr_val & ~(QSPI_SMPR_FSDLY_MASK |
> +                    QSPI_SMPR_FSPHS_MASK | QSPI_SMPR_HSENA_MASK));
> +       qspi_write32(&regs->mcr, QSPI_MCR_RESERVED_MASK);
> +
> +       /* flash size is still using macro definition. */
> +       if (qspi->flash_num == 2) {
> +               qspi_write32(&regs->sfa1ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +               qspi_write32(&regs->sfa2ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +               qspi_write32(&regs->sfb1ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE * 2);
> +               qspi_write32(&regs->sfb2ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE * 2);
> +       } else if (qspi->flash_num == 4) {
> +               qspi_write32(&regs->sfa1ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +               qspi_write32(&regs->sfa2ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE * 2);
> +               qspi_write32(&regs->sfb1ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE * 3);
> +               qspi_write32(&regs->sfb2ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE * 4);
> +       } else {
> +               qspi_write32(&regs->sfa1ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +               qspi_write32(&regs->sfa2ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +               qspi_write32(&regs->sfb1ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +               qspi_write32(&regs->sfb2ad, qspi->amba_base +
> +                            FSL_QSPI_FLASH_SIZE);
> +       }
> +
> +       qspi_set_lut(qspi);
> +
> +       smpr_val = qspi_read32(&regs->smpr);
> +       smpr_val &= ~QSPI_SMPR_DDRSMP_MASK;
> +       qspi_write32(&regs->smpr, smpr_val);
> +       qspi_write32(&regs->mcr, QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
> +
> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
> +       qspi_init_ahb_read(regs);
> +#endif
> +       return 0;
> +}
> +
> +static int fsl_qspi_probe(struct udevice *bus)
> +{
> +       struct fsl_qspi_platdata *plat = bus->platdata;
> +       struct fsl_qspi *priv = dev_get_priv(bus);
> +
> +       priv->reg_base = plat->reg_base;
> +       priv->amba_base = plat->amba_base;
> +       priv->flash_num = plat->flash_num;
> +
> +       if (!priv->qspi_is_init) {
> +               /* Initialization */
> +               fsl_qspi_init(priv);
> +               priv->qspi_is_init = 1;
> +       }
> +
> +       return 0;
> +}
> +
> +static int fsl_qspi_child_pre_probe(struct udevice *dev)
> +{
> +       struct udevice *bus = dev_get_parent(dev);
> +       struct fsl_qspi *priv = dev_get_priv(bus);
> +       struct spi_slave *slave = dev_get_parentdata(dev);
> +       int cs = spi_chip_select(dev);
> +
> +       slave->max_write_size = TX_BUFFER_SIZE;
> +
> +       priv->cs = cs;
> +       priv->amba_base = priv->bank_memmap_phy[priv->cs];
> +
> +       return 0;
> +}
> +
> +static int fsl_qspi_xfer(struct udevice *dev, unsigned int bitlen,
> +                        const void *dout, void *din, unsigned long flags)
> +{
> +       struct udevice *bus = dev->parent;
> +       struct fsl_qspi *q = dev_get_priv(bus);
> +       u32 len = DIV_ROUND_UP(bitlen, 8);
> +       int ret = 0;
> +       u8 *buf;
> +       static u32 addr;
> +       static u8 opcode;
> +
> +       if (!opcode && (flags & SPI_XFER_BEGIN)) {
> +               /* CMD PHASE */
> +               buf = (u8 *)dout;
> +               opcode = buf[0];
> +               if (len > 1) {
> +                       addr = buf[1] << 16 | buf[2] << 8 | buf[3];
> +                       q->sf_addr = addr;
> +               }
> +
> +               /* Only transfer CMD */
> +               if (flags & SPI_XFER_END)
> +                       ret = fsl_qspi_runcmd(q, opcode, 0);
> +
> +       } else if (opcode) {
> +               /* Data phase */
> +               if (din) {
> +                       /* read*/
> +                       buf = (u8 *)din;
> +#ifdef CONFIG_SYS_FSL_QSPI_AHB
> +                       if (QSPI_CMD_FAST_READ == opcode) {
> +                               qspi_ahb_read(q, buf, len);
> +                       } else {
> +                               ret = fsl_qspi_runcmd(q, opcode, len);
> +                               if (!ret)
> +                                       fsl_qspi_ip_read(q, len, buf);
> +                       }
> +#else
> +                       ret = fsl_qspi_runcmd(q, opcode, len);
> +                       if (!ret)
> +                               fsl_qspi_ip_read(q, len, buf);
> +#endif
> +               } else if (dout) {
> +                       /* Write data, First prepare data */
> +                       buf = (u8 *)dout;
> +                       fsl_qspi_write_data(q, len, buf);
> +                       /* Run page program cmd */
> +                       ret = fsl_qspi_runcmd(q, opcode, len);
> +               }
> +       }
> +
> +       if (ret || (flags & SPI_XFER_END)) {
> +               opcode = 0;
> +               addr = 0;
> +       }
> +
> +       return ret;
> +}
> +
> +static int fsl_qspi_ofdata_to_platdata(struct udevice *bus)
> +{
> +       struct fsl_qspi_platdata *plat = bus->platdata;
> +       const void *blob = gd->fdt_blob;
> +       int node = bus->of_offset;
> +       int subnode, flash_num = 0;
> +       u32 data[4];
> +       int ret;
> +
> +       ret = fdtdec_get_int_array(blob, node, "reg", data, ARRAY_SIZE(data));
> +       if (ret) {
> +               printf("Error: can't get base addresses (ret = %d)!\n", ret);
> +               return -ENODEV;

I think the device is there but it is wrong, so these should be -EINVAL

Also better to avoid printf() and use debug(), for code size reasons I think.
> +       }
> +
> +       /* Count flash numbers */
> +       fdt_for_each_subnode(blob, subnode, node)
> +               ++flash_num;
> +
> +       if (flash_num == 0) {
> +               printf("Error: Missing flashes!\n");
> +               return -ENODEV;
> +       }
> +
> +       plat->reg_base = data[0];
> +       plat->amba_base = data[2];
> +       plat->flash_num = flash_num;
> +
> +       plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
> +                                     50000000);
> +
> +       debug("%s: reg_base=0x%x amba_base=0x%x max-frequency=%d\n",
> +             __func__, plat->reg_base, plat->amba_base, plat->max_hz);
> +
> +       return 0;
> +}
> +
> +static int fsl_qspi_set_speed(struct udevice *bus, u32 speed)
> +{
> +       return 0;
> +}
> +
> +static int fsl_qspi_set_mode(struct udevice *bus, u32 mode)
> +{
> +       return 0;
> +}
> +
> +static const struct dm_spi_ops fsl_qspi_ops = {
> +       .xfer           = fsl_qspi_xfer,
> +       .set_speed      = fsl_qspi_set_speed,
> +       .set_mode       = fsl_qspi_set_mode,
> +};
> +
> +static const struct udevice_id fsl_qspi_ids[] = {
> +       { .compatible = "fsl,vf610-qspi" },
> +       { .compatible = "fsl,imx6sx-qspi" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(fsl_qspi) = {
> +       .name = "fsl_qspi",
> +       .id = UCLASS_SPI,
> +       .of_match = fsl_qspi_ids,
> +       .ops = &fsl_qspi_ops,
> +       .ofdata_to_platdata = fsl_qspi_ofdata_to_platdata,
> +       .platdata_auto_alloc_size = sizeof(struct fsl_qspi_platdata),
> +       .priv_auto_alloc_size = sizeof(struct fsl_qspi),
> +       .per_child_auto_alloc_size = sizeof(struct spi_slave),
> +       .probe = fsl_qspi_probe,
> +       .child_pre_probe = fsl_qspi_child_pre_probe,
> +};
> +
> +#else
> +static unsigned long spi_bases[] = {
> +       QSPI0_BASE_ADDR,
> +#ifdef CONFIG_MX6SX
> +       QSPI1_BASE_ADDR,
> +#endif
> +};
> +
> +static unsigned long amba_bases[] = {
> +       QSPI0_AMBA_BASE,
> +#ifdef CONFIG_MX6SX
> +       QSPI1_AMBA_BASE,
> +#endif
> +};
> +
> +static inline struct fsl_qspi *to_qspi_spi(struct spi_slave *slave)
> +{
> +       return container_of(slave, struct fsl_qspi, slave);
> +}
> +
>  void spi_init()
>  {
>         /* do nothing */
> @@ -782,3 +1169,4 @@ void spi_release_bus(struct spi_slave *slave)
>  {
>         /* Nothing to do */
>  }
> +#endif
> diff --git a/drivers/spi/fsl_qspi.h b/drivers/spi/fsl_qspi.h
> index 6cb3610..ef4d4b5 100644
> --- a/drivers/spi/fsl_qspi.h
> +++ b/drivers/spi/fsl_qspi.h
> @@ -104,6 +104,7 @@ struct fsl_qspi_regs {
>
>  #define QSPI_RBCT_RXBRD_SHIFT          8
>  #define QSPI_RBCT_RXBRD_USEIPS         (1 << QSPI_RBCT_RXBRD_SHIFT)
> +#define QSPI_RBCT_WMRK_MASK            0x1F
>
>  #define QSPI_SR_BUSY_SHIFT             0
>  #define QSPI_SR_BUSY_MASK              (1 << QSPI_SR_BUSY_SHIFT)
> --
> 1.8.4
>
>

Regards,
Simon


More information about the U-Boot mailing list