[U-Boot] [PATCH v3 3/3] spi: mxs_spi: DM conversion

[Marek Vasut <marek.vasut@gmail.com>]

On 05/10/2018 04:17 PM, Akash Gajjar wrote:
> This patch adds support for DM driver model 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 removed in
> this version.
> 
> Signed-off-by: Akash Gajjar <akash@openedev.com>
> ---
> changes in v2:
>  Register cs_info method 
>  Remove unused function __mxs_spi_setup
>  Merged __spi_xfer function to spi_xfer
>  printf replaced by debug
> 
> changes in v3:
>  changes made on top of previous patch is merged in v3
> ---
>  drivers/spi/Kconfig                |  12 +-
>  drivers/spi/mxs_spi.c              | 258 +++++++++++++++++++++++--------------
>  include/dm/platform_data/spi_mxs.h |  18 +++
>  3 files changed, 183 insertions(+), 105 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..3054438 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;

Type cleanup should be a separate patch

>  };
>  
> -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;
> -}
> -
> -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);
> @@ -156,7 +82,7 @@ static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
>  
>  		if (mxs_wait_mask_set(&ssp_regs->hw_ssp_ctrl0_reg,
>  			SSP_CTRL0_RUN, MXS_SPI_MAX_TIMEOUT)) {
> -			printf("MXS SPI: Timeout waiting for start\n");
> +			debug("MXS SPI: Timeout waiting for start\n");

printf , we don't want to hide errors

>  			return -ETIMEDOUT;
>  		}
>  
> @@ -168,7 +94,7 @@ static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
>  		if (!write) {
>  			if (mxs_wait_mask_clr(&ssp_regs->hw_ssp_status_reg,
>  				SSP_STATUS_FIFO_EMPTY, MXS_SPI_MAX_TIMEOUT)) {
> -				printf("MXS SPI: Timeout waiting for data\n");
> +				debug("MXS SPI: Timeout waiting for data\n");

DTTO

>  				return -ETIMEDOUT;
>  			}
>  
> @@ -178,7 +104,7 @@ static int mxs_spi_xfer_pio(struct mxs_spi_slave *slave,
>  
>  		if (mxs_wait_mask_clr(&ssp_regs->hw_ssp_ctrl0_reg,
>  			SSP_CTRL0_RUN, MXS_SPI_MAX_TIMEOUT)) {
> -			printf("MXS SPI: Timeout waiting for finish\n");
> +			debug("MXS SPI: Timeout waiting for finish\n");

DTTO

>  			return -ETIMEDOUT;
>  		}
>  	}
> @@ -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,67 @@ static int mxs_spi_xfer_dma(struct mxs_spi_slave *slave,
>  	return ret;
>  }
>  
> -int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> -		const void *dout, void *din, unsigned long flags)
> +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) |

plat->cs doesnt need parenthesis

> +			SSP_CTRL0_BUS_WIDTH_ONE_BIT,
> +			&ssp_regs->hw_ssp_ctrl0);
> +
> +	return 0;
> +}
> +
> +static int mxs_spi_release_bus(struct udevice *dev)
>  {
> -	struct mxs_spi_slave *mxs_slave = to_mxs_slave(slave);
> -	struct mxs_ssp_regs *ssp_regs = mxs_slave->regs;
> +	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;
> +	debug("%s speed %u\n", __func__, speed);
> +
> +	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;
> +
> +	priv->mode = mode;
> +	debug("%s mode %u\n", __func__, mode);
> +
> +	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);
> +	struct mxs_ssp_regs *ssp_regs = priv->regs;
>  	int len = bitlen / 8;
>  	char dummy;
>  	int write = 0;
> @@ -355,9 +337,87 @@ 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_cs_info(struct udevice *bus, uint cs,
> +				struct spi_cs_info *info)
> +{
> +	struct mxs_spi_priv *priv = dev_get_priv(bus);
> +
> +	if (cs >= priv->cs) {
> +		printf("%s: no cs %u\n", __func__, cs);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +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);
> +	int err;
> +
> +	priv->max_khz = (plat->max_hz) / 1000;
> +	priv->mode = plat->mode;
> +	priv->bus = plat->bus;
> +
> +	err = mxs_dma_init_channel(MXS_DMA_CHANNEL_AHB_APBH_SSP0 + priv->bus);
> +	if (err) {
> +		debug("%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,
> +	.cs_info 	= mxs_spi_cs_info,
> +};
> +
> +#if CONFIG_IS_ENABLED(OF_CONTROL)
> +static int mxs_ofdata_to_platadata(struct udevice *bus)
> +{
> +	struct mxs_spi_platdata *plat = bus->platdata;
> +	fdt_addr_t addr;
> +
> +	addr = devfdt_get_addr(bus);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	plat->regs = (struct mxs_ssp_regs *)addr;
> +	plat->cs = fdtdec_get_int(gd->fdt_blob,
> +			dev_of_offset(bus), "num-cs", 4);
> +
> +	return 0;
> +}
> +
> +/* TODO: update compatible device tree */
> +static const struct udevice_id mxs_spi_ids[] = {
> +	{ .compatible = " " },
> +	{ }
> +};
> +#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+
> + *
> + */
> +
> +#ifndef __SPI_MXS_H
> +#define __SPI_MXS_H
> +
> +struct mxs_spi_platdata {
> +	struct mxs_ssp_regs *regs;
> +	u32 bus;
> +	u32 max_hz;
> +	u32 cs;

Why is this header here at all ?

> +};
> +
> +#endif /* __SPI_MXS_H */
> 


-- 
Best regards,
Marek Vasut