[U-Boot] [PATCH v1 3/3] spi: mxs_spi: DM conversion
Jagan Teki
jagannadh.teki at gmail.com
Fri Apr 27 06:40:46 UTC 2018
+ add relevant board maintainers
On Thu, Apr 26, 2018 at 10:20 PM, Akash Gajjar <gajjar04akash at 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 at 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 at openedev.com>
> + *
> * Copyright (C) 2011 Marek Vasut <marek.vasut at 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 at openedev.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + *
no space.
Jagan.
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
More information about the U-Boot
mailing list