All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-18 10:47 ` Laxman Dewangan
  0 siblings, 0 replies; 18+ messages in thread
From: Laxman Dewangan @ 2012-10-18 10:47 UTC (permalink / raw)
  To: broonie, grant.likely, rob.herring
  Cc: spi-devel-general, linux-kernel, linux-tegra, Laxman Dewangan

Tegra20/Tegra30 supports the spi interface through its SLINK
controller. Add spi driver for SLINK controller.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/spi/Kconfig             |    6 +
 drivers/spi/Makefile            |    2 +-
 drivers/spi/spi-tegra20-slink.c | 1356 +++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi-tegra.h   |   40 ++
 4 files changed, 1403 insertions(+), 1 deletions(-)
 create mode 100644 drivers/spi/spi-tegra20-slink.c
 create mode 100644 include/linux/spi/spi-tegra.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 1acae35..25290d9 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -385,6 +385,12 @@ config SPI_MXS
 	help
 	  SPI driver for Freescale MXS devices.
 
+config SPI_TEGRA20_SLINK
+	tristate "Nvidia Tegra20/Tegra30 SLINK Controller"
+	depends on ARCH_TEGRA && TEGRA20_APB_DMA
+	help
+	  SPI driver for Nvidia Tegra20/Tegra30 SLINK Controller interface.
+
 config SPI_TI_SSP
 	tristate "TI Sequencer Serial Port - SPI Support"
 	depends on MFD_TI_SSP
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index c48df47..f87c0f1 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -60,10 +60,10 @@ obj-$(CONFIG_SPI_SH_MSIOF)		+= spi-sh-msiof.o
 obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
 obj-$(CONFIG_SPI_SIRF)		+= spi-sirf.o
 obj-$(CONFIG_SPI_STMP3XXX)		+= spi-stmp.o
+obj-$(CONFIG_SPI_TEGRA20_SLINK)		+= spi-tegra20-slink.o
 obj-$(CONFIG_SPI_TI_SSP)		+= spi-ti-ssp.o
 obj-$(CONFIG_SPI_TLE62X0)		+= spi-tle62x0.o
 obj-$(CONFIG_SPI_TOPCLIFF_PCH)		+= spi-topcliff-pch.o
 obj-$(CONFIG_SPI_TXX9)			+= spi-txx9.o
 obj-$(CONFIG_SPI_XCOMM)		+= spi-xcomm.o
 obj-$(CONFIG_SPI_XILINX)		+= spi-xilinx.o
-
diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
new file mode 100644
index 0000000..c8705d9
--- /dev/null
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -0,0 +1,1356 @@
+/*
+ * SPI driver for Nvidia's Tegra20/Tegra30 SLINK Controller.
+ *
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-tegra.h>
+#include <mach/clk.h>
+
+#define SLINK_COMMAND			0x000
+#define SLINK_BIT_LENGTH(x)		(((x) & 0x1f) << 0)
+#define SLINK_WORD_SIZE(x)		(((x) & 0x1f) << 5)
+#define SLINK_BOTH_EN			(1 << 10)
+#define SLINK_CS_SW			(1 << 11)
+#define SLINK_CS_VALUE			(1 << 12)
+#define SLINK_CS_POLARITY		(1 << 13)
+#define SLINK_IDLE_SDA_DRIVE_LOW	(0 << 16)
+#define SLINK_IDLE_SDA_DRIVE_HIGH	(1 << 16)
+#define SLINK_IDLE_SDA_PULL_LOW		(2 << 16)
+#define SLINK_IDLE_SDA_PULL_HIGH	(3 << 16)
+#define SLINK_IDLE_SDA_MASK		(3 << 16)
+#define SLINK_CS_POLARITY1		(1 << 20)
+#define SLINK_CK_SDA			(1 << 21)
+#define SLINK_CS_POLARITY2		(1 << 22)
+#define SLINK_CS_POLARITY3		(1 << 23)
+#define SLINK_IDLE_SCLK_DRIVE_LOW	(0 << 24)
+#define SLINK_IDLE_SCLK_DRIVE_HIGH	(1 << 24)
+#define SLINK_IDLE_SCLK_PULL_LOW	(2 << 24)
+#define SLINK_IDLE_SCLK_PULL_HIGH	(3 << 24)
+#define SLINK_IDLE_SCLK_MASK		(3 << 24)
+#define SLINK_M_S			(1 << 28)
+#define SLINK_WAIT			(1 << 29)
+#define SLINK_GO			(1 << 30)
+#define SLINK_ENB			(1 << 31)
+
+#define SLINK_MODES			(SLINK_IDLE_SCLK_MASK | SLINK_CK_SDA)
+
+#define SLINK_COMMAND2			0x004
+#define SLINK_LSBFE			(1 << 0)
+#define SLINK_SSOE			(1 << 1)
+#define SLINK_SPIE			(1 << 4)
+#define SLINK_BIDIROE			(1 << 6)
+#define SLINK_MODFEN			(1 << 7)
+#define SLINK_INT_SIZE(x)		(((x) & 0x1f) << 8)
+#define SLINK_CS_ACTIVE_BETWEEN		(1 << 17)
+#define SLINK_SS_EN_CS(x)		(((x) & 0x3) << 18)
+#define SLINK_SS_SETUP(x)		(((x) & 0x3) << 20)
+#define SLINK_FIFO_REFILLS_0		(0 << 22)
+#define SLINK_FIFO_REFILLS_1		(1 << 22)
+#define SLINK_FIFO_REFILLS_2		(2 << 22)
+#define SLINK_FIFO_REFILLS_3		(3 << 22)
+#define SLINK_FIFO_REFILLS_MASK		(3 << 22)
+#define SLINK_WAIT_PACK_INT(x)		(((x) & 0x7) << 26)
+#define SLINK_SPC0			(1 << 29)
+#define SLINK_TXEN			(1 << 30)
+#define SLINK_RXEN			(1 << 31)
+
+#define SLINK_STATUS			0x008
+#define SLINK_COUNT(val)		(((val) >> 0) & 0x1f)
+#define SLINK_WORD(val)			(((val) >> 5) & 0x1f)
+#define SLINK_BLK_CNT(val)		(((val) >> 0) & 0xffff)
+#define SLINK_MODF			(1 << 16)
+#define SLINK_RX_UNF			(1 << 18)
+#define SLINK_TX_OVF			(1 << 19)
+#define SLINK_TX_FULL			(1 << 20)
+#define SLINK_TX_EMPTY			(1 << 21)
+#define SLINK_RX_FULL			(1 << 22)
+#define SLINK_RX_EMPTY			(1 << 23)
+#define SLINK_TX_UNF			(1 << 24)
+#define SLINK_RX_OVF			(1 << 25)
+#define SLINK_TX_FLUSH			(1 << 26)
+#define SLINK_RX_FLUSH			(1 << 27)
+#define SLINK_SCLK			(1 << 28)
+#define SLINK_ERR			(1 << 29)
+#define SLINK_RDY			(1 << 30)
+#define SLINK_BSY			(1 << 31)
+#define SLINK_FIFO_ERROR		(SLINK_TX_OVF | SLINK_RX_UNF |	\
+					SLINK_TX_UNF | SLINK_RX_OVF)
+
+#define SLINK_FIFO_EMPTY		(SLINK_TX_EMPTY | SLINK_RX_EMPTY)
+
+#define SLINK_MAS_DATA			0x010
+#define SLINK_SLAVE_DATA		0x014
+
+#define SLINK_DMA_CTL			0x018
+#define SLINK_DMA_BLOCK_SIZE(x)		(((x) & 0xffff) << 0)
+#define SLINK_TX_TRIG_1			(0 << 16)
+#define SLINK_TX_TRIG_4			(1 << 16)
+#define SLINK_TX_TRIG_8			(2 << 16)
+#define SLINK_TX_TRIG_16		(3 << 16)
+#define SLINK_TX_TRIG_MASK		(3 << 16)
+#define SLINK_RX_TRIG_1			(0 << 18)
+#define SLINK_RX_TRIG_4			(1 << 18)
+#define SLINK_RX_TRIG_8			(2 << 18)
+#define SLINK_RX_TRIG_16		(3 << 18)
+#define SLINK_RX_TRIG_MASK		(3 << 18)
+#define SLINK_PACKED			(1 << 20)
+#define SLINK_PACK_SIZE_4		(0 << 21)
+#define SLINK_PACK_SIZE_8		(1 << 21)
+#define SLINK_PACK_SIZE_16		(2 << 21)
+#define SLINK_PACK_SIZE_32		(3 << 21)
+#define SLINK_PACK_SIZE_MASK		(3 << 21)
+#define SLINK_IE_TXC			(1 << 26)
+#define SLINK_IE_RXC			(1 << 27)
+#define SLINK_DMA_EN			(1 << 31)
+
+#define SLINK_STATUS2			0x01c
+#define SLINK_TX_FIFO_EMPTY_COUNT(val)	(((val) & 0x3f) >> 0)
+#define SLINK_RX_FIFO_FULL_COUNT(val)	(((val) & 0x3f0000) >> 16)
+#define SLINK_SS_HOLD_TIME(val)		(((val) & 0xF) << 6)
+
+#define SLINK_TX_FIFO			0x100
+#define SLINK_RX_FIFO			0x180
+
+#define DATA_DIR_TX			(1 << 0)
+#define DATA_DIR_RX			(1 << 1)
+
+#define SLINK_DMA_TIMEOUT		(msecs_to_jiffies(1000))
+
+#define DEFAULT_SPI_DMA_BUF_LEN		(16*1024)
+#define TX_FIFO_EMPTY_COUNT_MAX		SLINK_TX_FIFO_EMPTY_COUNT(0x20)
+#define RX_FIFO_FULL_COUNT_ZERO		SLINK_RX_FIFO_FULL_COUNT(0)
+
+#define SLINK_STATUS2_RESET \
+	(TX_FIFO_EMPTY_COUNT_MAX | RX_FIFO_FULL_COUNT_ZERO << 16)
+
+#define MAX_CHIP_SELECT			4
+#define SLINK_FIFO_DEPTH		32
+
+struct tegra_slink_chip_data {
+	bool cs_hold_time;
+};
+
+struct tegra_slink_data {
+	struct device				*dev;
+	struct spi_master			*master;
+	const struct tegra_slink_chip_data	*chip_data;
+	spinlock_t				lock;
+
+	struct clk				*clk;
+	void __iomem				*base;
+	phys_addr_t				phys;
+	unsigned				irq;
+	int					dma_req_sel;
+	u32					spi_max_frequency;
+	u32					cur_speed;
+
+	struct spi_device			*cur_spi;
+	unsigned				cur_pos;
+	unsigned				cur_len;
+	unsigned				words_per_32bit;
+	unsigned				bytes_per_word;
+	unsigned				curr_dma_words;
+	unsigned				cur_direction;
+
+	unsigned				cur_rx_pos;
+	unsigned				cur_tx_pos;
+
+	unsigned				dma_buf_size;
+	unsigned				max_buf_size;
+	bool					is_curr_dma_xfer;
+	bool					is_hw_based_cs;
+
+	struct completion			rx_dma_complete;
+	struct completion			tx_dma_complete;
+
+	u32					tx_status;
+	u32					rx_status;
+	u32					status_reg;
+	bool					is_packed;
+	unsigned long				packed_size;
+
+	u32					command_reg;
+	u32					command2_reg;
+	u32					dma_control_reg;
+	u32					def_command_reg;
+	u32					def_command2_reg;
+
+	struct completion			xfer_completion;
+	struct spi_transfer			*curr_xfer;
+	struct dma_chan				*rx_dma_chan;
+	u32					*rx_dma_buf;
+	dma_addr_t				rx_dma_phys;
+	struct dma_async_tx_descriptor		*rx_dma_desc;
+
+	struct dma_chan				*tx_dma_chan;
+	u32					*tx_dma_buf;
+	dma_addr_t				tx_dma_phys;
+	struct dma_async_tx_descriptor		*tx_dma_desc;
+};
+
+static inline unsigned long tegra_slink_readl(struct tegra_slink_data *tspi,
+		unsigned long reg)
+{
+	return readl(tspi->base + reg);
+}
+
+static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
+		unsigned long val, unsigned long reg)
+{
+	writel(val, tspi->base + reg);
+
+	/* Read back register to make sure that register writes completed */
+	if (reg != SLINK_TX_FIFO)
+		readl(tspi->base + SLINK_MAS_DATA);
+}
+
+static inline int tegra_slink_clk_disable(struct tegra_slink_data *tspi)
+{
+	/* Flush all write which are in PPSB queue by reading back */
+	tegra_slink_readl(tspi, SLINK_MAS_DATA);
+
+	clk_disable_unprepare(tspi->clk);
+	return 0;
+}
+
+static inline int tegra_slink_clk_enable(struct tegra_slink_data *tspi)
+{
+	clk_prepare_enable(tspi->clk);
+	return 0;
+}
+
+static void tegra_slink_clear_status(struct tegra_slink_data *tspi)
+{
+	unsigned long val;
+	unsigned long val_write = 0;
+
+	val = tegra_slink_readl(tspi, SLINK_STATUS);
+
+	/* Write 1 to clear status register */
+	val_write = SLINK_RDY;
+	val_write |= (val & SLINK_FIFO_ERROR);
+	tegra_slink_writel(tspi, val_write, SLINK_STATUS);
+}
+
+static unsigned long tegra_slink_get_packed_size(struct tegra_slink_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned long val;
+
+	switch (tspi->bytes_per_word) {
+	case 0:
+		val = SLINK_PACK_SIZE_4;
+		break;
+	case 1:
+		val = SLINK_PACK_SIZE_8;
+		break;
+	case 2:
+		val = SLINK_PACK_SIZE_16;
+		break;
+	case 4:
+		val = SLINK_PACK_SIZE_32;
+		break;
+	default:
+		val = 0;
+	}
+	return val;
+}
+
+static unsigned tegra_slink_calculate_curr_xfer_param(
+	struct spi_device *spi, struct tegra_slink_data *tspi,
+	struct spi_transfer *t)
+{
+	unsigned remain_len = t->len - tspi->cur_pos;
+	unsigned max_word;
+	unsigned bits_per_word ;
+	unsigned max_len;
+	unsigned total_fifo_words;
+
+	bits_per_word = t->bits_per_word ? t->bits_per_word :
+						spi->bits_per_word;
+	tspi->bytes_per_word = (bits_per_word - 1) / 8 + 1;
+
+	if (bits_per_word == 8 || bits_per_word == 16) {
+		tspi->is_packed = 1;
+		tspi->words_per_32bit = 32/bits_per_word;
+	} else {
+		tspi->is_packed = 0;
+		tspi->words_per_32bit = 1;
+	}
+	tspi->packed_size = tegra_slink_get_packed_size(tspi, t);
+
+	if (tspi->is_packed) {
+		max_len = min(remain_len, tspi->max_buf_size);
+		tspi->curr_dma_words = max_len/tspi->bytes_per_word;
+		total_fifo_words = max_len/4;
+	} else {
+		max_word = (remain_len - 1) / tspi->bytes_per_word + 1;
+		max_word = min(max_word, tspi->max_buf_size/4);
+		tspi->curr_dma_words = max_word;
+		total_fifo_words = max_word;
+	}
+	return total_fifo_words;
+}
+
+static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
+	struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned nbytes;
+	unsigned tx_empty_count;
+	unsigned long fifo_status;
+	unsigned max_n_32bit;
+	unsigned i, count;
+	unsigned long x;
+	unsigned int written_words;
+	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
+
+	fifo_status = tegra_slink_readl(tspi, SLINK_STATUS2);
+	tx_empty_count = SLINK_TX_FIFO_EMPTY_COUNT(fifo_status);
+
+	if (tspi->is_packed) {
+		nbytes = tspi->curr_dma_words * tspi->bytes_per_word;
+		max_n_32bit = (min(nbytes,  tx_empty_count*4) - 1)/4 + 1;
+		for (count = 0; count < max_n_32bit; ++count) {
+			x = 0;
+			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
+				x |= (*tx_buf++) << (i*8);
+			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
+		}
+		written_words =  min(max_n_32bit * tspi->words_per_32bit,
+					tspi->curr_dma_words);
+	} else {
+		max_n_32bit = min(tspi->curr_dma_words,  tx_empty_count);
+		nbytes = max_n_32bit * tspi->bytes_per_word;
+		for (count = 0; count < max_n_32bit; ++count) {
+			x = 0;
+			for (i = 0; nbytes && (i < tspi->bytes_per_word);
+							++i, nbytes--)
+				x |= ((*tx_buf++) << i*8);
+			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
+		}
+		written_words = max_n_32bit;
+	}
+	tspi->cur_tx_pos += written_words * tspi->bytes_per_word;
+	return written_words;
+}
+
+static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned rx_full_count;
+	unsigned long fifo_status;
+	unsigned i, count;
+	unsigned long x;
+	unsigned int read_words = 0;
+	unsigned len;
+	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_rx_pos;
+
+	fifo_status = tegra_slink_readl(tspi, SLINK_STATUS2);
+	rx_full_count = SLINK_RX_FIFO_FULL_COUNT(fifo_status);
+	if (tspi->is_packed) {
+		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		for (count = 0; count < rx_full_count; ++count) {
+			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
+			for (i = 0; len && (i < 4); ++i, len--)
+				*rx_buf++ = (x >> i*8) & 0xFF;
+		}
+		tspi->cur_rx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
+		read_words += tspi->curr_dma_words;
+	} else {
+		unsigned int rx_mask, bits_per_word;
+
+		bits_per_word = t->bits_per_word ? t->bits_per_word :
+						tspi->cur_spi->bits_per_word;
+		rx_mask = (1 << bits_per_word) - 1;
+		for (count = 0; count < rx_full_count; ++count) {
+			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
+			x &= rx_mask;
+			for (i = 0; (i < tspi->bytes_per_word); ++i)
+				*rx_buf++ = (x >> (i*8)) & 0xFF;
+		}
+		tspi->cur_rx_pos += rx_full_count * tspi->bytes_per_word;
+		read_words += rx_full_count;
+	}
+	return read_words;
+}
+
+static void tegra_slink_copy_client_txbuf_to_spi_txbuf(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned len;
+
+	/* Make the dma buffer to read by cpu */
+	dma_sync_single_for_cpu(tspi->dev, tspi->tx_dma_phys,
+				tspi->dma_buf_size, DMA_TO_DEVICE);
+
+	if (tspi->is_packed) {
+		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		memcpy(tspi->tx_dma_buf, t->tx_buf + tspi->cur_pos, len);
+	} else {
+		unsigned int i;
+		unsigned int count;
+		u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
+		unsigned consume = tspi->curr_dma_words * tspi->bytes_per_word;
+		unsigned int x;
+
+		for (count = 0; count < tspi->curr_dma_words; ++count) {
+			x = 0;
+			for (i = 0; consume && (i < tspi->bytes_per_word);
+							++i, consume--)
+				x |= ((*tx_buf++) << i*8);
+			tspi->tx_dma_buf[count] = x;
+		}
+	}
+	tspi->cur_tx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
+
+	/* Make the dma buffer to read by dma */
+	dma_sync_single_for_device(tspi->dev, tspi->tx_dma_phys,
+				tspi->dma_buf_size, DMA_TO_DEVICE);
+}
+
+static void tegra_slink_copy_spi_rxbuf_to_client_rxbuf(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned len;
+
+	/* Make the dma buffer to read by cpu */
+	dma_sync_single_for_cpu(tspi->dev, tspi->rx_dma_phys,
+		tspi->dma_buf_size, DMA_FROM_DEVICE);
+
+	if (tspi->is_packed) {
+		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		memcpy(t->rx_buf + tspi->cur_rx_pos, tspi->rx_dma_buf, len);
+	} else {
+		unsigned int i;
+		unsigned int count;
+		unsigned char *rx_buf = t->rx_buf + tspi->cur_rx_pos;
+		unsigned int x;
+		unsigned int rx_mask, bits_per_word;
+
+		bits_per_word = t->bits_per_word ? t->bits_per_word :
+						tspi->cur_spi->bits_per_word;
+		rx_mask = (1 << bits_per_word) - 1;
+		for (count = 0; count < tspi->curr_dma_words; ++count) {
+			x = tspi->rx_dma_buf[count];
+			x &= rx_mask;
+			for (i = 0; (i < tspi->bytes_per_word); ++i)
+				*rx_buf++ = (x >> (i*8)) & 0xFF;
+		}
+	}
+	tspi->cur_rx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
+
+	/* Make the dma buffer to read by dma */
+	dma_sync_single_for_device(tspi->dev, tspi->rx_dma_phys,
+		tspi->dma_buf_size, DMA_FROM_DEVICE);
+}
+
+static void tegra_slink_dma_complete(void *args)
+{
+	struct completion *dma_complete = args;
+
+	complete(dma_complete);
+}
+
+static int tegra_slink_start_tx_dma(struct tegra_slink_data *tspi, int len)
+{
+	INIT_COMPLETION(tspi->tx_dma_complete);
+	tspi->tx_dma_desc = dmaengine_prep_slave_single(tspi->tx_dma_chan,
+				tspi->tx_dma_phys, len, DMA_MEM_TO_DEV,
+				DMA_PREP_INTERRUPT |  DMA_CTRL_ACK);
+	if (!tspi->tx_dma_desc) {
+		dev_err(tspi->dev, "Not able to get desc for Tx\n");
+		return -EIO;
+	}
+
+	tspi->tx_dma_desc->callback = tegra_slink_dma_complete;
+	tspi->tx_dma_desc->callback_param = &tspi->tx_dma_complete;
+
+	dmaengine_submit(tspi->tx_dma_desc);
+	dma_async_issue_pending(tspi->tx_dma_chan);
+	return 0;
+}
+
+static int tegra_slink_start_rx_dma(struct tegra_slink_data *tspi, int len)
+{
+	INIT_COMPLETION(tspi->rx_dma_complete);
+	tspi->rx_dma_desc = dmaengine_prep_slave_single(tspi->rx_dma_chan,
+				tspi->rx_dma_phys, len, DMA_DEV_TO_MEM,
+				DMA_PREP_INTERRUPT |  DMA_CTRL_ACK);
+	if (!tspi->rx_dma_desc) {
+		dev_err(tspi->dev, "Not able to get desc for Rx\n");
+		return -EIO;
+	}
+
+	tspi->rx_dma_desc->callback = tegra_slink_dma_complete;
+	tspi->rx_dma_desc->callback_param = &tspi->rx_dma_complete;
+
+	dmaengine_submit(tspi->rx_dma_desc);
+	dma_async_issue_pending(tspi->rx_dma_chan);
+	return 0;
+}
+
+static int tegra_slink_start_dma_based_transfer(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned long val;
+	unsigned long test_val;
+	unsigned int len;
+	int ret = 0;
+	unsigned long status;
+
+	/* Make sure that Rx and Tx fifo are empty */
+	status = tegra_slink_readl(tspi, SLINK_STATUS);
+	if ((status & SLINK_FIFO_EMPTY) != SLINK_FIFO_EMPTY)
+		dev_err(tspi->dev,
+			"Rx/Tx fifo are not empty status 0x%08lx\n", status);
+
+	val = SLINK_DMA_BLOCK_SIZE(tspi->curr_dma_words - 1);
+	val |= tspi->packed_size;
+	if (tspi->is_packed)
+		len = DIV_ROUND_UP(tspi->curr_dma_words * tspi->bytes_per_word,
+					4) * 4;
+	else
+		len = tspi->curr_dma_words * 4;
+
+	/* Set attention level based on length of transfer */
+	if (len & 0xF)
+		val |= SLINK_TX_TRIG_1 | SLINK_RX_TRIG_1;
+	else if (((len) >> 4) & 0x1)
+		val |= SLINK_TX_TRIG_4 | SLINK_RX_TRIG_4;
+	else
+		val |= SLINK_TX_TRIG_8 | SLINK_RX_TRIG_8;
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		val |= SLINK_IE_TXC;
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		val |= SLINK_IE_RXC;
+
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	tspi->dma_control_reg = val;
+
+	if (tspi->cur_direction & DATA_DIR_TX) {
+		tegra_slink_copy_client_txbuf_to_spi_txbuf(tspi, t);
+		wmb();
+		ret = tegra_slink_start_tx_dma(tspi, len);
+		if (ret < 0) {
+			dev_err(tspi->dev,
+				"Starting tx dma failed, err %d\n", ret);
+			return ret;
+		}
+
+		/* Wait for tx fifo to be fill before starting slink */
+		test_val = tegra_slink_readl(tspi, SLINK_STATUS);
+		while (!(test_val & SLINK_TX_FULL))
+			test_val = tegra_slink_readl(tspi, SLINK_STATUS);
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX) {
+		/* Make the dma buffer to read by dma */
+		dma_sync_single_for_device(tspi->dev, tspi->rx_dma_phys,
+				tspi->dma_buf_size, DMA_FROM_DEVICE);
+
+		ret = tegra_slink_start_rx_dma(tspi, len);
+		if (ret < 0) {
+			dev_err(tspi->dev,
+				"Starting rx dma failed, err %d\n", ret);
+			if (tspi->cur_direction & DATA_DIR_TX)
+				dmaengine_terminate_all(tspi->tx_dma_chan);
+			return ret;
+		}
+	}
+	tspi->is_curr_dma_xfer = true;
+	if (tspi->is_packed) {
+		val |= SLINK_PACKED;
+		tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+		/* HW need small delay after settign Packed mode */
+		udelay(1);
+		wmb();
+	}
+	tspi->dma_control_reg = val;
+
+	val |= SLINK_DMA_EN;
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	return ret;
+}
+
+static int tegra_slink_start_cpu_based_transfer(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned long val;
+	unsigned cur_words;
+
+	val = tspi->packed_size;
+	if (tspi->cur_direction & DATA_DIR_TX)
+		val |= SLINK_IE_TXC;
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		val |= SLINK_IE_RXC;
+
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	tspi->dma_control_reg = val;
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		cur_words = tegra_slink_fill_tx_fifo_from_client_txbuf(tspi, t);
+	else
+		cur_words = tspi->curr_dma_words;
+	val |= SLINK_DMA_BLOCK_SIZE(cur_words - 1);
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	tspi->dma_control_reg = val;
+
+	tspi->is_curr_dma_xfer = false;
+	if (tspi->is_packed) {
+		val |= SLINK_PACKED;
+		tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+		udelay(1);
+		wmb();
+	}
+	tspi->dma_control_reg = val;
+	val |= SLINK_DMA_EN;
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	return 0;
+}
+static int tegra_slink_init_dma_param(struct tegra_slink_data *tspi,
+			bool dma_to_memory)
+{
+	struct dma_chan *dma_chan;
+	u32 *dma_buf;
+	dma_addr_t dma_phys;
+	int ret;
+	struct dma_slave_config dma_sconfig;
+	dma_cap_mask_t mask;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	dma_chan = dma_request_channel(mask, NULL, NULL);
+	if (!dma_chan) {
+		dev_err(tspi->dev,
+			"Dma channel is not available, will try later\n");
+		return -EPROBE_DEFER;
+	}
+
+	dma_buf = dma_alloc_coherent(tspi->dev, tspi->dma_buf_size,
+				&dma_phys, GFP_KERNEL);
+	if (!dma_buf) {
+		dev_err(tspi->dev, " Not able to allocate the dma buffer\n");
+		dma_release_channel(dma_chan);
+		return -ENOMEM;
+	}
+
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	dma_sconfig.slave_id = tspi->dma_req_sel;
+	dma_sconfig.src_maxburst = 0;
+	dma_sconfig.dst_maxburst = 0;
+	if (dma_to_memory) {
+		dma_sconfig.src_addr = tspi->phys + SLINK_RX_FIFO;
+		dma_sconfig.dst_addr = tspi->phys + SLINK_RX_FIFO;
+	} else {
+		dma_sconfig.src_addr = tspi->phys + SLINK_TX_FIFO;
+		dma_sconfig.dst_addr = tspi->phys + SLINK_TX_FIFO;
+	}
+
+	ret = dmaengine_slave_config(dma_chan, &dma_sconfig);
+	if (ret)
+		goto scrub;
+	if (dma_to_memory) {
+		tspi->rx_dma_chan = dma_chan;
+		tspi->rx_dma_buf = dma_buf;
+		tspi->rx_dma_phys = dma_phys;
+	} else {
+		tspi->tx_dma_chan = dma_chan;
+		tspi->tx_dma_buf = dma_buf;
+		tspi->tx_dma_phys = dma_phys;
+	}
+	return 0;
+
+scrub:
+	dma_free_coherent(tspi->dev, tspi->dma_buf_size, dma_buf, dma_phys);
+	dma_release_channel(dma_chan);
+	return ret;
+}
+
+static void tegra_slink_deinit_dma_param(struct tegra_slink_data *tspi,
+	bool dma_to_memory)
+{
+	u32 *dma_buf;
+	dma_addr_t dma_phys;
+	struct dma_chan *dma_chan;
+
+	if (dma_to_memory) {
+		dma_buf = tspi->rx_dma_buf;
+		dma_chan = tspi->rx_dma_chan;
+		dma_phys = tspi->rx_dma_phys;
+		tspi->rx_dma_chan = NULL;
+		tspi->rx_dma_buf = NULL;
+	} else {
+		dma_buf = tspi->tx_dma_buf;
+		dma_chan = tspi->tx_dma_chan;
+		dma_phys = tspi->tx_dma_phys;
+		tspi->tx_dma_buf = NULL;
+		tspi->tx_dma_chan = NULL;
+	}
+	if (!dma_chan)
+		return;
+
+	dma_free_coherent(tspi->dev, tspi->dma_buf_size, dma_buf, dma_phys);
+	dma_release_channel(dma_chan);
+}
+
+static int tegra_slink_start_transfer_one(struct spi_device *spi,
+		    struct spi_transfer *t, bool is_first_of_msg,
+		    bool is_single_xfer)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
+	u32 speed;
+	u8 bits_per_word;
+	unsigned total_fifo_words;
+	int ret;
+	struct tegra_spi_device_controller_data *cdata = spi->controller_data;
+	unsigned long command;
+	unsigned long command2;
+
+	bits_per_word = t->bits_per_word ? t->bits_per_word :
+					spi->bits_per_word;
+
+	speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
+	if (!speed)
+		speed = tspi->spi_max_frequency;
+	if (speed != tspi->cur_speed) {
+		clk_set_rate(tspi->clk, speed * 4);
+		tspi->cur_speed = speed;
+	}
+
+	tspi->cur_spi = spi;
+	tspi->cur_pos = 0;
+	tspi->cur_rx_pos = 0;
+	tspi->cur_tx_pos = 0;
+	tspi->curr_xfer = t;
+	total_fifo_words = tegra_slink_calculate_curr_xfer_param(spi, tspi, t);
+
+	if (is_first_of_msg) {
+		tegra_slink_clear_status(tspi);
+
+		command = tspi->def_command_reg;
+		command |= SLINK_BIT_LENGTH(bits_per_word - 1);
+
+		command2 = tspi->def_command2_reg;
+		command2 |= SLINK_SS_EN_CS(spi->chip_select);
+
+		/* possibly use the hw based chip select */
+		tspi->is_hw_based_cs = false;
+		if (cdata && cdata->is_hw_based_cs && is_single_xfer &&
+			((tspi->curr_dma_words * tspi->bytes_per_word) ==
+						(t->len - tspi->cur_pos))) {
+			int setup_count;
+			int sts2;
+
+			setup_count = cdata->cs_setup_clk_count >> 1;
+			setup_count = max(setup_count, 3);
+			command2 |= SLINK_SS_SETUP(setup_count);
+			if (tspi->chip_data->cs_hold_time) {
+				int hold_count;
+
+				hold_count = cdata->cs_hold_clk_count;
+				hold_count = max(hold_count, 0xF);
+				sts2 = tegra_slink_readl(tspi, SLINK_STATUS2);
+				sts2 &= ~SLINK_SS_HOLD_TIME(0xF);
+				sts2 |= SLINK_SS_HOLD_TIME(hold_count);
+				tegra_slink_writel(tspi, sts2, SLINK_STATUS2);
+			}
+			tspi->is_hw_based_cs = true;
+		}
+
+		if (tspi->is_hw_based_cs)
+			command &= ~SLINK_CS_SW;
+		else
+			command |= SLINK_CS_SW | SLINK_CS_VALUE;
+
+		command &= ~SLINK_MODES;
+		if (spi->mode & SPI_CPHA)
+			command |= SLINK_CK_SDA;
+
+		if (spi->mode & SPI_CPOL)
+			command |= SLINK_IDLE_SCLK_DRIVE_HIGH;
+		else
+			command |= SLINK_IDLE_SCLK_DRIVE_LOW;
+	} else {
+		command = tspi->command_reg;
+		command &= ~SLINK_BIT_LENGTH(~0);
+		command |= SLINK_BIT_LENGTH(bits_per_word - 1);
+
+		command2 = tspi->command2_reg;
+		command2 &= ~(SLINK_RXEN | SLINK_TXEN);
+	}
+
+	tegra_slink_writel(tspi, command, SLINK_COMMAND);
+	tspi->command_reg = command;
+
+	tspi->cur_direction = 0;
+	if (t->rx_buf) {
+		command2 |= SLINK_RXEN;
+		tspi->cur_direction |= DATA_DIR_RX;
+	}
+	if (t->tx_buf) {
+		command2 |= SLINK_TXEN;
+		tspi->cur_direction |= DATA_DIR_TX;
+	}
+	tegra_slink_writel(tspi, command2, SLINK_COMMAND2);
+	tspi->command2_reg = command2;
+
+	if (total_fifo_words > SLINK_FIFO_DEPTH)
+		ret = tegra_slink_start_dma_based_transfer(tspi, t);
+	else
+		ret = tegra_slink_start_cpu_based_transfer(tspi, t);
+	return ret;
+}
+
+static int tegra_slink_setup(struct spi_device *spi)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long val;
+	unsigned long flags;
+	unsigned int cs_pol_bit[MAX_CHIP_SELECT] = {
+			SLINK_CS_POLARITY,
+			SLINK_CS_POLARITY1,
+			SLINK_CS_POLARITY2,
+			SLINK_CS_POLARITY3,
+	};
+
+	dev_dbg(&spi->dev, "setup %d bpw, %scpol, %scpha, %dHz\n",
+		spi->bits_per_word,
+		spi->mode & SPI_CPOL ? "" : "~",
+		spi->mode & SPI_CPHA ? "" : "~",
+		spi->max_speed_hz);
+
+	BUG_ON(spi->chip_select >= MAX_CHIP_SELECT);
+
+	pm_runtime_get_sync(tspi->dev);
+	tegra_slink_clk_enable(tspi);
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	val = tspi->def_command_reg;
+	if (spi->mode & SPI_CS_HIGH)
+		val |= cs_pol_bit[spi->chip_select];
+	else
+		val &= ~cs_pol_bit[spi->chip_select];
+	tspi->def_command_reg = val;
+	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	tegra_slink_clk_disable(tspi);
+	pm_runtime_put_sync(tspi->dev);
+	return 0;
+}
+
+static int tegra_slink_prepare_transfer(struct spi_master *master)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
+	pm_runtime_get_sync(tspi->dev);
+	return 0;
+}
+
+static int tegra_slink_unprepare_transfer(struct spi_master *master)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
+	pm_runtime_put_sync(tspi->dev);
+	return 0;
+}
+
+static int tegra_slink_transfer_one_message(struct spi_master *master,
+			struct spi_message *msg)
+{
+
+	bool is_first_msg = true;
+	int single_xfer;
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	int ret;
+
+	msg->status = 0;
+	msg->actual_length = 0;
+	tegra_slink_clk_enable(tspi);
+	single_xfer = list_is_singular(&msg->transfers);
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		INIT_COMPLETION(tspi->xfer_completion);
+		ret = tegra_slink_start_transfer_one(spi, xfer,
+					is_first_msg, single_xfer);
+		if (ret < 0) {
+			dev_err(tspi->dev,
+				"spi can not start transfer, err %d\n", ret);
+			goto exit;
+		}
+		is_first_msg = false;
+		ret = wait_for_completion_timeout(&tspi->xfer_completion,
+						SLINK_DMA_TIMEOUT);
+		if (WARN_ON(ret == 0)) {
+			dev_err(tspi->dev,
+				"spi trasfer timeout, err %d\n", ret);
+			ret = -EIO;
+			goto exit;
+		}
+
+		if (tspi->tx_status ||  tspi->rx_status) {
+			dev_err(tspi->dev, "Error in Transfer\n");
+			ret = -EIO;
+			goto exit;
+		}
+		msg->actual_length += xfer->len;
+		if (xfer->cs_change && xfer->delay_usecs) {
+			tegra_slink_writel(tspi, tspi->def_command_reg,
+					SLINK_COMMAND);
+			udelay(xfer->delay_usecs);
+		}
+	}
+	ret = 0;
+exit:
+	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
+	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
+	tegra_slink_clk_disable(tspi);
+	msg->status = ret;
+	spi_finalize_current_message(master);
+	return ret;
+}
+
+static void handle_cpu_based_xfer(void *context_data)
+{
+	struct tegra_slink_data *tspi = context_data;
+	struct spi_transfer *t = tspi->curr_xfer;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	if (tspi->tx_status ||  tspi->rx_status ||
+				(tspi->status_reg & SLINK_BSY)) {
+		dev_err(tspi->dev,
+			"CpuXfer ERROR bit set 0x%x\n", tspi->status_reg);
+		dev_err(tspi->dev,
+			"CpuXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
+				tspi->command2_reg, tspi->dma_control_reg);
+		tegra_periph_reset_assert(tspi->clk);
+		udelay(2);
+		tegra_periph_reset_deassert(tspi->clk);
+		complete(&tspi->xfer_completion);
+		goto exit;
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		tegra_slink_read_rx_fifo_to_client_rxbuf(tspi, t);
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		tspi->cur_pos = tspi->cur_tx_pos;
+	else
+		tspi->cur_pos = tspi->cur_rx_pos;
+
+	if (tspi->cur_pos == t->len) {
+		complete(&tspi->xfer_completion);
+		goto exit;
+	}
+
+	tegra_slink_calculate_curr_xfer_param(tspi->cur_spi, tspi, t);
+	tegra_slink_start_cpu_based_transfer(tspi, t);
+exit:
+	spin_unlock_irqrestore(&tspi->lock, flags);
+	return;
+}
+
+static irqreturn_t tegra_slink_isr_thread(int irq, void *context_data)
+{
+	struct tegra_slink_data *tspi = context_data;
+	struct spi_transfer *t = tspi->curr_xfer;
+	long wait_status;
+	int err = 0;
+	unsigned total_fifo_words;
+	unsigned long flags;
+
+	if (!tspi->is_curr_dma_xfer) {
+		handle_cpu_based_xfer(context_data);
+		return IRQ_HANDLED;
+	}
+
+	/* Abort dmas if any error */
+	if (tspi->cur_direction & DATA_DIR_TX) {
+		if (tspi->tx_status) {
+			dmaengine_terminate_all(tspi->tx_dma_chan);
+			err += 1;
+		} else {
+			wait_status = wait_for_completion_interruptible_timeout(
+				&tspi->tx_dma_complete, SLINK_DMA_TIMEOUT);
+			if (wait_status <= 0) {
+				dmaengine_terminate_all(tspi->tx_dma_chan);
+				dev_err(tspi->dev, "TxDma Xfer failed\n");
+				err += 1;
+			}
+		}
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX) {
+		if (tspi->rx_status) {
+			dmaengine_terminate_all(tspi->rx_dma_chan);
+			err += 2;
+		} else {
+			wait_status = wait_for_completion_interruptible_timeout(
+				&tspi->rx_dma_complete, SLINK_DMA_TIMEOUT);
+			if (wait_status <= 0) {
+				dmaengine_terminate_all(tspi->rx_dma_chan);
+				dev_err(tspi->dev, "RxDma Xfer failed\n");
+				err += 2;
+			}
+		}
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	if (err) {
+		dev_err(tspi->dev,
+			"DmaXfer: ERROR bit set 0x%x\n", tspi->status_reg);
+		dev_err(tspi->dev,
+			"DmaXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
+				tspi->command2_reg, tspi->dma_control_reg);
+		tegra_periph_reset_assert(tspi->clk);
+		udelay(2);
+		tegra_periph_reset_deassert(tspi->clk);
+		complete(&tspi->xfer_completion);
+		spin_unlock_irqrestore(&tspi->lock, flags);
+		return IRQ_HANDLED;
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		tegra_slink_copy_spi_rxbuf_to_client_rxbuf(tspi, t);
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		tspi->cur_pos = tspi->cur_tx_pos;
+	else
+		tspi->cur_pos = tspi->cur_rx_pos;
+
+	if (tspi->cur_pos == t->len) {
+		complete(&tspi->xfer_completion);
+		goto exit;
+	}
+
+	/* Continue transfer in current message */
+	total_fifo_words = tegra_slink_calculate_curr_xfer_param(tspi->cur_spi,
+							tspi, t);
+	if (total_fifo_words > SLINK_FIFO_DEPTH)
+		err = tegra_slink_start_dma_based_transfer(tspi, t);
+	else
+		err = tegra_slink_start_cpu_based_transfer(tspi, t);
+
+exit:
+	spin_unlock_irqrestore(&tspi->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t tegra_slink_isr(int irq, void *context_data)
+{
+	struct tegra_slink_data *tspi = context_data;
+
+	tspi->status_reg = tegra_slink_readl(tspi, SLINK_STATUS);
+	if (tspi->cur_direction & DATA_DIR_TX)
+		tspi->tx_status = tspi->status_reg &
+					(SLINK_TX_OVF | SLINK_TX_UNF);
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		tspi->rx_status = tspi->status_reg &
+					(SLINK_RX_OVF | SLINK_RX_UNF);
+	tegra_slink_clear_status(tspi);
+
+	return IRQ_WAKE_THREAD;
+}
+
+#ifdef CONFIG_OF
+static struct tegra_spi_platform_data *tegra_slink_parese_dt(
+		struct platform_device *pdev)
+{
+	struct tegra_spi_platform_data *pdata;
+	const unsigned int *prop;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(&pdev->dev, "Memory alloc for pdata failed\n");
+		return NULL;
+	}
+
+	prop = of_get_property(pdev->dev.of_node, "nvidia,dma-req-sel", NULL);
+	if (prop)
+		pdata->dma_req_sel = be32_to_cpup(prop);
+
+	prop = of_get_property(pdev->dev.of_node, "spi_max_frequency", NULL);
+	if (prop)
+		pdata->spi_max_frequency = be32_to_cpup(prop);
+	else
+		pdata->spi_max_frequency = 25000000; /* 25MHz */
+
+	return pdata;
+}
+#else
+static struct tegra_spi_platform_data *tegra_slink_parese_dt(
+		struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
+
+const struct tegra_slink_chip_data tegra30_spi_cdata = {
+	.cs_hold_time = true,
+};
+
+#ifdef CONFIG_OF
+const struct tegra_slink_chip_data tegra20_spi_cdata = {
+	.cs_hold_time = false,
+};
+
+static struct of_device_id tegra_slink_of_match[] __devinitconst = {
+	{ .compatible = "nvidia,tegra20-slink", .data = &tegra20_spi_cdata, },
+	{ .compatible = "nvidia,tegra30-slink", .data = &tegra30_spi_cdata, },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tegra_slink_of_match);
+#endif
+
+static int __devinit tegra_slink_probe(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct tegra_slink_data	*tspi;
+	struct resource		*r;
+	struct tegra_spi_platform_data *pdata = pdev->dev.platform_data;
+	int ret, spi_irq;
+	const struct tegra_slink_chip_data *cdata = NULL;
+
+	if (!pdata && pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_device(of_match_ptr(tegra_slink_of_match),
+				&pdev->dev);
+		if (!match) {
+			dev_err(&pdev->dev, "Error: No device match found\n");
+			return -ENODEV;
+		}
+		cdata = match->data;
+		pdata = tegra_slink_parese_dt(pdev);
+	} else {
+		cdata = &tegra30_spi_cdata;
+	}
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data, exiting\n");
+		return -ENODEV;
+	}
+
+	master = spi_alloc_master(&pdev->dev, sizeof(*tspi));
+	if (!master) {
+		dev_err(&pdev->dev, "master allocation failed\n");
+		return -ENOMEM;
+	}
+
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+
+	/* the spi->mode bits understood by this driver: */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	master->setup = tegra_slink_setup;
+	master->prepare_transfer_hardware = tegra_slink_prepare_transfer;
+	master->transfer_one_message = tegra_slink_transfer_one_message;
+	master->unprepare_transfer_hardware = tegra_slink_unprepare_transfer;
+	master->num_chipselect = MAX_CHIP_SELECT;
+
+	dev_set_drvdata(&pdev->dev, master);
+	tspi = spi_master_get_devdata(master);
+	tspi->master = master;
+	tspi->dma_req_sel = pdata->dma_req_sel;
+	tspi->dev = &pdev->dev;
+	tspi->chip_data = cdata;
+	spin_lock_init(&tspi->lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "No IO memory resource\n");
+		ret = -ENODEV;
+		goto exit_free_master;
+	}
+	tspi->phys = r->start;
+	tspi->base = devm_request_and_ioremap(&pdev->dev, r);
+	if (!tspi->base) {
+		dev_err(&pdev->dev,
+			"Cannot request memregion/iomap dma address\n");
+		ret = -EADDRNOTAVAIL;
+		goto exit_free_master;
+	}
+
+	spi_irq = platform_get_irq(pdev, 0);
+	if (unlikely(spi_irq < 0)) {
+		dev_err(&pdev->dev, "can't find irq resource\n");
+		ret = -ENXIO;
+		goto exit_free_master;
+	}
+	tspi->irq = spi_irq;
+
+	ret = devm_request_threaded_irq(&pdev->dev, tspi->irq,
+			tegra_slink_isr, tegra_slink_isr_thread, IRQF_ONESHOT,
+			dev_name(&pdev->dev), tspi);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register ISR for IRQ %d\n",
+					tspi->irq);
+		goto exit_free_master;
+	}
+
+	tspi->clk = devm_clk_get(&pdev->dev, "slink");
+	if (IS_ERR(tspi->clk)) {
+		dev_err(&pdev->dev, "can not get clock\n");
+		ret = PTR_ERR(tspi->clk);
+		goto exit_free_master;
+	}
+
+	tspi->max_buf_size = SLINK_FIFO_DEPTH << 2;
+	tspi->dma_buf_size = DEFAULT_SPI_DMA_BUF_LEN;
+	tspi->spi_max_frequency = pdata->spi_max_frequency;
+
+	if (pdata->dma_req_sel) {
+		ret = tegra_slink_init_dma_param(tspi, true);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "RxDma Init failed, err %d\n", ret);
+			goto exit_free_master;
+		}
+
+		ret = tegra_slink_init_dma_param(tspi, false);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "TxDma Init failed, err %d\n", ret);
+			goto exit_rx_dma_free;
+		}
+		tspi->max_buf_size = tspi->dma_buf_size;
+		init_completion(&tspi->tx_dma_complete);
+		init_completion(&tspi->rx_dma_complete);
+	}
+
+	init_completion(&tspi->xfer_completion);
+
+	tspi->def_command_reg  = SLINK_M_S;
+	tspi->def_command2_reg = SLINK_CS_ACTIVE_BETWEEN;
+	tegra_slink_clk_enable(tspi);
+	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
+	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
+	tegra_slink_clk_disable(tspi);
+
+	pm_runtime_enable(&pdev->dev);
+
+	master->dev.of_node = pdev->dev.of_node;
+	ret = spi_register_master(master);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "can not register to master err %d\n", ret);
+		goto exit_fail_master_reg;
+	}
+	return ret;
+
+exit_fail_master_reg:
+	pm_runtime_disable(&pdev->dev);
+	tegra_slink_deinit_dma_param(tspi, false);
+exit_rx_dma_free:
+	tegra_slink_deinit_dma_param(tspi, true);
+exit_free_master:
+	spi_master_put(master);
+	return ret;
+}
+
+static int __devexit tegra_slink_remove(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct tegra_slink_data	*tspi;
+
+	master = dev_get_drvdata(&pdev->dev);
+	tspi = spi_master_get_devdata(master);
+
+	spi_unregister_master(master);
+
+	if (tspi->tx_dma_chan)
+		tegra_slink_deinit_dma_param(tspi, false);
+
+	if (tspi->rx_dma_chan)
+		tegra_slink_deinit_dma_param(tspi, true);
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_slink_suspend(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+
+	spi_master_suspend(master);
+	return 0;
+}
+
+static int tegra_slink_resume(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
+	pm_runtime_get_sync(dev);
+	tegra_slink_clk_enable(tspi);
+	tegra_slink_writel(tspi, tspi->command_reg, SLINK_COMMAND);
+	tegra_slink_writel(tspi, tspi->command2_reg, SLINK_COMMAND2);
+	tegra_slink_clk_disable(tspi);
+	pm_runtime_put_sync(dev);
+
+	spi_master_resume(master);
+	return 0;
+}
+
+static const struct dev_pm_ops tegra_slink_dev_pm_ops = {
+	.suspend = tegra_slink_suspend,
+	.resume = tegra_slink_resume,
+};
+#define TEGRA_SPI_PM	(&tegra_slink_dev_pm_ops)
+#else
+#define TEGRA_SPI_PM	NULL
+#endif
+
+static struct platform_driver tegra_slink_driver = {
+	.driver = {
+		.name =		"spi-tegra-slink",
+		.owner =	THIS_MODULE,
+		.pm =		&tegra_slink_dev_pm_ops,
+		.of_match_table = of_match_ptr(tegra_slink_of_match),
+	},
+	.probe =	tegra_slink_probe,
+	.remove =	__devexit_p(tegra_slink_remove),
+};
+module_platform_driver(tegra_slink_driver);
+
+MODULE_ALIAS("platform:tegra_slink-slink");
+MODULE_DESCRIPTION("NVIDIA Tegra SLINK Controller Driver");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/spi/spi-tegra.h b/include/linux/spi/spi-tegra.h
new file mode 100644
index 0000000..786932c
--- /dev/null
+++ b/include/linux/spi/spi-tegra.h
@@ -0,0 +1,40 @@
+/*
+ * spi-tegra.h: SPI interface for Nvidia Tegra20 SLINK controller.
+ *
+ * Copyright (C) 2011 NVIDIA Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#ifndef _LINUX_SPI_TEGRA_H
+#define _LINUX_SPI_TEGRA_H
+
+struct tegra_spi_platform_data {
+	int dma_req_sel;
+	unsigned int spi_max_frequency;
+};
+
+/*
+ * Controller data from device to pass some info like
+ * hw based chip select can be used or not and if yes
+ * then CS hold and setup time.
+ */
+struct tegra_spi_device_controller_data {
+	bool is_hw_based_cs;
+	int cs_setup_clk_count;
+	int cs_hold_clk_count;
+};
+
+#endif /* _LINUX_SPI_TEGRA_H */
-- 
1.7.1.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-18 10:47 ` Laxman Dewangan
  0 siblings, 0 replies; 18+ messages in thread
From: Laxman Dewangan @ 2012-10-18 10:47 UTC (permalink / raw)
  To: broonie, grant.likely, rob.herring
  Cc: spi-devel-general, linux-kernel, linux-tegra, Laxman Dewangan

Tegra20/Tegra30 supports the spi interface through its SLINK
controller. Add spi driver for SLINK controller.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/spi/Kconfig             |    6 +
 drivers/spi/Makefile            |    2 +-
 drivers/spi/spi-tegra20-slink.c | 1356 +++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi-tegra.h   |   40 ++
 4 files changed, 1403 insertions(+), 1 deletions(-)
 create mode 100644 drivers/spi/spi-tegra20-slink.c
 create mode 100644 include/linux/spi/spi-tegra.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 1acae35..25290d9 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -385,6 +385,12 @@ config SPI_MXS
 	help
 	  SPI driver for Freescale MXS devices.
 
+config SPI_TEGRA20_SLINK
+	tristate "Nvidia Tegra20/Tegra30 SLINK Controller"
+	depends on ARCH_TEGRA && TEGRA20_APB_DMA
+	help
+	  SPI driver for Nvidia Tegra20/Tegra30 SLINK Controller interface.
+
 config SPI_TI_SSP
 	tristate "TI Sequencer Serial Port - SPI Support"
 	depends on MFD_TI_SSP
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index c48df47..f87c0f1 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -60,10 +60,10 @@ obj-$(CONFIG_SPI_SH_MSIOF)		+= spi-sh-msiof.o
 obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
 obj-$(CONFIG_SPI_SIRF)		+= spi-sirf.o
 obj-$(CONFIG_SPI_STMP3XXX)		+= spi-stmp.o
+obj-$(CONFIG_SPI_TEGRA20_SLINK)		+= spi-tegra20-slink.o
 obj-$(CONFIG_SPI_TI_SSP)		+= spi-ti-ssp.o
 obj-$(CONFIG_SPI_TLE62X0)		+= spi-tle62x0.o
 obj-$(CONFIG_SPI_TOPCLIFF_PCH)		+= spi-topcliff-pch.o
 obj-$(CONFIG_SPI_TXX9)			+= spi-txx9.o
 obj-$(CONFIG_SPI_XCOMM)		+= spi-xcomm.o
 obj-$(CONFIG_SPI_XILINX)		+= spi-xilinx.o
-
diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
new file mode 100644
index 0000000..c8705d9
--- /dev/null
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -0,0 +1,1356 @@
+/*
+ * SPI driver for Nvidia's Tegra20/Tegra30 SLINK Controller.
+ *
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-tegra.h>
+#include <mach/clk.h>
+
+#define SLINK_COMMAND			0x000
+#define SLINK_BIT_LENGTH(x)		(((x) & 0x1f) << 0)
+#define SLINK_WORD_SIZE(x)		(((x) & 0x1f) << 5)
+#define SLINK_BOTH_EN			(1 << 10)
+#define SLINK_CS_SW			(1 << 11)
+#define SLINK_CS_VALUE			(1 << 12)
+#define SLINK_CS_POLARITY		(1 << 13)
+#define SLINK_IDLE_SDA_DRIVE_LOW	(0 << 16)
+#define SLINK_IDLE_SDA_DRIVE_HIGH	(1 << 16)
+#define SLINK_IDLE_SDA_PULL_LOW		(2 << 16)
+#define SLINK_IDLE_SDA_PULL_HIGH	(3 << 16)
+#define SLINK_IDLE_SDA_MASK		(3 << 16)
+#define SLINK_CS_POLARITY1		(1 << 20)
+#define SLINK_CK_SDA			(1 << 21)
+#define SLINK_CS_POLARITY2		(1 << 22)
+#define SLINK_CS_POLARITY3		(1 << 23)
+#define SLINK_IDLE_SCLK_DRIVE_LOW	(0 << 24)
+#define SLINK_IDLE_SCLK_DRIVE_HIGH	(1 << 24)
+#define SLINK_IDLE_SCLK_PULL_LOW	(2 << 24)
+#define SLINK_IDLE_SCLK_PULL_HIGH	(3 << 24)
+#define SLINK_IDLE_SCLK_MASK		(3 << 24)
+#define SLINK_M_S			(1 << 28)
+#define SLINK_WAIT			(1 << 29)
+#define SLINK_GO			(1 << 30)
+#define SLINK_ENB			(1 << 31)
+
+#define SLINK_MODES			(SLINK_IDLE_SCLK_MASK | SLINK_CK_SDA)
+
+#define SLINK_COMMAND2			0x004
+#define SLINK_LSBFE			(1 << 0)
+#define SLINK_SSOE			(1 << 1)
+#define SLINK_SPIE			(1 << 4)
+#define SLINK_BIDIROE			(1 << 6)
+#define SLINK_MODFEN			(1 << 7)
+#define SLINK_INT_SIZE(x)		(((x) & 0x1f) << 8)
+#define SLINK_CS_ACTIVE_BETWEEN		(1 << 17)
+#define SLINK_SS_EN_CS(x)		(((x) & 0x3) << 18)
+#define SLINK_SS_SETUP(x)		(((x) & 0x3) << 20)
+#define SLINK_FIFO_REFILLS_0		(0 << 22)
+#define SLINK_FIFO_REFILLS_1		(1 << 22)
+#define SLINK_FIFO_REFILLS_2		(2 << 22)
+#define SLINK_FIFO_REFILLS_3		(3 << 22)
+#define SLINK_FIFO_REFILLS_MASK		(3 << 22)
+#define SLINK_WAIT_PACK_INT(x)		(((x) & 0x7) << 26)
+#define SLINK_SPC0			(1 << 29)
+#define SLINK_TXEN			(1 << 30)
+#define SLINK_RXEN			(1 << 31)
+
+#define SLINK_STATUS			0x008
+#define SLINK_COUNT(val)		(((val) >> 0) & 0x1f)
+#define SLINK_WORD(val)			(((val) >> 5) & 0x1f)
+#define SLINK_BLK_CNT(val)		(((val) >> 0) & 0xffff)
+#define SLINK_MODF			(1 << 16)
+#define SLINK_RX_UNF			(1 << 18)
+#define SLINK_TX_OVF			(1 << 19)
+#define SLINK_TX_FULL			(1 << 20)
+#define SLINK_TX_EMPTY			(1 << 21)
+#define SLINK_RX_FULL			(1 << 22)
+#define SLINK_RX_EMPTY			(1 << 23)
+#define SLINK_TX_UNF			(1 << 24)
+#define SLINK_RX_OVF			(1 << 25)
+#define SLINK_TX_FLUSH			(1 << 26)
+#define SLINK_RX_FLUSH			(1 << 27)
+#define SLINK_SCLK			(1 << 28)
+#define SLINK_ERR			(1 << 29)
+#define SLINK_RDY			(1 << 30)
+#define SLINK_BSY			(1 << 31)
+#define SLINK_FIFO_ERROR		(SLINK_TX_OVF | SLINK_RX_UNF |	\
+					SLINK_TX_UNF | SLINK_RX_OVF)
+
+#define SLINK_FIFO_EMPTY		(SLINK_TX_EMPTY | SLINK_RX_EMPTY)
+
+#define SLINK_MAS_DATA			0x010
+#define SLINK_SLAVE_DATA		0x014
+
+#define SLINK_DMA_CTL			0x018
+#define SLINK_DMA_BLOCK_SIZE(x)		(((x) & 0xffff) << 0)
+#define SLINK_TX_TRIG_1			(0 << 16)
+#define SLINK_TX_TRIG_4			(1 << 16)
+#define SLINK_TX_TRIG_8			(2 << 16)
+#define SLINK_TX_TRIG_16		(3 << 16)
+#define SLINK_TX_TRIG_MASK		(3 << 16)
+#define SLINK_RX_TRIG_1			(0 << 18)
+#define SLINK_RX_TRIG_4			(1 << 18)
+#define SLINK_RX_TRIG_8			(2 << 18)
+#define SLINK_RX_TRIG_16		(3 << 18)
+#define SLINK_RX_TRIG_MASK		(3 << 18)
+#define SLINK_PACKED			(1 << 20)
+#define SLINK_PACK_SIZE_4		(0 << 21)
+#define SLINK_PACK_SIZE_8		(1 << 21)
+#define SLINK_PACK_SIZE_16		(2 << 21)
+#define SLINK_PACK_SIZE_32		(3 << 21)
+#define SLINK_PACK_SIZE_MASK		(3 << 21)
+#define SLINK_IE_TXC			(1 << 26)
+#define SLINK_IE_RXC			(1 << 27)
+#define SLINK_DMA_EN			(1 << 31)
+
+#define SLINK_STATUS2			0x01c
+#define SLINK_TX_FIFO_EMPTY_COUNT(val)	(((val) & 0x3f) >> 0)
+#define SLINK_RX_FIFO_FULL_COUNT(val)	(((val) & 0x3f0000) >> 16)
+#define SLINK_SS_HOLD_TIME(val)		(((val) & 0xF) << 6)
+
+#define SLINK_TX_FIFO			0x100
+#define SLINK_RX_FIFO			0x180
+
+#define DATA_DIR_TX			(1 << 0)
+#define DATA_DIR_RX			(1 << 1)
+
+#define SLINK_DMA_TIMEOUT		(msecs_to_jiffies(1000))
+
+#define DEFAULT_SPI_DMA_BUF_LEN		(16*1024)
+#define TX_FIFO_EMPTY_COUNT_MAX		SLINK_TX_FIFO_EMPTY_COUNT(0x20)
+#define RX_FIFO_FULL_COUNT_ZERO		SLINK_RX_FIFO_FULL_COUNT(0)
+
+#define SLINK_STATUS2_RESET \
+	(TX_FIFO_EMPTY_COUNT_MAX | RX_FIFO_FULL_COUNT_ZERO << 16)
+
+#define MAX_CHIP_SELECT			4
+#define SLINK_FIFO_DEPTH		32
+
+struct tegra_slink_chip_data {
+	bool cs_hold_time;
+};
+
+struct tegra_slink_data {
+	struct device				*dev;
+	struct spi_master			*master;
+	const struct tegra_slink_chip_data	*chip_data;
+	spinlock_t				lock;
+
+	struct clk				*clk;
+	void __iomem				*base;
+	phys_addr_t				phys;
+	unsigned				irq;
+	int					dma_req_sel;
+	u32					spi_max_frequency;
+	u32					cur_speed;
+
+	struct spi_device			*cur_spi;
+	unsigned				cur_pos;
+	unsigned				cur_len;
+	unsigned				words_per_32bit;
+	unsigned				bytes_per_word;
+	unsigned				curr_dma_words;
+	unsigned				cur_direction;
+
+	unsigned				cur_rx_pos;
+	unsigned				cur_tx_pos;
+
+	unsigned				dma_buf_size;
+	unsigned				max_buf_size;
+	bool					is_curr_dma_xfer;
+	bool					is_hw_based_cs;
+
+	struct completion			rx_dma_complete;
+	struct completion			tx_dma_complete;
+
+	u32					tx_status;
+	u32					rx_status;
+	u32					status_reg;
+	bool					is_packed;
+	unsigned long				packed_size;
+
+	u32					command_reg;
+	u32					command2_reg;
+	u32					dma_control_reg;
+	u32					def_command_reg;
+	u32					def_command2_reg;
+
+	struct completion			xfer_completion;
+	struct spi_transfer			*curr_xfer;
+	struct dma_chan				*rx_dma_chan;
+	u32					*rx_dma_buf;
+	dma_addr_t				rx_dma_phys;
+	struct dma_async_tx_descriptor		*rx_dma_desc;
+
+	struct dma_chan				*tx_dma_chan;
+	u32					*tx_dma_buf;
+	dma_addr_t				tx_dma_phys;
+	struct dma_async_tx_descriptor		*tx_dma_desc;
+};
+
+static inline unsigned long tegra_slink_readl(struct tegra_slink_data *tspi,
+		unsigned long reg)
+{
+	return readl(tspi->base + reg);
+}
+
+static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
+		unsigned long val, unsigned long reg)
+{
+	writel(val, tspi->base + reg);
+
+	/* Read back register to make sure that register writes completed */
+	if (reg != SLINK_TX_FIFO)
+		readl(tspi->base + SLINK_MAS_DATA);
+}
+
+static inline int tegra_slink_clk_disable(struct tegra_slink_data *tspi)
+{
+	/* Flush all write which are in PPSB queue by reading back */
+	tegra_slink_readl(tspi, SLINK_MAS_DATA);
+
+	clk_disable_unprepare(tspi->clk);
+	return 0;
+}
+
+static inline int tegra_slink_clk_enable(struct tegra_slink_data *tspi)
+{
+	clk_prepare_enable(tspi->clk);
+	return 0;
+}
+
+static void tegra_slink_clear_status(struct tegra_slink_data *tspi)
+{
+	unsigned long val;
+	unsigned long val_write = 0;
+
+	val = tegra_slink_readl(tspi, SLINK_STATUS);
+
+	/* Write 1 to clear status register */
+	val_write = SLINK_RDY;
+	val_write |= (val & SLINK_FIFO_ERROR);
+	tegra_slink_writel(tspi, val_write, SLINK_STATUS);
+}
+
+static unsigned long tegra_slink_get_packed_size(struct tegra_slink_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned long val;
+
+	switch (tspi->bytes_per_word) {
+	case 0:
+		val = SLINK_PACK_SIZE_4;
+		break;
+	case 1:
+		val = SLINK_PACK_SIZE_8;
+		break;
+	case 2:
+		val = SLINK_PACK_SIZE_16;
+		break;
+	case 4:
+		val = SLINK_PACK_SIZE_32;
+		break;
+	default:
+		val = 0;
+	}
+	return val;
+}
+
+static unsigned tegra_slink_calculate_curr_xfer_param(
+	struct spi_device *spi, struct tegra_slink_data *tspi,
+	struct spi_transfer *t)
+{
+	unsigned remain_len = t->len - tspi->cur_pos;
+	unsigned max_word;
+	unsigned bits_per_word ;
+	unsigned max_len;
+	unsigned total_fifo_words;
+
+	bits_per_word = t->bits_per_word ? t->bits_per_word :
+						spi->bits_per_word;
+	tspi->bytes_per_word = (bits_per_word - 1) / 8 + 1;
+
+	if (bits_per_word == 8 || bits_per_word == 16) {
+		tspi->is_packed = 1;
+		tspi->words_per_32bit = 32/bits_per_word;
+	} else {
+		tspi->is_packed = 0;
+		tspi->words_per_32bit = 1;
+	}
+	tspi->packed_size = tegra_slink_get_packed_size(tspi, t);
+
+	if (tspi->is_packed) {
+		max_len = min(remain_len, tspi->max_buf_size);
+		tspi->curr_dma_words = max_len/tspi->bytes_per_word;
+		total_fifo_words = max_len/4;
+	} else {
+		max_word = (remain_len - 1) / tspi->bytes_per_word + 1;
+		max_word = min(max_word, tspi->max_buf_size/4);
+		tspi->curr_dma_words = max_word;
+		total_fifo_words = max_word;
+	}
+	return total_fifo_words;
+}
+
+static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(
+	struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned nbytes;
+	unsigned tx_empty_count;
+	unsigned long fifo_status;
+	unsigned max_n_32bit;
+	unsigned i, count;
+	unsigned long x;
+	unsigned int written_words;
+	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
+
+	fifo_status = tegra_slink_readl(tspi, SLINK_STATUS2);
+	tx_empty_count = SLINK_TX_FIFO_EMPTY_COUNT(fifo_status);
+
+	if (tspi->is_packed) {
+		nbytes = tspi->curr_dma_words * tspi->bytes_per_word;
+		max_n_32bit = (min(nbytes,  tx_empty_count*4) - 1)/4 + 1;
+		for (count = 0; count < max_n_32bit; ++count) {
+			x = 0;
+			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
+				x |= (*tx_buf++) << (i*8);
+			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
+		}
+		written_words =  min(max_n_32bit * tspi->words_per_32bit,
+					tspi->curr_dma_words);
+	} else {
+		max_n_32bit = min(tspi->curr_dma_words,  tx_empty_count);
+		nbytes = max_n_32bit * tspi->bytes_per_word;
+		for (count = 0; count < max_n_32bit; ++count) {
+			x = 0;
+			for (i = 0; nbytes && (i < tspi->bytes_per_word);
+							++i, nbytes--)
+				x |= ((*tx_buf++) << i*8);
+			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
+		}
+		written_words = max_n_32bit;
+	}
+	tspi->cur_tx_pos += written_words * tspi->bytes_per_word;
+	return written_words;
+}
+
+static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned rx_full_count;
+	unsigned long fifo_status;
+	unsigned i, count;
+	unsigned long x;
+	unsigned int read_words = 0;
+	unsigned len;
+	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_rx_pos;
+
+	fifo_status = tegra_slink_readl(tspi, SLINK_STATUS2);
+	rx_full_count = SLINK_RX_FIFO_FULL_COUNT(fifo_status);
+	if (tspi->is_packed) {
+		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		for (count = 0; count < rx_full_count; ++count) {
+			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
+			for (i = 0; len && (i < 4); ++i, len--)
+				*rx_buf++ = (x >> i*8) & 0xFF;
+		}
+		tspi->cur_rx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
+		read_words += tspi->curr_dma_words;
+	} else {
+		unsigned int rx_mask, bits_per_word;
+
+		bits_per_word = t->bits_per_word ? t->bits_per_word :
+						tspi->cur_spi->bits_per_word;
+		rx_mask = (1 << bits_per_word) - 1;
+		for (count = 0; count < rx_full_count; ++count) {
+			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
+			x &= rx_mask;
+			for (i = 0; (i < tspi->bytes_per_word); ++i)
+				*rx_buf++ = (x >> (i*8)) & 0xFF;
+		}
+		tspi->cur_rx_pos += rx_full_count * tspi->bytes_per_word;
+		read_words += rx_full_count;
+	}
+	return read_words;
+}
+
+static void tegra_slink_copy_client_txbuf_to_spi_txbuf(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned len;
+
+	/* Make the dma buffer to read by cpu */
+	dma_sync_single_for_cpu(tspi->dev, tspi->tx_dma_phys,
+				tspi->dma_buf_size, DMA_TO_DEVICE);
+
+	if (tspi->is_packed) {
+		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		memcpy(tspi->tx_dma_buf, t->tx_buf + tspi->cur_pos, len);
+	} else {
+		unsigned int i;
+		unsigned int count;
+		u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_tx_pos;
+		unsigned consume = tspi->curr_dma_words * tspi->bytes_per_word;
+		unsigned int x;
+
+		for (count = 0; count < tspi->curr_dma_words; ++count) {
+			x = 0;
+			for (i = 0; consume && (i < tspi->bytes_per_word);
+							++i, consume--)
+				x |= ((*tx_buf++) << i*8);
+			tspi->tx_dma_buf[count] = x;
+		}
+	}
+	tspi->cur_tx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
+
+	/* Make the dma buffer to read by dma */
+	dma_sync_single_for_device(tspi->dev, tspi->tx_dma_phys,
+				tspi->dma_buf_size, DMA_TO_DEVICE);
+}
+
+static void tegra_slink_copy_spi_rxbuf_to_client_rxbuf(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned len;
+
+	/* Make the dma buffer to read by cpu */
+	dma_sync_single_for_cpu(tspi->dev, tspi->rx_dma_phys,
+		tspi->dma_buf_size, DMA_FROM_DEVICE);
+
+	if (tspi->is_packed) {
+		len = tspi->curr_dma_words * tspi->bytes_per_word;
+		memcpy(t->rx_buf + tspi->cur_rx_pos, tspi->rx_dma_buf, len);
+	} else {
+		unsigned int i;
+		unsigned int count;
+		unsigned char *rx_buf = t->rx_buf + tspi->cur_rx_pos;
+		unsigned int x;
+		unsigned int rx_mask, bits_per_word;
+
+		bits_per_word = t->bits_per_word ? t->bits_per_word :
+						tspi->cur_spi->bits_per_word;
+		rx_mask = (1 << bits_per_word) - 1;
+		for (count = 0; count < tspi->curr_dma_words; ++count) {
+			x = tspi->rx_dma_buf[count];
+			x &= rx_mask;
+			for (i = 0; (i < tspi->bytes_per_word); ++i)
+				*rx_buf++ = (x >> (i*8)) & 0xFF;
+		}
+	}
+	tspi->cur_rx_pos += tspi->curr_dma_words * tspi->bytes_per_word;
+
+	/* Make the dma buffer to read by dma */
+	dma_sync_single_for_device(tspi->dev, tspi->rx_dma_phys,
+		tspi->dma_buf_size, DMA_FROM_DEVICE);
+}
+
+static void tegra_slink_dma_complete(void *args)
+{
+	struct completion *dma_complete = args;
+
+	complete(dma_complete);
+}
+
+static int tegra_slink_start_tx_dma(struct tegra_slink_data *tspi, int len)
+{
+	INIT_COMPLETION(tspi->tx_dma_complete);
+	tspi->tx_dma_desc = dmaengine_prep_slave_single(tspi->tx_dma_chan,
+				tspi->tx_dma_phys, len, DMA_MEM_TO_DEV,
+				DMA_PREP_INTERRUPT |  DMA_CTRL_ACK);
+	if (!tspi->tx_dma_desc) {
+		dev_err(tspi->dev, "Not able to get desc for Tx\n");
+		return -EIO;
+	}
+
+	tspi->tx_dma_desc->callback = tegra_slink_dma_complete;
+	tspi->tx_dma_desc->callback_param = &tspi->tx_dma_complete;
+
+	dmaengine_submit(tspi->tx_dma_desc);
+	dma_async_issue_pending(tspi->tx_dma_chan);
+	return 0;
+}
+
+static int tegra_slink_start_rx_dma(struct tegra_slink_data *tspi, int len)
+{
+	INIT_COMPLETION(tspi->rx_dma_complete);
+	tspi->rx_dma_desc = dmaengine_prep_slave_single(tspi->rx_dma_chan,
+				tspi->rx_dma_phys, len, DMA_DEV_TO_MEM,
+				DMA_PREP_INTERRUPT |  DMA_CTRL_ACK);
+	if (!tspi->rx_dma_desc) {
+		dev_err(tspi->dev, "Not able to get desc for Rx\n");
+		return -EIO;
+	}
+
+	tspi->rx_dma_desc->callback = tegra_slink_dma_complete;
+	tspi->rx_dma_desc->callback_param = &tspi->rx_dma_complete;
+
+	dmaengine_submit(tspi->rx_dma_desc);
+	dma_async_issue_pending(tspi->rx_dma_chan);
+	return 0;
+}
+
+static int tegra_slink_start_dma_based_transfer(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned long val;
+	unsigned long test_val;
+	unsigned int len;
+	int ret = 0;
+	unsigned long status;
+
+	/* Make sure that Rx and Tx fifo are empty */
+	status = tegra_slink_readl(tspi, SLINK_STATUS);
+	if ((status & SLINK_FIFO_EMPTY) != SLINK_FIFO_EMPTY)
+		dev_err(tspi->dev,
+			"Rx/Tx fifo are not empty status 0x%08lx\n", status);
+
+	val = SLINK_DMA_BLOCK_SIZE(tspi->curr_dma_words - 1);
+	val |= tspi->packed_size;
+	if (tspi->is_packed)
+		len = DIV_ROUND_UP(tspi->curr_dma_words * tspi->bytes_per_word,
+					4) * 4;
+	else
+		len = tspi->curr_dma_words * 4;
+
+	/* Set attention level based on length of transfer */
+	if (len & 0xF)
+		val |= SLINK_TX_TRIG_1 | SLINK_RX_TRIG_1;
+	else if (((len) >> 4) & 0x1)
+		val |= SLINK_TX_TRIG_4 | SLINK_RX_TRIG_4;
+	else
+		val |= SLINK_TX_TRIG_8 | SLINK_RX_TRIG_8;
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		val |= SLINK_IE_TXC;
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		val |= SLINK_IE_RXC;
+
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	tspi->dma_control_reg = val;
+
+	if (tspi->cur_direction & DATA_DIR_TX) {
+		tegra_slink_copy_client_txbuf_to_spi_txbuf(tspi, t);
+		wmb();
+		ret = tegra_slink_start_tx_dma(tspi, len);
+		if (ret < 0) {
+			dev_err(tspi->dev,
+				"Starting tx dma failed, err %d\n", ret);
+			return ret;
+		}
+
+		/* Wait for tx fifo to be fill before starting slink */
+		test_val = tegra_slink_readl(tspi, SLINK_STATUS);
+		while (!(test_val & SLINK_TX_FULL))
+			test_val = tegra_slink_readl(tspi, SLINK_STATUS);
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX) {
+		/* Make the dma buffer to read by dma */
+		dma_sync_single_for_device(tspi->dev, tspi->rx_dma_phys,
+				tspi->dma_buf_size, DMA_FROM_DEVICE);
+
+		ret = tegra_slink_start_rx_dma(tspi, len);
+		if (ret < 0) {
+			dev_err(tspi->dev,
+				"Starting rx dma failed, err %d\n", ret);
+			if (tspi->cur_direction & DATA_DIR_TX)
+				dmaengine_terminate_all(tspi->tx_dma_chan);
+			return ret;
+		}
+	}
+	tspi->is_curr_dma_xfer = true;
+	if (tspi->is_packed) {
+		val |= SLINK_PACKED;
+		tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+		/* HW need small delay after settign Packed mode */
+		udelay(1);
+		wmb();
+	}
+	tspi->dma_control_reg = val;
+
+	val |= SLINK_DMA_EN;
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	return ret;
+}
+
+static int tegra_slink_start_cpu_based_transfer(
+		struct tegra_slink_data *tspi, struct spi_transfer *t)
+{
+	unsigned long val;
+	unsigned cur_words;
+
+	val = tspi->packed_size;
+	if (tspi->cur_direction & DATA_DIR_TX)
+		val |= SLINK_IE_TXC;
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		val |= SLINK_IE_RXC;
+
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	tspi->dma_control_reg = val;
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		cur_words = tegra_slink_fill_tx_fifo_from_client_txbuf(tspi, t);
+	else
+		cur_words = tspi->curr_dma_words;
+	val |= SLINK_DMA_BLOCK_SIZE(cur_words - 1);
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	tspi->dma_control_reg = val;
+
+	tspi->is_curr_dma_xfer = false;
+	if (tspi->is_packed) {
+		val |= SLINK_PACKED;
+		tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+		udelay(1);
+		wmb();
+	}
+	tspi->dma_control_reg = val;
+	val |= SLINK_DMA_EN;
+	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
+	return 0;
+}
+static int tegra_slink_init_dma_param(struct tegra_slink_data *tspi,
+			bool dma_to_memory)
+{
+	struct dma_chan *dma_chan;
+	u32 *dma_buf;
+	dma_addr_t dma_phys;
+	int ret;
+	struct dma_slave_config dma_sconfig;
+	dma_cap_mask_t mask;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	dma_chan = dma_request_channel(mask, NULL, NULL);
+	if (!dma_chan) {
+		dev_err(tspi->dev,
+			"Dma channel is not available, will try later\n");
+		return -EPROBE_DEFER;
+	}
+
+	dma_buf = dma_alloc_coherent(tspi->dev, tspi->dma_buf_size,
+				&dma_phys, GFP_KERNEL);
+	if (!dma_buf) {
+		dev_err(tspi->dev, " Not able to allocate the dma buffer\n");
+		dma_release_channel(dma_chan);
+		return -ENOMEM;
+	}
+
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	dma_sconfig.slave_id = tspi->dma_req_sel;
+	dma_sconfig.src_maxburst = 0;
+	dma_sconfig.dst_maxburst = 0;
+	if (dma_to_memory) {
+		dma_sconfig.src_addr = tspi->phys + SLINK_RX_FIFO;
+		dma_sconfig.dst_addr = tspi->phys + SLINK_RX_FIFO;
+	} else {
+		dma_sconfig.src_addr = tspi->phys + SLINK_TX_FIFO;
+		dma_sconfig.dst_addr = tspi->phys + SLINK_TX_FIFO;
+	}
+
+	ret = dmaengine_slave_config(dma_chan, &dma_sconfig);
+	if (ret)
+		goto scrub;
+	if (dma_to_memory) {
+		tspi->rx_dma_chan = dma_chan;
+		tspi->rx_dma_buf = dma_buf;
+		tspi->rx_dma_phys = dma_phys;
+	} else {
+		tspi->tx_dma_chan = dma_chan;
+		tspi->tx_dma_buf = dma_buf;
+		tspi->tx_dma_phys = dma_phys;
+	}
+	return 0;
+
+scrub:
+	dma_free_coherent(tspi->dev, tspi->dma_buf_size, dma_buf, dma_phys);
+	dma_release_channel(dma_chan);
+	return ret;
+}
+
+static void tegra_slink_deinit_dma_param(struct tegra_slink_data *tspi,
+	bool dma_to_memory)
+{
+	u32 *dma_buf;
+	dma_addr_t dma_phys;
+	struct dma_chan *dma_chan;
+
+	if (dma_to_memory) {
+		dma_buf = tspi->rx_dma_buf;
+		dma_chan = tspi->rx_dma_chan;
+		dma_phys = tspi->rx_dma_phys;
+		tspi->rx_dma_chan = NULL;
+		tspi->rx_dma_buf = NULL;
+	} else {
+		dma_buf = tspi->tx_dma_buf;
+		dma_chan = tspi->tx_dma_chan;
+		dma_phys = tspi->tx_dma_phys;
+		tspi->tx_dma_buf = NULL;
+		tspi->tx_dma_chan = NULL;
+	}
+	if (!dma_chan)
+		return;
+
+	dma_free_coherent(tspi->dev, tspi->dma_buf_size, dma_buf, dma_phys);
+	dma_release_channel(dma_chan);
+}
+
+static int tegra_slink_start_transfer_one(struct spi_device *spi,
+		    struct spi_transfer *t, bool is_first_of_msg,
+		    bool is_single_xfer)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
+	u32 speed;
+	u8 bits_per_word;
+	unsigned total_fifo_words;
+	int ret;
+	struct tegra_spi_device_controller_data *cdata = spi->controller_data;
+	unsigned long command;
+	unsigned long command2;
+
+	bits_per_word = t->bits_per_word ? t->bits_per_word :
+					spi->bits_per_word;
+
+	speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
+	if (!speed)
+		speed = tspi->spi_max_frequency;
+	if (speed != tspi->cur_speed) {
+		clk_set_rate(tspi->clk, speed * 4);
+		tspi->cur_speed = speed;
+	}
+
+	tspi->cur_spi = spi;
+	tspi->cur_pos = 0;
+	tspi->cur_rx_pos = 0;
+	tspi->cur_tx_pos = 0;
+	tspi->curr_xfer = t;
+	total_fifo_words = tegra_slink_calculate_curr_xfer_param(spi, tspi, t);
+
+	if (is_first_of_msg) {
+		tegra_slink_clear_status(tspi);
+
+		command = tspi->def_command_reg;
+		command |= SLINK_BIT_LENGTH(bits_per_word - 1);
+
+		command2 = tspi->def_command2_reg;
+		command2 |= SLINK_SS_EN_CS(spi->chip_select);
+
+		/* possibly use the hw based chip select */
+		tspi->is_hw_based_cs = false;
+		if (cdata && cdata->is_hw_based_cs && is_single_xfer &&
+			((tspi->curr_dma_words * tspi->bytes_per_word) ==
+						(t->len - tspi->cur_pos))) {
+			int setup_count;
+			int sts2;
+
+			setup_count = cdata->cs_setup_clk_count >> 1;
+			setup_count = max(setup_count, 3);
+			command2 |= SLINK_SS_SETUP(setup_count);
+			if (tspi->chip_data->cs_hold_time) {
+				int hold_count;
+
+				hold_count = cdata->cs_hold_clk_count;
+				hold_count = max(hold_count, 0xF);
+				sts2 = tegra_slink_readl(tspi, SLINK_STATUS2);
+				sts2 &= ~SLINK_SS_HOLD_TIME(0xF);
+				sts2 |= SLINK_SS_HOLD_TIME(hold_count);
+				tegra_slink_writel(tspi, sts2, SLINK_STATUS2);
+			}
+			tspi->is_hw_based_cs = true;
+		}
+
+		if (tspi->is_hw_based_cs)
+			command &= ~SLINK_CS_SW;
+		else
+			command |= SLINK_CS_SW | SLINK_CS_VALUE;
+
+		command &= ~SLINK_MODES;
+		if (spi->mode & SPI_CPHA)
+			command |= SLINK_CK_SDA;
+
+		if (spi->mode & SPI_CPOL)
+			command |= SLINK_IDLE_SCLK_DRIVE_HIGH;
+		else
+			command |= SLINK_IDLE_SCLK_DRIVE_LOW;
+	} else {
+		command = tspi->command_reg;
+		command &= ~SLINK_BIT_LENGTH(~0);
+		command |= SLINK_BIT_LENGTH(bits_per_word - 1);
+
+		command2 = tspi->command2_reg;
+		command2 &= ~(SLINK_RXEN | SLINK_TXEN);
+	}
+
+	tegra_slink_writel(tspi, command, SLINK_COMMAND);
+	tspi->command_reg = command;
+
+	tspi->cur_direction = 0;
+	if (t->rx_buf) {
+		command2 |= SLINK_RXEN;
+		tspi->cur_direction |= DATA_DIR_RX;
+	}
+	if (t->tx_buf) {
+		command2 |= SLINK_TXEN;
+		tspi->cur_direction |= DATA_DIR_TX;
+	}
+	tegra_slink_writel(tspi, command2, SLINK_COMMAND2);
+	tspi->command2_reg = command2;
+
+	if (total_fifo_words > SLINK_FIFO_DEPTH)
+		ret = tegra_slink_start_dma_based_transfer(tspi, t);
+	else
+		ret = tegra_slink_start_cpu_based_transfer(tspi, t);
+	return ret;
+}
+
+static int tegra_slink_setup(struct spi_device *spi)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long val;
+	unsigned long flags;
+	unsigned int cs_pol_bit[MAX_CHIP_SELECT] = {
+			SLINK_CS_POLARITY,
+			SLINK_CS_POLARITY1,
+			SLINK_CS_POLARITY2,
+			SLINK_CS_POLARITY3,
+	};
+
+	dev_dbg(&spi->dev, "setup %d bpw, %scpol, %scpha, %dHz\n",
+		spi->bits_per_word,
+		spi->mode & SPI_CPOL ? "" : "~",
+		spi->mode & SPI_CPHA ? "" : "~",
+		spi->max_speed_hz);
+
+	BUG_ON(spi->chip_select >= MAX_CHIP_SELECT);
+
+	pm_runtime_get_sync(tspi->dev);
+	tegra_slink_clk_enable(tspi);
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	val = tspi->def_command_reg;
+	if (spi->mode & SPI_CS_HIGH)
+		val |= cs_pol_bit[spi->chip_select];
+	else
+		val &= ~cs_pol_bit[spi->chip_select];
+	tspi->def_command_reg = val;
+	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	tegra_slink_clk_disable(tspi);
+	pm_runtime_put_sync(tspi->dev);
+	return 0;
+}
+
+static int tegra_slink_prepare_transfer(struct spi_master *master)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
+	pm_runtime_get_sync(tspi->dev);
+	return 0;
+}
+
+static int tegra_slink_unprepare_transfer(struct spi_master *master)
+{
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
+	pm_runtime_put_sync(tspi->dev);
+	return 0;
+}
+
+static int tegra_slink_transfer_one_message(struct spi_master *master,
+			struct spi_message *msg)
+{
+
+	bool is_first_msg = true;
+	int single_xfer;
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	int ret;
+
+	msg->status = 0;
+	msg->actual_length = 0;
+	tegra_slink_clk_enable(tspi);
+	single_xfer = list_is_singular(&msg->transfers);
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		INIT_COMPLETION(tspi->xfer_completion);
+		ret = tegra_slink_start_transfer_one(spi, xfer,
+					is_first_msg, single_xfer);
+		if (ret < 0) {
+			dev_err(tspi->dev,
+				"spi can not start transfer, err %d\n", ret);
+			goto exit;
+		}
+		is_first_msg = false;
+		ret = wait_for_completion_timeout(&tspi->xfer_completion,
+						SLINK_DMA_TIMEOUT);
+		if (WARN_ON(ret == 0)) {
+			dev_err(tspi->dev,
+				"spi trasfer timeout, err %d\n", ret);
+			ret = -EIO;
+			goto exit;
+		}
+
+		if (tspi->tx_status ||  tspi->rx_status) {
+			dev_err(tspi->dev, "Error in Transfer\n");
+			ret = -EIO;
+			goto exit;
+		}
+		msg->actual_length += xfer->len;
+		if (xfer->cs_change && xfer->delay_usecs) {
+			tegra_slink_writel(tspi, tspi->def_command_reg,
+					SLINK_COMMAND);
+			udelay(xfer->delay_usecs);
+		}
+	}
+	ret = 0;
+exit:
+	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
+	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
+	tegra_slink_clk_disable(tspi);
+	msg->status = ret;
+	spi_finalize_current_message(master);
+	return ret;
+}
+
+static void handle_cpu_based_xfer(void *context_data)
+{
+	struct tegra_slink_data *tspi = context_data;
+	struct spi_transfer *t = tspi->curr_xfer;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	if (tspi->tx_status ||  tspi->rx_status ||
+				(tspi->status_reg & SLINK_BSY)) {
+		dev_err(tspi->dev,
+			"CpuXfer ERROR bit set 0x%x\n", tspi->status_reg);
+		dev_err(tspi->dev,
+			"CpuXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
+				tspi->command2_reg, tspi->dma_control_reg);
+		tegra_periph_reset_assert(tspi->clk);
+		udelay(2);
+		tegra_periph_reset_deassert(tspi->clk);
+		complete(&tspi->xfer_completion);
+		goto exit;
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		tegra_slink_read_rx_fifo_to_client_rxbuf(tspi, t);
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		tspi->cur_pos = tspi->cur_tx_pos;
+	else
+		tspi->cur_pos = tspi->cur_rx_pos;
+
+	if (tspi->cur_pos == t->len) {
+		complete(&tspi->xfer_completion);
+		goto exit;
+	}
+
+	tegra_slink_calculate_curr_xfer_param(tspi->cur_spi, tspi, t);
+	tegra_slink_start_cpu_based_transfer(tspi, t);
+exit:
+	spin_unlock_irqrestore(&tspi->lock, flags);
+	return;
+}
+
+static irqreturn_t tegra_slink_isr_thread(int irq, void *context_data)
+{
+	struct tegra_slink_data *tspi = context_data;
+	struct spi_transfer *t = tspi->curr_xfer;
+	long wait_status;
+	int err = 0;
+	unsigned total_fifo_words;
+	unsigned long flags;
+
+	if (!tspi->is_curr_dma_xfer) {
+		handle_cpu_based_xfer(context_data);
+		return IRQ_HANDLED;
+	}
+
+	/* Abort dmas if any error */
+	if (tspi->cur_direction & DATA_DIR_TX) {
+		if (tspi->tx_status) {
+			dmaengine_terminate_all(tspi->tx_dma_chan);
+			err += 1;
+		} else {
+			wait_status = wait_for_completion_interruptible_timeout(
+				&tspi->tx_dma_complete, SLINK_DMA_TIMEOUT);
+			if (wait_status <= 0) {
+				dmaengine_terminate_all(tspi->tx_dma_chan);
+				dev_err(tspi->dev, "TxDma Xfer failed\n");
+				err += 1;
+			}
+		}
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX) {
+		if (tspi->rx_status) {
+			dmaengine_terminate_all(tspi->rx_dma_chan);
+			err += 2;
+		} else {
+			wait_status = wait_for_completion_interruptible_timeout(
+				&tspi->rx_dma_complete, SLINK_DMA_TIMEOUT);
+			if (wait_status <= 0) {
+				dmaengine_terminate_all(tspi->rx_dma_chan);
+				dev_err(tspi->dev, "RxDma Xfer failed\n");
+				err += 2;
+			}
+		}
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	if (err) {
+		dev_err(tspi->dev,
+			"DmaXfer: ERROR bit set 0x%x\n", tspi->status_reg);
+		dev_err(tspi->dev,
+			"DmaXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
+				tspi->command2_reg, tspi->dma_control_reg);
+		tegra_periph_reset_assert(tspi->clk);
+		udelay(2);
+		tegra_periph_reset_deassert(tspi->clk);
+		complete(&tspi->xfer_completion);
+		spin_unlock_irqrestore(&tspi->lock, flags);
+		return IRQ_HANDLED;
+	}
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		tegra_slink_copy_spi_rxbuf_to_client_rxbuf(tspi, t);
+
+	if (tspi->cur_direction & DATA_DIR_TX)
+		tspi->cur_pos = tspi->cur_tx_pos;
+	else
+		tspi->cur_pos = tspi->cur_rx_pos;
+
+	if (tspi->cur_pos == t->len) {
+		complete(&tspi->xfer_completion);
+		goto exit;
+	}
+
+	/* Continue transfer in current message */
+	total_fifo_words = tegra_slink_calculate_curr_xfer_param(tspi->cur_spi,
+							tspi, t);
+	if (total_fifo_words > SLINK_FIFO_DEPTH)
+		err = tegra_slink_start_dma_based_transfer(tspi, t);
+	else
+		err = tegra_slink_start_cpu_based_transfer(tspi, t);
+
+exit:
+	spin_unlock_irqrestore(&tspi->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t tegra_slink_isr(int irq, void *context_data)
+{
+	struct tegra_slink_data *tspi = context_data;
+
+	tspi->status_reg = tegra_slink_readl(tspi, SLINK_STATUS);
+	if (tspi->cur_direction & DATA_DIR_TX)
+		tspi->tx_status = tspi->status_reg &
+					(SLINK_TX_OVF | SLINK_TX_UNF);
+
+	if (tspi->cur_direction & DATA_DIR_RX)
+		tspi->rx_status = tspi->status_reg &
+					(SLINK_RX_OVF | SLINK_RX_UNF);
+	tegra_slink_clear_status(tspi);
+
+	return IRQ_WAKE_THREAD;
+}
+
+#ifdef CONFIG_OF
+static struct tegra_spi_platform_data *tegra_slink_parese_dt(
+		struct platform_device *pdev)
+{
+	struct tegra_spi_platform_data *pdata;
+	const unsigned int *prop;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(&pdev->dev, "Memory alloc for pdata failed\n");
+		return NULL;
+	}
+
+	prop = of_get_property(pdev->dev.of_node, "nvidia,dma-req-sel", NULL);
+	if (prop)
+		pdata->dma_req_sel = be32_to_cpup(prop);
+
+	prop = of_get_property(pdev->dev.of_node, "spi_max_frequency", NULL);
+	if (prop)
+		pdata->spi_max_frequency = be32_to_cpup(prop);
+	else
+		pdata->spi_max_frequency = 25000000; /* 25MHz */
+
+	return pdata;
+}
+#else
+static struct tegra_spi_platform_data *tegra_slink_parese_dt(
+		struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
+
+const struct tegra_slink_chip_data tegra30_spi_cdata = {
+	.cs_hold_time = true,
+};
+
+#ifdef CONFIG_OF
+const struct tegra_slink_chip_data tegra20_spi_cdata = {
+	.cs_hold_time = false,
+};
+
+static struct of_device_id tegra_slink_of_match[] __devinitconst = {
+	{ .compatible = "nvidia,tegra20-slink", .data = &tegra20_spi_cdata, },
+	{ .compatible = "nvidia,tegra30-slink", .data = &tegra30_spi_cdata, },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tegra_slink_of_match);
+#endif
+
+static int __devinit tegra_slink_probe(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct tegra_slink_data	*tspi;
+	struct resource		*r;
+	struct tegra_spi_platform_data *pdata = pdev->dev.platform_data;
+	int ret, spi_irq;
+	const struct tegra_slink_chip_data *cdata = NULL;
+
+	if (!pdata && pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_device(of_match_ptr(tegra_slink_of_match),
+				&pdev->dev);
+		if (!match) {
+			dev_err(&pdev->dev, "Error: No device match found\n");
+			return -ENODEV;
+		}
+		cdata = match->data;
+		pdata = tegra_slink_parese_dt(pdev);
+	} else {
+		cdata = &tegra30_spi_cdata;
+	}
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data, exiting\n");
+		return -ENODEV;
+	}
+
+	master = spi_alloc_master(&pdev->dev, sizeof(*tspi));
+	if (!master) {
+		dev_err(&pdev->dev, "master allocation failed\n");
+		return -ENOMEM;
+	}
+
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+
+	/* the spi->mode bits understood by this driver: */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	master->setup = tegra_slink_setup;
+	master->prepare_transfer_hardware = tegra_slink_prepare_transfer;
+	master->transfer_one_message = tegra_slink_transfer_one_message;
+	master->unprepare_transfer_hardware = tegra_slink_unprepare_transfer;
+	master->num_chipselect = MAX_CHIP_SELECT;
+
+	dev_set_drvdata(&pdev->dev, master);
+	tspi = spi_master_get_devdata(master);
+	tspi->master = master;
+	tspi->dma_req_sel = pdata->dma_req_sel;
+	tspi->dev = &pdev->dev;
+	tspi->chip_data = cdata;
+	spin_lock_init(&tspi->lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "No IO memory resource\n");
+		ret = -ENODEV;
+		goto exit_free_master;
+	}
+	tspi->phys = r->start;
+	tspi->base = devm_request_and_ioremap(&pdev->dev, r);
+	if (!tspi->base) {
+		dev_err(&pdev->dev,
+			"Cannot request memregion/iomap dma address\n");
+		ret = -EADDRNOTAVAIL;
+		goto exit_free_master;
+	}
+
+	spi_irq = platform_get_irq(pdev, 0);
+	if (unlikely(spi_irq < 0)) {
+		dev_err(&pdev->dev, "can't find irq resource\n");
+		ret = -ENXIO;
+		goto exit_free_master;
+	}
+	tspi->irq = spi_irq;
+
+	ret = devm_request_threaded_irq(&pdev->dev, tspi->irq,
+			tegra_slink_isr, tegra_slink_isr_thread, IRQF_ONESHOT,
+			dev_name(&pdev->dev), tspi);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register ISR for IRQ %d\n",
+					tspi->irq);
+		goto exit_free_master;
+	}
+
+	tspi->clk = devm_clk_get(&pdev->dev, "slink");
+	if (IS_ERR(tspi->clk)) {
+		dev_err(&pdev->dev, "can not get clock\n");
+		ret = PTR_ERR(tspi->clk);
+		goto exit_free_master;
+	}
+
+	tspi->max_buf_size = SLINK_FIFO_DEPTH << 2;
+	tspi->dma_buf_size = DEFAULT_SPI_DMA_BUF_LEN;
+	tspi->spi_max_frequency = pdata->spi_max_frequency;
+
+	if (pdata->dma_req_sel) {
+		ret = tegra_slink_init_dma_param(tspi, true);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "RxDma Init failed, err %d\n", ret);
+			goto exit_free_master;
+		}
+
+		ret = tegra_slink_init_dma_param(tspi, false);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "TxDma Init failed, err %d\n", ret);
+			goto exit_rx_dma_free;
+		}
+		tspi->max_buf_size = tspi->dma_buf_size;
+		init_completion(&tspi->tx_dma_complete);
+		init_completion(&tspi->rx_dma_complete);
+	}
+
+	init_completion(&tspi->xfer_completion);
+
+	tspi->def_command_reg  = SLINK_M_S;
+	tspi->def_command2_reg = SLINK_CS_ACTIVE_BETWEEN;
+	tegra_slink_clk_enable(tspi);
+	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
+	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
+	tegra_slink_clk_disable(tspi);
+
+	pm_runtime_enable(&pdev->dev);
+
+	master->dev.of_node = pdev->dev.of_node;
+	ret = spi_register_master(master);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "can not register to master err %d\n", ret);
+		goto exit_fail_master_reg;
+	}
+	return ret;
+
+exit_fail_master_reg:
+	pm_runtime_disable(&pdev->dev);
+	tegra_slink_deinit_dma_param(tspi, false);
+exit_rx_dma_free:
+	tegra_slink_deinit_dma_param(tspi, true);
+exit_free_master:
+	spi_master_put(master);
+	return ret;
+}
+
+static int __devexit tegra_slink_remove(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct tegra_slink_data	*tspi;
+
+	master = dev_get_drvdata(&pdev->dev);
+	tspi = spi_master_get_devdata(master);
+
+	spi_unregister_master(master);
+
+	if (tspi->tx_dma_chan)
+		tegra_slink_deinit_dma_param(tspi, false);
+
+	if (tspi->rx_dma_chan)
+		tegra_slink_deinit_dma_param(tspi, true);
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_slink_suspend(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+
+	spi_master_suspend(master);
+	return 0;
+}
+
+static int tegra_slink_resume(struct device *dev)
+{
+	struct spi_master *master = dev_get_drvdata(dev);
+	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
+
+	pm_runtime_get_sync(dev);
+	tegra_slink_clk_enable(tspi);
+	tegra_slink_writel(tspi, tspi->command_reg, SLINK_COMMAND);
+	tegra_slink_writel(tspi, tspi->command2_reg, SLINK_COMMAND2);
+	tegra_slink_clk_disable(tspi);
+	pm_runtime_put_sync(dev);
+
+	spi_master_resume(master);
+	return 0;
+}
+
+static const struct dev_pm_ops tegra_slink_dev_pm_ops = {
+	.suspend = tegra_slink_suspend,
+	.resume = tegra_slink_resume,
+};
+#define TEGRA_SPI_PM	(&tegra_slink_dev_pm_ops)
+#else
+#define TEGRA_SPI_PM	NULL
+#endif
+
+static struct platform_driver tegra_slink_driver = {
+	.driver = {
+		.name =		"spi-tegra-slink",
+		.owner =	THIS_MODULE,
+		.pm =		&tegra_slink_dev_pm_ops,
+		.of_match_table = of_match_ptr(tegra_slink_of_match),
+	},
+	.probe =	tegra_slink_probe,
+	.remove =	__devexit_p(tegra_slink_remove),
+};
+module_platform_driver(tegra_slink_driver);
+
+MODULE_ALIAS("platform:tegra_slink-slink");
+MODULE_DESCRIPTION("NVIDIA Tegra SLINK Controller Driver");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/spi/spi-tegra.h b/include/linux/spi/spi-tegra.h
new file mode 100644
index 0000000..786932c
--- /dev/null
+++ b/include/linux/spi/spi-tegra.h
@@ -0,0 +1,40 @@
+/*
+ * spi-tegra.h: SPI interface for Nvidia Tegra20 SLINK controller.
+ *
+ * Copyright (C) 2011 NVIDIA Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#ifndef _LINUX_SPI_TEGRA_H
+#define _LINUX_SPI_TEGRA_H
+
+struct tegra_spi_platform_data {
+	int dma_req_sel;
+	unsigned int spi_max_frequency;
+};
+
+/*
+ * Controller data from device to pass some info like
+ * hw based chip select can be used or not and if yes
+ * then CS hold and setup time.
+ */
+struct tegra_spi_device_controller_data {
+	bool is_hw_based_cs;
+	int cs_setup_clk_count;
+	int cs_hold_clk_count;
+};
+
+#endif /* _LINUX_SPI_TEGRA_H */
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-18 10:47 ` Laxman Dewangan
@ 2012-10-22 12:28     ` Mark Brown
  -1 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2012-10-22 12:28 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]

On Thu, Oct 18, 2012 at 04:17:13PM +0530, Laxman Dewangan wrote:

> +		udelay(1);
> +		wmb();
> +	}
> +	tspi->dma_control_reg = val;
> +	val |= SLINK_DMA_EN;
> +	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
> +	return 0;
> +}
> +static int tegra_slink_init_dma_param(struct tegra_slink_data *tspi,
> +			bool dma_to_memory)
> +{

Blank line between functions.

> +	tegra_slink_clk_disable(tspi);
> +	pm_runtime_put_sync(tspi->dev);
> +	return 0;

Throughout the code you're using pm_runtime_put_sync() - why does this
need to be _sync()?  Can't we just use a regular put()?  If we can't we
should document why.

> +static int tegra_slink_prepare_transfer(struct spi_master *master)
> +{
> +	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
> +
> +	pm_runtime_get_sync(tspi->dev);
> +	return 0;

Should really check the error code of the pm_runtime call here.

> +static struct tegra_spi_platform_data *tegra_slink_parese_dt(
> +		struct platform_device *pdev)
> +{

There doens't seem to be any binding documentation; binding
documentation is mandatory.

> +	prop = of_get_property(pdev->dev.of_node, "spi_max_frequency", NULL);
> +	if (prop)
> +		pdata->spi_max_frequency = be32_to_cpup(prop);
> +	else
> +		pdata->spi_max_frequency = 25000000; /* 25MHz */

Why is the default not being picked in the general non-OF case?

> +const struct tegra_slink_chip_data tegra30_spi_cdata = {
> +	.cs_hold_time = true,
> +};
> +
> +#ifdef CONFIG_OF
> +const struct tegra_slink_chip_data tegra20_spi_cdata = {
> +	.cs_hold_time = false,
> +};
> +
> +static struct of_device_id tegra_slink_of_match[] __devinitconst = {
> +	{ .compatible = "nvidia,tegra20-slink", .data = &tegra20_spi_cdata, },
> +	{ .compatible = "nvidia,tegra30-slink", .data = &tegra30_spi_cdata, },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, tegra_slink_of_match);
> +#endif

The ifdefs here look misplaced?

> +	spi_irq = platform_get_irq(pdev, 0);
> +	if (unlikely(spi_irq < 0)) {

Putting optimisation hints like unlikely() here is pointless.

> +	ret = devm_request_threaded_irq(&pdev->dev, tspi->irq,
> +			tegra_slink_isr, tegra_slink_isr_thread, IRQF_ONESHOT,
> +			dev_name(&pdev->dev), tspi);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to register ISR for IRQ %d\n",
> +					tspi->irq);
> +		goto exit_free_master;
> +	}

Any use of devm_ IRQ functions is suspicous... why is this safe against
an interrupt firing after the SPI device has started to deallocate?

> +#ifdef CONFIG_PM_SLEEP
> +static int tegra_slink_suspend(struct device *dev)
> +{
> +	struct spi_master *master = dev_get_drvdata(dev);
> +
> +	spi_master_suspend(master);
> +	return 0;

Should use the return code of spi_master_suspend().

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-22 12:28     ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2012-10-22 12:28 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: grant.likely, rob.herring, spi-devel-general, linux-kernel, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]

On Thu, Oct 18, 2012 at 04:17:13PM +0530, Laxman Dewangan wrote:

> +		udelay(1);
> +		wmb();
> +	}
> +	tspi->dma_control_reg = val;
> +	val |= SLINK_DMA_EN;
> +	tegra_slink_writel(tspi, val, SLINK_DMA_CTL);
> +	return 0;
> +}
> +static int tegra_slink_init_dma_param(struct tegra_slink_data *tspi,
> +			bool dma_to_memory)
> +{

Blank line between functions.

> +	tegra_slink_clk_disable(tspi);
> +	pm_runtime_put_sync(tspi->dev);
> +	return 0;

Throughout the code you're using pm_runtime_put_sync() - why does this
need to be _sync()?  Can't we just use a regular put()?  If we can't we
should document why.

> +static int tegra_slink_prepare_transfer(struct spi_master *master)
> +{
> +	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
> +
> +	pm_runtime_get_sync(tspi->dev);
> +	return 0;

Should really check the error code of the pm_runtime call here.

> +static struct tegra_spi_platform_data *tegra_slink_parese_dt(
> +		struct platform_device *pdev)
> +{

There doens't seem to be any binding documentation; binding
documentation is mandatory.

> +	prop = of_get_property(pdev->dev.of_node, "spi_max_frequency", NULL);
> +	if (prop)
> +		pdata->spi_max_frequency = be32_to_cpup(prop);
> +	else
> +		pdata->spi_max_frequency = 25000000; /* 25MHz */

Why is the default not being picked in the general non-OF case?

> +const struct tegra_slink_chip_data tegra30_spi_cdata = {
> +	.cs_hold_time = true,
> +};
> +
> +#ifdef CONFIG_OF
> +const struct tegra_slink_chip_data tegra20_spi_cdata = {
> +	.cs_hold_time = false,
> +};
> +
> +static struct of_device_id tegra_slink_of_match[] __devinitconst = {
> +	{ .compatible = "nvidia,tegra20-slink", .data = &tegra20_spi_cdata, },
> +	{ .compatible = "nvidia,tegra30-slink", .data = &tegra30_spi_cdata, },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, tegra_slink_of_match);
> +#endif

The ifdefs here look misplaced?

> +	spi_irq = platform_get_irq(pdev, 0);
> +	if (unlikely(spi_irq < 0)) {

Putting optimisation hints like unlikely() here is pointless.

> +	ret = devm_request_threaded_irq(&pdev->dev, tspi->irq,
> +			tegra_slink_isr, tegra_slink_isr_thread, IRQF_ONESHOT,
> +			dev_name(&pdev->dev), tspi);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to register ISR for IRQ %d\n",
> +					tspi->irq);
> +		goto exit_free_master;
> +	}

Any use of devm_ IRQ functions is suspicous... why is this safe against
an interrupt firing after the SPI device has started to deallocate?

> +#ifdef CONFIG_PM_SLEEP
> +static int tegra_slink_suspend(struct device *dev)
> +{
> +	struct spi_master *master = dev_get_drvdata(dev);
> +
> +	spi_master_suspend(master);
> +	return 0;

Should use the return code of spi_master_suspend().

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-22 12:28     ` Mark Brown
@ 2012-10-22 18:04         ` Thierry Reding
  -1 siblings, 0 replies; 18+ messages in thread
From: Thierry Reding @ 2012-10-22 18:04 UTC (permalink / raw)
  To: Mark Brown
  Cc: Laxman Dewangan, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

On Mon, Oct 22, 2012 at 01:28:26PM +0100, Mark Brown wrote:
> On Thu, Oct 18, 2012 at 04:17:13PM +0530, Laxman Dewangan wrote:
[...]
> > +static struct tegra_spi_platform_data *tegra_slink_parese_dt(
> > +		struct platform_device *pdev)
> > +{
> 
> There doens't seem to be any binding documentation; binding
> documentation is mandatory.

Also this should probably be renamed to tegra_slink_parse_dt().

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-22 18:04         ` Thierry Reding
  0 siblings, 0 replies; 18+ messages in thread
From: Thierry Reding @ 2012-10-22 18:04 UTC (permalink / raw)
  To: Mark Brown
  Cc: Laxman Dewangan, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

On Mon, Oct 22, 2012 at 01:28:26PM +0100, Mark Brown wrote:
> On Thu, Oct 18, 2012 at 04:17:13PM +0530, Laxman Dewangan wrote:
[...]
> > +static struct tegra_spi_platform_data *tegra_slink_parese_dt(
> > +		struct platform_device *pdev)
> > +{
> 
> There doens't seem to be any binding documentation; binding
> documentation is mandatory.

Also this should probably be renamed to tegra_slink_parse_dt().

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-18 10:47 ` Laxman Dewangan
@ 2012-10-22 20:02     ` Stephen Warren
  -1 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-10-22 20:02 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
> Tegra20/Tegra30 supports the spi interface through its SLINK
> controller. Add spi driver for SLINK controller.

> diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c

> +static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
> +		unsigned long val, unsigned long reg)
> +{
> +	writel(val, tspi->base + reg);
> +
> +	/* Read back register to make sure that register writes completed */
> +	if (reg != SLINK_TX_FIFO)
> +		readl(tspi->base + SLINK_MAS_DATA);

Is that really necessary on every single write, or only for certain
specific operations? This seems rather heavy-weight.

> +static void tegra_slink_clear_status(struct tegra_slink_data *tspi)
> +{
> +	unsigned long val;
> +	unsigned long val_write = 0;
> +
> +	val = tegra_slink_readl(tspi, SLINK_STATUS);
> +
> +	/* Write 1 to clear status register */
> +	val_write = SLINK_RDY;
> +	val_write |= (val & SLINK_FIFO_ERROR);

Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
write only if the status was previously asserted? If that is true, how
do you avoid a race condition where the bit gets set in SLINK_STATUS
after you read it but before you write to clear it?

> +	tegra_slink_writel(tspi, val_write, SLINK_STATUS);
> +}

> +static unsigned tegra_slink_calculate_curr_xfer_param(

> +	if (bits_per_word == 8 || bits_per_word == 16) {
> +		tspi->is_packed = 1;
> +		tspi->words_per_32bit = 32/bits_per_word;

Spaces required around all operators. Does this pass checkpatch? No:
total: 1405 errors, 11 warnings, 1418 lines checked

> +static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(

> +	if (tspi->is_packed) {
> +		nbytes = tspi->curr_dma_words * tspi->bytes_per_word;
> +		max_n_32bit = (min(nbytes,  tx_empty_count*4) - 1)/4 + 1;
> +		for (count = 0; count < max_n_32bit; ++count) {

Very minor almost irrelevant nit: count++ would be much more typical here.

> +			x = 0;
> +			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
> +				x |= (*tx_buf++) << (i*8);
> +			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
> +		}
> +		written_words =  min(max_n_32bit * tspi->words_per_32bit,
> +					tspi->curr_dma_words);

The calculations here seem a little over-complex. Why not calculate the
FIFO space in words above, and just set written words based on that.
Something like:

fifo_words_left = tx_empty_count * spi->words_per_32bit;
written_words = min(fifo_words_left, tspi->curr_dma_words);
nbytes = written_words * spi->bytes_per_word;
max_n_32bit = (nbytes + 3) / 4;

Most likely a similar comment applies to all the other similar functions
for filling FIFOs and buffers.

In fact, I suspect you can completely get rid of the if (is_packed)
statement here by appropriately parameterising the code using variables
in *spi...

> +static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(

> +		bits_per_word = t->bits_per_word ? t->bits_per_word :
> +						tspi->cur_spi->bits_per_word;

That logic is repeated a few times. Shouldn't there be a macro to avoid
cut/paste. Perhaps even in the SPI core rather than this driver.

> +		rx_mask = (1 << bits_per_word) - 1;
> +		for (count = 0; count < rx_full_count; ++count) {
> +			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
> +			x &= rx_mask;

I don't think you need that mask; the loop below only extracts bytes
that actually exist in the FIFO right?

> +			for (i = 0; (i < tspi->bytes_per_word); ++i)
> +				*rx_buf++ = (x >> (i*8)) & 0xFF;

> +static void tegra_slink_copy_client_txbuf_to_spi_txbuf(
> +		struct tegra_slink_data *tspi, struct spi_transfer *t)

Are there no cases where we can simply DMA straight into the client
buffer? I suppose it would be limited to cache-line-aligned
cache-line-size-aligned buffers which is probably unlikely in general:-(

> +static int tegra_slink_start_dma_based_transfer(
> +		struct tegra_slink_data *tspi, struct spi_transfer *t)

> +	/* Make sure that Rx and Tx fifo are empty */
> +	status = tegra_slink_readl(tspi, SLINK_STATUS);
> +	if ((status & SLINK_FIFO_EMPTY) != SLINK_FIFO_EMPTY)
> +		dev_err(tspi->dev,
> +			"Rx/Tx fifo are not empty status 0x%08lx\n", status);

Shouldn't this return an error, or drain the FIFO, or something?

> +static int tegra_slink_init_dma_param(struct tegra_slink_data *tspi,

> +	if (dma_to_memory) {
> +		dma_sconfig.src_addr = tspi->phys + SLINK_RX_FIFO;
> +		dma_sconfig.dst_addr = tspi->phys + SLINK_RX_FIFO;
> +	} else {
> +		dma_sconfig.src_addr = tspi->phys + SLINK_TX_FIFO;
> +		dma_sconfig.dst_addr = tspi->phys + SLINK_TX_FIFO;
> +	}

Surely each branch of that if should only initialize one of {src,dst}_addr?

> +static int tegra_slink_prepare_transfer(struct spi_master *master)
> +{
> +	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
> +
> +	pm_runtime_get_sync(tspi->dev);
> +	return 0;
> +}
> +
> +static int tegra_slink_unprepare_transfer(struct spi_master *master)
> +{
> +	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
> +
> +	pm_runtime_put_sync(tspi->dev);
> +	return 0;
> +}

Does this driver actually implement any runtime PM ops? I certainly
don't see any in struct dev_pm_ops tegra_slink_dev_pm_ops. Related, why
do tegra_slink_clk_{en,dis}able exist; shouldn't those functions be the
implementation of the runtime PM ops? Related, why are
tegra_slink_clk_{en,dis}able called all over the place, rather than
relying on runtime PM API calls, or tegra_slink_{,un}prepare_transfer
having been called?

> +static int tegra_slink_transfer_one_message(struct spi_master *master,
> +			struct spi_message *msg)
> +{
> +
> +	bool is_first_msg = true;

Unnecessary blank line there.

> +static irqreturn_t tegra_slink_isr_thread(int irq, void *context_data)

> +	if (!tspi->is_curr_dma_xfer) {
> +		handle_cpu_based_xfer(context_data);
> +		return IRQ_HANDLED;
> +	}

Minor nit: That implies the rest of the body of this function should be
in another function named handle_dma_based_xfer() for symmetry.

> +	/* Abort dmas if any error */
> +	if (tspi->cur_direction & DATA_DIR_TX) {
> +		if (tspi->tx_status) {
> +			dmaengine_terminate_all(tspi->tx_dma_chan);
> +			err += 1;
> +		} else {
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&tspi->tx_dma_complete, SLINK_DMA_TIMEOUT);
> +			if (wait_status <= 0) {
> +				dmaengine_terminate_all(tspi->tx_dma_chan);
> +				dev_err(tspi->dev, "TxDma Xfer failed\n");
> +				err += 1;
> +			}
> +		}
> +	}
> +
> +	if (tspi->cur_direction & DATA_DIR_RX) {
> +		if (tspi->rx_status) {
> +			dmaengine_terminate_all(tspi->rx_dma_chan);
> +			err += 2;
> +		} else {
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&tspi->rx_dma_complete, SLINK_DMA_TIMEOUT);
> +			if (wait_status <= 0) {
> +				dmaengine_terminate_all(tspi->rx_dma_chan);
> +				dev_err(tspi->dev, "RxDma Xfer failed\n");
> +				err += 2;
> +			}
> +		}
> +	}

So that all implies that we wait for the very first interrupt from the
SPI peripheral for a transfer, and then wait for the DMA controller to
complete all DMA (which would probably entail actually waiting for DMA
to drain the RX FIFO in the RX case). Does it make sense to simply
return from the ISR if not all conditions that we will wait on have
occurred, and so avoid blocking this ISR thread? I suppose since this
thread gets blocked rather than spinning, it's not a big deal though.

> +	spin_lock_irqsave(&tspi->lock, flags);
> +	if (err) {
> +		dev_err(tspi->dev,
> +			"DmaXfer: ERROR bit set 0x%x\n", tspi->status_reg);
> +		dev_err(tspi->dev,
> +			"DmaXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
> +				tspi->command2_reg, tspi->dma_control_reg);
> +		tegra_periph_reset_assert(tspi->clk);
> +		udelay(2);
> +		tegra_periph_reset_deassert(tspi->clk);

Do we /have/ to reset the SPI block; can't we just disable it in the
control register, clear all status, and re-program it from scratch?

If at all possible, I would like to avoid introducing any new use of
tegra_periph_reset_{,de}assert, since that API has no standard subsystem
equivalent (or if it does, isn't hooked into the standard subsystem
yet), and hence means this driver relies on a header file currently in
arch/arm/mach-tegra/include/mach, and we need to move or delete all such
headers in order to support single zImage.

> +#ifdef CONFIG_OF

No need for this ifdef; Tegra always has CONFIG_OF enabled and always
boots using DT now in mainline.

> +static struct tegra_spi_platform_data *tegra_slink_parese_dt(

Typo, as thierry pointed out.

> +	prop = of_get_property(pdev->dev.of_node, "nvidia,dma-req-sel", NULL);
> +	if (prop)
> +		pdata->dma_req_sel = be32_to_cpup(prop);

At least, please lets use the standardize format for this, which is
<phandle select> not just <select>; see the I2S driver code. I thought
that's what the .dts patch you posted did...

> +	prop = of_get_property(pdev->dev.of_node, "spi_max_frequency", NULL);

Property names use - not _ as word separators. Isn't there a
standardized property for this? Yes, in fact it's spi-max-frequency
according to Documentation/devicetree/bindings/spi/spi-bus.txt.

Doesn't this function need to parse all the other standardized
properties from spi-bus.txt, or does the SPI core handle that?

> +const struct tegra_slink_chip_data tegra30_spi_cdata = {
> +	.cs_hold_time = true,
> +};
> +
> +#ifdef CONFIG_OF
> +const struct tegra_slink_chip_data tegra20_spi_cdata = {
> +	.cs_hold_time = false,
> +};

Again, no need for that ifdef. Why would the Tegra30 data be outside the
ifdef and the Tegra20 data be inside it anyway?

> +static int __devinit tegra_slink_probe(struct platform_device *pdev)

> +	if (!pdata && pdev->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_device(of_match_ptr(tegra_slink_of_match),
> +				&pdev->dev);
> +		if (!match) {
> +			dev_err(&pdev->dev, "Error: No device match found\n");
> +			return -ENODEV;
> +		}
> +		cdata = match->data;
> +		pdata = tegra_slink_parese_dt(pdev);
> +	} else {
> +		cdata = &tegra30_spi_cdata;
> +	}

That can be simplified since we know DT is enabled and used to boot, so
of_match_device() can always be called. The only conditional that's
needed is to check if explicit platform data was supplied to override
the DT.

> +	if (pdev->id != -1)
> +		master->bus_num = pdev->id;

That will never be the case, since we boot using DT.

> +	spi_irq = platform_get_irq(pdev, 0);
> +	if (unlikely(spi_irq < 0)) {

That should be <= or ==; 0 is an invalid IRQ. I imagine that
request_irq() checks that anyway, so you probably don't need to test it
here too.

> +	tspi->def_command_reg  = SLINK_M_S;
> +	tspi->def_command2_reg = SLINK_CS_ACTIVE_BETWEEN;
> +	tegra_slink_clk_enable(tspi);
> +	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
> +	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
> +	tegra_slink_clk_disable(tspi);
> +
> +	pm_runtime_enable(&pdev->dev);

There are register writes before that, so the runtime PM setup probably
should happen earlier. Also, once runtime PM is actually implemented,
you'll need to handle the case where kernel runtime PM support is
disabled. Check out tegra20_i2s.c for what I believe is the correct way
to handle this.


> +static int __devexit tegra_slink_remove(struct platform_device *pdev)
> +{
> +	struct spi_master	*master;
> +	struct tegra_slink_data	*tspi;
> +
> +	master = dev_get_drvdata(&pdev->dev);
> +	tspi = spi_master_get_devdata(master);

You can probably wrap those two assignments only the same line as the
declaration.

> +static const struct dev_pm_ops tegra_slink_dev_pm_ops = {
> +	.suspend = tegra_slink_suspend,
> +	.resume = tegra_slink_resume,
> +};
> +#define TEGRA_SPI_PM	(&tegra_slink_dev_pm_ops)
> +#else
> +#define TEGRA_SPI_PM	NULL
> +#endif
> +
> +static struct platform_driver tegra_slink_driver = {
> +	.driver = {
> +		.name =		"spi-tegra-slink",
> +		.owner =	THIS_MODULE,
> +		.pm =		&tegra_slink_dev_pm_ops,

Can't you use SET_SYSTEM_SLEEP_PM_OPS() to assign the ops, so you don't
have to #define TEGRA_SPI_PM (which you ended up not even using)?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-22 20:02     ` Stephen Warren
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-10-22 20:02 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: broonie, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
> Tegra20/Tegra30 supports the spi interface through its SLINK
> controller. Add spi driver for SLINK controller.

> diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c

> +static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
> +		unsigned long val, unsigned long reg)
> +{
> +	writel(val, tspi->base + reg);
> +
> +	/* Read back register to make sure that register writes completed */
> +	if (reg != SLINK_TX_FIFO)
> +		readl(tspi->base + SLINK_MAS_DATA);

Is that really necessary on every single write, or only for certain
specific operations? This seems rather heavy-weight.

> +static void tegra_slink_clear_status(struct tegra_slink_data *tspi)
> +{
> +	unsigned long val;
> +	unsigned long val_write = 0;
> +
> +	val = tegra_slink_readl(tspi, SLINK_STATUS);
> +
> +	/* Write 1 to clear status register */
> +	val_write = SLINK_RDY;
> +	val_write |= (val & SLINK_FIFO_ERROR);

Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
write only if the status was previously asserted? If that is true, how
do you avoid a race condition where the bit gets set in SLINK_STATUS
after you read it but before you write to clear it?

> +	tegra_slink_writel(tspi, val_write, SLINK_STATUS);
> +}

> +static unsigned tegra_slink_calculate_curr_xfer_param(

> +	if (bits_per_word == 8 || bits_per_word == 16) {
> +		tspi->is_packed = 1;
> +		tspi->words_per_32bit = 32/bits_per_word;

Spaces required around all operators. Does this pass checkpatch? No:
total: 1405 errors, 11 warnings, 1418 lines checked

> +static unsigned tegra_slink_fill_tx_fifo_from_client_txbuf(

> +	if (tspi->is_packed) {
> +		nbytes = tspi->curr_dma_words * tspi->bytes_per_word;
> +		max_n_32bit = (min(nbytes,  tx_empty_count*4) - 1)/4 + 1;
> +		for (count = 0; count < max_n_32bit; ++count) {

Very minor almost irrelevant nit: count++ would be much more typical here.

> +			x = 0;
> +			for (i = 0; (i < 4) && nbytes; i++, nbytes--)
> +				x |= (*tx_buf++) << (i*8);
> +			tegra_slink_writel(tspi, x, SLINK_TX_FIFO);
> +		}
> +		written_words =  min(max_n_32bit * tspi->words_per_32bit,
> +					tspi->curr_dma_words);

The calculations here seem a little over-complex. Why not calculate the
FIFO space in words above, and just set written words based on that.
Something like:

fifo_words_left = tx_empty_count * spi->words_per_32bit;
written_words = min(fifo_words_left, tspi->curr_dma_words);
nbytes = written_words * spi->bytes_per_word;
max_n_32bit = (nbytes + 3) / 4;

Most likely a similar comment applies to all the other similar functions
for filling FIFOs and buffers.

In fact, I suspect you can completely get rid of the if (is_packed)
statement here by appropriately parameterising the code using variables
in *spi...

> +static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(

> +		bits_per_word = t->bits_per_word ? t->bits_per_word :
> +						tspi->cur_spi->bits_per_word;

That logic is repeated a few times. Shouldn't there be a macro to avoid
cut/paste. Perhaps even in the SPI core rather than this driver.

> +		rx_mask = (1 << bits_per_word) - 1;
> +		for (count = 0; count < rx_full_count; ++count) {
> +			x = tegra_slink_readl(tspi, SLINK_RX_FIFO);
> +			x &= rx_mask;

I don't think you need that mask; the loop below only extracts bytes
that actually exist in the FIFO right?

> +			for (i = 0; (i < tspi->bytes_per_word); ++i)
> +				*rx_buf++ = (x >> (i*8)) & 0xFF;

> +static void tegra_slink_copy_client_txbuf_to_spi_txbuf(
> +		struct tegra_slink_data *tspi, struct spi_transfer *t)

Are there no cases where we can simply DMA straight into the client
buffer? I suppose it would be limited to cache-line-aligned
cache-line-size-aligned buffers which is probably unlikely in general:-(

> +static int tegra_slink_start_dma_based_transfer(
> +		struct tegra_slink_data *tspi, struct spi_transfer *t)

> +	/* Make sure that Rx and Tx fifo are empty */
> +	status = tegra_slink_readl(tspi, SLINK_STATUS);
> +	if ((status & SLINK_FIFO_EMPTY) != SLINK_FIFO_EMPTY)
> +		dev_err(tspi->dev,
> +			"Rx/Tx fifo are not empty status 0x%08lx\n", status);

Shouldn't this return an error, or drain the FIFO, or something?

> +static int tegra_slink_init_dma_param(struct tegra_slink_data *tspi,

> +	if (dma_to_memory) {
> +		dma_sconfig.src_addr = tspi->phys + SLINK_RX_FIFO;
> +		dma_sconfig.dst_addr = tspi->phys + SLINK_RX_FIFO;
> +	} else {
> +		dma_sconfig.src_addr = tspi->phys + SLINK_TX_FIFO;
> +		dma_sconfig.dst_addr = tspi->phys + SLINK_TX_FIFO;
> +	}

Surely each branch of that if should only initialize one of {src,dst}_addr?

> +static int tegra_slink_prepare_transfer(struct spi_master *master)
> +{
> +	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
> +
> +	pm_runtime_get_sync(tspi->dev);
> +	return 0;
> +}
> +
> +static int tegra_slink_unprepare_transfer(struct spi_master *master)
> +{
> +	struct tegra_slink_data *tspi = spi_master_get_devdata(master);
> +
> +	pm_runtime_put_sync(tspi->dev);
> +	return 0;
> +}

Does this driver actually implement any runtime PM ops? I certainly
don't see any in struct dev_pm_ops tegra_slink_dev_pm_ops. Related, why
do tegra_slink_clk_{en,dis}able exist; shouldn't those functions be the
implementation of the runtime PM ops? Related, why are
tegra_slink_clk_{en,dis}able called all over the place, rather than
relying on runtime PM API calls, or tegra_slink_{,un}prepare_transfer
having been called?

> +static int tegra_slink_transfer_one_message(struct spi_master *master,
> +			struct spi_message *msg)
> +{
> +
> +	bool is_first_msg = true;

Unnecessary blank line there.

> +static irqreturn_t tegra_slink_isr_thread(int irq, void *context_data)

> +	if (!tspi->is_curr_dma_xfer) {
> +		handle_cpu_based_xfer(context_data);
> +		return IRQ_HANDLED;
> +	}

Minor nit: That implies the rest of the body of this function should be
in another function named handle_dma_based_xfer() for symmetry.

> +	/* Abort dmas if any error */
> +	if (tspi->cur_direction & DATA_DIR_TX) {
> +		if (tspi->tx_status) {
> +			dmaengine_terminate_all(tspi->tx_dma_chan);
> +			err += 1;
> +		} else {
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&tspi->tx_dma_complete, SLINK_DMA_TIMEOUT);
> +			if (wait_status <= 0) {
> +				dmaengine_terminate_all(tspi->tx_dma_chan);
> +				dev_err(tspi->dev, "TxDma Xfer failed\n");
> +				err += 1;
> +			}
> +		}
> +	}
> +
> +	if (tspi->cur_direction & DATA_DIR_RX) {
> +		if (tspi->rx_status) {
> +			dmaengine_terminate_all(tspi->rx_dma_chan);
> +			err += 2;
> +		} else {
> +			wait_status = wait_for_completion_interruptible_timeout(
> +				&tspi->rx_dma_complete, SLINK_DMA_TIMEOUT);
> +			if (wait_status <= 0) {
> +				dmaengine_terminate_all(tspi->rx_dma_chan);
> +				dev_err(tspi->dev, "RxDma Xfer failed\n");
> +				err += 2;
> +			}
> +		}
> +	}

So that all implies that we wait for the very first interrupt from the
SPI peripheral for a transfer, and then wait for the DMA controller to
complete all DMA (which would probably entail actually waiting for DMA
to drain the RX FIFO in the RX case). Does it make sense to simply
return from the ISR if not all conditions that we will wait on have
occurred, and so avoid blocking this ISR thread? I suppose since this
thread gets blocked rather than spinning, it's not a big deal though.

> +	spin_lock_irqsave(&tspi->lock, flags);
> +	if (err) {
> +		dev_err(tspi->dev,
> +			"DmaXfer: ERROR bit set 0x%x\n", tspi->status_reg);
> +		dev_err(tspi->dev,
> +			"DmaXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
> +				tspi->command2_reg, tspi->dma_control_reg);
> +		tegra_periph_reset_assert(tspi->clk);
> +		udelay(2);
> +		tegra_periph_reset_deassert(tspi->clk);

Do we /have/ to reset the SPI block; can't we just disable it in the
control register, clear all status, and re-program it from scratch?

If at all possible, I would like to avoid introducing any new use of
tegra_periph_reset_{,de}assert, since that API has no standard subsystem
equivalent (or if it does, isn't hooked into the standard subsystem
yet), and hence means this driver relies on a header file currently in
arch/arm/mach-tegra/include/mach, and we need to move or delete all such
headers in order to support single zImage.

> +#ifdef CONFIG_OF

No need for this ifdef; Tegra always has CONFIG_OF enabled and always
boots using DT now in mainline.

> +static struct tegra_spi_platform_data *tegra_slink_parese_dt(

Typo, as thierry pointed out.

> +	prop = of_get_property(pdev->dev.of_node, "nvidia,dma-req-sel", NULL);
> +	if (prop)
> +		pdata->dma_req_sel = be32_to_cpup(prop);

At least, please lets use the standardize format for this, which is
<phandle select> not just <select>; see the I2S driver code. I thought
that's what the .dts patch you posted did...

> +	prop = of_get_property(pdev->dev.of_node, "spi_max_frequency", NULL);

Property names use - not _ as word separators. Isn't there a
standardized property for this? Yes, in fact it's spi-max-frequency
according to Documentation/devicetree/bindings/spi/spi-bus.txt.

Doesn't this function need to parse all the other standardized
properties from spi-bus.txt, or does the SPI core handle that?

> +const struct tegra_slink_chip_data tegra30_spi_cdata = {
> +	.cs_hold_time = true,
> +};
> +
> +#ifdef CONFIG_OF
> +const struct tegra_slink_chip_data tegra20_spi_cdata = {
> +	.cs_hold_time = false,
> +};

Again, no need for that ifdef. Why would the Tegra30 data be outside the
ifdef and the Tegra20 data be inside it anyway?

> +static int __devinit tegra_slink_probe(struct platform_device *pdev)

> +	if (!pdata && pdev->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_device(of_match_ptr(tegra_slink_of_match),
> +				&pdev->dev);
> +		if (!match) {
> +			dev_err(&pdev->dev, "Error: No device match found\n");
> +			return -ENODEV;
> +		}
> +		cdata = match->data;
> +		pdata = tegra_slink_parese_dt(pdev);
> +	} else {
> +		cdata = &tegra30_spi_cdata;
> +	}

That can be simplified since we know DT is enabled and used to boot, so
of_match_device() can always be called. The only conditional that's
needed is to check if explicit platform data was supplied to override
the DT.

> +	if (pdev->id != -1)
> +		master->bus_num = pdev->id;

That will never be the case, since we boot using DT.

> +	spi_irq = platform_get_irq(pdev, 0);
> +	if (unlikely(spi_irq < 0)) {

That should be <= or ==; 0 is an invalid IRQ. I imagine that
request_irq() checks that anyway, so you probably don't need to test it
here too.

> +	tspi->def_command_reg  = SLINK_M_S;
> +	tspi->def_command2_reg = SLINK_CS_ACTIVE_BETWEEN;
> +	tegra_slink_clk_enable(tspi);
> +	tegra_slink_writel(tspi, tspi->def_command_reg, SLINK_COMMAND);
> +	tegra_slink_writel(tspi, tspi->def_command2_reg, SLINK_COMMAND2);
> +	tegra_slink_clk_disable(tspi);
> +
> +	pm_runtime_enable(&pdev->dev);

There are register writes before that, so the runtime PM setup probably
should happen earlier. Also, once runtime PM is actually implemented,
you'll need to handle the case where kernel runtime PM support is
disabled. Check out tegra20_i2s.c for what I believe is the correct way
to handle this.


> +static int __devexit tegra_slink_remove(struct platform_device *pdev)
> +{
> +	struct spi_master	*master;
> +	struct tegra_slink_data	*tspi;
> +
> +	master = dev_get_drvdata(&pdev->dev);
> +	tspi = spi_master_get_devdata(master);

You can probably wrap those two assignments only the same line as the
declaration.

> +static const struct dev_pm_ops tegra_slink_dev_pm_ops = {
> +	.suspend = tegra_slink_suspend,
> +	.resume = tegra_slink_resume,
> +};
> +#define TEGRA_SPI_PM	(&tegra_slink_dev_pm_ops)
> +#else
> +#define TEGRA_SPI_PM	NULL
> +#endif
> +
> +static struct platform_driver tegra_slink_driver = {
> +	.driver = {
> +		.name =		"spi-tegra-slink",
> +		.owner =	THIS_MODULE,
> +		.pm =		&tegra_slink_dev_pm_ops,

Can't you use SET_SYSTEM_SLEEP_PM_OPS() to assign the ops, so you don't
have to #define TEGRA_SPI_PM (which you ended up not even using)?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-22 20:02     ` Stephen Warren
@ 2012-10-23  9:17         ` Mark Brown
  -1 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2012-10-23  9:17 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Laxman Dewangan, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]

On Mon, Oct 22, 2012 at 02:02:47PM -0600, Stephen Warren wrote:
> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:

> > +	/* Read back register to make sure that register writes completed */
> > +	if (reg != SLINK_TX_FIFO)
> > +		readl(tspi->base + SLINK_MAS_DATA);

> Is that really necessary on every single write, or only for certain
> specific operations? This seems rather heavy-weight.

I saw that myself - I figured that since SPI is (relatively) slow itself
the simplicity gained from doing the flush unconditionally was
reasonable, further optimisation can always be done later.

> > +	val = tegra_slink_readl(tspi, SLINK_STATUS);

> > +	/* Write 1 to clear status register */
> > +	val_write = SLINK_RDY;
> > +	val_write |= (val & SLINK_FIFO_ERROR);

> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
> write only if the status was previously asserted? If that is true, how
> do you avoid a race condition where the bit gets set in SLINK_STATUS
> after you read it but before you write to clear it?

The whole point with this sort of interrupt scheme is usually that the
interrupt is level triggered and will just continue to be asserted if
some source within the interrupt isn't acked; the race is usually the
other way around where you may ack a status you didn't see.

> > +static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
> 
> > +		bits_per_word = t->bits_per_word ? t->bits_per_word :
> > +						tspi->cur_spi->bits_per_word;

> That logic is repeated a few times. Shouldn't there be a macro to avoid
> cut/paste. Perhaps even in the SPI core rather than this driver.

Yes, should be in the SPI core as this pattern exists for all SPI
drivers.

> Doesn't this function need to parse all the other standardized
> properties from spi-bus.txt, or does the SPI core handle that?

No, it doesn't.  But perhaps it should (see the thing above about the
per-transfer max frequency too...).  In general SPI hasn't done much
about factoring code out of drivers until very recently.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-23  9:17         ` Mark Brown
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Brown @ 2012-10-23  9:17 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Laxman Dewangan, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]

On Mon, Oct 22, 2012 at 02:02:47PM -0600, Stephen Warren wrote:
> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:

> > +	/* Read back register to make sure that register writes completed */
> > +	if (reg != SLINK_TX_FIFO)
> > +		readl(tspi->base + SLINK_MAS_DATA);

> Is that really necessary on every single write, or only for certain
> specific operations? This seems rather heavy-weight.

I saw that myself - I figured that since SPI is (relatively) slow itself
the simplicity gained from doing the flush unconditionally was
reasonable, further optimisation can always be done later.

> > +	val = tegra_slink_readl(tspi, SLINK_STATUS);

> > +	/* Write 1 to clear status register */
> > +	val_write = SLINK_RDY;
> > +	val_write |= (val & SLINK_FIFO_ERROR);

> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
> write only if the status was previously asserted? If that is true, how
> do you avoid a race condition where the bit gets set in SLINK_STATUS
> after you read it but before you write to clear it?

The whole point with this sort of interrupt scheme is usually that the
interrupt is level triggered and will just continue to be asserted if
some source within the interrupt isn't acked; the race is usually the
other way around where you may ack a status you didn't see.

> > +static unsigned int tegra_slink_read_rx_fifo_to_client_rxbuf(
> 
> > +		bits_per_word = t->bits_per_word ? t->bits_per_word :
> > +						tspi->cur_spi->bits_per_word;

> That logic is repeated a few times. Shouldn't there be a macro to avoid
> cut/paste. Perhaps even in the SPI core rather than this driver.

Yes, should be in the SPI core as this pattern exists for all SPI
drivers.

> Doesn't this function need to parse all the other standardized
> properties from spi-bus.txt, or does the SPI core handle that?

No, it doesn't.  But perhaps it should (see the thing above about the
per-transfer max frequency too...).  In general SPI hasn't done much
about factoring code out of drivers until very recently.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-22 20:02     ` Stephen Warren
@ 2012-10-26 18:49         ` Laxman Dewangan
  -1 siblings, 0 replies; 18+ messages in thread
From: Laxman Dewangan @ 2012-10-26 18:49 UTC (permalink / raw)
  To: Stephen Warren
  Cc: broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Thanks Stephen for review.
I have taken care of almost all feedback. Some of having my below comments.



On Tuesday 23 October 2012 01:32 AM, Stephen Warren wrote:
> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
>> Tegra20/Tegra30 supports the spi interface through its SLINK
>> controller. Add spi driver for SLINK controller.
>
>> +static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
>> +             unsigned long val, unsigned long reg)
>> +{
>> +     writel(val, tspi->base + reg);
>> +
>> +     /* Read back register to make sure that register writes completed */
>> +     if (reg != SLINK_TX_FIFO)
>> +             readl(tspi->base + SLINK_MAS_DATA);
> Is that really necessary on every single write, or only for certain
> specific operations? This seems rather heavy-weight.
>

We do write register very less (as we  shadow the register locally), I 
do not see any perf degradation here.


>> +     val_write = SLINK_RDY;
>> +     val_write |= (val&  SLINK_FIFO_ERROR);
> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
> write only if the status was previously asserted? If that is true, how
> do you avoid a race condition where the bit gets set in SLINK_STATUS
> after you read it but before you write to clear it?
>
Status gets updated together. There is no steps of updating status.


>
>> +     if (bits_per_word == 8 || bits_per_word == 16) {
>> +             tspi->is_packed = 1;
>> +             tspi->words_per_32bit = 32/bits_per_word;
> Spaces required around all operators. Does this pass checkpatch? No:
> total: 1405 errors, 11 warnings, 1418 lines checked
>

I saw the issue by my checkpatch is not caching the issue. Let me 
recheck my command.

>> +             bits_per_word = t->bits_per_word ? t->bits_per_word :
>> +                                             tspi->cur_spi->bits_per_word;
> That logic is repeated a few times. Shouldn't there be a macro to avoid
> cut/paste. Perhaps even in the SPI core rather than this driver.
>

I will post the change for moving the code to core later. I will keep 
this for now.

>> +             struct tegra_slink_data *tspi, struct spi_transfer *t)
> Are there no cases where we can simply DMA straight into the client
> buffer? I suppose it would be limited to cache-line-aligned
> cache-line-size-aligned buffers which is probably unlikely in general:-(
>

I think it is possible by checking some flags, I will explore and will 
post the fix later.

>> +static int tegra_slink_unprepare_transfer(struct spi_master *master)
>> +{
>> +     struct tegra_slink_data *tspi = spi_master_get_devdata(master);
>> +
>> +     pm_runtime_put_sync(tspi->dev);
>> +     return 0;
>> +}
> Does this driver actually implement any runtime PM ops? I certainly
> don't see any in struct dev_pm_ops tegra_slink_dev_pm_ops. Related, why
> do tegra_slink_clk_{en,dis}able exist; shouldn't those functions be the
> implementation of the runtime PM ops? Related, why are
> tegra_slink_clk_{en,dis}able called all over the place, rather than
> relying on runtime PM API calls, or tegra_slink_{,un}prepare_transfer
> having been called?
>


OK, I move the clock control in runtime callbacks.


> So that all implies that we wait for the very first interrupt from the
> SPI peripheral for a transfer, and then wait for the DMA controller to
> complete all DMA (which would probably entail actually waiting for DMA
> to drain the RX FIFO in the RX case). Does it make sense to simply
> return from the ISR if not all conditions that we will wait on have
> occurred, and so avoid blocking this ISR thread? I suppose since this
> thread gets blocked rather than spinning, it's not a big deal though.
>

In this case, we can get rid of isr thread and move the wait/status 
check to caller thread.
I need to do some more stress on this and if it is fine then will post 
the patch later for this, not as part of this patch.


>> +     spin_lock_irqsave(&tspi->lock, flags);
>> +     if (err) {
>> +             dev_err(tspi->dev,
>> +                     "DmaXfer: ERROR bit set 0x%x\n", tspi->status_reg);
>> +             dev_err(tspi->dev,
>> +                     "DmaXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
>> +                             tspi->command2_reg, tspi->dma_control_reg);
>> +             tegra_periph_reset_assert(tspi->clk);
>> +             udelay(2);
>> +             tegra_periph_reset_deassert(tspi->clk);
> Do we /have/ to reset the SPI block; can't we just disable it in the
> control register, clear all status, and re-program it from scratch?
>
> If at all possible, I would like to avoid introducing any new use of
> tegra_periph_reset_{,de}assert, since that API has no standard subsystem
> equivalent (or if it does, isn't hooked into the standard subsystem
> yet), and hence means this driver relies on a header file currently in
> arch/arm/mach-tegra/include/mach, and we need to move or delete all such
> headers in order to support single zImage.

Is there a way to support the reset of controller. We will need this 
functionality.


------------------------------------------------------------------------------
The Windows 8 Center 
In partnership with Sourceforge
Your idea - your app - 30 days. Get started!
http://windows8center.sourceforge.net/
what-html-developers-need-to-know-about-coding-windows-8-metro-style-apps/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-26 18:49         ` Laxman Dewangan
  0 siblings, 0 replies; 18+ messages in thread
From: Laxman Dewangan @ 2012-10-26 18:49 UTC (permalink / raw)
  To: Stephen Warren
  Cc: broonie, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

Thanks Stephen for review.
I have taken care of almost all feedback. Some of having my below comments.



On Tuesday 23 October 2012 01:32 AM, Stephen Warren wrote:
> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
>> Tegra20/Tegra30 supports the spi interface through its SLINK
>> controller. Add spi driver for SLINK controller.
>
>> +static inline void tegra_slink_writel(struct tegra_slink_data *tspi,
>> +             unsigned long val, unsigned long reg)
>> +{
>> +     writel(val, tspi->base + reg);
>> +
>> +     /* Read back register to make sure that register writes completed */
>> +     if (reg != SLINK_TX_FIFO)
>> +             readl(tspi->base + SLINK_MAS_DATA);
> Is that really necessary on every single write, or only for certain
> specific operations? This seems rather heavy-weight.
>

We do write register very less (as we  shadow the register locally), I 
do not see any perf degradation here.


>> +     val_write = SLINK_RDY;
>> +     val_write |= (val&  SLINK_FIFO_ERROR);
> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
> write only if the status was previously asserted? If that is true, how
> do you avoid a race condition where the bit gets set in SLINK_STATUS
> after you read it but before you write to clear it?
>
Status gets updated together. There is no steps of updating status.


>
>> +     if (bits_per_word == 8 || bits_per_word == 16) {
>> +             tspi->is_packed = 1;
>> +             tspi->words_per_32bit = 32/bits_per_word;
> Spaces required around all operators. Does this pass checkpatch? No:
> total: 1405 errors, 11 warnings, 1418 lines checked
>

I saw the issue by my checkpatch is not caching the issue. Let me 
recheck my command.

>> +             bits_per_word = t->bits_per_word ? t->bits_per_word :
>> +                                             tspi->cur_spi->bits_per_word;
> That logic is repeated a few times. Shouldn't there be a macro to avoid
> cut/paste. Perhaps even in the SPI core rather than this driver.
>

I will post the change for moving the code to core later. I will keep 
this for now.

>> +             struct tegra_slink_data *tspi, struct spi_transfer *t)
> Are there no cases where we can simply DMA straight into the client
> buffer? I suppose it would be limited to cache-line-aligned
> cache-line-size-aligned buffers which is probably unlikely in general:-(
>

I think it is possible by checking some flags, I will explore and will 
post the fix later.

>> +static int tegra_slink_unprepare_transfer(struct spi_master *master)
>> +{
>> +     struct tegra_slink_data *tspi = spi_master_get_devdata(master);
>> +
>> +     pm_runtime_put_sync(tspi->dev);
>> +     return 0;
>> +}
> Does this driver actually implement any runtime PM ops? I certainly
> don't see any in struct dev_pm_ops tegra_slink_dev_pm_ops. Related, why
> do tegra_slink_clk_{en,dis}able exist; shouldn't those functions be the
> implementation of the runtime PM ops? Related, why are
> tegra_slink_clk_{en,dis}able called all over the place, rather than
> relying on runtime PM API calls, or tegra_slink_{,un}prepare_transfer
> having been called?
>


OK, I move the clock control in runtime callbacks.


> So that all implies that we wait for the very first interrupt from the
> SPI peripheral for a transfer, and then wait for the DMA controller to
> complete all DMA (which would probably entail actually waiting for DMA
> to drain the RX FIFO in the RX case). Does it make sense to simply
> return from the ISR if not all conditions that we will wait on have
> occurred, and so avoid blocking this ISR thread? I suppose since this
> thread gets blocked rather than spinning, it's not a big deal though.
>

In this case, we can get rid of isr thread and move the wait/status 
check to caller thread.
I need to do some more stress on this and if it is fine then will post 
the patch later for this, not as part of this patch.


>> +     spin_lock_irqsave(&tspi->lock, flags);
>> +     if (err) {
>> +             dev_err(tspi->dev,
>> +                     "DmaXfer: ERROR bit set 0x%x\n", tspi->status_reg);
>> +             dev_err(tspi->dev,
>> +                     "DmaXfer 0x%08x:0x%08x:0x%08x\n", tspi->command_reg,
>> +                             tspi->command2_reg, tspi->dma_control_reg);
>> +             tegra_periph_reset_assert(tspi->clk);
>> +             udelay(2);
>> +             tegra_periph_reset_deassert(tspi->clk);
> Do we /have/ to reset the SPI block; can't we just disable it in the
> control register, clear all status, and re-program it from scratch?
>
> If at all possible, I would like to avoid introducing any new use of
> tegra_periph_reset_{,de}assert, since that API has no standard subsystem
> equivalent (or if it does, isn't hooked into the standard subsystem
> yet), and hence means this driver relies on a header file currently in
> arch/arm/mach-tegra/include/mach, and we need to move or delete all such
> headers in order to support single zImage.

Is there a way to support the reset of controller. We will need this 
functionality.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-26 18:49         ` Laxman Dewangan
@ 2012-10-29 15:17             ` Stephen Warren
  -1 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-10-29 15:17 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
> Thanks Stephen for review.
> I have taken care of almost all feedback. Some of having my below comments.
> 
> On Tuesday 23 October 2012 01:32 AM, Stephen Warren wrote:
>> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
>>> Tegra20/Tegra30 supports the spi interface through its SLINK
>>> controller. Add spi driver for SLINK controller.

>>> +     val_write = SLINK_RDY;
>>> +     val_write |= (val&  SLINK_FIFO_ERROR);
>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>> write only if the status was previously asserted? If that is true, how
>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>> after you read it but before you write to clear it?
>
> Status gets updated together. There is no steps of updating status.

Sorry, I don't understand this answer.

>>> +     spin_lock_irqsave(&tspi->lock, flags);
>>> +     if (err) {
>>> +             dev_err(tspi->dev,
>>> +                     "DmaXfer: ERROR bit set 0x%x\n",
>>> tspi->status_reg);
>>> +             dev_err(tspi->dev,
>>> +                     "DmaXfer 0x%08x:0x%08x:0x%08x\n",
>>> tspi->command_reg,
>>> +                             tspi->command2_reg,
>>> tspi->dma_control_reg);
>>> +             tegra_periph_reset_assert(tspi->clk);
>>> +             udelay(2);
>>> +             tegra_periph_reset_deassert(tspi->clk);
>> Do we /have/ to reset the SPI block; can't we just disable it in the
>> control register, clear all status, and re-program it from scratch?
>>
>> If at all possible, I would like to avoid introducing any new use of
>> tegra_periph_reset_{,de}assert, since that API has no standard subsystem
>> equivalent (or if it does, isn't hooked into the standard subsystem
>> yet), and hence means this driver relies on a header file currently in
>> arch/arm/mach-tegra/include/mach, and we need to move or delete all such
>> headers in order to support single zImage.
> 
> Is there a way to support the reset of controller. We will need this
> functionality.

Why do we need to reset the controller at all; can't we simply program
all the (few) configuration registers? Are there HW bugs that hang the
controller and require a reset or something?

------------------------------------------------------------------------------
The Windows 8 Center - In partnership with Sourceforge
Your idea - your app - 30 days.
Get started!
http://windows8center.sourceforge.net/
what-html-developers-need-to-know-about-coding-windows-8-metro-style-apps/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-29 15:17             ` Stephen Warren
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-10-29 15:17 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: broonie, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
> Thanks Stephen for review.
> I have taken care of almost all feedback. Some of having my below comments.
> 
> On Tuesday 23 October 2012 01:32 AM, Stephen Warren wrote:
>> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
>>> Tegra20/Tegra30 supports the spi interface through its SLINK
>>> controller. Add spi driver for SLINK controller.

>>> +     val_write = SLINK_RDY;
>>> +     val_write |= (val&  SLINK_FIFO_ERROR);
>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>> write only if the status was previously asserted? If that is true, how
>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>> after you read it but before you write to clear it?
>
> Status gets updated together. There is no steps of updating status.

Sorry, I don't understand this answer.

>>> +     spin_lock_irqsave(&tspi->lock, flags);
>>> +     if (err) {
>>> +             dev_err(tspi->dev,
>>> +                     "DmaXfer: ERROR bit set 0x%x\n",
>>> tspi->status_reg);
>>> +             dev_err(tspi->dev,
>>> +                     "DmaXfer 0x%08x:0x%08x:0x%08x\n",
>>> tspi->command_reg,
>>> +                             tspi->command2_reg,
>>> tspi->dma_control_reg);
>>> +             tegra_periph_reset_assert(tspi->clk);
>>> +             udelay(2);
>>> +             tegra_periph_reset_deassert(tspi->clk);
>> Do we /have/ to reset the SPI block; can't we just disable it in the
>> control register, clear all status, and re-program it from scratch?
>>
>> If at all possible, I would like to avoid introducing any new use of
>> tegra_periph_reset_{,de}assert, since that API has no standard subsystem
>> equivalent (or if it does, isn't hooked into the standard subsystem
>> yet), and hence means this driver relies on a header file currently in
>> arch/arm/mach-tegra/include/mach, and we need to move or delete all such
>> headers in order to support single zImage.
> 
> Is there a way to support the reset of controller. We will need this
> functionality.

Why do we need to reset the controller at all; can't we simply program
all the (few) configuration registers? Are there HW bugs that hang the
controller and require a reset or something?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-29 15:17             ` Stephen Warren
@ 2012-10-29 16:18                 ` Laxman Dewangan
  -1 siblings, 0 replies; 18+ messages in thread
From: Laxman Dewangan @ 2012-10-29 16:18 UTC (permalink / raw)
  To: Stephen Warren
  Cc: broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA

On Monday 29 October 2012 08:47 PM, Stephen Warren wrote:
> On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
>>>
>>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>>> write only if the status was previously asserted? If that is true, how
>>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>>> after you read it but before you write to clear it?
>> Status gets updated together. There is no steps of updating status.
> Sorry, I don't understand this answer.

The status should be updated once by HW and so there is no race condition.
HW behavior is that if the tx or Rx error occurs, it updates the status, 
generates interrupt and still continue transfer and later on, it 
generates the ready.
In first isr, we read status, error status found and so in isr thread, 
we reset controller to stop the transfer.

So in good state, only ready bit will be set and hence writing 1 to 
clear it.
In error state, clearing error first in ISR and in isr thread resetting 
the controller to stop the controller engine.


>>
>> Is there a way to support the reset of controller. We will need this
>> functionality.
> Why do we need to reset the controller at all; can't we simply program
> all the (few) configuration registers? Are there HW bugs that hang the
> controller and require a reset or something?

HW generates error,  then interrupt and still continue transfer and 
later point of time it generates the transfer done.
We want to stop the transfer once error get detected. For this we need 
to reset controller.
I did disabling rx and tx but still controller shows as busy.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-29 16:18                 ` Laxman Dewangan
  0 siblings, 0 replies; 18+ messages in thread
From: Laxman Dewangan @ 2012-10-29 16:18 UTC (permalink / raw)
  To: Stephen Warren
  Cc: broonie, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

On Monday 29 October 2012 08:47 PM, Stephen Warren wrote:
> On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
>>>
>>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>>> write only if the status was previously asserted? If that is true, how
>>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>>> after you read it but before you write to clear it?
>> Status gets updated together. There is no steps of updating status.
> Sorry, I don't understand this answer.

The status should be updated once by HW and so there is no race condition.
HW behavior is that if the tx or Rx error occurs, it updates the status, 
generates interrupt and still continue transfer and later on, it 
generates the ready.
In first isr, we read status, error status found and so in isr thread, 
we reset controller to stop the transfer.

So in good state, only ready bit will be set and hence writing 1 to 
clear it.
In error state, clearing error first in ISR and in isr thread resetting 
the controller to stop the controller engine.


>>
>> Is there a way to support the reset of controller. We will need this
>> functionality.
> Why do we need to reset the controller at all; can't we simply program
> all the (few) configuration registers? Are there HW bugs that hang the
> controller and require a reset or something?

HW generates error,  then interrupt and still continue transfer and 
later point of time it generates the transfer done.
We want to stop the transfer once error get detected. For this we need 
to reset controller.
I did disabling rx and tx but still controller shows as busy.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
  2012-10-29 16:18                 ` Laxman Dewangan
@ 2012-10-29 18:55                   ` Stephen Warren
  -1 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-10-29 18:55 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: broonie, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

On 10/29/2012 10:18 AM, Laxman Dewangan wrote:
> On Monday 29 October 2012 08:47 PM, Stephen Warren wrote:
>> On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
>>>>
>>>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>>>> write only if the status was previously asserted? If that is true, how
>>>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>>>> after you read it but before you write to clear it?
>>> Status gets updated together. There is no steps of updating status.
>> Sorry, I don't understand this answer.
> 
> The status should be updated once by HW and so there is no race condition.
> HW behavior is that if the tx or Rx error occurs, it updates the status,
> generates interrupt and still continue transfer and later on, it
> generates the ready.
> In first isr, we read status, error status found and so in isr thread,
> we reset controller to stop the transfer.
> 
> So in good state, only ready bit will be set and hence writing 1 to
> clear it.
> In error state, clearing error first in ISR and in isr thread resetting
> the controller to stop the controller engine.

OK, I see why there's no race. It still seems simply to me if
tegra_slink_clear_status() just always writes all the status bits, but I
suppose it isn't a correctness issue.

>>> Is there a way to support the reset of controller. We will need this
>>> functionality.
>>
>> Why do we need to reset the controller at all; can't we simply program
>> all the (few) configuration registers? Are there HW bugs that hang the
>> controller and require a reset or something?
> 
> HW generates error,  then interrupt and still continue transfer and
> later point of time it generates the transfer done.
> We want to stop the transfer once error get detected. For this we need
> to reset controller.
> I did disabling rx and tx but still controller shows as busy.

Oh dear. Well, I guess it'll have to be OK then; we'll just have to find
a way of decoupling this API from the mach-tegra directory later:-(

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] spi: tegra: add spi driver for SLINK controller
@ 2012-10-29 18:55                   ` Stephen Warren
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Warren @ 2012-10-29 18:55 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: broonie, grant.likely, rob.herring, spi-devel-general,
	linux-kernel, linux-tegra

On 10/29/2012 10:18 AM, Laxman Dewangan wrote:
> On Monday 29 October 2012 08:47 PM, Stephen Warren wrote:
>> On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
>>>>
>>>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>>>> write only if the status was previously asserted? If that is true, how
>>>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>>>> after you read it but before you write to clear it?
>>> Status gets updated together. There is no steps of updating status.
>> Sorry, I don't understand this answer.
> 
> The status should be updated once by HW and so there is no race condition.
> HW behavior is that if the tx or Rx error occurs, it updates the status,
> generates interrupt and still continue transfer and later on, it
> generates the ready.
> In first isr, we read status, error status found and so in isr thread,
> we reset controller to stop the transfer.
> 
> So in good state, only ready bit will be set and hence writing 1 to
> clear it.
> In error state, clearing error first in ISR and in isr thread resetting
> the controller to stop the controller engine.

OK, I see why there's no race. It still seems simply to me if
tegra_slink_clear_status() just always writes all the status bits, but I
suppose it isn't a correctness issue.

>>> Is there a way to support the reset of controller. We will need this
>>> functionality.
>>
>> Why do we need to reset the controller at all; can't we simply program
>> all the (few) configuration registers? Are there HW bugs that hang the
>> controller and require a reset or something?
> 
> HW generates error,  then interrupt and still continue transfer and
> later point of time it generates the transfer done.
> We want to stop the transfer once error get detected. For this we need
> to reset controller.
> I did disabling rx and tx but still controller shows as busy.

Oh dear. Well, I guess it'll have to be OK then; we'll just have to find
a way of decoupling this API from the mach-tegra directory later:-(

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-10-29 18:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-18 10:47 [PATCH] spi: tegra: add spi driver for SLINK controller Laxman Dewangan
2012-10-18 10:47 ` Laxman Dewangan
     [not found] ` <1350557233-31234-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-10-22 12:28   ` Mark Brown
2012-10-22 12:28     ` Mark Brown
     [not found]     ` <20121022122825.GD4477-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
2012-10-22 18:04       ` Thierry Reding
2012-10-22 18:04         ` Thierry Reding
2012-10-22 20:02   ` Stephen Warren
2012-10-22 20:02     ` Stephen Warren
     [not found]     ` <5085A667.2000100-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-10-23  9:17       ` Mark Brown
2012-10-23  9:17         ` Mark Brown
2012-10-26 18:49       ` Laxman Dewangan
2012-10-26 18:49         ` Laxman Dewangan
     [not found]         ` <508ADB1C.6040602-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-10-29 15:17           ` Stephen Warren
2012-10-29 15:17             ` Stephen Warren
     [not found]             ` <508E9E22.6030201-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-10-29 16:18               ` Laxman Dewangan
2012-10-29 16:18                 ` Laxman Dewangan
2012-10-29 18:55                 ` Stephen Warren
2012-10-29 18:55                   ` Stephen Warren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.