[U-Boot] [U-Boot, v4] spi: add Faraday FTSPI010 SPI controller support

[Jagan Teki <jagannadh.teki@gmail.com>]

Hi,

Few comments, please get back your inputs.

Use commit header as "spi: ftssp010_spi: "

On 07-05-2013 12:04, Kuo-Jung Su wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>
> The Faraday FTSSP010 is a multi-function controller
> which supports I2S/SPI/SSP/AC97/SPDIF. However This
> patch implements only the SPI mode.
>
> NOTE:
> The DMA and CS/Clock control logic has been altered
> since hardware revision 1.19.0. So this patch
> would first detects the revision id of the underlying
> chip, and then switch to the corresponding software
> control routines.
>
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> CC: Tom Rini <trini@ti.com>
>
> ---
> Changes for v4:
>     - Coding Style cleanup.
>     - Make it a separate patch, rather then a part of
>       Faraday A36x patch series
>     - Use macro constants for timeout control
>
> Changes for v3:
>     - Coding Style cleanup.
>     - Drop macros for wirtel()/readl(), call them directly.
>     - Always insert a blank line between declarations and code.
>     - Replace all the infinite wait loop with a timeout.
>     - Add '__iomem' to all the declaration of HW register pointers.
>
> Changes for v2:
>     - Coding Style cleanup.
>     - Use readl(), writel(), clrsetbits_le32() to replace REG() macros.
>     - Use structure based hardware registers to replace the macro constants.
>     - Replace BIT() with BIT_MASK().
>
>   drivers/spi/Makefile        |    1 +
>   drivers/spi/ftssp010_spi.c  |  385 +++++++++++++++++++++++++++++++++++++++++++
>   drivers/spi/ftssp010_spi.h  |   86 ++++++++++
>   include/faraday/ftgpio010.h |   25 +++
>   4 files changed, 497 insertions(+)
>   create mode 100644 drivers/spi/ftssp010_spi.c
>   create mode 100644 drivers/spi/ftssp010_spi.h
>   create mode 100644 include/faraday/ftgpio010.h
>
> --
> 1.7.9.5
>
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index d08609e..947d60e 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_BFIN_SPI6XX) += bfin_spi6xx.o
>   COBJS-$(CONFIG_CF_SPI) += cf_spi.o
>   COBJS-$(CONFIG_CF_QSPI) += cf_qspi.o
>   COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
> +COBJS-$(CONFIG_FTSSP010_SPI) += ftssp010_spi.o

Place into in alphabetic order, to make sure some kind of coding style.

>   COBJS-$(CONFIG_EXYNOS_SPI) += exynos_spi.o
>   COBJS-$(CONFIG_ICH_SPI) +=  ich.o
>   COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
> diff --git a/drivers/spi/ftssp010_spi.c b/drivers/spi/ftssp010_spi.c
> new file mode 100644
> index 0000000..d401ecc
> --- /dev/null
> +++ b/drivers/spi/ftssp010_spi.c
> @@ -0,0 +1,385 @@
> +/*
> + * Faraday Multi-function Controller - SPI Mode
> + *
> + * (C) Copyright 2010 Faraday Technology
> + * Dante Su <dantesu@faraday-tech.com>
> + *
> + * This file is released under the terms of GPL v2 and any later version.
> + * See the file COPYING in the root directory of the source tree for details.
> + */
Little bit uneasy with the above license note, any reason for this GPL 
w.r.t your company style. refer license note on drivers/spi/exynos_spi.c

> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <spi.h>
> +#include <malloc.h>
> +#include <faraday/ftgpio010.h>
Does this include file is an arch' specific, means struct ftgpio010_regs
is used by some other drivers on ur board?

> +#include "ftssp010_spi.h"
Please don't use extra include file, use the same structure's in driver 
it self, IMHO.

> +
> +#define CFG_PIO_TIMEOUT (CONFIG_SYS_HZ >> 3) /* 125 ms */
> +#define CFG_CS_TIMEOUT  (CONFIG_SYS_HZ >> 2) /* 250 ms */
> +

----- TAG+
> +struct ftssp010_chip {
> +	void __iomem *regs;
> +	uint32_t fifo;
> +	uint32_t rev;
> +	uint32_t div;
> +	uint32_t mode;
> +
> +	struct {
> +		void __iomem *regs;
> +		uint32_t      pin;
> +	} gpio;
> +};
> +
> +static struct ftssp010_chip chip_list[] = {
> +#ifdef CONFIG_FTSSP010_BASE
> +	{
> +		.regs = (void __iomem *)CONFIG_FTSSP010_BASE,
> +# ifdef CONFIG_FTSSP010_GPIO_BASE
> +		.gpio = {
> +			(void __iomem *)CONFIG_FTSSP010_GPIO_BASE,
> +			CONFIG_FTSSP010_GPIO_PIN
> +		},
> +# endif
> +	},
> +#endif /* #ifdef CONFIG_FTSSP010_BASE */
> +#ifdef CONFIG_FTSSP010_BASE1
> +	{ .regs = (void __iomem *)CONFIG_FTSSP010_BASE1, },
> +# ifdef CONFIG_FTSSP010_GPIO_BASE1
> +		.gpio = {
> +			(void __iomem *)CONFIG_FTSSP010_GPIO_BASE1,
> +			CONFIG_FTSSP010_GPIO_PIN1
> +		},
> +# endif
> +#endif
> +#ifdef CONFIG_FTSSP010_BASE2
> +	{ .regs = (void __iomem *)CONFIG_FTSSP010_BASE2, },
> +# ifdef CONFIG_FTSSP010_GPIO_BASE2
> +		.gpio = {
> +			(void __iomem *)CONFIG_FTSSP010_GPIO_BASE2,
> +			CONFIG_FTSSP010_GPIO_PIN2
> +		},
> +# endif
> +#endif
> +#ifdef CONFIG_FTSSP010_BASE3
> +	{ .regs = (void __iomem *)CONFIG_FTSSP010_BASE3, },
> +# ifdef CONFIG_FTSSP010_GPIO_BASE3
> +		.gpio = {
> +			(void __iomem *)CONFIG_FTSSP010_GPIO_BASE3,
> +			CONFIG_FTSSP010_GPIO_PIN3
> +		},
> +# endif
> +#endif
> +};
---- TAG-

TAG+ ... TAG- this type of assignment is old way.
please use the structure macro assignment instead of void __iomem *

ex:
#define CONFIG_MY_REG_BASE	0xF1000000

struct my_reg {
	u32 r1;
	u32 r2;
	u32 r3;
	u32 r4;
}

#define my_reg_base ((struct my_reg *) CONFIG_MY_REG_BASE)

then use readl(&my_reg_base->r3);

see the example usage on
http://patchwork.ozlabs.org/patch/246468/
https://github.com/Xilinx/u-boot-xlnx/blob/master/drivers/spi/zynq_qspips.c

> +
> +static inline int ftssp010_wait_tx(struct ftssp010_chip *chip)
> +{
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +	int ret = -1;
> +	ulong ts;
> +
> +	for (ts = get_timer(0); get_timer(ts) < CFG_PIO_TIMEOUT; ) {
> +		if (!(readl(&regs->sr) & SR_TFNF))
> +			continue;
> +		ret = 0;
> +		break;
> +	}
> +
> +	if (ret)
> +		printf("ftssp010: tx timeout\n");
> +
> +	return ret;
> +}
> +
> +static inline int ftssp010_wait_rx(struct ftssp010_chip *chip)
> +{
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +	int ret = -1;
> +	ulong ts;
> +
> +	for (ts = get_timer(0); get_timer(ts) < CFG_PIO_TIMEOUT; ) {
> +		if (!SR_RFVE(readl(&regs->sr)))
> +			continue;
> +		ret = 0;
> +		break;
> +	}
> +
> +	if (ret)
> +		printf("ftssp010: rx timeout\n");
> +
> +	return ret;
> +}
> +
> +static int ftssp010_spi_work_transfer_v1_19(struct ftssp010_chip *chip,
> +	const void *tx_buf, void *rx_buf, int len, uint flags)
> +{
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +	const uint8_t *txb = tx_buf;
> +	uint8_t       *rxb = rx_buf;
> +
> +	while (len > 0) {
> +		int i, depth = min(chip->fifo >> 2, len);
> +		uint32_t xmsk = 0;
> +
> +		if (tx_buf) {
> +			for (i = 0; i < depth; ++i) {
> +				ftssp010_wait_tx(chip);
> +				writel(*txb++, &regs->dr);
> +			}
> +			xmsk |= CR2_TXEN | CR2_TXDOE;
> +			if ((readl(&regs->cr[2]) & xmsk) != xmsk)
> +				setbits_le32(&regs->cr[2], xmsk);
> +		}
> +		if (rx_buf) {
> +			xmsk |= CR2_RXEN;
> +			if ((readl(&regs->cr[2]) & xmsk) != xmsk)
> +				setbits_le32(&regs->cr[2], xmsk);
> +			for (i = 0; i < depth; ++i) {
> +				ftssp010_wait_rx(chip);
> +				*rxb++ = (uint8_t)readl(&regs->dr);
> +			}
> +		}
> +
> +		len -= depth;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ftssp010_spi_work_transfer(struct ftssp010_chip *chip,
> +	const void *tx_buf, void *rx_buf, int len, uint flags)
> +{
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +	const uint8_t *txb = tx_buf;
> +	uint8_t       *rxb = rx_buf;
> +
> +	while (len > 0) {
> +		int i, depth = min(chip->fifo >> 2, len);
> +		uint32_t tmp;
> +
> +		for (i = 0; i < depth; ++i) {
> +			ftssp010_wait_tx(chip);
> +			writel(txb ? (*txb++) : 0, &regs->dr);
> +		}
> +		for (i = 0; i < depth; ++i) {
> +			ftssp010_wait_rx(chip);
> +			tmp = readl(&regs->dr);
> +			if (rxb)
> +				*rxb++ = (uint8_t)tmp;
> +		}
> +
> +		len -= depth;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Determine if a SPI chipselect is valid.
> + * This function is provided by the board if the low-level SPI driver
> + * needs it to determine if a given chipselect is actually valid.
> + *
> + * Returns: 1 if bus:cs identifies a valid chip on this board, 0
> + * otherwise.
> + */
> +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> +{
> +	int ret = 0;
> +	struct ftssp010_chip *chip;
> +	struct ftssp010_regs __iomem *regs;
> +	uint32_t txfifo, rxfifo;
> +
> +	if (bus >= ARRAY_SIZE(chip_list))
> +		return ret;
> +
> +	chip = chip_list + bus;
> +	regs = chip->regs;
> +	chip->rev = readl(&regs->revr);
> +	txfifo = FEAR_TXFIFO(readl(&regs->fear));
> +	rxfifo = FEAR_RXFIFO(readl(&regs->fear));
> +	chip->fifo = min(txfifo, rxfifo);
> +
> +	debug("ftssp010: rev=0x%08X, fifo=%d\n", chip->rev, chip->fifo);
> +
> +	if (chip->rev >= 0x00011900) {
> +		if (cs < 4)
> +			ret = 1;
> +	} else if (!cs) {
> +		struct ftgpio010_regs *gpio = chip->gpio.regs;
> +		uint32_t mask = BIT_MASK(chip->gpio.pin);
> +
> +		if (gpio) {
> +			/* setup gpio pin as an output pin */
> +			setbits_le32(&gpio->dir, mask);
> +			ret = 1;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Activate a SPI chipselect.
> + * This function is provided by the board code when using a driver
> + * that can't control its chipselects automatically (e.g.
> + * common/soft_spi.c). When called, it should activate the chip select
> + * to the device identified by "slave".
> + */
> +void spi_cs_activate(struct spi_slave *slave)
> +{
> +	struct ftssp010_chip *chip = chip_list + slave->bus;
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +
> +	/* cs pull low */
> +	if (chip->rev >= 0x00011900) {
> +		writel((slave->cs << 10) | CR2_SSPEN | CR2_TXFCLR
> +			| CR2_RXFCLR, &regs->cr[2]);
> +	} else {
> +		struct ftgpio010_regs *gpio = chip->gpio.regs;
> +		uint32_t mask = BIT_MASK(chip->gpio.pin);
> +
> +		if (gpio)
> +			setbits_le32(&gpio->clr, mask);
> +	}
> +	udelay_masked(1);
> +}
> +
> +/*
> + * Deactivate a SPI chipselect.
> + * This function is provided by the board code when using a driver
> + * that can't control its chipselects automatically (e.g.
> + * common/soft_spi.c). When called, it should deactivate the chip
> + * select to the device identified by "slave".
> + */
> +void spi_cs_deactivate(struct spi_slave *slave)
> +{
> +	struct ftssp010_chip *chip = chip_list + slave->bus;
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +	ulong ts;
> +
> +	/* wait until device idle */
> +	for (ts = get_timer(0); get_timer(ts) < CFG_CS_TIMEOUT; ) {
> +		if (readl(&regs->sr) & SR_BUSY)
> +			continue;
> +		break;
> +	}
> +	if (readl(&regs->sr) & SR_BUSY)
> +		printf("ftspi010: busy timeout at cs deactivate\n");
> +
> +	/* cs pull high */
> +	if (chip->rev >= 0x00011900) {
> +		writel((slave->cs << 10) | CR2_FS, &regs->cr[2]);
> +	} else {
> +		struct ftgpio010_regs *gpio = chip->gpio.regs;
> +		uint32_t mask = BIT_MASK(chip->gpio.pin);
> +
> +		if (gpio)
> +			setbits_le32(&gpio->set, mask);
> +	}
> +	udelay_masked(1);
> +}
> +
> +void spi_init(void)
> +{
> +}
> +
> +struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
> +{
> +	uint32_t clk, div;
> +	struct spi_slave *ss;
> +	struct ftssp010_chip *chip;
> +
> +	if (!spi_cs_is_valid(bus, cs))
> +		return NULL;
> +
> +	if (mode != SPI_MODE_0) {
> +		printf("ftssp010: MODE%d is not supported\n", mode);
> +		return NULL;
> +	}
> +

----- TAG+
> +	ss = malloc(sizeof(struct spi_slave));
> +	if (!ss)
> +		return NULL;
> +
> +	ss->bus = bus;
> +	ss->cs  = cs;
----- TAG-
Pleas use spi_alloc_slave() instead of malloc()
see the sample code on "drivers/spi/exynos_spi.c"

> +
> +#ifdef CONFIG_FTSSP010_SCLK
> +	clk = CONFIG_FTSSP010_SCLK;
> +#else
> +	clk = clk_get_rate("SSP");
> +#endif
> +	if (max_hz > 0) {
> +		for (div = 0; div < 0xFFFF; ++div) {
> +			if ((clk / (2 * (div + 1))) <= max_hz)
> +				break;
> +		}
> +	} else {
> +		div = 7;
> +	}
> +
> +	chip = chip_list + bus;
> +	chip->div  = div;
> +	chip->mode = mode;
> +
> +	debug("ftssp010: bus=%d, hz=%u\n", bus, (clk / (2 * (div + 1))));
> +
> +	return ss;
> +}
> +
> +void spi_free_slave(struct spi_slave *slave)
> +{
> +	free(slave);
> +}
> +
> +int spi_claim_bus(struct spi_slave *slave)
> +{
> +	struct ftssp010_chip *chip = chip_list + slave->bus;
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +
> +	writel(CR1_SDL(8) | CR1_CLKDIV(chip->div), &regs->cr[1]);
> +
> +	if (chip->rev >= 0x00011900) {
> +		writel(CR0_OPM_MASTER | CR0_FFMT_SPI | CR0_FSPO | CR0_FLASH,
> +			&regs->cr[0]);
> +		writel(CR2_TXFCLR | CR2_RXFCLR,
> +			&regs->cr[2]);
> +	} else {
> +		writel(CR0_OPM_MASTER | CR0_FFMT_SPI | CR0_FSPO,
> +			&regs->cr[0]);
> +		writel(CR2_TXFCLR | CR2_RXFCLR | CR2_SSPEN | CR2_TXDOE,
> +			&regs->cr[2]);
> +	}
> +
> +	spi_cs_deactivate(slave);
> +
> +	return 0;
> +}
> +
> +void spi_release_bus(struct spi_slave *slave)
> +{
> +	struct ftssp010_chip *chip = chip_list + slave->bus;
> +	struct ftssp010_regs __iomem *regs = chip->regs;
> +
> +	writel(0, &regs->cr[2]);
> +}
> +
> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> +			 const void *dout, void *din, unsigned long flags)
> +{
> +	struct ftssp010_chip *chip = chip_list + slave->bus;
> +	uint32_t len = bitlen >> 3;
> +
> +	if (flags & SPI_XFER_BEGIN)
> +		spi_cs_activate(slave);
> +
> +	if (chip->rev >= 0x00011900)
> +		ftssp010_spi_work_transfer_v1_19(chip, dout, din, len, flags);
> +	else
> +		ftssp010_spi_work_transfer(chip, dout, din, len, flags);
> +
> +	if (flags & SPI_XFER_END)
> +		spi_cs_deactivate(slave);
> +
> +	return 0;
> +}

---- TAG+
> diff --git a/drivers/spi/ftssp010_spi.h b/drivers/spi/ftssp010_spi.h
> new file mode 100644
> index 0000000..5258edc
> --- /dev/null
> +++ b/drivers/spi/ftssp010_spi.h
> @@ -0,0 +1,86 @@
> +/*
> + * Faraday Multi-function Controller - SPI Mode
> + *
> + * (C) Copyright 2010 Faraday Technology
> + * Dante Su <dantesu@faraday-tech.com>
> + *
> + * This file is released under the terms of GPL v2 and any later version.
> + * See the file COPYING in the root directory of the source tree for details.
> + */
> +
> +#ifndef _FTSSP010_H
> +#define _FTSSP010_H
> +
> +/* FTSSP010 HW Registers */
> +struct ftssp010_regs {
> +	uint32_t cr[3];/* control register */
> +	uint32_t sr;   /* status register */
> +	uint32_t icr;  /* interrupt control register */
> +	uint32_t isr;  /* interrupt status register */
> +	uint32_t dr;   /* data register */
> +	uint32_t rsvd[17];
> +	uint32_t revr; /* revision register */
> +	uint32_t fear; /* feature register */
> +};
> +
> +/* Control Register 0  */
> +#define CR0_FFMT_MASK       (7 << 12)
> +#define CR0_FFMT_SSP        (0 << 12)
> +#define CR0_FFMT_SPI        (1 << 12)
> +#define CR0_FFMT_MICROWIRE  (2 << 12)
> +#define CR0_FFMT_I2S        (3 << 12)
> +#define CR0_FFMT_AC97       (4 << 12)
> +#define CR0_FLASH           (1 << 11)
> +#define CR0_FSDIST(x)       (((x) & 0x03) << 8)
> +#define CR0_LBM             (1 << 7)  /* Loopback mode */
> +#define CR0_LSB             (1 << 6)  /* LSB first */
> +#define CR0_FSPO            (1 << 5)  /* Frame sync atcive low */
> +#define CR0_FSJUSTIFY       (1 << 4)
> +#define CR0_OPM_SLAVE       (0 << 2)
> +#define CR0_OPM_MASTER      (3 << 2)
> +#define CR0_OPM_I2S_MSST    (3 << 2)  /* Master stereo mode */
> +#define CR0_OPM_I2S_MSMO    (2 << 2)  /* Master mono mode */
> +#define CR0_OPM_I2S_SLST    (1 << 2)  /* Slave stereo mode */
> +#define CR0_OPM_I2S_SLMO    (0 << 2)  /* Slave mono mode */
> +#define CR0_SCLKPO          (1 << 1)  /* SCLK Remain HIGH */
> +#define CR0_SCLKPH          (1 << 0)  /* Half CLK cycle */
> +
> +/* Control Register 1 */
> +
> +#define CR1_PDL(x)          (((x) & 0xff) << 24) /* padding length */
> +#define CR1_SDL(x)          ((((x) - 1) & 0x1f) << 16) /* data length */
> +#define CR1_CLKDIV(x)       ((x) & 0xffff) /*  clk divider */
> +
> +/* Control Register 2 */
> +#define CR2_FSOS(x)         (((x) & 0x03) << 10)	/* FS/CS Select */
> +#define CR2_FS              (1 << 9)	/* FS/CS Signal Level */
> +#define CR2_TXEN            (1 << 8)	/* Tx Enable */
> +#define CR2_RXEN            (1 << 7)	/* Rx Enable */
> +#define CR2_SSPRST          (1 << 6)	/* SSP reset */
> +#define CR2_TXFCLR          (1 << 3)	/* TX FIFO Clear */
> +#define CR2_RXFCLR          (1 << 2)	/* RX FIFO Clear */
> +#define CR2_TXDOE           (1 << 1)	/* TX Data Output Enable */
> +#define CR2_SSPEN           (1 << 0)	/* SSP Enable */
> +
> +/*
> + * Status Register
> + */
> +#define SR_RFF       (1 << 0) /* receive FIFO full */
> +#define SR_TFNF      (1 << 1) /* transmit FIFO not full */
> +#define SR_BUSY      (1 << 2) /* bus is busy */
> +#define SR_RFVE(reg) (((reg) >> 4) & 0x1f)  /* receive  FIFO valid entries */
> +#define SR_TFVE(reg) (((reg) >> 12) & 0x1f) /* transmit FIFO valid entries */
> +
> +/*
> + * Feature Register
> + */
> +#define FEAR_WIDTH(reg)  ((((reg) >>  0) & 0xff) + 1)
> +#define FEAR_RXFIFO(reg) ((((reg) >>  8) & 0xff) + 1)
> +#define FEAR_TXFIFO(reg) ((((reg) >> 16) & 0xff) + 1)
> +#define FEAR_AC97        (1 << 24)
> +#define FEAR_I2S         (1 << 25)
> +#define FEAR_SPI_MWR     (1 << 26)
> +#define FEAR_SSP         (1 << 27)
> +#define FEAR_SPDIF       (1 << 28)
> +
> +#endif /* EOF */
---- TAG-

Please use the same defination on driver code itself.

Thanks,
Jagan.