[U-Boot,v1,3/3] spi: mxs_spi: DM conversion

This patch adds support for DM to the mxs spi driver.

Some TODOs are left over for later, These would be enhancements to the
original functionality, and can come later.

The legacy functionality is present in this version, so old boards in the tree
is working with legacy SPI driver functionality.

Signed-off-by: Akash Gajjar <akash@openedev.com>
---
 drivers/spi/Kconfig                |  12 +-
 drivers/spi/mxs_spi.c              | 257 +++++++++++++++++++++++--------------
 include/dm/platform_data/spi_mxs.h |  18 +++
 3 files changed, 186 insertions(+), 101 deletions(-)
 create mode 100644 include/dm/platform_data/spi_mxs.h

+ add relevant board maintainers

On Thu, Apr 26, 2018 at 10:20 PM, Akash Gajjar <gajjar04akash@gmail.com> wrote:
> This patch adds support for DM to the mxs spi driver.
>
> Some TODOs are left over for later, These would be enhancements to the
> original functionality, and can come later.
>
> The legacy functionality is present in this version, so old boards in the tree
> is working with legacy SPI driver functionality.
>
> Signed-off-by: Akash Gajjar <akash@openedev.com>
> ---
>  drivers/spi/Kconfig                |  12 +-
>  drivers/spi/mxs_spi.c              | 257 +++++++++++++++++++++++--------------
>  include/dm/platform_data/spi_mxs.h |  18 +++
>  3 files changed, 186 insertions(+), 101 deletions(-)
>  create mode 100644 include/dm/platform_data/spi_mxs.h
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index ec92b84..5d3e152 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -106,6 +106,12 @@ config MVEBU_A3700_SPI
>           used to access the SPI NOR flash on platforms embedding this
>           Marvell IP core.
>
> +config MXS_SPI
> +       bool "MXS SPI Driver"
> +       help
> +         Enable the MXS SPI controller driver. This driver can be used
> +         on the i.MX23 and i.MX28 SoCs.
> +
>  config PIC32_SPI
>         bool "Microchip PIC32 SPI driver"
>         depends on MACH_PIC32
> @@ -299,12 +305,6 @@ config MXC_SPI
>           Enable the MXC SPI controller driver. This driver can be used
>           on various i.MX SoCs such as i.MX31/35/51/6/7.
>
> -config MXS_SPI
> -       bool "MXS SPI Driver"
> -       help
> -         Enable the MXS SPI controller driver. This driver can be used
> -         on the i.MX23 and i.MX28 SoCs.
> -
>  config OMAP3_SPI
>         bool "McSPI driver for OMAP"
>         help
> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> index 790db78..b48ecbf 100644
> --- a/drivers/spi/mxs_spi.c
> +++ b/drivers/spi/mxs_spi.c
> @@ -1,6 +1,9 @@
>  /*
>   * Freescale i.MX28 SPI driver
>   *
> + * Support for device model:
> + * Copyright (C) 2018 Akash Gajjar <akash@openedev.com>
> + *
>   * Copyright (C) 2011 Marek Vasut <marek.vasut@gmail.com>
>   * on behalf of DENX Software Engineering GmbH
>   *
> @@ -20,6 +23,8 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/sys_proto.h>
>  #include <asm/mach-imx/dma.h>
> +#include <dm.h>
> +#include <dm/platform_data/spi_mxs.h>
>
>  #define        MXS_SPI_MAX_TIMEOUT     1000000
>  #define        MXS_SPI_PORT_OFFSET     0x2000
> @@ -28,93 +33,14 @@
>
>  #define MXSSSP_SMALL_TRANSFER  512
>
> -struct mxs_spi_slave {
> -       struct spi_slave        slave;
> -       uint32_t                max_khz;
> -       uint32_t                mode;
> -       struct mxs_ssp_regs     *regs;
> +struct mxs_spi_priv {
> +       struct mxs_ssp_regs *regs;
> +       u32     max_khz;
> +       u32     mode;
> +       u32     bus;
> +       u32     cs;

I think we can't maintain this private structure since we have
platdata both in DT and PLATDATA. just assign regs to plat->regs or
any other members.

>  };
>
> -static inline struct mxs_spi_slave *to_mxs_slave(struct spi_slave *slave)
> -{
> -       return container_of(slave, struct mxs_spi_slave, slave);
> -}
> -
> -void spi_init(void)
> -{
> -}
> -
> -int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> -{
> -       /* MXS SPI: 4 ports and 3 chip selects maximum */
> -       if (!mxs_ssp_bus_id_valid(bus) || cs > 2)
> -               return 0;
> -       else
> -               return 1;
> -}

No, we need to maintain this with .cs_info

> -
> -struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> -                                 unsigned int max_hz, unsigned int mode)
> -{
> -       struct mxs_spi_slave *mxs_slave;
> -
> -       if (!spi_cs_is_valid(bus, cs)) {
> -               printf("mxs_spi: invalid bus %d / chip select %d\n", bus, cs);
> -               return NULL;
> -       }
> -
> -       mxs_slave = spi_alloc_slave(struct mxs_spi_slave, bus, cs);
> -       if (!mxs_slave)
> -               return NULL;
> -
> -       if (mxs_dma_init_channel(MXS_DMA_CHANNEL_AHB_APBH_SSP0 + bus))
> -               goto err_init;
> -
> -       mxs_slave->max_khz = max_hz / 1000;
> -       mxs_slave->mode = mode;
> -       mxs_slave->regs = mxs_ssp_regs_by_bus(bus);
> -
> -       return &mxs_slave->slave;
> -
> -err_init:
> -       free(mxs_slave);
> -       return NULL;
> -}
> -
> -void spi_free_slave(struct spi_slave *slave)
> -{
> -       struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
> -       free(mxs_slave);
> -}
> -
> -int spi_claim_bus(struct spi_slave *slave)
> -{
> -       struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
> -       struct mxs_ssp_regs *ssp_regs = mxs_slave->regs;
> -       uint32_t reg = 0;
> -
> -       mxs_reset_block(&ssp_regs->hw_ssp_ctrl0_reg);
> -
> -       writel((slave->cs << MXS_SSP_CHIPSELECT_SHIFT) |
> -              SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> -              &ssp_regs->hw_ssp_ctrl0);
> -
> -       reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
> -       reg |= (mxs_slave->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
> -       reg |= (mxs_slave->mode & SPI_CPHA) ? SSP_CTRL1_PHASE : 0;
> -       writel(reg, &ssp_regs->hw_ssp_ctrl1);
> -
> -       writel(0, &ssp_regs->hw_ssp_cmd0);
> -
> -       mxs_set_ssp_busclock(slave->bus, mxs_slave->max_khz);
> -
> -       return 0;
> -}
> -
> -void spi_release_bus(struct spi_slave *slave)
> -{
> -}
> -
>  static void mxs_spi_start_xfer(struct mxs_ssp_regs *ssp_regs)
>  {
>         writel(SSP_CTRL0_LOCK_CS, &ssp_regs->hw_ssp_ctrl0_set);
> @@ -127,10 +53,10 @@ static void mxs_spi_end_xfer(struct mxs_ssp_regs *ssp_regs)
>         writel(SSP_CTRL0_IGNORE_CRC, &ssp_regs->hw_ssp_ctrl0_set);
>  }
>
> -static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
> +static int mxs_spi_xfer_pio(struct mxs_spi_priv *priv,
>                         char *data, int length, int write, unsigned long flags)
>  {
> -       struct mxs_ssp_regs *ssp_regs = slave->regs;
> +       struct mxs_ssp_regs *ssp_regs = priv->regs;
>
>         if (flags & SPI_XFER_BEGIN)
>                 mxs_spi_start_xfer(ssp_regs);
> @@ -186,12 +112,12 @@ static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
>         return 0;
>  }
>
> -static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
> +static int mxs_spi_xfer_dma(struct mxs_spi_priv *priv,
>                         char *data, int length, int write, unsigned long flags)
>  {
> +       struct mxs_ssp_regs *ssp_regs = priv->regs;
>         const int xfer_max_sz = 0xff00;
>         const int desc_count = DIV_ROUND_UP(length, xfer_max_sz) + 1;
> -       struct mxs_ssp_regs *ssp_regs = slave->regs;
>         struct mxs_dma_desc *dp;
>         uint32_t ctrl0;
>         uint32_t cache_data_count;
> @@ -230,7 +156,7 @@ static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
>         /* Invalidate the area, so no writeback into the RAM races with DMA */
>         invalidate_dcache_range(dstart, dstart + cache_data_count);
>
> -       dmach = MXS_DMA_CHANNEL_AHB_APBH_SSP0 + slave->slave.bus;
> +       dmach = MXS_DMA_CHANNEL_AHB_APBH_SSP0 + priv->bus;
>
>         dp = desc;
>         while (length) {
> @@ -307,11 +233,10 @@ static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
>         return ret;
>  }
>
> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> +static int __spi_xfer(struct mxs_spi_priv *priv, unsigned int bitlen,
>                 const void *dout, void *din, unsigned long flags)
>  {
> -       struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
> -       struct mxs_ssp_regs *ssp_regs = mxs_slave->regs;
> +       struct mxs_ssp_regs *ssp_regs = priv->regs;
>         int len = bitlen / 8;
>         char dummy;
>         int write = 0;
> @@ -355,9 +280,151 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
>
>         if (!dma || (len < MXSSSP_SMALL_TRANSFER)) {
>                 writel(SSP_CTRL1_DMA_ENABLE, &ssp_regs->hw_ssp_ctrl1_clr);
> -               return mxs_spi_xfer_pio(mxs_slave, data, len, write, flags);
> +               return mxs_spi_xfer_pio(priv, data, len, write, flags);
>         } else {
>                 writel(SSP_CTRL1_DMA_ENABLE, &ssp_regs->hw_ssp_ctrl1_set);
> -               return mxs_spi_xfer_dma(mxs_slave, data, len, write, flags);
> +               return mxs_spi_xfer_dma(priv, data, len, write, flags);
>         }
>  }
> +
> +static int __mxs_spi_setup(struct mxs_spi_priv *mxs_spi, uint bus)

who is calling this?

> +{
> +       struct mxs_spi_priv *priv = mxs_spi;
> +       int err;
> +
> +       priv->max_khz = max_hz / 1000;
> +       priv->mode = mode;
> +       priv->regs = mxs_ssp_regs_by_bus(bus);
> +       priv->bus = bus;
> +
> +       if (mxs_dma_init_channel(MXS_DMA_CHANNEL_AHB_APBH_SSP0 + priv->bus)) {
> +               printf("%s: DMA init channel error %d\n", __func__, err);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +static int mxs_spi_claim_bus(struct udevice *dev)
> +{
> +       struct udevice *bus = dev_get_parent(dev);
> +       struct mxs_spi_priv *priv = dev_get_priv(bus);
> +       struct mxs_ssp_regs *ssp_regs = priv->regs;
> +       struct mxs_spi_platdata *plat = dev_get_platdata(bus);
> +
> +       writel(((plat->cs) << MXS_SSP_CHIPSELECT_SHIFT) |
> +                       SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> +                       &ssp_regs->hw_ssp_ctrl0);
> +
> +       return 0;
> +}
> +
> +static int mxs_spi_release_bus(struct udevice *dev)
> +{
> +       /* TODO */
> +
> +       return 0;
> +}
> +
> +static int mxs_spi_set_speed(struct udevice *bus, uint speed)
> +{
> +       struct mxs_spi_priv *priv = dev_get_priv(bus);
> +       struct mxs_spi_platdata *plat = dev_get_platdata(bus);
> +
> +       if (speed > plat->max_khz)
> +               speed = plat->max_khz;
> +
> +       priv->max_khz = speed;

assign this at last

> +       printf("%s speed %u\n", __func__, speed);
> +

make it debug

> +       mxs_set_ssp_busclock(plat->bus, priv->max_khz);
> +
> +       return 0;
> +}
> +
> +static int mxs_spi_set_mode(struct udevice *bus, uint mode)
> +{
> +       struct mxs_spi_priv *priv = dev_get_priv(bus);
> +       struct mxs_ssp_regs *ssp_regs = priv->regs;
> +       u32 reg;
> +
> +       printf("%s mode %u\n", __func__, mode);
> +       priv->mode = mode;

same comments like .set_speed

> +
> +       reg = SSP_CTRL1_SSP_MODE_SPI | SSP_CTRL1_WORD_LENGTH_EIGHT_BITS;
> +       reg |= (priv->mode & SPI_CPOL) ? SSP_CTRL1_POLARITY : 0;
> +       reg |= (priv->mode & SPI_CPHA) ? SSP_CTRL1_PHASE : 0;
> +       writel(reg, &ssp_regs->hw_ssp_ctrl1);
> +
> +       /* go in idle state */
> +       writel(0, &ssp_regs->hw_ssp_cmd0);
> +
> +       return 0;
> +}
> +
> +static int mxs_spi_xfer(struct udevice *dev, unsigned int bitlen,
> +                       const void *dout, void *din, unsigned long flags)
> +{
> +       struct udevice *bus = dev_get_parent(dev);
> +       struct mxs_spi_priv *priv = dev_get_priv(bus);
> +
> +       return __spi_xfer(priv, bitlen, dout, din, flags);

Don't use another function, place the old code.

> +}
> +
> +static int mxs_spi_probe(struct udevice *dev)
> +{
> +       struct mxs_spi_platdata *plat = dev_get_platdata(dev);
> +       struct mxs_spi_priv *priv = dev_get_priv(bus);
> +       struct mxs_spi_priv *priv = mxs_spi;
> +       int err;
> +
> +       priv->max_khz = max_hz / 1000;
> +       priv->mode = mode;
> +       priv->bus = bus;
> +       priv->regs = mxs_ssp_regs_by_bus(bus);

These will go out, if we retrive platdata, and remember don't use
mxs_ssp_regs_by_bus you have get this from DT.

> +
> +       if (mxs_dma_init_channel(MXS_DMA_CHANNEL_AHB_APBH_SSP0 + priv->bus)) {
> +               printf("%s: DMA init channel error %d\n", __func__, err);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct dm_spi_ops mxs_spi_ops = {
> +       .claim_bus      = mxs_spi_claim_bus,
> +       .release_bus    = mxs_spi_release_bus,
> +       .xfer           = mxs_spi_xfer,
> +       .set_speed      = mxs_spi_set_speed,
> +       .set_mode       = mxs_spi_set_mode,
> +};
> +
> +#if CONFIG_IS_ENABLED(OF_CONTROL)
> +static int mxs_ofdata_to_platadata(struct udevice *bus)
> +{
> +       struct mxs_spi_platdata *plat = bus->platdata;
> +
> +       plat->cs = fdtdec_get_int(gd->fdt_blob,
> +                       dev_of_offset(bus), "num-cs", 4);

Missing other plat members assignment?

> +
> +       return 0;
> +}
> +
> +static const struct udevice_id mxs_spi_ids[] = {
> +       { .compatible = "fsl,mxs-spi" },

wrong use, use Linux driver binding?

> +       { }
> +};
> +#endif
> +
> +U_BOOT_DRIVER(mxs_spi) = {
> +       .name   = "mxs_spi",
> +       .id     = UCLASS_SPI,
> +#if CONFIG_IS_ENABLED(OF_CONTROL)
> +       .of_match = mxs_spi_ids,
> +       .ofdata_to_platdata = mxs_ofdata_to_platadata,
> +       .platdata_auto_alloc_size = sizeof(struct mxs_spi_platdata),
> +#endif
> +       .ops    = &mxs_spi_ops,
> +       .priv_auto_alloc_size = sizeof(struct mxs_spi_priv),
> +       .probe  = mxs_spi_probe,
> +};
> diff --git a/include/dm/platform_data/spi_mxs.h b/include/dm/platform_data/spi_mxs.h
> new file mode 100644
> index 0000000..5164834
> --- /dev/null
> +++ b/include/dm/platform_data/spi_mxs.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2018 Akash Gajjar <akash@openedev.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + *

no space.

Jagan.