linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] spi: add spi_tegra driver
@ 2010-07-30  0:37 Colin Cross
  2010-07-30  7:54 ` Thierry Reding
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Colin Cross @ 2010-07-30  0:37 UTC (permalink / raw)
  To: linux-tegra; +Cc: spi-devel-general, Erik Gilling, linux-arm-kernel

From: Erik Gilling <konkers@android.com>

CC: spi-devel-general@lists.sourceforge.net
Signed-off-by: Erik Gilling <konkers@android.com>
---
This patch depends on the Tegra APB DMA driver:
[PATCH] [ARM] tegra: Add APB DMA support
---
 drivers/spi/Kconfig     |    6 +
 drivers/spi/Makefile    |    1 +
 drivers/spi/spi_tegra.c |  651 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 658 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/spi_tegra.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 91c2f4f..c07e0de 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -298,6 +298,12 @@ config SPI_STMP3XXX
 	help
 	  SPI driver for Freescale STMP37xx/378x SoC SSP interface
 
+config SPI_TEGRA
+	tristate "Nvidia Tegra SPI controller"
+	depends on ARCH_TEGRA
+	help
+	  SPI driver for NVidia Tegra SoCs
+
 config SPI_TXX9
 	tristate "Toshiba TXx9 SPI controller"
 	depends on GENERIC_GPIO && CPU_TX49XX
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index e9cbd18..b6573d8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)		+= spi_ppc4xx.o
 obj-$(CONFIG_SPI_S3C24XX_GPIO)		+= spi_s3c24xx_gpio.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi_s3c24xx_hw.o
 obj-$(CONFIG_SPI_S3C64XX)		+= spi_s3c64xx.o
+obj-$(CONFIG_SPI_TEGRA)			+= spi_tegra.o
 obj-$(CONFIG_SPI_TXX9)			+= spi_txx9.o
 obj-$(CONFIG_SPI_XILINX)		+= xilinx_spi.o
 obj-$(CONFIG_SPI_XILINX_OF)		+= xilinx_spi_of.o
diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
new file mode 100644
index 0000000..72fc4d4
--- /dev/null
+++ b/drivers/spi/spi_tegra.c
@@ -0,0 +1,651 @@
+/*
+ * drivers/spi/spi_tegra.c
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * Author:
+ *     Erik Gilling <konkers@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#undef DEBUG
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+
+#include <linux/spi/spi.h>
+
+#include <mach/dma.h>
+
+#define DRIVER_NAME		"spi_tegra"
+
+#define BUSY_TIMEOUT		1000
+
+#define SLINK_COMMAND		0x000
+#define   BIT_LENGTH(x)			(((x) & 0x1f) << 0)
+#define   WORD_SIZE(x)			(((x) & 0x1f) << 5)
+#define   BOTH_EN			(1 << 10)
+#define   CS_SW				(1 << 11)
+#define   CS_VALUE			(1 << 12)
+#define   CS_POLARITY			(1 << 13)
+#define   IDLE_SDA_DRIVE_LOW		(0 << 16)
+#define   IDLE_SDA_DRIVE_HIGH		(1 << 16)
+#define   IDLE_SDA_PULL_LOW		(2 << 16)
+#define   IDLE_SDA_PULL_HIGH		(3 << 16)
+#define   IDLE_SDA_MASK			(3 << 16)
+#define   CS_POLARITY1			(1 << 20)
+#define   CK_SDA			(1 << 21)
+#define   CS_POLARITY2			(1 << 22)
+#define   CS_POLARITY3			(1 << 23)
+#define   IDLE_SCLK_DRIVE_LOW		(0 << 24)
+#define   IDLE_SCLK_DRIVE_HIGH		(1 << 24)
+#define   IDLE_SCLK_PULL_LOW		(2 << 24)
+#define   IDLE_SCLK_PULL_HIGH		(3 << 24)
+#define   IDLE_SCLK_MASK		(3 << 24)
+#define   M_S				(1 << 28)
+#define   WAIT				(1 << 29)
+#define   GO				(1 << 30)
+#define   ENB				(1 << 31)
+
+#define SLINK_COMMAND2		0x004
+#define   LSBFE				(1 << 0)
+#define   SSOE				(1 << 1)
+#define   SPIE				(1 << 4)
+#define   BIDIROE			(1 << 6)
+#define   MODFEN			(1 << 7)
+#define   INT_SIZE(x)			(((x) & 0x1f) << 8)
+#define   CS_ACTIVE_BETWEEN		(1 << 17)
+#define   SS_EN_CS(x)			(((x) & 0x3) << 18)
+#define   SS_SETUP(x)			(((x) & 0x3) << 20)
+#define   FIFO_REFILLS_0		(0 << 22)
+#define   FIFO_REFILLS_1		(1 << 22)
+#define   FIFO_REFILLS_2		(2 << 22)
+#define   FIFO_REFILLS_3		(3 << 22)
+#define   FIFO_REFILLS_MASK		(3 << 22)
+#define   WAIT_PACK_INT(x)		(((x) & 0x7) << 26)
+#define   SPC0				(1 << 29)
+#define   TXEN				(1 << 30)
+#define   RXEN				(1 << 31)
+
+#define SLINK_STATUS		0x008
+#define   COUNT(val)			(((val) >> 0) & 0x1f)
+#define   WORD(val)			(((val) >> 5) & 0x1f)
+#define   BLK_CNT(val)			(((val) >> 0) & 0xffff)
+#define   MODF				(1 << 16)
+#define   RX_UNF			(1 << 18)
+#define   TX_OVF			(1 << 19)
+#define   TX_FULL			(1 << 20)
+#define   TX_EMPTY			(1 << 21)
+#define   RX_FULL			(1 << 22)
+#define   RX_EMPTY			(1 << 23)
+#define   TX_UNF			(1 << 24)
+#define   RX_OVF			(1 << 25)
+#define   TX_FLUSH			(1 << 26)
+#define   RX_FLUSH			(1 << 27)
+#define   SCLK				(1 << 28)
+#define   ERR				(1 << 29)
+#define   RDY				(1 << 30)
+#define   BSY				(1 << 31)
+
+#define SLINK_MAS_DATA		0x010
+#define SLINK_SLAVE_DATA	0x014
+
+#define SLINK_DMA_CTL		0x018
+#define   DMA_BLOCK_SIZE(x)		(((x) & 0xffff) << 0)
+#define   TX_TRIG_1			(0 << 16)
+#define   TX_TRIG_4			(1 << 16)
+#define   TX_TRIG_8			(2 << 16)
+#define   TX_TRIG_16			(3 << 16)
+#define   TX_TRIG_MASK			(3 << 16)
+#define   RX_TRIG_1			(0 << 18)
+#define   RX_TRIG_4			(1 << 18)
+#define   RX_TRIG_8			(2 << 18)
+#define   RX_TRIG_16			(3 << 18)
+#define   RX_TRIG_MASK			(3 << 18)
+#define   PACKED			(1 << 20)
+#define   PACK_SIZE_4			(0 << 21)
+#define   PACK_SIZE_8			(1 << 21)
+#define   PACK_SIZE_16			(2 << 21)
+#define   PACK_SIZE_32			(3 << 21)
+#define   PACK_SIZE_MASK		(3 << 21)
+#define   IE_TXC			(1 << 26)
+#define   IE_RXC			(1 << 27)
+#define   DMA_EN			(1 << 31)
+
+#define SLINK_STATUS2		0x01c
+#define   TX_FIFO_EMPTY_COUNT(val)	(((val) & 0x3f) >> 0)
+#define   RX_FIFO_FULL_COUNT(val)	(((val) & 0x3f) >> 16)
+
+#define SLINK_TX_FIFO		0x100
+#define SLINK_RX_FIFO		0x180
+
+static const unsigned long spi_tegra_req_sels[] = {
+	TEGRA_DMA_REQ_SEL_SL2B1,
+	TEGRA_DMA_REQ_SEL_SL2B2,
+	TEGRA_DMA_REQ_SEL_SL2B3,
+	TEGRA_DMA_REQ_SEL_SL2B4,
+};
+
+static inline unsigned bytes_per_word(u8 bits)
+{
+	WARN_ON((bits < 1) || bits > 32);
+	if (bits <= 8)
+		return 1;
+	else if (bits <= 16)
+		return 2;
+	else if (bits <= 24)
+		return 3;
+	else
+		return 4;
+}
+
+#define BB_LEN			32
+
+struct spi_tegra_data {
+	struct spi_master	*master;
+	struct platform_device	*pdev;
+	spinlock_t		lock;
+
+	struct clk		*clk;
+	void __iomem		*base;
+	unsigned long		phys;
+
+	u32			cur_speed;
+
+	struct list_head	queue;
+	struct spi_transfer	*cur;
+	unsigned		cur_pos;
+	unsigned		cur_len;
+	unsigned		cur_bytes_per_word;
+
+	/* The tegra spi controller has a bug which causes the first word
+	 * in PIO transactions to be garbage.  Since packed DMA transactions
+	 * require transfers to be 4 byte aligned we need a bounce buffer
+	 * for the generic case.
+	 */
+	struct tegra_dma_req	rx_dma_req;
+	struct tegra_dma_channel *rx_dma;
+	u32			*rx_bb;
+	dma_addr_t		rx_bb_phys;
+};
+
+
+static unsigned long spi_readl(struct spi_tegra_data *tspi, unsigned long reg)
+{
+	return readl(tspi->base + reg);
+}
+
+static void spi_writel(struct spi_tegra_data *tspi, unsigned long val,
+		       unsigned long reg)
+{
+	writel(val, tspi->base + reg);
+}
+
+static void spi_tegra_go(struct spi_tegra_data *tspi)
+{
+	unsigned long val;
+
+	wmb();
+
+	val = spi_readl(tspi, SLINK_DMA_CTL);
+	val &= ~DMA_BLOCK_SIZE(~0) & ~DMA_EN;
+	val |= DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
+	spi_writel(tspi, val, SLINK_DMA_CTL);
+
+	tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
+
+	val |= DMA_EN;
+	spi_writel(tspi, val, SLINK_DMA_CTL);
+}
+
+static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
+			   tspi->cur_bytes_per_word);
+	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	val = spi_readl(tspi, SLINK_COMMAND);
+	val &= ~WORD_SIZE(~0);
+	val |= WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
+	spi_writel(tspi, val, SLINK_COMMAND);
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = 0;
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			val |= tx_buf[i + j] << j * 8;
+
+		spi_writel(tspi, val, SLINK_TX_FIFO);
+	}
+
+	tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
+
+	return len;
+}
+
+static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = tspi->cur_len;
+	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			rx_buf[i + j] = (val >> (j * 8)) & 0xff;
+	}
+
+	return len;
+}
+
+static int spi_tegra_start_transfer(struct spi_device *spi,
+				    struct spi_transfer *t)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	u32 speed;
+	u8 bits_per_word;
+	unsigned long val;
+
+	speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
+	bits_per_word = t->bits_per_word ? t->bits_per_word  :
+		spi->bits_per_word;
+
+	if (bits_per_word < 1 || bits_per_word > 32)
+		return -EINVAL;
+
+	if (t->len == 0)
+		return -EINVAL;
+
+	if (!t->rx_buf && !t->tx_buf)
+		return -EINVAL;
+
+	tspi->cur_bytes_per_word = bytes_per_word(bits_per_word);
+
+	if (speed != tspi->cur_speed)
+		clk_set_rate(tspi->clk, speed);
+
+	if (tspi->cur_speed == 0)
+		clk_enable(tspi->clk);
+
+	tspi->cur_speed = speed;
+
+	val = spi_readl(tspi, SLINK_COMMAND2);
+	if (t->rx_buf)
+		val |= RXEN;
+	else
+		val &= ~RXEN;
+
+	if (t->tx_buf)
+		val |= TXEN;
+	else
+		val &= ~TXEN;
+
+	val &= ~SS_EN_CS(~0);
+	val |= SS_EN_CS(spi->chip_select);
+	val |= SPIE;
+
+	spi_writel(tspi, val, SLINK_COMMAND2);
+
+	val = spi_readl(tspi, SLINK_COMMAND);
+	val &= ~BIT_LENGTH(~0);
+	val |= BIT_LENGTH(bits_per_word - 1);
+
+	/* FIXME: should probably control CS manually so that we can be sure
+	 * it does not go low between transfer and to support delay_usecs
+	 * correctly.
+	 */
+	val &= ~IDLE_SCLK_MASK & ~CK_SDA & ~CS_SW;
+
+	if (spi->mode & SPI_CPHA)
+		val |= CK_SDA;
+
+	if (spi->mode & SPI_CPOL)
+		val |= IDLE_SCLK_DRIVE_HIGH;
+	else
+		val |= IDLE_SCLK_DRIVE_LOW;
+
+	val |= M_S;
+
+	spi_writel(tspi, val, SLINK_COMMAND);
+
+	spi_writel(tspi, RX_FLUSH | TX_FLUSH, SLINK_STATUS);
+
+	tspi->cur = t;
+	tspi->cur_pos = 0;
+	tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
+
+	spi_tegra_go(tspi);
+
+	return 0;
+}
+
+static void spi_tegra_start_message(struct spi_device *spi,
+				    struct spi_message *m)
+{
+	struct spi_transfer *t;
+
+	m->actual_length = 0;
+
+	t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
+	spi_tegra_start_transfer(spi, t);
+}
+
+static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
+{
+	struct spi_tegra_data *tspi = req->dev;
+	unsigned long flags;
+	struct spi_message *m;
+	struct spi_device *spi;
+	int complete = 0;
+	int timeout = 0;
+	unsigned long val;
+
+	/* the SPI controller may come back with both the BSY and RDY bits
+	 * set.  In this case we need to wait for the BSY bit to clear so
+	 * that we are sure the DMA is finished.
+	 */
+	while (timeout++ < BUSY_TIMEOUT) {
+		if (!(spi_readl(tspi, SLINK_STATUS) & BSY))
+			break;
+	}
+
+	val = spi_readl(tspi, SLINK_STATUS);
+	val |= RDY;
+	spi_writel(tspi, val, SLINK_STATUS);
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	m = list_first_entry(&tspi->queue, struct spi_message, queue);
+	spi = m->state;
+
+	tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
+	m->actual_length += tspi->cur_pos;
+
+	if (tspi->cur_pos < tspi->cur->len) {
+		tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
+		spi_tegra_go(tspi);
+	} else if (!list_is_last(&tspi->cur->transfer_list,
+				 &m->transfers)) {
+		tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
+					      struct spi_transfer,
+					      transfer_list);
+		spi_tegra_start_transfer(spi, tspi->cur);
+	} else {
+		complete = 1;
+	}
+
+	if (complete) {
+		list_del(&m->queue);
+
+		m->status = 0;
+		m->complete(m->context);
+
+		if (!list_empty(&tspi->queue)) {
+			m = list_first_entry(&tspi->queue, struct spi_message,
+					     queue);
+			spi = m->state;
+			spi_tegra_start_message(spi, m);
+		} else {
+			clk_disable(tspi->clk);
+			tspi->cur_speed = 0;
+		}
+	}
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+}
+
+static int spi_tegra_setup(struct spi_device *spi)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long cs_bit;
+	unsigned long val;
+	unsigned long flags;
+
+	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);
+
+
+	switch (spi->chip_select) {
+	case 0:
+		cs_bit = CS_POLARITY;
+		break;
+
+	case 1:
+		cs_bit = CS_POLARITY1;
+		break;
+
+	case 2:
+		cs_bit = CS_POLARITY2;
+		break;
+
+	case 4:
+		cs_bit = CS_POLARITY3;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	val = spi_readl(tspi, SLINK_COMMAND);
+	if (spi->mode & SPI_CS_HIGH)
+		val |= cs_bit;
+	else
+		val &= ~cs_bit;
+	spi_writel(tspi, val, SLINK_COMMAND);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long flags;
+	int was_empty;
+
+	if (list_empty(&m->transfers) || !m->complete)
+		return -EINVAL;
+
+	m->state = spi;
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	was_empty = list_empty(&tspi->queue);
+	list_add_tail(&m->queue, &tspi->queue);
+
+	if (was_empty)
+		spi_tegra_start_message(spi, m);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static void spi_tegra_cleanup(struct spi_device *spi)
+{
+	dev_dbg(&spi->dev, "cleanup\n");
+}
+
+static int __init spi_tegra_probe(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof *tspi);
+	if (master == NULL) {
+		dev_err(&pdev->dev, "master allocation failed\n");
+		return -ENOMEM;
+	}
+
+	/* the spi->mode bits understood by this driver: */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+
+	master->setup = spi_tegra_setup;
+	master->transfer = spi_tegra_transfer;
+	master->cleanup = spi_tegra_cleanup;
+	master->num_chipselect = 4;
+
+	dev_set_drvdata(&pdev->dev, master);
+	tspi = spi_master_get_devdata(master);
+	tspi->master = master;
+	tspi->pdev = pdev;
+	spin_lock_init(&tspi->lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		ret = -ENODEV;
+		goto err0;
+	}
+
+	if (!request_mem_region(r->start, (r->end - r->start) + 1,
+			dev_name(&pdev->dev))) {
+		ret = -EBUSY;
+		goto err0;
+	}
+
+	tspi->phys = r->start;
+	tspi->base = ioremap(r->start, r->end - r->start + 1);
+	if (!tspi->base) {
+		dev_err(&pdev->dev, "can't ioremap iomem\n");
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	tspi->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR_OR_NULL(tspi->clk)) {
+		dev_err(&pdev->dev, "can not get clock\n");
+		ret = PTR_ERR(tspi->clk);
+		goto err2;
+	}
+
+	INIT_LIST_HEAD(&tspi->queue);
+
+	tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
+	if (IS_ERR(tspi->rx_dma)) {
+		dev_err(&pdev->dev, "can not allocate rx dma channel\n");
+		ret = PTR_ERR(tspi->rx_dma);
+		goto err3;
+	}
+
+	tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+					 &tspi->rx_bb_phys, GFP_KERNEL);
+	if (!tspi->rx_bb) {
+		dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
+		ret = -ENOMEM;
+		goto err4;
+	}
+
+	tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
+	tspi->rx_dma_req.to_memory = 1;
+	tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
+	tspi->rx_dma_req.dest_bus_width = 32;
+	tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
+	tspi->rx_dma_req.source_bus_width = 32;
+	tspi->rx_dma_req.source_wrap = 4;
+	tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
+	tspi->rx_dma_req.dev = tspi;
+
+	ret = spi_register_master(master);
+
+	if (ret < 0)
+		goto err5;
+
+	return ret;
+
+err5:
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+err4:
+	tegra_dma_free_channel(tspi->rx_dma);
+err3:
+	clk_put(tspi->clk);
+err2:
+	iounmap(tspi->base);
+err1:
+	release_mem_region(r->start, (r->end - r->start) + 1);
+err0:
+	spi_master_put(master);
+	return ret;
+}
+
+static int __exit spi_tegra_remove(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+
+	master = dev_get_drvdata(&pdev->dev);
+	tspi = spi_master_get_devdata(master);
+
+	tegra_dma_free_channel(tspi->rx_dma);
+
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+
+	clk_put(tspi->clk);
+	iounmap(tspi->base);
+
+	spi_master_put(master);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(r->start, (r->end - r->start) + 1);
+
+	return 0;
+}
+
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+
+static struct platform_driver spi_tegra_driver = {
+	.driver = {
+		.name =		DRIVER_NAME,
+		.owner =	THIS_MODULE,
+	},
+	.remove =	__exit_p(spi_tegra_remove),
+};
+
+
+static int __init spi_tegra_init(void)
+{
+	return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
+}
+subsys_initcall(spi_tegra_init);
+
+static void __exit spi_tegra_exit(void)
+{
+	platform_driver_unregister(&spi_tegra_driver);
+}
+module_exit(spi_tegra_exit);
+
+MODULE_LICENSE("GPL");
-- 
1.7.1

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

* Re: [PATCH] spi: add spi_tegra driver
  2010-07-30  0:37 [PATCH] spi: add spi_tegra driver Colin Cross
@ 2010-07-30  7:54 ` Thierry Reding
       [not found]   ` <20100730075443.GA17814-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
  2010-08-01  6:21 ` Grant Likely
       [not found] ` <1280450224-25118-1-git-send-email-ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
  2 siblings, 1 reply; 18+ messages in thread
From: Thierry Reding @ 2010-07-30  7:54 UTC (permalink / raw)
  To: Colin Cross
  Cc: linux-tegra, spi-devel-general, Erik Gilling, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1138 bytes --]

* Colin Cross wrote:
> From: Erik Gilling <konkers@android.com>
> 
> CC: spi-devel-general@lists.sourceforge.net
> Signed-off-by: Erik Gilling <konkers@android.com>
> ---
> This patch depends on the Tegra APB DMA driver:
> [PATCH] [ARM] tegra: Add APB DMA support
> ---
>  drivers/spi/Kconfig     |    6 +
>  drivers/spi/Makefile    |    1 +
>  drivers/spi/spi_tegra.c |  651 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 658 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/spi_tegra.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 91c2f4f..c07e0de 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -298,6 +298,12 @@ config SPI_STMP3XXX
>  	help
>  	  SPI driver for Freescale STMP37xx/378x SoC SSP interface
>  
> +config SPI_TEGRA
> +	tristate "Nvidia Tegra SPI controller"
> +	depends on ARCH_TEGRA
> +	help
> +	  SPI driver for NVidia Tegra SoCs
> +
[...]

Since you say this driver depends on the APB DMA driver, should this not be
expressed as a Kconfig dependency? Something like:

	select TEGRA_SYSTEM_DMA

Thierry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] spi: add spi_tegra driver
  2010-07-30  0:37 [PATCH] spi: add spi_tegra driver Colin Cross
  2010-07-30  7:54 ` Thierry Reding
@ 2010-08-01  6:21 ` Grant Likely
       [not found]   ` <AANLkTimUm_wj+gyxTa=X+845kLj1sZ9GF+jsmsBwxnLN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
       [not found] ` <1280450224-25118-1-git-send-email-ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
  2 siblings, 1 reply; 18+ messages in thread
From: Grant Likely @ 2010-08-01  6:21 UTC (permalink / raw)
  To: Colin Cross
  Cc: linux-tegra, spi-devel-general, linux-arm-kernel, Erik Gilling

Hi Colin,

Looks like a pretty nice driver.  Some comments/questions below.

Cheers,
g.

On Thu, Jul 29, 2010 at 6:37 PM, Colin Cross <ccross@google.com> wrote:
> From: Erik Gilling <konkers@android.com>
>
> CC: spi-devel-general@lists.sourceforge.net
> Signed-off-by: Erik Gilling <konkers@android.com>

You should also add your s-o-b line when passing on patches.

> ---
> This patch depends on the Tegra APB DMA driver:
> [PATCH] [ARM] tegra: Add APB DMA support


What is the status of the APB DMA driver?  Is it going to be merged in 2.6.36?

> ---
>  drivers/spi/Kconfig     |    6 +
>  drivers/spi/Makefile    |    1 +
>  drivers/spi/spi_tegra.c |  651 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 658 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/spi_tegra.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 91c2f4f..c07e0de 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -298,6 +298,12 @@ config SPI_STMP3XXX
>        help
>          SPI driver for Freescale STMP37xx/378x SoC SSP interface
>
> +config SPI_TEGRA
> +       tristate "Nvidia Tegra SPI controller"
> +       depends on ARCH_TEGRA
> +       help
> +         SPI driver for NVidia Tegra SoCs
> +
>  config SPI_TXX9
>        tristate "Toshiba TXx9 SPI controller"
>        depends on GENERIC_GPIO && CPU_TX49XX
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index e9cbd18..b6573d8 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)              += spi_ppc4xx.o
>  obj-$(CONFIG_SPI_S3C24XX_GPIO)         += spi_s3c24xx_gpio.o
>  obj-$(CONFIG_SPI_S3C24XX)              += spi_s3c24xx_hw.o
>  obj-$(CONFIG_SPI_S3C64XX)              += spi_s3c64xx.o
> +obj-$(CONFIG_SPI_TEGRA)                        += spi_tegra.o
>  obj-$(CONFIG_SPI_TXX9)                 += spi_txx9.o
>  obj-$(CONFIG_SPI_XILINX)               += xilinx_spi.o
>  obj-$(CONFIG_SPI_XILINX_OF)            += xilinx_spi_of.o
> diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
> new file mode 100644
> index 0000000..72fc4d4
> --- /dev/null
> +++ b/drivers/spi/spi_tegra.c
> @@ -0,0 +1,651 @@
> +/*
> + * drivers/spi/spi_tegra.c

I find it is more useful, and it is my preference, to have a one-line
description of what the file actually is here.  Showing the filename
is just redundant and doesn't tell a reader anything new.

> + *
> + * Copyright (C) 2010 Google, Inc.
> + *
> + * Author:
> + *     Erik Gilling <konkers@android.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + *
> + */
> +
> +#undef DEBUG

trim

> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +
> +#include <linux/spi/spi.h>
> +
> +#include <mach/dma.h>
> +
> +#define DRIVER_NAME            "spi_tegra"

Used only twice in 2 side-by-side locations.  I'd trim, or at least
move to where it is actually used.

> +
> +#define BUSY_TIMEOUT           1000

Used exactly once.  May be better defined and documented at the usage site

> +
> +#define SLINK_COMMAND          0x000
> +#define   BIT_LENGTH(x)                        (((x) & 0x1f) << 0)
> +#define   WORD_SIZE(x)                 (((x) & 0x1f) << 5)
> +#define   BOTH_EN                      (1 << 10)
> +#define   CS_SW                                (1 << 11)
> +#define   CS_VALUE                     (1 << 12)
> +#define   CS_POLARITY                  (1 << 13)
> +#define   IDLE_SDA_DRIVE_LOW           (0 << 16)
> +#define   IDLE_SDA_DRIVE_HIGH          (1 << 16)
> +#define   IDLE_SDA_PULL_LOW            (2 << 16)
> +#define   IDLE_SDA_PULL_HIGH           (3 << 16)
> +#define   IDLE_SDA_MASK                        (3 << 16)
> +#define   CS_POLARITY1                 (1 << 20)
> +#define   CK_SDA                       (1 << 21)
> +#define   CS_POLARITY2                 (1 << 22)
> +#define   CS_POLARITY3                 (1 << 23)
> +#define   IDLE_SCLK_DRIVE_LOW          (0 << 24)
> +#define   IDLE_SCLK_DRIVE_HIGH         (1 << 24)
> +#define   IDLE_SCLK_PULL_LOW           (2 << 24)
> +#define   IDLE_SCLK_PULL_HIGH          (3 << 24)
> +#define   IDLE_SCLK_MASK               (3 << 24)
> +#define   M_S                          (1 << 28)
> +#define   WAIT                         (1 << 29)
> +#define   GO                           (1 << 30)
> +#define   ENB                          (1 << 31)
> +
> +#define SLINK_COMMAND2         0x004
> +#define   LSBFE                                (1 << 0)
> +#define   SSOE                         (1 << 1)
> +#define   SPIE                         (1 << 4)
> +#define   BIDIROE                      (1 << 6)
> +#define   MODFEN                       (1 << 7)
> +#define   INT_SIZE(x)                  (((x) & 0x1f) << 8)
> +#define   CS_ACTIVE_BETWEEN            (1 << 17)
> +#define   SS_EN_CS(x)                  (((x) & 0x3) << 18)
> +#define   SS_SETUP(x)                  (((x) & 0x3) << 20)
> +#define   FIFO_REFILLS_0               (0 << 22)
> +#define   FIFO_REFILLS_1               (1 << 22)
> +#define   FIFO_REFILLS_2               (2 << 22)
> +#define   FIFO_REFILLS_3               (3 << 22)
> +#define   FIFO_REFILLS_MASK            (3 << 22)
> +#define   WAIT_PACK_INT(x)             (((x) & 0x7) << 26)
> +#define   SPC0                         (1 << 29)
> +#define   TXEN                         (1 << 30)
> +#define   RXEN                         (1 << 31)
> +
> +#define SLINK_STATUS           0x008
> +#define   COUNT(val)                   (((val) >> 0) & 0x1f)
> +#define   WORD(val)                    (((val) >> 5) & 0x1f)
> +#define   BLK_CNT(val)                 (((val) >> 0) & 0xffff)
> +#define   MODF                         (1 << 16)
> +#define   RX_UNF                       (1 << 18)
> +#define   TX_OVF                       (1 << 19)
> +#define   TX_FULL                      (1 << 20)
> +#define   TX_EMPTY                     (1 << 21)
> +#define   RX_FULL                      (1 << 22)
> +#define   RX_EMPTY                     (1 << 23)
> +#define   TX_UNF                       (1 << 24)
> +#define   RX_OVF                       (1 << 25)
> +#define   TX_FLUSH                     (1 << 26)
> +#define   RX_FLUSH                     (1 << 27)
> +#define   SCLK                         (1 << 28)
> +#define   ERR                          (1 << 29)
> +#define   RDY                          (1 << 30)
> +#define   BSY                          (1 << 31)
> +
> +#define SLINK_MAS_DATA         0x010
> +#define SLINK_SLAVE_DATA       0x014
> +
> +#define SLINK_DMA_CTL          0x018
> +#define   DMA_BLOCK_SIZE(x)            (((x) & 0xffff) << 0)
> +#define   TX_TRIG_1                    (0 << 16)
> +#define   TX_TRIG_4                    (1 << 16)
> +#define   TX_TRIG_8                    (2 << 16)
> +#define   TX_TRIG_16                   (3 << 16)
> +#define   TX_TRIG_MASK                 (3 << 16)
> +#define   RX_TRIG_1                    (0 << 18)
> +#define   RX_TRIG_4                    (1 << 18)
> +#define   RX_TRIG_8                    (2 << 18)
> +#define   RX_TRIG_16                   (3 << 18)
> +#define   RX_TRIG_MASK                 (3 << 18)
> +#define   PACKED                       (1 << 20)
> +#define   PACK_SIZE_4                  (0 << 21)
> +#define   PACK_SIZE_8                  (1 << 21)
> +#define   PACK_SIZE_16                 (2 << 21)
> +#define   PACK_SIZE_32                 (3 << 21)
> +#define   PACK_SIZE_MASK               (3 << 21)
> +#define   IE_TXC                       (1 << 26)
> +#define   IE_RXC                       (1 << 27)
> +#define   DMA_EN                       (1 << 31)
> +
> +#define SLINK_STATUS2          0x01c
> +#define   TX_FIFO_EMPTY_COUNT(val)     (((val) & 0x3f) >> 0)
> +#define   RX_FIFO_FULL_COUNT(val)      (((val) & 0x3f) >> 16)
> +
> +#define SLINK_TX_FIFO          0x100
> +#define SLINK_RX_FIFO          0x180
> +
> +static const unsigned long spi_tegra_req_sels[] = {
> +       TEGRA_DMA_REQ_SEL_SL2B1,
> +       TEGRA_DMA_REQ_SEL_SL2B2,
> +       TEGRA_DMA_REQ_SEL_SL2B3,
> +       TEGRA_DMA_REQ_SEL_SL2B4,
> +};
> +
> +static inline unsigned bytes_per_word(u8 bits)
> +{
> +       WARN_ON((bits < 1) || bits > 32);

This condition has already been tested for in spi_tegra_start_transfer()

> +       if (bits <= 8)
> +               return 1;
> +       else if (bits <= 16)
> +               return 2;
> +       else if (bits <= 24)
> +               return 3;
> +       else
> +               return 4;

return ((bits - 1) / 8) + 1

Also, this function is used in exactly 1 place.  Consider moving it
down to nearer where it is called, or open coding it instead.

> +}
> +
> +#define BB_LEN                 32
> +
> +struct spi_tegra_data {
> +       struct spi_master       *master;
> +       struct platform_device  *pdev;
> +       spinlock_t              lock;
> +
> +       struct clk              *clk;
> +       void __iomem            *base;
> +       unsigned long           phys;
> +
> +       u32                     cur_speed;
> +
> +       struct list_head        queue;
> +       struct spi_transfer     *cur;
> +       unsigned                cur_pos;
> +       unsigned                cur_len;
> +       unsigned                cur_bytes_per_word;
> +
> +       /* The tegra spi controller has a bug which causes the first word
> +        * in PIO transactions to be garbage.  Since packed DMA transactions
> +        * require transfers to be 4 byte aligned we need a bounce buffer
> +        * for the generic case.
> +        */
> +       struct tegra_dma_req    rx_dma_req;
> +       struct tegra_dma_channel *rx_dma;
> +       u32                     *rx_bb;
> +       dma_addr_t              rx_bb_phys;
> +};
> +
> +
> +static unsigned long spi_readl(struct spi_tegra_data *tspi, unsigned long reg)

To avoid global namespace collisions, please prefix all static symbols
with something like tegra_spi_.

> +{
> +       return readl(tspi->base + reg);
> +}
> +
> +static void spi_writel(struct spi_tegra_data *tspi, unsigned long val,
> +                      unsigned long reg)
> +{
> +       writel(val, tspi->base + reg);
> +}

Are the accessor redirects really necessary?  Is there any likelyhood
that anything other than readl/writel will ever be used to access the
device regs?

> +
> +static void spi_tegra_go(struct spi_tegra_data *tspi)
> +{
> +       unsigned long val;
> +
> +       wmb();
> +
> +       val = spi_readl(tspi, SLINK_DMA_CTL);
> +       val &= ~DMA_BLOCK_SIZE(~0) & ~DMA_EN;
> +       val |= DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
> +       spi_writel(tspi, val, SLINK_DMA_CTL);
> +
> +       tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
> +
> +       val |= DMA_EN;
> +       spi_writel(tspi, val, SLINK_DMA_CTL);
> +}
> +
> +static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
> +                                 struct spi_transfer *t)
> +{
> +       unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
> +                          tspi->cur_bytes_per_word);
> +       u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
> +       int i, j;
> +       unsigned long val;
> +
> +       val = spi_readl(tspi, SLINK_COMMAND);
> +       val &= ~WORD_SIZE(~0);
> +       val |= WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
> +       spi_writel(tspi, val, SLINK_COMMAND);
> +
> +       for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
> +               val = 0;
> +               for (j = 0; j < tspi->cur_bytes_per_word; j++)
> +                       val |= tx_buf[i + j] << j * 8;
> +
> +               spi_writel(tspi, val, SLINK_TX_FIFO);
> +       }
> +
> +       tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
> +
> +       return len;
> +}
> +
> +static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
> +                                 struct spi_transfer *t)
> +{
> +       unsigned len = tspi->cur_len;
> +       u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
> +       int i, j;
> +       unsigned long val;
> +
> +       for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
> +               val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
> +               for (j = 0; j < tspi->cur_bytes_per_word; j++)
> +                       rx_buf[i + j] = (val >> (j * 8)) & 0xff;
> +       }
> +
> +       return len;
> +}
> +
> +static int spi_tegra_start_transfer(struct spi_device *spi,
> +                                   struct spi_transfer *t)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       u32 speed;
> +       u8 bits_per_word;
> +       unsigned long val;
> +
> +       speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
> +       bits_per_word = t->bits_per_word ? t->bits_per_word  :
> +               spi->bits_per_word;
> +
> +       if (bits_per_word < 1 || bits_per_word > 32)
> +               return -EINVAL;
> +
> +       if (t->len == 0)
> +               return -EINVAL;
> +
> +       if (!t->rx_buf && !t->tx_buf)
> +               return -EINVAL;

Looks to me like this validation should be performed before accepting
the transfer (see comments below)

> +
> +       tspi->cur_bytes_per_word = bytes_per_word(bits_per_word);
> +
> +       if (speed != tspi->cur_speed)
> +               clk_set_rate(tspi->clk, speed);
> +
> +       if (tspi->cur_speed == 0)
> +               clk_enable(tspi->clk);
> +
> +       tspi->cur_speed = speed;
> +
> +       val = spi_readl(tspi, SLINK_COMMAND2);
> +       if (t->rx_buf)
> +               val |= RXEN;
> +       else
> +               val &= ~RXEN;
> +
> +       if (t->tx_buf)
> +               val |= TXEN;
> +       else
> +               val &= ~TXEN;
> +
> +       val &= ~SS_EN_CS(~0);
> +       val |= SS_EN_CS(spi->chip_select);
> +       val |= SPIE;
> +
> +       spi_writel(tspi, val, SLINK_COMMAND2);
> +
> +       val = spi_readl(tspi, SLINK_COMMAND);
> +       val &= ~BIT_LENGTH(~0);
> +       val |= BIT_LENGTH(bits_per_word - 1);
> +
> +       /* FIXME: should probably control CS manually so that we can be sure
> +        * it does not go low between transfer and to support delay_usecs
> +        * correctly.
> +        */
> +       val &= ~IDLE_SCLK_MASK & ~CK_SDA & ~CS_SW;
> +
> +       if (spi->mode & SPI_CPHA)
> +               val |= CK_SDA;
> +
> +       if (spi->mode & SPI_CPOL)
> +               val |= IDLE_SCLK_DRIVE_HIGH;
> +       else
> +               val |= IDLE_SCLK_DRIVE_LOW;
> +
> +       val |= M_S;
> +
> +       spi_writel(tspi, val, SLINK_COMMAND);
> +
> +       spi_writel(tspi, RX_FLUSH | TX_FLUSH, SLINK_STATUS);
> +
> +       tspi->cur = t;
> +       tspi->cur_pos = 0;
> +       tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
> +
> +       spi_tegra_go(tspi);
> +
> +       return 0;
> +}
> +
> +static void spi_tegra_start_message(struct spi_device *spi,
> +                                   struct spi_message *m)
> +{
> +       struct spi_transfer *t;
> +
> +       m->actual_length = 0;
> +
> +       t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
> +       spi_tegra_start_transfer(spi, t);
> +}
> +
> +static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
> +{
> +       struct spi_tegra_data *tspi = req->dev;
> +       unsigned long flags;
> +       struct spi_message *m;
> +       struct spi_device *spi;
> +       int complete = 0;
> +       int timeout = 0;
> +       unsigned long val;
> +
> +       /* the SPI controller may come back with both the BSY and RDY bits
> +        * set.  In this case we need to wait for the BSY bit to clear so
> +        * that we are sure the DMA is finished.
> +        */
> +       while (timeout++ < BUSY_TIMEOUT) {
> +               if (!(spi_readl(tspi, SLINK_STATUS) & BSY))
> +                       break;
> +       }
> +
> +       val = spi_readl(tspi, SLINK_STATUS);
> +       val |= RDY;
> +       spi_writel(tspi, val, SLINK_STATUS);
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +
> +       m = list_first_entry(&tspi->queue, struct spi_message, queue);
> +       spi = m->state;
> +
> +       tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
> +       m->actual_length += tspi->cur_pos;
> +
> +       if (tspi->cur_pos < tspi->cur->len) {
> +               tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
> +               spi_tegra_go(tspi);
> +       } else if (!list_is_last(&tspi->cur->transfer_list,
> +                                &m->transfers)) {
> +               tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
> +                                             struct spi_transfer,
> +                                             transfer_list);
> +               spi_tegra_start_transfer(spi, tspi->cur);

Not checking the returned error code.  If an error occurs, then the
callback needs to be called to notify that the message did not
complete.  However, this comment may be moot if the validation is
moved out of spi_tegra_start_transfer().

> +       } else {
> +               complete = 1;
> +       }
> +
> +       if (complete) {
> +               list_del(&m->queue);
> +
> +               m->status = 0;
> +               m->complete(m->context);
> +
> +               if (!list_empty(&tspi->queue)) {
> +                       m = list_first_entry(&tspi->queue, struct spi_message,
> +                                            queue);
> +                       spi = m->state;
> +                       spi_tegra_start_message(spi, m);
> +               } else {
> +                       clk_disable(tspi->clk);
> +                       tspi->cur_speed = 0;
> +               }
> +       }
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +}
> +
> +static int spi_tegra_setup(struct spi_device *spi)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       unsigned long cs_bit;
> +       unsigned long val;
> +       unsigned long flags;
> +
> +       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);
> +
> +
> +       switch (spi->chip_select) {
> +       case 0:
> +               cs_bit = CS_POLARITY;
> +               break;
> +
> +       case 1:
> +               cs_bit = CS_POLARITY1;
> +               break;
> +
> +       case 2:
> +               cs_bit = CS_POLARITY2;
> +               break;
> +
> +       case 4:
> +               cs_bit = CS_POLARITY3;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +
> +       val = spi_readl(tspi, SLINK_COMMAND);
> +       if (spi->mode & SPI_CS_HIGH)
> +               val |= cs_bit;
> +       else
> +               val &= ~cs_bit;
> +       spi_writel(tspi, val, SLINK_COMMAND);
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       unsigned long flags;
> +       int was_empty;
> +
> +       if (list_empty(&m->transfers) || !m->complete)
> +               return -EINVAL;

It's a good idea to move most of the validation of the message from
spi_tegra_start_message() and spi_tegra_start_transfer() to here so
that the validation occurs outside of the spinlock.  It also means the
driver doesn't discover badly formed messages part way through the
transfer.

> +
> +       m->state = spi;
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +       was_empty = list_empty(&tspi->queue);
> +       list_add_tail(&m->queue, &tspi->queue);
> +
> +       if (was_empty)
> +               spi_tegra_start_message(spi, m);
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void spi_tegra_cleanup(struct spi_device *spi)
> +{
> +       dev_dbg(&spi->dev, "cleanup\n");
> +}
> +
> +static int __init spi_tegra_probe(struct platform_device *pdev)
> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +       struct resource         *r;
> +       int ret;
> +
> +       master = spi_alloc_master(&pdev->dev, sizeof *tspi);
> +       if (master == NULL) {
> +               dev_err(&pdev->dev, "master allocation failed\n");
> +               return -ENOMEM;
> +       }
> +
> +       /* the spi->mode bits understood by this driver: */
> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> +
> +       if (pdev->id != -1)
> +               master->bus_num = pdev->id;
> +
> +       master->setup = spi_tegra_setup;
> +       master->transfer = spi_tegra_transfer;
> +       master->cleanup = spi_tegra_cleanup;
> +       master->num_chipselect = 4;
> +
> +       dev_set_drvdata(&pdev->dev, master);
> +       tspi = spi_master_get_devdata(master);
> +       tspi->master = master;
> +       tspi->pdev = pdev;
> +       spin_lock_init(&tspi->lock);
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (r == NULL) {
> +               ret = -ENODEV;
> +               goto err0;
> +       }
> +
> +       if (!request_mem_region(r->start, (r->end - r->start) + 1,
> +                       dev_name(&pdev->dev))) {
> +               ret = -EBUSY;
> +               goto err0;
> +       }

I believe the platform bus does this for you already by calling
insert_resource() on all the platform bus ranges.  See if /proc/iomem
is any different with or without this call to verify.

> +
> +       tspi->phys = r->start;
> +       tspi->base = ioremap(r->start, r->end - r->start + 1);
> +       if (!tspi->base) {
> +               dev_err(&pdev->dev, "can't ioremap iomem\n");
> +               ret = -ENOMEM;
> +               goto err1;
> +       }
> +
> +       tspi->clk = clk_get(&pdev->dev, NULL);
> +       if (IS_ERR_OR_NULL(tspi->clk)) {
> +               dev_err(&pdev->dev, "can not get clock\n");
> +               ret = PTR_ERR(tspi->clk);
> +               goto err2;
> +       }
> +
> +       INIT_LIST_HEAD(&tspi->queue);
> +
> +       tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
> +       if (IS_ERR(tspi->rx_dma)) {
> +               dev_err(&pdev->dev, "can not allocate rx dma channel\n");
> +               ret = PTR_ERR(tspi->rx_dma);
> +               goto err3;
> +       }
> +
> +       tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                                        &tspi->rx_bb_phys, GFP_KERNEL);
> +       if (!tspi->rx_bb) {
> +               dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
> +               ret = -ENOMEM;
> +               goto err4;
> +       }
> +
> +       tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
> +       tspi->rx_dma_req.to_memory = 1;
> +       tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
> +       tspi->rx_dma_req.dest_bus_width = 32;
> +       tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
> +       tspi->rx_dma_req.source_bus_width = 32;
> +       tspi->rx_dma_req.source_wrap = 4;
> +       tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
> +       tspi->rx_dma_req.dev = tspi;
> +
> +       ret = spi_register_master(master);
> +
> +       if (ret < 0)
> +               goto err5;
> +
> +       return ret;
> +
> +err5:
> +       dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                         tspi->rx_bb, tspi->rx_bb_phys);
> +err4:
> +       tegra_dma_free_channel(tspi->rx_dma);
> +err3:
> +       clk_put(tspi->clk);
> +err2:
> +       iounmap(tspi->base);
> +err1:
> +       release_mem_region(r->start, (r->end - r->start) + 1);
> +err0:
> +       spi_master_put(master);
> +       return ret;
> +}
> +
> +static int __exit spi_tegra_remove(struct platform_device *pdev)

__devexit

> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +       struct resource         *r;
> +
> +       master = dev_get_drvdata(&pdev->dev);
> +       tspi = spi_master_get_devdata(master);
> +
> +       tegra_dma_free_channel(tspi->rx_dma);
> +
> +       dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                         tspi->rx_bb, tspi->rx_bb_phys);
> +
> +       clk_put(tspi->clk);
> +       iounmap(tspi->base);
> +
> +       spi_master_put(master);
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       release_mem_region(r->start, (r->end - r->start) + 1);

Ditto to comment earlier.

> +
> +       return 0;
> +}
> +
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +
> +static struct platform_driver spi_tegra_driver = {
> +       .driver = {
> +               .name =         DRIVER_NAME,
> +               .owner =        THIS_MODULE,
> +       },
> +       .remove =       __exit_p(spi_tegra_remove),

__devexit_p

> +};
> +
> +
> +static int __init spi_tegra_init(void)
> +{
> +       return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
> +}
> +subsys_initcall(spi_tegra_init);

Is module_init() not sufficient?

> +
> +static void __exit spi_tegra_exit(void)
> +{
> +       platform_driver_unregister(&spi_tegra_driver);
> +}
> +module_exit(spi_tegra_exit);
> +
> +MODULE_LICENSE("GPL");
> --
> 1.7.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

* Re: [PATCH] spi: add spi_tegra driver
       [not found]   ` <20100730075443.GA17814-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
@ 2010-08-02 18:48     ` Erik Gilling
  0 siblings, 0 replies; 18+ messages in thread
From: Erik Gilling @ 2010-08-02 18:48 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Colin Cross

On Fri, Jul 30, 2010 at 12:54 AM, Thierry Reding
<thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> wrote:
>
> Since you say this driver depends on the APB DMA driver, should this not be
> expressed as a Kconfig dependency? Something like:
>
>        select TEGRA_SYSTEM_DMA

yep, will do.

------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://p.sf.net/sfu/dev2dev-palm

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

* Re: [PATCH] spi: add spi_tegra driver
       [not found]   ` <AANLkTimUm_wj+gyxTa=X+845kLj1sZ9GF+jsmsBwxnLN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-08-12  0:31     ` Erik Gilling
  2010-08-12 15:24       ` Russell King - ARM Linux
  0 siblings, 1 reply; 18+ messages in thread
From: Erik Gilling @ 2010-08-12  0:31 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Colin Cross

On Sat, Jul 31, 2010 at 11:21 PM, Grant Likely
<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> What is the status of the APB DMA driver?  Is it going to be merged in 2.6.36?

Since we were too late on these patches, they will go into the 37
merge window.  The APB DMA driver will go in then along with SPI.

>> + * drivers/spi/spi_tegra.c
>
> I find it is more useful, and it is my preference, to have a one-line
> description of what the file actually is here.  Showing the filename
> is just redundant and doesn't tell a reader anything new.

I'll change it so it matches the rest of the files in the directory

>> +#define DRIVER_NAME            "spi_tegra"
>
> Used only twice in 2 side-by-side locations.  I'd trim, or at least
> move to where it is actually used.

done


>> +#define BUSY_TIMEOUT           1000
>
> Used exactly once.  May be better defined and documented at the usage site

done

>> +
>> +static inline unsigned bytes_per_word(u8 bits)
>> +{
>> +       WARN_ON((bits < 1) || bits > 32);
>
> This condition has already been tested for in spi_tegra_start_transfer()
>
>> +       if (bits <= 8)
>> +               return 1;
>> +       else if (bits <= 16)
>> +               return 2;
>> +       else if (bits <= 24)
>> +               return 3;
>> +       else
>> +               return 4;
>
> return ((bits - 1) / 8) + 1
>
> Also, this function is used in exactly 1 place.  Consider moving it
> down to nearer where it is called, or open coding it instead.

moved to spi_tegra_start_transfer

>> +static unsigned long spi_readl(struct spi_tegra_data *tspi, unsigned long reg)
>
> To avoid global namespace collisions, please prefix all static symbols
> with something like tegra_spi_.

changed

>> +{
>> +       return readl(tspi->base + reg);
>> +}
>> +
>> +static void spi_writel(struct spi_tegra_data *tspi, unsigned long val,
>> +                      unsigned long reg)
>> +{
>> +       writel(val, tspi->base + reg);
>> +}
>
> Are the accessor redirects really necessary?  Is there any likelyhood
> that anything other than readl/writel will ever be used to access the
> device regs?

They are there to make reset of the code easier to read and less error
probe by eliminating the tspi->base + reg from every call.

>> +
>> +       speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
>> +       bits_per_word = t->bits_per_word ? t->bits_per_word  :
>> +               spi->bits_per_word;
>> +
>> +       if (bits_per_word < 1 || bits_per_word > 32)
>> +               return -EINVAL;
>> +
>> +       if (t->len == 0)
>> +               return -EINVAL;
>> +
>> +       if (!t->rx_buf && !t->tx_buf)
>> +               return -EINVAL;
>
> Looks to me like this validation should be performed before accepting
> the transfer (see comments below)

moved


>> +               tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
>> +                                             struct spi_transfer,
>> +                                             transfer_list);
>> +               spi_tegra_start_transfer(spi, tspi->cur);
>
> Not checking the returned error code.  If an error occurs, then the
> callback needs to be called to notify that the message did not
> complete.  However, this comment may be moot if the validation is
> moved out of spi_tegra_start_transfer().

moved


>> +static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
>> +{
>> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
>> +       unsigned long flags;
>> +       int was_empty;
>> +
>> +       if (list_empty(&m->transfers) || !m->complete)
>> +               return -EINVAL;
>
> It's a good idea to move most of the validation of the message from
> spi_tegra_start_message() and spi_tegra_start_transfer() to here so
> that the validation occurs outside of the spinlock.  It also means the
> driver doesn't discover badly formed messages part way through the
> transfer.

moved

>> +       if (!request_mem_region(r->start, (r->end - r->start) + 1,
>> +                       dev_name(&pdev->dev))) {
>> +               ret = -EBUSY;
>> +               goto err0;
>> +       }
>
> I believe the platform bus does this for you already by calling
> insert_resource() on all the platform bus ranges.  See if /proc/iomem
> is any different with or without this call to verify.

You're right.  Didn't realize this


>> +static int __exit spi_tegra_remove(struct platform_device *pdev)
>
> __devexit

done

>> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       release_mem_region(r->start, (r->end - r->start) + 1);
>
> Ditto to comment earlier.

done


>> +       .remove =       __exit_p(spi_tegra_remove),
>
> __devexit_p

done


>> +subsys_initcall(spi_tegra_init);
>
> Is module_init() not sufficient?

All the other spi drivers use subsys_initcall

New patch coming soon.

Thanks,
    Erik

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

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

* [PATCH v2] spi: add spi_tegra driver
       [not found] ` <1280450224-25118-1-git-send-email-ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
@ 2010-08-12  0:47   ` Erik Gilling
       [not found]     ` <1281574023-25160-1-git-send-email-konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Erik Gilling @ 2010-08-12  0:47 UTC (permalink / raw)
  To: linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Erik Gilling, ccross-hpIqsD4AKlfQT0dZR+AlfA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

v2 changes:
  from Thierry Reding:
    * add "select TEGRA_SYSTEM_DMA" to Kconfig
  from Grant Likely:
    * add oneline description to header
    * inline references to DRIVER_NAME
    * inline references to BUSY_TIMEOUT
    * open coded bytes_per_word()
    * spi_readl/writel -> spi_tegra_readl/writel
    * move transfer validation to spi_tegra_transfer
    * don't request_mem_region iomem as platform bus does that for us
    * __exit -> __devexit

Signed-off-by: Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Cc: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---
 drivers/spi/Kconfig     |    7 +
 drivers/spi/Makefile    |    1 +
 drivers/spi/spi_tegra.c |  638 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 646 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/spi_tegra.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 91c2f4f..9fdb309 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -298,6 +298,13 @@ config SPI_STMP3XXX
 	help
 	  SPI driver for Freescale STMP37xx/378x SoC SSP interface
 
+config SPI_TEGRA
+	tristate "Nvidia Tegra SPI controller"
+	depends on ARCH_TEGRA
+	select TEGRA_SYSTEM_DMA
+	help
+	  SPI driver for NVidia Tegra SoCs
+
 config SPI_TXX9
 	tristate "Toshiba TXx9 SPI controller"
 	depends on GENERIC_GPIO && CPU_TX49XX
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index e9cbd18..b6573d8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)		+= spi_ppc4xx.o
 obj-$(CONFIG_SPI_S3C24XX_GPIO)		+= spi_s3c24xx_gpio.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi_s3c24xx_hw.o
 obj-$(CONFIG_SPI_S3C64XX)		+= spi_s3c64xx.o
+obj-$(CONFIG_SPI_TEGRA)			+= spi_tegra.o
 obj-$(CONFIG_SPI_TXX9)			+= spi_txx9.o
 obj-$(CONFIG_SPI_XILINX)		+= xilinx_spi.o
 obj-$(CONFIG_SPI_XILINX_OF)		+= xilinx_spi_of.o
diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
new file mode 100644
index 0000000..5afb7a7
--- /dev/null
+++ b/drivers/spi/spi_tegra.c
@@ -0,0 +1,638 @@
+/*
+ * Driver for Nvidia TEGRA spi controller.
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * Author:
+ *     Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#undef DEBUG
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+
+#include <linux/spi/spi.h>
+
+#include <mach/dma.h>
+
+#define SLINK_COMMAND		0x000
+#define   BIT_LENGTH(x)			(((x) & 0x1f) << 0)
+#define   WORD_SIZE(x)			(((x) & 0x1f) << 5)
+#define   BOTH_EN			(1 << 10)
+#define   CS_SW				(1 << 11)
+#define   CS_VALUE			(1 << 12)
+#define   CS_POLARITY			(1 << 13)
+#define   IDLE_SDA_DRIVE_LOW		(0 << 16)
+#define   IDLE_SDA_DRIVE_HIGH		(1 << 16)
+#define   IDLE_SDA_PULL_LOW		(2 << 16)
+#define   IDLE_SDA_PULL_HIGH		(3 << 16)
+#define   IDLE_SDA_MASK			(3 << 16)
+#define   CS_POLARITY1			(1 << 20)
+#define   CK_SDA			(1 << 21)
+#define   CS_POLARITY2			(1 << 22)
+#define   CS_POLARITY3			(1 << 23)
+#define   IDLE_SCLK_DRIVE_LOW		(0 << 24)
+#define   IDLE_SCLK_DRIVE_HIGH		(1 << 24)
+#define   IDLE_SCLK_PULL_LOW		(2 << 24)
+#define   IDLE_SCLK_PULL_HIGH		(3 << 24)
+#define   IDLE_SCLK_MASK		(3 << 24)
+#define   M_S				(1 << 28)
+#define   WAIT				(1 << 29)
+#define   GO				(1 << 30)
+#define   ENB				(1 << 31)
+
+#define SLINK_COMMAND2		0x004
+#define   LSBFE				(1 << 0)
+#define   SSOE				(1 << 1)
+#define   SPIE				(1 << 4)
+#define   BIDIROE			(1 << 6)
+#define   MODFEN			(1 << 7)
+#define   INT_SIZE(x)			(((x) & 0x1f) << 8)
+#define   CS_ACTIVE_BETWEEN		(1 << 17)
+#define   SS_EN_CS(x)			(((x) & 0x3) << 18)
+#define   SS_SETUP(x)			(((x) & 0x3) << 20)
+#define   FIFO_REFILLS_0		(0 << 22)
+#define   FIFO_REFILLS_1		(1 << 22)
+#define   FIFO_REFILLS_2		(2 << 22)
+#define   FIFO_REFILLS_3		(3 << 22)
+#define   FIFO_REFILLS_MASK		(3 << 22)
+#define   WAIT_PACK_INT(x)		(((x) & 0x7) << 26)
+#define   SPC0				(1 << 29)
+#define   TXEN				(1 << 30)
+#define   RXEN				(1 << 31)
+
+#define SLINK_STATUS		0x008
+#define   COUNT(val)			(((val) >> 0) & 0x1f)
+#define   WORD(val)			(((val) >> 5) & 0x1f)
+#define   BLK_CNT(val)			(((val) >> 0) & 0xffff)
+#define   MODF				(1 << 16)
+#define   RX_UNF			(1 << 18)
+#define   TX_OVF			(1 << 19)
+#define   TX_FULL			(1 << 20)
+#define   TX_EMPTY			(1 << 21)
+#define   RX_FULL			(1 << 22)
+#define   RX_EMPTY			(1 << 23)
+#define   TX_UNF			(1 << 24)
+#define   RX_OVF			(1 << 25)
+#define   TX_FLUSH			(1 << 26)
+#define   RX_FLUSH			(1 << 27)
+#define   SCLK				(1 << 28)
+#define   ERR				(1 << 29)
+#define   RDY				(1 << 30)
+#define   BSY				(1 << 31)
+
+#define SLINK_MAS_DATA		0x010
+#define SLINK_SLAVE_DATA	0x014
+
+#define SLINK_DMA_CTL		0x018
+#define   DMA_BLOCK_SIZE(x)		(((x) & 0xffff) << 0)
+#define   TX_TRIG_1			(0 << 16)
+#define   TX_TRIG_4			(1 << 16)
+#define   TX_TRIG_8			(2 << 16)
+#define   TX_TRIG_16			(3 << 16)
+#define   TX_TRIG_MASK			(3 << 16)
+#define   RX_TRIG_1			(0 << 18)
+#define   RX_TRIG_4			(1 << 18)
+#define   RX_TRIG_8			(2 << 18)
+#define   RX_TRIG_16			(3 << 18)
+#define   RX_TRIG_MASK			(3 << 18)
+#define   PACKED			(1 << 20)
+#define   PACK_SIZE_4			(0 << 21)
+#define   PACK_SIZE_8			(1 << 21)
+#define   PACK_SIZE_16			(2 << 21)
+#define   PACK_SIZE_32			(3 << 21)
+#define   PACK_SIZE_MASK		(3 << 21)
+#define   IE_TXC			(1 << 26)
+#define   IE_RXC			(1 << 27)
+#define   DMA_EN			(1 << 31)
+
+#define SLINK_STATUS2		0x01c
+#define   TX_FIFO_EMPTY_COUNT(val)	(((val) & 0x3f) >> 0)
+#define   RX_FIFO_FULL_COUNT(val)	(((val) & 0x3f) >> 16)
+
+#define SLINK_TX_FIFO		0x100
+#define SLINK_RX_FIFO		0x180
+
+static const unsigned long spi_tegra_req_sels[] = {
+	TEGRA_DMA_REQ_SEL_SL2B1,
+	TEGRA_DMA_REQ_SEL_SL2B2,
+	TEGRA_DMA_REQ_SEL_SL2B3,
+	TEGRA_DMA_REQ_SEL_SL2B4,
+};
+
+static inline unsigned bytes_per_word(u8 bits)
+{
+	WARN_ON((bits < 1) || bits > 32);
+	if (bits <= 8)
+		return 1;
+	else if (bits <= 16)
+		return 2;
+	else if (bits <= 24)
+		return 3;
+	else
+		return 4;
+}
+
+#define BB_LEN			32
+
+struct spi_tegra_data {
+	struct spi_master	*master;
+	struct platform_device	*pdev;
+	spinlock_t		lock;
+
+	struct clk		*clk;
+	void __iomem		*base;
+	unsigned long		phys;
+
+	u32			cur_speed;
+
+	struct list_head	queue;
+	struct spi_transfer	*cur;
+	unsigned		cur_pos;
+	unsigned		cur_len;
+	unsigned		cur_bytes_per_word;
+
+	/* The tegra spi controller has a bug which causes the first word
+	 * in PIO transactions to be garbage.  Since packed DMA transactions
+	 * require transfers to be 4 byte aligned we need a bounce buffer
+	 * for the generic case.
+	 */
+	struct tegra_dma_req	rx_dma_req;
+	struct tegra_dma_channel *rx_dma;
+	u32			*rx_bb;
+	dma_addr_t		rx_bb_phys;
+};
+
+
+static unsigned long spi_tegra_readl(struct spi_tegra_data *tspi,
+				     unsigned long reg)
+{
+	return readl(tspi->base + reg);
+}
+
+static void spi_tegra_writel(struct spi_tegra_data *tspi, unsigned long val,
+		       unsigned long reg)
+{
+	writel(val, tspi->base + reg);
+}
+
+static void spi_tegra_go(struct spi_tegra_data *tspi)
+{
+	unsigned long val;
+
+	wmb();
+
+	val = spi_tegra_readl(tspi, SLINK_DMA_CTL);
+	val &= ~DMA_BLOCK_SIZE(~0) & ~DMA_EN;
+	val |= DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
+	spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
+
+	tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
+
+	val |= DMA_EN;
+	spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
+}
+
+static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
+			   tspi->cur_bytes_per_word);
+	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	val &= ~WORD_SIZE(~0);
+	val |= WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = 0;
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			val |= tx_buf[i + j] << j * 8;
+
+		spi_tegra_writel(tspi, val, SLINK_TX_FIFO);
+	}
+
+	tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
+
+	return len;
+}
+
+static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = tspi->cur_len;
+	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			rx_buf[i + j] = (val >> (j * 8)) & 0xff;
+	}
+
+	return len;
+}
+
+static void spi_tegra_start_transfer(struct spi_device *spi,
+				    struct spi_transfer *t)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	u32 speed;
+	u8 bits_per_word;
+	unsigned long val;
+
+	speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
+	bits_per_word = t->bits_per_word ? t->bits_per_word  :
+		spi->bits_per_word;
+
+	tspi->cur_bytes_per_word = (bits_per_word - 1) / 8 + 1;
+
+	if (speed != tspi->cur_speed)
+		clk_set_rate(tspi->clk, speed);
+
+	if (tspi->cur_speed == 0)
+		clk_enable(tspi->clk);
+
+	tspi->cur_speed = speed;
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND2);
+	if (t->rx_buf)
+		val |= RXEN;
+	else
+		val &= ~RXEN;
+
+	if (t->tx_buf)
+		val |= TXEN;
+	else
+		val &= ~TXEN;
+
+	val &= ~SS_EN_CS(~0);
+	val |= SS_EN_CS(spi->chip_select);
+	val |= SPIE;
+
+	spi_tegra_writel(tspi, val, SLINK_COMMAND2);
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	val &= ~BIT_LENGTH(~0);
+	val |= BIT_LENGTH(bits_per_word - 1);
+
+	/* FIXME: should probably control CS manually so that we can be sure
+	 * it does not go low between transfer and to support delay_usecs
+	 * correctly.
+	 */
+	val &= ~IDLE_SCLK_MASK & ~CK_SDA & ~CS_SW;
+
+	if (spi->mode & SPI_CPHA)
+		val |= CK_SDA;
+
+	if (spi->mode & SPI_CPOL)
+		val |= IDLE_SCLK_DRIVE_HIGH;
+	else
+		val |= IDLE_SCLK_DRIVE_LOW;
+
+	val |= M_S;
+
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	spi_tegra_writel(tspi, RX_FLUSH | TX_FLUSH, SLINK_STATUS);
+
+	tspi->cur = t;
+	tspi->cur_pos = 0;
+	tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
+
+	spi_tegra_go(tspi);
+}
+
+static void spi_tegra_start_message(struct spi_device *spi,
+				    struct spi_message *m)
+{
+	struct spi_transfer *t;
+
+	m->actual_length = 0;
+
+	t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
+	spi_tegra_start_transfer(spi, t);
+}
+
+static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
+{
+	struct spi_tegra_data *tspi = req->dev;
+	unsigned long flags;
+	struct spi_message *m;
+	struct spi_device *spi;
+	int complete = 0;
+	int timeout = 0;
+	unsigned long val;
+
+	/* the SPI controller may come back with both the BSY and RDY bits
+	 * set.  In this case we need to wait for the BSY bit to clear so
+	 * that we are sure the DMA is finished.  1000 reads was empirically
+	 * determined to be long enough.
+	 */
+	while (timeout++ < 1000) {
+		if (!(spi_tegra_readl(tspi, SLINK_STATUS) & BSY))
+			break;
+	}
+
+	val = spi_tegra_readl(tspi, SLINK_STATUS);
+	val |= RDY;
+	spi_tegra_writel(tspi, val, SLINK_STATUS);
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	m = list_first_entry(&tspi->queue, struct spi_message, queue);
+	spi = m->state;
+
+	tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
+	m->actual_length += tspi->cur_pos;
+
+	if (tspi->cur_pos < tspi->cur->len) {
+		tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
+		spi_tegra_go(tspi);
+	} else if (!list_is_last(&tspi->cur->transfer_list,
+				 &m->transfers)) {
+		tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
+					      struct spi_transfer,
+					      transfer_list);
+		spi_tegra_start_transfer(spi, tspi->cur);
+	} else {
+		complete = 1;
+	}
+
+	if (complete) {
+		list_del(&m->queue);
+
+		m->status = 0;
+		m->complete(m->context);
+
+		if (!list_empty(&tspi->queue)) {
+			m = list_first_entry(&tspi->queue, struct spi_message,
+					     queue);
+			spi = m->state;
+			spi_tegra_start_message(spi, m);
+		} else {
+			clk_disable(tspi->clk);
+			tspi->cur_speed = 0;
+		}
+	}
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+}
+
+static int spi_tegra_setup(struct spi_device *spi)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long cs_bit;
+	unsigned long val;
+	unsigned long flags;
+
+	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);
+
+
+	switch (spi->chip_select) {
+	case 0:
+		cs_bit = CS_POLARITY;
+		break;
+
+	case 1:
+		cs_bit = CS_POLARITY1;
+		break;
+
+	case 2:
+		cs_bit = CS_POLARITY2;
+		break;
+
+	case 4:
+		cs_bit = CS_POLARITY3;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	if (spi->mode & SPI_CS_HIGH)
+		val |= cs_bit;
+	else
+		val &= ~cs_bit;
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	struct spi_transfer *t;
+	unsigned long flags;
+	int was_empty;
+
+	if (list_empty(&m->transfers) || !m->complete)
+		return -EINVAL;
+
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		if (t->bits_per_word < 0 || t->bits_per_word > 32)
+			return -EINVAL;
+
+		if (t->len == 0)
+			return -EINVAL;
+
+		if (!t->rx_buf && !t->tx_buf)
+			return -EINVAL;
+	}
+
+	m->state = spi;
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	was_empty = list_empty(&tspi->queue);
+	list_add_tail(&m->queue, &tspi->queue);
+
+	if (was_empty)
+		spi_tegra_start_message(spi, m);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static void spi_tegra_cleanup(struct spi_device *spi)
+{
+	dev_dbg(&spi->dev, "cleanup\n");
+}
+
+static int __init spi_tegra_probe(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof *tspi);
+	if (master == NULL) {
+		dev_err(&pdev->dev, "master allocation failed\n");
+		return -ENOMEM;
+	}
+
+	/* the spi->mode bits understood by this driver: */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+
+	master->setup = spi_tegra_setup;
+	master->transfer = spi_tegra_transfer;
+	master->cleanup = spi_tegra_cleanup;
+	master->num_chipselect = 4;
+
+	dev_set_drvdata(&pdev->dev, master);
+	tspi = spi_master_get_devdata(master);
+	tspi->master = master;
+	tspi->pdev = pdev;
+	spin_lock_init(&tspi->lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		ret = -ENODEV;
+		goto err0;
+	}
+
+	tspi->phys = r->start;
+	tspi->base = ioremap(r->start, r->end - r->start + 1);
+	if (!tspi->base) {
+		dev_err(&pdev->dev, "can't ioremap iomem\n");
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	tspi->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR_OR_NULL(tspi->clk)) {
+		dev_err(&pdev->dev, "can not get clock\n");
+		ret = PTR_ERR(tspi->clk);
+		goto err2;
+	}
+
+	INIT_LIST_HEAD(&tspi->queue);
+
+	tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
+	if (IS_ERR(tspi->rx_dma)) {
+		dev_err(&pdev->dev, "can not allocate rx dma channel\n");
+		ret = PTR_ERR(tspi->rx_dma);
+		goto err3;
+	}
+
+	tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+					 &tspi->rx_bb_phys, GFP_KERNEL);
+	if (!tspi->rx_bb) {
+		dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
+		ret = -ENOMEM;
+		goto err4;
+	}
+
+	tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
+	tspi->rx_dma_req.to_memory = 1;
+	tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
+	tspi->rx_dma_req.dest_bus_width = 32;
+	tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
+	tspi->rx_dma_req.source_bus_width = 32;
+	tspi->rx_dma_req.source_wrap = 4;
+	tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
+	tspi->rx_dma_req.dev = tspi;
+
+	ret = spi_register_master(master);
+
+	if (ret < 0)
+		goto err5;
+
+	return ret;
+
+err5:
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+err4:
+	tegra_dma_free_channel(tspi->rx_dma);
+err3:
+	clk_put(tspi->clk);
+err2:
+	iounmap(tspi->base);
+err1:
+	release_mem_region(r->start, (r->end - r->start) + 1);
+err0:
+	spi_master_put(master);
+	return ret;
+}
+
+static int __devexit spi_tegra_remove(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+
+	master = dev_get_drvdata(&pdev->dev);
+	tspi = spi_master_get_devdata(master);
+
+	tegra_dma_free_channel(tspi->rx_dma);
+
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+
+	clk_put(tspi->clk);
+	iounmap(tspi->base);
+
+	spi_master_put(master);
+
+	return 0;
+}
+
+MODULE_ALIAS("platform:spi_tegra");
+
+static struct platform_driver spi_tegra_driver = {
+	.driver = {
+		.name =		"spi_tegra",
+		.owner =	THIS_MODULE,
+	},
+	.remove =	__devexit_p(spi_tegra_remove),
+};
+
+static int __init spi_tegra_init(void)
+{
+	return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
+}
+subsys_initcall(spi_tegra_init);
+
+static void __exit spi_tegra_exit(void)
+{
+	platform_driver_unregister(&spi_tegra_driver);
+}
+module_exit(spi_tegra_exit);
+
+MODULE_LICENSE("GPL");
-- 
1.6.5.6


------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

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

* Re: [PATCH] spi: add spi_tegra driver
  2010-08-12  0:31     ` Erik Gilling
@ 2010-08-12 15:24       ` Russell King - ARM Linux
       [not found]         ` <20100812152424.GB31982-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Russell King - ARM Linux @ 2010-08-12 15:24 UTC (permalink / raw)
  To: Erik Gilling
  Cc: Grant Likely, linux-tegra, Colin Cross, linux-arm-kernel,
	spi-devel-general

On Wed, Aug 11, 2010 at 05:31:00PM -0700, Erik Gilling wrote:
> >> +       if (!request_mem_region(r->start, (r->end - r->start) + 1,
> >> +                       dev_name(&pdev->dev))) {
> >> +               ret = -EBUSY;
> >> +               goto err0;
> >> +       }
> >
> > I believe the platform bus does this for you already by calling
> > insert_resource() on all the platform bus ranges.  See if /proc/iomem
> > is any different with or without this call to verify.
> 
> You're right.  Didn't realize this

There is a big difference between what the platform bus does and what
drivers do.

The platform bus adds the resources to the resource tree as they stand
without marking them _busy_.  This means that they can be sub-divided
and other resources registered within their range.

request_mem_region() adds resources to the resource tree marking them
busy, detecting overlapping resources and indicating whether two
drivers are trying to claim the same resource region.  In other words,
it's a preventative measure against two device drivers accessing the
same region.

You're absolutely right to do what you're doing above, please continue
to do so.

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

* Re: [PATCH] spi: add spi_tegra driver
       [not found]         ` <20100812152424.GB31982-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
@ 2010-08-12 18:44           ` Erik Gilling
  0 siblings, 0 replies; 18+ messages in thread
From: Erik Gilling @ 2010-08-12 18:44 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, Colin Cross,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Thu, Aug 12, 2010 at 8:24 AM, Russell King - ARM Linux
<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> On Wed, Aug 11, 2010 at 05:31:00PM -0700, Erik Gilling wrote:
>> >> +       if (!request_mem_region(r->start, (r->end - r->start) + 1,
>> >> +                       dev_name(&pdev->dev))) {
>> >> +               ret = -EBUSY;
>> >> +               goto err0;
>> >> +       }
>> >
>> > I believe the platform bus does this for you already by calling
>> > insert_resource() on all the platform bus ranges.  See if /proc/iomem
>> > is any different with or without this call to verify.
>>
>> You're right.  Didn't realize this
>
> There is a big difference between what the platform bus does and what
> drivers do.
>
> The platform bus adds the resources to the resource tree as they stand
> without marking them _busy_.  This means that they can be sub-divided
> and other resources registered within their range.
>
> request_mem_region() adds resources to the resource tree marking them
> busy, detecting overlapping resources and indicating whether two
> drivers are trying to claim the same resource region.  In other words,
> it's a preventative measure against two device drivers accessing the
> same region.
>
> You're absolutely right to do what you're doing above, please continue
> to do so.
>

Thanks for the explanation. I'll add it back in.

Cheers,
    Erik

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

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

* Re: [PATCH v2] spi: add spi_tegra driver
       [not found]     ` <1281574023-25160-1-git-send-email-konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
@ 2010-08-12 19:54       ` Grant Likely
       [not found]         ` <AANLkTim4nL+Re=qc55CGS+ksq+ACtGe=vqOeGQ3HbQWM-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2010-08-25 21:23       ` [PATCH v3] " Erik Gilling
  1 sibling, 1 reply; 18+ messages in thread
From: Grant Likely @ 2010-08-12 19:54 UTC (permalink / raw)
  To: Erik Gilling
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	ccross-hpIqsD4AKlfQT0dZR+AlfA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Erik,

Thanks for the quick respin.  I've got some more comments below, most
of them minor, but there are some questions about the locking model.
I don't see any problem with me picking this driver up into my
next-spi tree once the .26 merge window closes.

g.

On Wed, Aug 11, 2010 at 6:47 PM, Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> wrote:

Missing patch description.  Give reviewers and future readers some
more details about what hardware this patch adds support for.

> v2 changes:
>  from Thierry Reding:
>    * add "select TEGRA_SYSTEM_DMA" to Kconfig
>  from Grant Likely:
>    * add oneline description to header
>    * inline references to DRIVER_NAME
>    * inline references to BUSY_TIMEOUT
>    * open coded bytes_per_word()
>    * spi_readl/writel -> spi_tegra_readl/writel
>    * move transfer validation to spi_tegra_transfer
>    * don't request_mem_region iomem as platform bus does that for us
>    * __exit -> __devexit
>
> Signed-off-by: Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Cc: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

> ---
>  drivers/spi/Kconfig     |    7 +
>  drivers/spi/Makefile    |    1 +
>  drivers/spi/spi_tegra.c |  638 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 646 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/spi_tegra.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 91c2f4f..9fdb309 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -298,6 +298,13 @@ config SPI_STMP3XXX
>        help
>          SPI driver for Freescale STMP37xx/378x SoC SSP interface
>
> +config SPI_TEGRA
> +       tristate "Nvidia Tegra SPI controller"
> +       depends on ARCH_TEGRA
> +       select TEGRA_SYSTEM_DMA
> +       help
> +         SPI driver for NVidia Tegra SoCs
> +
>  config SPI_TXX9
>        tristate "Toshiba TXx9 SPI controller"
>        depends on GENERIC_GPIO && CPU_TX49XX
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index e9cbd18..b6573d8 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)              += spi_ppc4xx.o
>  obj-$(CONFIG_SPI_S3C24XX_GPIO)         += spi_s3c24xx_gpio.o
>  obj-$(CONFIG_SPI_S3C24XX)              += spi_s3c24xx_hw.o
>  obj-$(CONFIG_SPI_S3C64XX)              += spi_s3c64xx.o
> +obj-$(CONFIG_SPI_TEGRA)                        += spi_tegra.o
>  obj-$(CONFIG_SPI_TXX9)                 += spi_txx9.o
>  obj-$(CONFIG_SPI_XILINX)               += xilinx_spi.o
>  obj-$(CONFIG_SPI_XILINX_OF)            += xilinx_spi_of.o
> diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
> new file mode 100644
> index 0000000..5afb7a7
> --- /dev/null
> +++ b/drivers/spi/spi_tegra.c
> @@ -0,0 +1,638 @@
> +/*
> + * Driver for Nvidia TEGRA spi controller.
> + *
> + * Copyright (C) 2010 Google, Inc.
> + *
> + * Author:
> + *     Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + *
> + */
> +
> +#undef DEBUG

Still need to trim this #undef

> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +
> +#include <linux/spi/spi.h>
> +
> +#include <mach/dma.h>
> +
> +#define SLINK_COMMAND          0x000
> +#define   BIT_LENGTH(x)                        (((x) & 0x1f) << 0)
> +#define   WORD_SIZE(x)                 (((x) & 0x1f) << 5)
> +#define   BOTH_EN                      (1 << 10)
> +#define   CS_SW                                (1 << 11)
> +#define   CS_VALUE                     (1 << 12)
> +#define   CS_POLARITY                  (1 << 13)
> +#define   IDLE_SDA_DRIVE_LOW           (0 << 16)
> +#define   IDLE_SDA_DRIVE_HIGH          (1 << 16)
> +#define   IDLE_SDA_PULL_LOW            (2 << 16)
> +#define   IDLE_SDA_PULL_HIGH           (3 << 16)
> +#define   IDLE_SDA_MASK                        (3 << 16)
> +#define   CS_POLARITY1                 (1 << 20)
> +#define   CK_SDA                       (1 << 21)
> +#define   CS_POLARITY2                 (1 << 22)
> +#define   CS_POLARITY3                 (1 << 23)
> +#define   IDLE_SCLK_DRIVE_LOW          (0 << 24)
> +#define   IDLE_SCLK_DRIVE_HIGH         (1 << 24)
> +#define   IDLE_SCLK_PULL_LOW           (2 << 24)
> +#define   IDLE_SCLK_PULL_HIGH          (3 << 24)
> +#define   IDLE_SCLK_MASK               (3 << 24)
> +#define   M_S                          (1 << 28)
> +#define   WAIT                         (1 << 29)
> +#define   GO                           (1 << 30)
> +#define   ENB                          (1 << 31)

It's a good idea to prefix all these register #defines with something
driver specific to avoid the possibility of collisions with the global
namespace.

> +
> +#define SLINK_COMMAND2         0x004
> +#define   LSBFE                                (1 << 0)
> +#define   SSOE                         (1 << 1)
> +#define   SPIE                         (1 << 4)
> +#define   BIDIROE                      (1 << 6)
> +#define   MODFEN                       (1 << 7)
> +#define   INT_SIZE(x)                  (((x) & 0x1f) << 8)
> +#define   CS_ACTIVE_BETWEEN            (1 << 17)
> +#define   SS_EN_CS(x)                  (((x) & 0x3) << 18)
> +#define   SS_SETUP(x)                  (((x) & 0x3) << 20)
> +#define   FIFO_REFILLS_0               (0 << 22)
> +#define   FIFO_REFILLS_1               (1 << 22)
> +#define   FIFO_REFILLS_2               (2 << 22)
> +#define   FIFO_REFILLS_3               (3 << 22)
> +#define   FIFO_REFILLS_MASK            (3 << 22)
> +#define   WAIT_PACK_INT(x)             (((x) & 0x7) << 26)
> +#define   SPC0                         (1 << 29)
> +#define   TXEN                         (1 << 30)
> +#define   RXEN                         (1 << 31)
> +
> +#define SLINK_STATUS           0x008
> +#define   COUNT(val)                   (((val) >> 0) & 0x1f)
> +#define   WORD(val)                    (((val) >> 5) & 0x1f)
> +#define   BLK_CNT(val)                 (((val) >> 0) & 0xffff)
> +#define   MODF                         (1 << 16)
> +#define   RX_UNF                       (1 << 18)
> +#define   TX_OVF                       (1 << 19)
> +#define   TX_FULL                      (1 << 20)
> +#define   TX_EMPTY                     (1 << 21)
> +#define   RX_FULL                      (1 << 22)
> +#define   RX_EMPTY                     (1 << 23)
> +#define   TX_UNF                       (1 << 24)
> +#define   RX_OVF                       (1 << 25)
> +#define   TX_FLUSH                     (1 << 26)
> +#define   RX_FLUSH                     (1 << 27)
> +#define   SCLK                         (1 << 28)
> +#define   ERR                          (1 << 29)
> +#define   RDY                          (1 << 30)
> +#define   BSY                          (1 << 31)
> +
> +#define SLINK_MAS_DATA         0x010
> +#define SLINK_SLAVE_DATA       0x014
> +
> +#define SLINK_DMA_CTL          0x018
> +#define   DMA_BLOCK_SIZE(x)            (((x) & 0xffff) << 0)
> +#define   TX_TRIG_1                    (0 << 16)
> +#define   TX_TRIG_4                    (1 << 16)
> +#define   TX_TRIG_8                    (2 << 16)
> +#define   TX_TRIG_16                   (3 << 16)
> +#define   TX_TRIG_MASK                 (3 << 16)
> +#define   RX_TRIG_1                    (0 << 18)
> +#define   RX_TRIG_4                    (1 << 18)
> +#define   RX_TRIG_8                    (2 << 18)
> +#define   RX_TRIG_16                   (3 << 18)
> +#define   RX_TRIG_MASK                 (3 << 18)
> +#define   PACKED                       (1 << 20)
> +#define   PACK_SIZE_4                  (0 << 21)
> +#define   PACK_SIZE_8                  (1 << 21)
> +#define   PACK_SIZE_16                 (2 << 21)
> +#define   PACK_SIZE_32                 (3 << 21)
> +#define   PACK_SIZE_MASK               (3 << 21)
> +#define   IE_TXC                       (1 << 26)
> +#define   IE_RXC                       (1 << 27)
> +#define   DMA_EN                       (1 << 31)
> +
> +#define SLINK_STATUS2          0x01c
> +#define   TX_FIFO_EMPTY_COUNT(val)     (((val) & 0x3f) >> 0)
> +#define   RX_FIFO_FULL_COUNT(val)      (((val) & 0x3f) >> 16)
> +
> +#define SLINK_TX_FIFO          0x100
> +#define SLINK_RX_FIFO          0x180
> +
> +static const unsigned long spi_tegra_req_sels[] = {
> +       TEGRA_DMA_REQ_SEL_SL2B1,
> +       TEGRA_DMA_REQ_SEL_SL2B2,
> +       TEGRA_DMA_REQ_SEL_SL2B3,
> +       TEGRA_DMA_REQ_SEL_SL2B4,
> +};
> +
> +static inline unsigned bytes_per_word(u8 bits)
> +{
> +       WARN_ON((bits < 1) || bits > 32);
> +       if (bits <= 8)
> +               return 1;
> +       else if (bits <= 16)
> +               return 2;
> +       else if (bits <= 24)
> +               return 3;
> +       else
> +               return 4;
> +}

No longer used, can be removed.

> +
> +#define BB_LEN                 32
> +
> +struct spi_tegra_data {
> +       struct spi_master       *master;
> +       struct platform_device  *pdev;
> +       spinlock_t              lock;
> +
> +       struct clk              *clk;
> +       void __iomem            *base;
> +       unsigned long           phys;
> +
> +       u32                     cur_speed;
> +
> +       struct list_head        queue;
> +       struct spi_transfer     *cur;
> +       unsigned                cur_pos;
> +       unsigned                cur_len;
> +       unsigned                cur_bytes_per_word;
> +
> +       /* The tegra spi controller has a bug which causes the first word
> +        * in PIO transactions to be garbage.  Since packed DMA transactions
> +        * require transfers to be 4 byte aligned we need a bounce buffer
> +        * for the generic case.
> +        */
> +       struct tegra_dma_req    rx_dma_req;
> +       struct tegra_dma_channel *rx_dma;
> +       u32                     *rx_bb;
> +       dma_addr_t              rx_bb_phys;
> +};
> +
> +
> +static unsigned long spi_tegra_readl(struct spi_tegra_data *tspi,
> +                                    unsigned long reg)
> +{
> +       return readl(tspi->base + reg);
> +}

static inline

> +
> +static void spi_tegra_writel(struct spi_tegra_data *tspi, unsigned long val,
> +                      unsigned long reg)
> +{
> +       writel(val, tspi->base + reg);
> +}

ditto

> +
> +static void spi_tegra_go(struct spi_tegra_data *tspi)
> +{
> +       unsigned long val;
> +
> +       wmb();
> +
> +       val = spi_tegra_readl(tspi, SLINK_DMA_CTL);
> +       val &= ~DMA_BLOCK_SIZE(~0) & ~DMA_EN;
> +       val |= DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
> +       spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
> +
> +       tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
> +
> +       val |= DMA_EN;
> +       spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
> +}
> +
> +static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
> +                                 struct spi_transfer *t)
> +{
> +       unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
> +                          tspi->cur_bytes_per_word);
> +       u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
> +       int i, j;
> +       unsigned long val;
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND);
> +       val &= ~WORD_SIZE(~0);
> +       val |= WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);
> +
> +       for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
> +               val = 0;
> +               for (j = 0; j < tspi->cur_bytes_per_word; j++)
> +                       val |= tx_buf[i + j] << j * 8;
> +
> +               spi_tegra_writel(tspi, val, SLINK_TX_FIFO);
> +       }
> +
> +       tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
> +
> +       return len;
> +}
> +
> +static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
> +                                 struct spi_transfer *t)
> +{
> +       unsigned len = tspi->cur_len;
> +       u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
> +       int i, j;
> +       unsigned long val;
> +
> +       for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
> +               val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
> +               for (j = 0; j < tspi->cur_bytes_per_word; j++)
> +                       rx_buf[i + j] = (val >> (j * 8)) & 0xff;
> +       }
> +
> +       return len;
> +}
> +
> +static void spi_tegra_start_transfer(struct spi_device *spi,
> +                                   struct spi_transfer *t)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       u32 speed;
> +       u8 bits_per_word;
> +       unsigned long val;
> +
> +       speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
> +       bits_per_word = t->bits_per_word ? t->bits_per_word  :
> +               spi->bits_per_word;
> +
> +       tspi->cur_bytes_per_word = (bits_per_word - 1) / 8 + 1;
> +
> +       if (speed != tspi->cur_speed)
> +               clk_set_rate(tspi->clk, speed);
> +
> +       if (tspi->cur_speed == 0)
> +               clk_enable(tspi->clk);
> +
> +       tspi->cur_speed = speed;
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND2);
> +       if (t->rx_buf)
> +               val |= RXEN;
> +       else
> +               val &= ~RXEN;
> +
> +       if (t->tx_buf)
> +               val |= TXEN;
> +       else
> +               val &= ~TXEN;
> +
> +       val &= ~SS_EN_CS(~0);
> +       val |= SS_EN_CS(spi->chip_select);
> +       val |= SPIE;

Nit: This would be more concise:

+       val &= ~(SS_EN_CS(~0) | RXEN | TXEN);
+       if (t->rx_buf)
+               val |= RXEN;
+       if (t->tx_buf)
+               val |= TXEN;
+       val |= SS_EN_CS(spi->chip_select);
+       val |= SPIE;


> +
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND2);
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND);
> +       val &= ~BIT_LENGTH(~0);
> +       val |= BIT_LENGTH(bits_per_word - 1);
> +
> +       /* FIXME: should probably control CS manually so that we can be sure
> +        * it does not go low between transfer and to support delay_usecs
> +        * correctly.
> +        */
> +       val &= ~IDLE_SCLK_MASK & ~CK_SDA & ~CS_SW;
> +
> +       if (spi->mode & SPI_CPHA)
> +               val |= CK_SDA;
> +
> +       if (spi->mode & SPI_CPOL)
> +               val |= IDLE_SCLK_DRIVE_HIGH;
> +       else
> +               val |= IDLE_SCLK_DRIVE_LOW;

nit: IDLE_SCLK_DRIVE_LOW == 0.  The driver will be more concise if the
above 2 lines are dropped.
> +
> +       val |= M_S;
> +
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);

The read/modify/write register blocks are quite common.  Would it be
clearer and more concise to have a spi_tegra_clrsetbits() static
inline?

> +
> +       spi_tegra_writel(tspi, RX_FLUSH | TX_FLUSH, SLINK_STATUS);
> +
> +       tspi->cur = t;
> +       tspi->cur_pos = 0;
> +       tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
> +
> +       spi_tegra_go(tspi);
> +}
> +
> +static void spi_tegra_start_message(struct spi_device *spi,
> +                                   struct spi_message *m)
> +{
> +       struct spi_transfer *t;
> +
> +       m->actual_length = 0;
> +
> +       t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
> +       spi_tegra_start_transfer(spi, t);
> +}
> +
> +static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
> +{
> +       struct spi_tegra_data *tspi = req->dev;
> +       unsigned long flags;
> +       struct spi_message *m;
> +       struct spi_device *spi;
> +       int complete = 0;
> +       int timeout = 0;
> +       unsigned long val;
> +
> +       /* the SPI controller may come back with both the BSY and RDY bits
> +        * set.  In this case we need to wait for the BSY bit to clear so
> +        * that we are sure the DMA is finished.  1000 reads was empirically
> +        * determined to be long enough.
> +        */
> +       while (timeout++ < 1000) {
> +               if (!(spi_tegra_readl(tspi, SLINK_STATUS) & BSY))
> +                       break;
> +       }

What happens if the BSY bit is still set when the loop exits?

> +
> +       val = spi_tegra_readl(tspi, SLINK_STATUS);
> +       val |= RDY;
> +       spi_tegra_writel(tspi, val, SLINK_STATUS);

The spin lock isn't held at this point.  Will that make this
read-modify-write operation risky?

> +
> +       spin_lock_irqsave(&tspi->lock, flags);

Is there any condition/event which could cause the spi controller to
go busy before obtaining the spinlock?

> +
> +       m = list_first_entry(&tspi->queue, struct spi_message, queue);
> +       spi = m->state;
> +
> +       tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
> +       m->actual_length += tspi->cur_pos;
> +
> +       if (tspi->cur_pos < tspi->cur->len) {
> +               tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
> +               spi_tegra_go(tspi);
> +       } else if (!list_is_last(&tspi->cur->transfer_list,
> +                                &m->transfers)) {
> +               tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
> +                                             struct spi_transfer,
> +                                             transfer_list);
> +               spi_tegra_start_transfer(spi, tspi->cur);
> +       } else {
> +               complete = 1;
> +       }
> +
> +       if (complete) {

Looks like the above else clause is the only thing that can set
complete.  You could delete the above three lines to get the same
behaviour.

> +               list_del(&m->queue);
> +
> +               m->status = 0;
> +               m->complete(m->context);
> +
> +               if (!list_empty(&tspi->queue)) {
> +                       m = list_first_entry(&tspi->queue, struct spi_message,
> +                                            queue);
> +                       spi = m->state;
> +                       spi_tegra_start_message(spi, m);
> +               } else {
> +                       clk_disable(tspi->clk);
> +                       tspi->cur_speed = 0;
> +               }
> +       }
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +}
> +
> +static int spi_tegra_setup(struct spi_device *spi)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       unsigned long cs_bit;
> +       unsigned long val;
> +       unsigned long flags;
> +
> +       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);
> +
> +
> +       switch (spi->chip_select) {
> +       case 0:
> +               cs_bit = CS_POLARITY;
> +               break;
> +
> +       case 1:
> +               cs_bit = CS_POLARITY1;
> +               break;
> +
> +       case 2:
> +               cs_bit = CS_POLARITY2;
> +               break;
> +
> +       case 4:
> +               cs_bit = CS_POLARITY3;
> +               break;

CS_POLARITY is named a little oddly as it seems to indicate the bit,
not the polarity of the bit.  Would this better be named
CS_BIT0..CS_BIT3?

> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND);
> +       if (spi->mode & SPI_CS_HIGH)
> +               val |= cs_bit;
> +       else
> +               val &= ~cs_bit;
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       struct spi_transfer *t;
> +       unsigned long flags;
> +       int was_empty;
> +
> +       if (list_empty(&m->transfers) || !m->complete)
> +               return -EINVAL;
> +
> +       list_for_each_entry(t, &m->transfers, transfer_list) {
> +               if (t->bits_per_word < 0 || t->bits_per_word > 32)
> +                       return -EINVAL;
> +
> +               if (t->len == 0)
> +                       return -EINVAL;
> +
> +               if (!t->rx_buf && !t->tx_buf)
> +                       return -EINVAL;
> +       }
> +
> +       m->state = spi;
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +       was_empty = list_empty(&tspi->queue);
> +       list_add_tail(&m->queue, &tspi->queue);
> +
> +       if (was_empty)
> +               spi_tegra_start_message(spi, m);
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void spi_tegra_cleanup(struct spi_device *spi)
> +{
> +       dev_dbg(&spi->dev, "cleanup\n");
> +}
> +
> +static int __init spi_tegra_probe(struct platform_device *pdev)
> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +       struct resource         *r;
> +       int ret;
> +
> +       master = spi_alloc_master(&pdev->dev, sizeof *tspi);
> +       if (master == NULL) {
> +               dev_err(&pdev->dev, "master allocation failed\n");
> +               return -ENOMEM;
> +       }
> +
> +       /* the spi->mode bits understood by this driver: */
> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> +
> +       if (pdev->id != -1)
> +               master->bus_num = pdev->id;
> +
> +       master->setup = spi_tegra_setup;
> +       master->transfer = spi_tegra_transfer;
> +       master->cleanup = spi_tegra_cleanup;
> +       master->num_chipselect = 4;
> +
> +       dev_set_drvdata(&pdev->dev, master);
> +       tspi = spi_master_get_devdata(master);
> +       tspi->master = master;
> +       tspi->pdev = pdev;
> +       spin_lock_init(&tspi->lock);
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (r == NULL) {
> +               ret = -ENODEV;
> +               goto err0;
> +       }
> +
> +       tspi->phys = r->start;
> +       tspi->base = ioremap(r->start, r->end - r->start + 1);
> +       if (!tspi->base) {
> +               dev_err(&pdev->dev, "can't ioremap iomem\n");
> +               ret = -ENOMEM;
> +               goto err1;
> +       }
> +
> +       tspi->clk = clk_get(&pdev->dev, NULL);
> +       if (IS_ERR_OR_NULL(tspi->clk)) {
> +               dev_err(&pdev->dev, "can not get clock\n");
> +               ret = PTR_ERR(tspi->clk);
> +               goto err2;
> +       }
> +
> +       INIT_LIST_HEAD(&tspi->queue);
> +
> +       tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
> +       if (IS_ERR(tspi->rx_dma)) {

As an aside, and commenting on the tegra DMA api... Is the IS_ERR()
pattern in the tegra_dma_allocate_channel() return really a good idea
here?  Personally I find that pattern isn't very useful, especially
when the results is either "yes, allocated" or "no, not-alocated", and
it is very easy to write code that tests the wrong thing on the return
value because the compiler cannot catch it.  In other words, if it
isn't vital to return a specific error code, the using the PTR_ERR()
pattern adds unnecessary risk to the code because developers are used
to seeing and writing this pattern:

tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
if (!tspi->rx_dma)
	return -ENOMEM;

The compiler won't catch it if the return value is an PTR_ERR.

(I know that some subsystems make heavy use of it, particularly
filesystems and the block layer which do need to return specific error
codes to userspace.  My argument is that the specific error code is
probably irrelevant when allocating DMA channels)

> +               dev_err(&pdev->dev, "can not allocate rx dma channel\n");
> +               ret = PTR_ERR(tspi->rx_dma);
> +               goto err3;
> +       }
> +
> +       tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                                        &tspi->rx_bb_phys, GFP_KERNEL);
> +       if (!tspi->rx_bb) {
> +               dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
> +               ret = -ENOMEM;
> +               goto err4;
> +       }
> +
> +       tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
> +       tspi->rx_dma_req.to_memory = 1;
> +       tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
> +       tspi->rx_dma_req.dest_bus_width = 32;
> +       tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
> +       tspi->rx_dma_req.source_bus_width = 32;
> +       tspi->rx_dma_req.source_wrap = 4;
> +       tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
> +       tspi->rx_dma_req.dev = tspi;
> +
> +       ret = spi_register_master(master);
> +
> +       if (ret < 0)
> +               goto err5;
> +
> +       return ret;
> +
> +err5:
> +       dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                         tspi->rx_bb, tspi->rx_bb_phys);
> +err4:
> +       tegra_dma_free_channel(tspi->rx_dma);
> +err3:
> +       clk_put(tspi->clk);
> +err2:
> +       iounmap(tspi->base);
> +err1:
> +       release_mem_region(r->start, (r->end - r->start) + 1);

Since this version doesn't request_mem_region(), this release is
dangling.  (However, as rmk pointed out the request_mem_region() was
completely fine and the driver should continue to use it.  Sorry for
the noise).

> +err0:
> +       spi_master_put(master);
> +       return ret;
> +}
> +
> +static int __devexit spi_tegra_remove(struct platform_device *pdev)
> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +
> +       master = dev_get_drvdata(&pdev->dev);
> +       tspi = spi_master_get_devdata(master);
> +
> +       tegra_dma_free_channel(tspi->rx_dma);
> +
> +       dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                         tspi->rx_bb, tspi->rx_bb_phys);
> +
> +       clk_put(tspi->clk);
> +       iounmap(tspi->base);
> +
> +       spi_master_put(master);
> +
> +       return 0;
> +}
> +
> +MODULE_ALIAS("platform:spi_tegra");
> +
> +static struct platform_driver spi_tegra_driver = {
> +       .driver = {
> +               .name =         "spi_tegra",
> +               .owner =        THIS_MODULE,
> +       },
> +       .remove =       __devexit_p(spi_tegra_remove),
> +};
> +
> +static int __init spi_tegra_init(void)
> +{
> +       return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
> +}
> +subsys_initcall(spi_tegra_init);

Unless there is a specific reason that this driver needs to be
initialized earlier, it should use the module_init() call.  In which
case the reason it is initialized early should be documented with a
comment.  (for reference: 4 of the mainlined spi bus drivers use
subsys_initcall() whereas 30 of them use module_init()..  1 uses
device_initcall() which is the same as module_init.  None of the ones
using subsys_initcall() give a reason why.)

g.

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 

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

* Re: [PATCH v2] spi: add spi_tegra driver
       [not found]         ` <AANLkTim4nL+Re=qc55CGS+ksq+ACtGe=vqOeGQ3HbQWM-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-08-25 21:22           ` Erik Gilling
       [not found]             ` <AANLkTincgapUK5ncs_t3ax=1Kt_56GdR_RVwRqdCm5ge-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Erik Gilling @ 2010-08-25 21:22 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	ccross-hpIqsD4AKlfQT0dZR+AlfA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Aug 12, 2010 at 12:54 PM, Grant Likely
<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> Missing patch description.  Give reviewers and future readers some
> more details about what hardware this patch adds support for.

I'll add one

>> +#undef DEBUG
>
> Still need to trim this #undef

Done.

> It's a good idea to prefix all these register #defines with something
> driver specific to avoid the possibility of collisions with the global
> namespace.

I'll add SLINK_ to all the bit definitions.

>> +static inline unsigned bytes_per_word(u8 bits)
>> +{
>> +       WARN_ON((bits < 1) || bits > 32);
>> +       if (bits <= 8)
>> +               return 1;
>> +       else if (bits <= 16)
>> +               return 2;
>> +       else if (bits <= 24)
>> +               return 3;
>> +       else
>> +               return 4;
>> +}
>
> No longer used, can be removed.

done

>> +static unsigned long spi_tegra_readl(struct spi_tegra_data *tspi,
>> +                                    unsigned long reg)
>> +{
>> +       return readl(tspi->base + reg);
>> +}
>
> static inline

done

>> +
>> +static void spi_tegra_writel(struct spi_tegra_data *tspi, unsigned long val,
>> +                      unsigned long reg)
>> +{
>> +       writel(val, tspi->base + reg);
>> +}
>
> ditto

done

>> +       val = spi_tegra_readl(tspi, SLINK_COMMAND2);
>> +       if (t->rx_buf)
>> +               val |= RXEN;
>> +       else
>> +               val &= ~RXEN;
>> +
>> +       if (t->tx_buf)
>> +               val |= TXEN;
>> +       else
>> +               val &= ~TXEN;
>> +
>> +       val &= ~SS_EN_CS(~0);
>> +       val |= SS_EN_CS(spi->chip_select);
>> +       val |= SPIE;
>
> Nit: This would be more concise:
>
> +       val &= ~(SS_EN_CS(~0) | RXEN | TXEN);
> +       if (t->rx_buf)
> +               val |= RXEN;
> +       if (t->tx_buf)
> +               val |= TXEN;
> +       val |= SS_EN_CS(spi->chip_select);
> +       val |= SPIE;

done

>> +       if (spi->mode & SPI_CPOL)
>> +               val |= IDLE_SCLK_DRIVE_HIGH;
>> +       else
>> +               val |= IDLE_SCLK_DRIVE_LOW;
>
> nit: IDLE_SCLK_DRIVE_LOW == 0.  The driver will be more concise if the
> above 2 lines are dropped.

I did it this way because it's not obvious from this code which sets
the bit and which does not without decoding the #defines.  I expect
the complier optimizes this out.

>> +
>> +       val |= M_S;
>> +
>> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);
>
> The read/modify/write register blocks are quite common.  Would it be
> clearer and more concise to have a spi_tegra_clrsetbits() static
> inline?

More concise, yes.  Clearer?  I don't think so.  It seems that
clrsetbits is mostly used in ppc code.

>> +       /* the SPI controller may come back with both the BSY and RDY bits
>> +        * set.  In this case we need to wait for the BSY bit to clear so
>> +        * that we are sure the DMA is finished.  1000 reads was empirically
>> +        * determined to be long enough.
>> +        */
>> +       while (timeout++ < 1000) {
>> +               if (!(spi_tegra_readl(tspi, SLINK_STATUS) & BSY))
>> +                       break;
>> +       }
>
> What happens if the BSY bit is still set when the loop exits?

Given how I've seen the hardware act, I don't think it's possible for
BSY to still be set after 1000 reads.  I forget the average number of
loops but 1000 was very conservative.  Worst case some of the data
might be corrupt.  I'll put an error message in and set status to
-EIO.

>> +
>> +       val = spi_tegra_readl(tspi, SLINK_STATUS);
>> +       val |= RDY;
>> +       spi_tegra_writel(tspi, val, SLINK_STATUS);
>
> The spin lock isn't held at this point.  Will that make this
> read-modify-write operation risky?

move spinlock up.

>> +
>> +       spin_lock_irqsave(&tspi->lock, flags);
>
> Is there any condition/event which could cause the spi controller to
> go busy before obtaining the spinlock?

No.  The controller only goes busy when a transaction is started by
calling spi_tegra_go().

>> +
>> +       m = list_first_entry(&tspi->queue, struct spi_message, queue);
>> +       spi = m->state;
>> +
>> +       tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
>> +       m->actual_length += tspi->cur_pos;
>> +
>> +       if (tspi->cur_pos < tspi->cur->len) {
>> +               tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
>> +               spi_tegra_go(tspi);
>> +       } else if (!list_is_last(&tspi->cur->transfer_list,
>> +                                &m->transfers)) {
>> +               tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
>> +                                             struct spi_transfer,
>> +                                             transfer_list);
>> +               spi_tegra_start_transfer(spi, tspi->cur);
>> +       } else {
>> +               complete = 1;
>> +       }
>> +
>> +       if (complete) {
>
> Looks like the above else clause is the only thing that can set
> complete.  You could delete the above three lines to get the same
> behaviour.

deleted

>> +               list_del(&m->queue);
>> +
>> +               m->status = 0;
>> +               m->complete(m->context);


>> +       switch (spi->chip_select) {
>> +       case 0:
>> +               cs_bit = CS_POLARITY;
>> +               break;
>> +
>> +       case 1:
>> +               cs_bit = CS_POLARITY1;
>> +               break;
>> +
>> +       case 2:
>> +               cs_bit = CS_POLARITY2;
>> +               break;
>> +
>> +       case 4:
>> +               cs_bit = CS_POLARITY3;
>> +               break;
>
> CS_POLARITY is named a little oddly as it seems to indicate the bit,
> not the polarity of the bit.  Would this better be named
> CS_BIT0..CS_BIT3?

That's how Nvidia named them in the TRM.


>> +       tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
>> +       if (IS_ERR(tspi->rx_dma)) {
>
> As an aside, and commenting on the tegra DMA api... Is the IS_ERR()
> pattern in the tegra_dma_allocate_channel() return really a good idea
> here?  Personally I find that pattern isn't very useful, especially
> when the results is either "yes, allocated" or "no, not-alocated", and
> it is very easy to write code that tests the wrong thing on the return
> value because the compiler cannot catch it.  In other words, if it
> isn't vital to return a specific error code, the using the PTR_ERR()
> pattern adds unnecessary risk to the code because developers are used
> to seeing and writing this pattern:
>
> tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
> if (!tspi->rx_dma)
>        return -ENOMEM;
>
> The compiler won't catch it if the return value is an PTR_ERR.
>
> (I know that some subsystems make heavy use of it, particularly
> filesystems and the block layer which do need to return specific error
> codes to userspace.  My argument is that the specific error code is
> probably irrelevant when allocating DMA channels)

I don't have a strong opinion either way but that's the way Colin's
code is written.  I'll let him speak to that.

>> +static int __init spi_tegra_init(void)
>> +{
>> +       return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
>> +}
>> +subsys_initcall(spi_tegra_init);
>
> Unless there is a specific reason that this driver needs to be
> initialized earlier, it should use the module_init() call.  In which
> case the reason it is initialized early should be documented with a
> comment.  (for reference: 4 of the mainlined spi bus drivers use
> subsys_initcall() whereas 30 of them use module_init()..  1 uses
> device_initcall() which is the same as module_init.  None of the ones
> using subsys_initcall() give a reason why.)

heh.. I missed the module_inits because i grepped for initcall.
changed to module_init.

patch incoming.

-Erik

------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d

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

* [PATCH v3] spi: add spi_tegra driver
       [not found]     ` <1281574023-25160-1-git-send-email-konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
  2010-08-12 19:54       ` Grant Likely
@ 2010-08-25 21:23       ` Erik Gilling
  2010-09-01 22:16         ` [PATCH v4] " Erik Gilling
  1 sibling, 1 reply; 18+ messages in thread
From: Erik Gilling @ 2010-08-25 21:23 UTC (permalink / raw)
  To: linux-tegra-u79uwXL29TY76Z2rM5mHXA
  Cc: Russell King, Erik Gilling,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

v2 changes:
  from Thierry Reding:
    * add "select TEGRA_SYSTEM_DMA" to Kconfig
  from Grant Likely:
    * add oneline description to header
    * inline references to DRIVER_NAME
    * inline references to BUSY_TIMEOUT
    * open coded bytes_per_word()
    * spi_readl/writel -> spi_tegra_readl/writel
    * move transfer validation to spi_tegra_transfer
    * don't request_mem_region iomem as platform bus does that for us
    * __exit -> __devexit

v3 changes:
  from Russell King:
    * put request_mem_region back int
  from Grant Likely:
    * remove #undef DEBUG
    * add SLINK_ to register bit defines
    * remove unused bytes_per_word
    * make spi_tegra_readl/writel static linine
    * various refactoring for clarity
    * mark err if BSY bit is not cleared after 1000 retries
    * move spinlock to protect setting of RDY bit
    * subsys_initcall -> module_init

Signed-off-by: Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Cc: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/spi/Kconfig     |    7 +
 drivers/spi/Makefile    |    1 +
 drivers/spi/spi_tegra.c |  625 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 633 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/spi_tegra.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 91c2f4f..9fdb309 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -298,6 +298,13 @@ config SPI_STMP3XXX
 	help
 	  SPI driver for Freescale STMP37xx/378x SoC SSP interface
 
+config SPI_TEGRA
+	tristate "Nvidia Tegra SPI controller"
+	depends on ARCH_TEGRA
+	select TEGRA_SYSTEM_DMA
+	help
+	  SPI driver for NVidia Tegra SoCs
+
 config SPI_TXX9
 	tristate "Toshiba TXx9 SPI controller"
 	depends on GENERIC_GPIO && CPU_TX49XX
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index e9cbd18..b6573d8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)		+= spi_ppc4xx.o
 obj-$(CONFIG_SPI_S3C24XX_GPIO)		+= spi_s3c24xx_gpio.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi_s3c24xx_hw.o
 obj-$(CONFIG_SPI_S3C64XX)		+= spi_s3c64xx.o
+obj-$(CONFIG_SPI_TEGRA)			+= spi_tegra.o
 obj-$(CONFIG_SPI_TXX9)			+= spi_txx9.o
 obj-$(CONFIG_SPI_XILINX)		+= xilinx_spi.o
 obj-$(CONFIG_SPI_XILINX_OF)		+= xilinx_spi_of.o
diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
new file mode 100644
index 0000000..cd9f19b
--- /dev/null
+++ b/drivers/spi/spi_tegra.c
@@ -0,0 +1,625 @@
+/*
+ * Driver for Nvidia TEGRA spi controller.
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * Author:
+ *     Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+
+#include <linux/spi/spi.h>
+
+#include <mach/dma.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_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_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) & 0x3f) >> 16)
+
+#define SLINK_TX_FIFO		0x100
+#define SLINK_RX_FIFO		0x180
+
+static const unsigned long spi_tegra_req_sels[] = {
+	TEGRA_DMA_REQ_SEL_SL2B1,
+	TEGRA_DMA_REQ_SEL_SL2B2,
+	TEGRA_DMA_REQ_SEL_SL2B3,
+	TEGRA_DMA_REQ_SEL_SL2B4,
+};
+
+#define BB_LEN			32
+
+struct spi_tegra_data {
+	struct spi_master	*master;
+	struct platform_device	*pdev;
+	spinlock_t		lock;
+
+	struct clk		*clk;
+	void __iomem		*base;
+	unsigned long		phys;
+
+	u32			cur_speed;
+
+	struct list_head	queue;
+	struct spi_transfer	*cur;
+	unsigned		cur_pos;
+	unsigned		cur_len;
+	unsigned		cur_bytes_per_word;
+
+	/* The tegra spi controller has a bug which causes the first word
+	 * in PIO transactions to be garbage.  Since packed DMA transactions
+	 * require transfers to be 4 byte aligned we need a bounce buffer
+	 * for the generic case.
+	 */
+	struct tegra_dma_req	rx_dma_req;
+	struct tegra_dma_channel *rx_dma;
+	u32			*rx_bb;
+	dma_addr_t		rx_bb_phys;
+};
+
+
+static inline unsigned long spi_tegra_readl(struct spi_tegra_data *tspi,
+					    unsigned long reg)
+{
+	return readl(tspi->base + reg);
+}
+
+static inline void spi_tegra_writel(struct spi_tegra_data *tspi,
+				    unsigned long val,
+				    unsigned long reg)
+{
+	writel(val, tspi->base + reg);
+}
+
+static void spi_tegra_go(struct spi_tegra_data *tspi)
+{
+	unsigned long val;
+
+	wmb();
+
+	val = spi_tegra_readl(tspi, SLINK_DMA_CTL);
+	val &= ~SLINK_DMA_BLOCK_SIZE(~0) & ~SLINK_DMA_EN;
+	val |= SLINK_DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
+	spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
+
+	tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
+
+	val |= SLINK_DMA_EN;
+	spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
+}
+
+static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
+			   tspi->cur_bytes_per_word);
+	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	val &= ~SLINK_WORD_SIZE(~0);
+	val |= SLINK_WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = 0;
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			val |= tx_buf[i + j] << j * 8;
+
+		spi_tegra_writel(tspi, val, SLINK_TX_FIFO);
+	}
+
+	tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
+
+	return len;
+}
+
+static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = tspi->cur_len;
+	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			rx_buf[i + j] = (val >> (j * 8)) & 0xff;
+	}
+
+	return len;
+}
+
+static void spi_tegra_start_transfer(struct spi_device *spi,
+				    struct spi_transfer *t)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	u32 speed;
+	u8 bits_per_word;
+	unsigned long val;
+
+	speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
+	bits_per_word = t->bits_per_word ? t->bits_per_word  :
+		spi->bits_per_word;
+
+	tspi->cur_bytes_per_word = (bits_per_word - 1) / 8 + 1;
+
+	if (speed != tspi->cur_speed)
+		clk_set_rate(tspi->clk, speed);
+
+	if (tspi->cur_speed == 0)
+		clk_enable(tspi->clk);
+
+	tspi->cur_speed = speed;
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND2);
+	val &= ~SLINK_SS_EN_CS(~0) | SLINK_RXEN | SLINK_TXEN;
+	if (t->rx_buf)
+		val |= SLINK_RXEN;
+	if (t->tx_buf)
+		val |= SLINK_TXEN;
+	val |= SLINK_SS_EN_CS(spi->chip_select);
+	val |= SLINK_SPIE;
+	spi_tegra_writel(tspi, val, SLINK_COMMAND2);
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	val &= ~SLINK_BIT_LENGTH(~0);
+	val |= SLINK_BIT_LENGTH(bits_per_word - 1);
+
+	/* FIXME: should probably control CS manually so that we can be sure
+	 * it does not go low between transfer and to support delay_usecs
+	 * correctly.
+	 */
+	val &= ~SLINK_IDLE_SCLK_MASK & ~SLINK_CK_SDA & ~SLINK_CS_SW;
+
+	if (spi->mode & SPI_CPHA)
+		val |= SLINK_CK_SDA;
+
+	if (spi->mode & SPI_CPOL)
+		val |= SLINK_IDLE_SCLK_DRIVE_HIGH;
+	else
+		val |= SLINK_IDLE_SCLK_DRIVE_LOW;
+
+	val |= SLINK_M_S;
+
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	spi_tegra_writel(tspi, SLINK_RX_FLUSH | SLINK_TX_FLUSH, SLINK_STATUS);
+
+	tspi->cur = t;
+	tspi->cur_pos = 0;
+	tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
+
+	spi_tegra_go(tspi);
+}
+
+static void spi_tegra_start_message(struct spi_device *spi,
+				    struct spi_message *m)
+{
+	struct spi_transfer *t;
+
+	m->actual_length = 0;
+	m->status = 0;
+
+	t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
+	spi_tegra_start_transfer(spi, t);
+}
+
+static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
+{
+	struct spi_tegra_data *tspi = req->dev;
+	unsigned long flags;
+	struct spi_message *m;
+	struct spi_device *spi;
+	int timeout = 0;
+	unsigned long val;
+
+	/* the SPI controller may come back with both the BSY and RDY bits
+	 * set.  In this case we need to wait for the BSY bit to clear so
+	 * that we are sure the DMA is finished.  1000 reads was empirically
+	 * determined to be long enough.
+	 */
+	while (timeout++ < 1000) {
+		if (!(spi_tegra_readl(tspi, SLINK_STATUS) & SLINK_BSY))
+			break;
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	if (timeout >= 1000)
+		m->status = -EIO;
+
+	val = spi_tegra_readl(tspi, SLINK_STATUS);
+	val |= SLINK_RDY;
+	spi_tegra_writel(tspi, val, SLINK_STATUS);
+
+
+	m = list_first_entry(&tspi->queue, struct spi_message, queue);
+	spi = m->state;
+
+	tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
+	m->actual_length += tspi->cur_pos;
+
+	if (tspi->cur_pos < tspi->cur->len) {
+		tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
+		spi_tegra_go(tspi);
+	} else if (!list_is_last(&tspi->cur->transfer_list,
+				 &m->transfers)) {
+		tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
+					      struct spi_transfer,
+					      transfer_list);
+		spi_tegra_start_transfer(spi, tspi->cur);
+	} else {
+		list_del(&m->queue);
+
+		m->complete(m->context);
+
+		if (!list_empty(&tspi->queue)) {
+			m = list_first_entry(&tspi->queue, struct spi_message,
+					     queue);
+			spi = m->state;
+			spi_tegra_start_message(spi, m);
+		} else {
+			clk_disable(tspi->clk);
+			tspi->cur_speed = 0;
+		}
+	}
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+}
+
+static int spi_tegra_setup(struct spi_device *spi)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long cs_bit;
+	unsigned long val;
+	unsigned long flags;
+
+	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);
+
+
+	switch (spi->chip_select) {
+	case 0:
+		cs_bit = SLINK_CS_POLARITY;
+		break;
+
+	case 1:
+		cs_bit = SLINK_CS_POLARITY1;
+		break;
+
+	case 2:
+		cs_bit = SLINK_CS_POLARITY2;
+		break;
+
+	case 4:
+		cs_bit = SLINK_CS_POLARITY3;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	if (spi->mode & SPI_CS_HIGH)
+		val |= cs_bit;
+	else
+		val &= ~cs_bit;
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	struct spi_transfer *t;
+	unsigned long flags;
+	int was_empty;
+
+	if (list_empty(&m->transfers) || !m->complete)
+		return -EINVAL;
+
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		if (t->bits_per_word < 0 || t->bits_per_word > 32)
+			return -EINVAL;
+
+		if (t->len == 0)
+			return -EINVAL;
+
+		if (!t->rx_buf && !t->tx_buf)
+			return -EINVAL;
+	}
+
+	m->state = spi;
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	was_empty = list_empty(&tspi->queue);
+	list_add_tail(&m->queue, &tspi->queue);
+
+	if (was_empty)
+		spi_tegra_start_message(spi, m);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static void spi_tegra_cleanup(struct spi_device *spi)
+{
+	dev_dbg(&spi->dev, "cleanup\n");
+}
+
+static int __init spi_tegra_probe(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof *tspi);
+	if (master == NULL) {
+		dev_err(&pdev->dev, "master allocation failed\n");
+		return -ENOMEM;
+	}
+
+	/* the spi->mode bits understood by this driver: */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+
+	master->setup = spi_tegra_setup;
+	master->transfer = spi_tegra_transfer;
+	master->cleanup = spi_tegra_cleanup;
+	master->num_chipselect = 4;
+
+	dev_set_drvdata(&pdev->dev, master);
+	tspi = spi_master_get_devdata(master);
+	tspi->master = master;
+	tspi->pdev = pdev;
+	spin_lock_init(&tspi->lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		ret = -ENODEV;
+		goto err0;
+	}
+
+	if (!request_mem_region(r->start, (r->end - r->start) + 1,
+				dev_name(&pdev->dev))) {
+		ret = -EBUSY;
+		goto err0;
+	}
+
+	tspi->phys = r->start;
+	tspi->base = ioremap(r->start, r->end - r->start + 1);
+	if (!tspi->base) {
+		dev_err(&pdev->dev, "can't ioremap iomem\n");
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	tspi->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR_OR_NULL(tspi->clk)) {
+		dev_err(&pdev->dev, "can not get clock\n");
+		ret = PTR_ERR(tspi->clk);
+		goto err2;
+	}
+
+	INIT_LIST_HEAD(&tspi->queue);
+
+	tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
+	if (IS_ERR(tspi->rx_dma)) {
+		dev_err(&pdev->dev, "can not allocate rx dma channel\n");
+		ret = PTR_ERR(tspi->rx_dma);
+		goto err3;
+	}
+
+	tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+					 &tspi->rx_bb_phys, GFP_KERNEL);
+	if (!tspi->rx_bb) {
+		dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
+		ret = -ENOMEM;
+		goto err4;
+	}
+
+	tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
+	tspi->rx_dma_req.to_memory = 1;
+	tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
+	tspi->rx_dma_req.dest_bus_width = 32;
+	tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
+	tspi->rx_dma_req.source_bus_width = 32;
+	tspi->rx_dma_req.source_wrap = 4;
+	tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
+	tspi->rx_dma_req.dev = tspi;
+
+	ret = spi_register_master(master);
+
+	if (ret < 0)
+		goto err5;
+
+	return ret;
+
+err5:
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+err4:
+	tegra_dma_free_channel(tspi->rx_dma);
+err3:
+	clk_put(tspi->clk);
+err2:
+	iounmap(tspi->base);
+err1:
+	release_mem_region(r->start, (r->end - r->start) + 1);
+err0:
+	spi_master_put(master);
+	return ret;
+}
+
+static int __devexit spi_tegra_remove(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+
+	master = dev_get_drvdata(&pdev->dev);
+	tspi = spi_master_get_devdata(master);
+
+	tegra_dma_free_channel(tspi->rx_dma);
+
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+
+	clk_put(tspi->clk);
+	iounmap(tspi->base);
+
+	spi_master_put(master);
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(r->start, (r->end - r->start) + 1);
+
+	return 0;
+}
+
+MODULE_ALIAS("platform:spi_tegra");
+
+static struct platform_driver spi_tegra_driver = {
+	.driver = {
+		.name =		"spi_tegra",
+		.owner =	THIS_MODULE,
+	},
+	.remove =	__devexit_p(spi_tegra_remove),
+};
+
+static int __init spi_tegra_init(void)
+{
+	return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
+}
+module_init(spi_tegra_init);
+
+static void __exit spi_tegra_exit(void)
+{
+	platform_driver_unregister(&spi_tegra_driver);
+}
+module_exit(spi_tegra_exit);
+
+MODULE_LICENSE("GPL");
-- 
1.6.5.6


------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d

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

* Re: [PATCH v2] spi: add spi_tegra driver
       [not found]             ` <AANLkTincgapUK5ncs_t3ax=1Kt_56GdR_RVwRqdCm5ge-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-08-27 19:26               ` Erik Gilling
       [not found]                 ` <AANLkTin4HxVLt=4VH-NQWhB-p_2EpX_yoTcwEOTJP3m_-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Erik Gilling @ 2010-08-27 19:26 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	ccross-hpIqsD4AKlfQT0dZR+AlfA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Aug 25, 2010 at 2:22 PM, Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> wrote:
>> As an aside, and commenting on the tegra DMA api... Is the IS_ERR()
>> pattern in the tegra_dma_allocate_channel() return really a good idea
>> here?  Personally I find that pattern isn't very useful, especially
>> when the results is either "yes, allocated" or "no, not-alocated", and
>> it is very easy to write code that tests the wrong thing on the return
>> value because the compiler cannot catch it.  In other words, if it
>> isn't vital to return a specific error code, the using the PTR_ERR()
>> pattern adds unnecessary risk to the code because developers are used
>> to seeing and writing this pattern:
>>
>> tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
>> if (!tspi->rx_dma)
>>        return -ENOMEM;
>>
>> The compiler won't catch it if the return value is an PTR_ERR.
>>
>> (I know that some subsystems make heavy use of it, particularly
>> filesystems and the block layer which do need to return specific error
>> codes to userspace.  My argument is that the specific error code is
>> probably irrelevant when allocating DMA channels)
>
> I don't have a strong opinion either way but that's the way Colin's
> code is written.  I'll let him speak to that.

I spoke with colin about this.  He's fine getting rid of the PTR_ERR.
I'll post a DMA and SPI patch when that's done.

-Erik

------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d

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

* Re: [PATCH v2] spi: add spi_tegra driver
       [not found]                 ` <AANLkTin4HxVLt=4VH-NQWhB-p_2EpX_yoTcwEOTJP3m_-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-08-27 20:13                   ` Grant Likely
  0 siblings, 0 replies; 18+ messages in thread
From: Grant Likely @ 2010-08-27 20:13 UTC (permalink / raw)
  To: Erik Gilling
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	ccross-hpIqsD4AKlfQT0dZR+AlfA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, Aug 27, 2010 at 1:26 PM, Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> wrote:
> On Wed, Aug 25, 2010 at 2:22 PM, Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> wrote:
>>> As an aside, and commenting on the tegra DMA api... Is the IS_ERR()
>>> pattern in the tegra_dma_allocate_channel() return really a good idea
>>> here?  Personally I find that pattern isn't very useful, especially
>>> when the results is either "yes, allocated" or "no, not-alocated", and
>>> it is very easy to write code that tests the wrong thing on the return
>>> value because the compiler cannot catch it.  In other words, if it
>>> isn't vital to return a specific error code, the using the PTR_ERR()
>>> pattern adds unnecessary risk to the code because developers are used
>>> to seeing and writing this pattern:
>>>
>>> tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
>>> if (!tspi->rx_dma)
>>>        return -ENOMEM;
>>>
>>> The compiler won't catch it if the return value is an PTR_ERR.
>>>
>>> (I know that some subsystems make heavy use of it, particularly
>>> filesystems and the block layer which do need to return specific error
>>> codes to userspace.  My argument is that the specific error code is
>>> probably irrelevant when allocating DMA channels)
>>
>> I don't have a strong opinion either way but that's the way Colin's
>> code is written.  I'll let him speak to that.
>
> I spoke with colin about this.  He's fine getting rid of the PTR_ERR.
> I'll post a DMA and SPI patch when that's done.

Okay, thanks.

g.

------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d

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

* [PATCH v4] spi: add spi_tegra driver
  2010-08-25 21:23       ` [PATCH v3] " Erik Gilling
@ 2010-09-01 22:16         ` Erik Gilling
  2010-09-01 22:18           ` Erik Gilling
  2010-09-02 14:22           ` Grant Likely
  0 siblings, 2 replies; 18+ messages in thread
From: Erik Gilling @ 2010-09-01 22:16 UTC (permalink / raw)
  To: linux-tegra
  Cc: Russell King, Erik Gilling, Thierry Reding, Grant Likely,
	spi-devel-general, linux-arm-kernel

v2 changes:
  from Thierry Reding:
    * add "select TEGRA_SYSTEM_DMA" to Kconfig
  from Grant Likely:
    * add oneline description to header
    * inline references to DRIVER_NAME
    * inline references to BUSY_TIMEOUT
    * open coded bytes_per_word()
    * spi_readl/writel -> spi_tegra_readl/writel
    * move transfer validation to spi_tegra_transfer
    * don't request_mem_region iomem as platform bus does that for us
    * __exit -> __devexit

v3 changes:
  from Russell King:
    * put request_mem_region back int
  from Grant Likely:
    * remove #undef DEBUG
    * add SLINK_ to register bit defines
    * remove unused bytes_per_word
    * make spi_tegra_readl/writel static linine
    * various refactoring for clarity
    * mark err if BSY bit is not cleared after 1000 retries
    * move spinlock to protect setting of RDY bit
    * subsys_initcall -> module_init

v3 changes:
  from Grant Likely:
    * update spi_tegra to use PTR_ERRless dma API

Signed-off-by: Erik Gilling <konkers@android.com>
Cc: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Russell King <linux@arm.linux.org.uk>
---
 drivers/spi/Kconfig     |    7 +
 drivers/spi/Makefile    |    1 +
 drivers/spi/spi_tegra.c |  625 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 633 insertions(+), 0 deletions(-)
 create mode 100644 drivers/spi/spi_tegra.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 91c2f4f..9fdb309 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -298,6 +298,13 @@ config SPI_STMP3XXX
 	help
 	  SPI driver for Freescale STMP37xx/378x SoC SSP interface
 
+config SPI_TEGRA
+	tristate "Nvidia Tegra SPI controller"
+	depends on ARCH_TEGRA
+	select TEGRA_SYSTEM_DMA
+	help
+	  SPI driver for NVidia Tegra SoCs
+
 config SPI_TXX9
 	tristate "Toshiba TXx9 SPI controller"
 	depends on GENERIC_GPIO && CPU_TX49XX
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index e9cbd18..b6573d8 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)		+= spi_ppc4xx.o
 obj-$(CONFIG_SPI_S3C24XX_GPIO)		+= spi_s3c24xx_gpio.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi_s3c24xx_hw.o
 obj-$(CONFIG_SPI_S3C64XX)		+= spi_s3c64xx.o
+obj-$(CONFIG_SPI_TEGRA)			+= spi_tegra.o
 obj-$(CONFIG_SPI_TXX9)			+= spi_txx9.o
 obj-$(CONFIG_SPI_XILINX)		+= xilinx_spi.o
 obj-$(CONFIG_SPI_XILINX_OF)		+= xilinx_spi_of.o
diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
new file mode 100644
index 0000000..b277734
--- /dev/null
+++ b/drivers/spi/spi_tegra.c
@@ -0,0 +1,625 @@
+/*
+ * Driver for Nvidia TEGRA spi controller.
+ *
+ * Copyright (C) 2010 Google, Inc.
+ *
+ * Author:
+ *     Erik Gilling <konkers@android.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+
+#include <linux/spi/spi.h>
+
+#include <mach/dma.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_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_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) & 0x3f) >> 16)
+
+#define SLINK_TX_FIFO		0x100
+#define SLINK_RX_FIFO		0x180
+
+static const unsigned long spi_tegra_req_sels[] = {
+	TEGRA_DMA_REQ_SEL_SL2B1,
+	TEGRA_DMA_REQ_SEL_SL2B2,
+	TEGRA_DMA_REQ_SEL_SL2B3,
+	TEGRA_DMA_REQ_SEL_SL2B4,
+};
+
+#define BB_LEN			32
+
+struct spi_tegra_data {
+	struct spi_master	*master;
+	struct platform_device	*pdev;
+	spinlock_t		lock;
+
+	struct clk		*clk;
+	void __iomem		*base;
+	unsigned long		phys;
+
+	u32			cur_speed;
+
+	struct list_head	queue;
+	struct spi_transfer	*cur;
+	unsigned		cur_pos;
+	unsigned		cur_len;
+	unsigned		cur_bytes_per_word;
+
+	/* The tegra spi controller has a bug which causes the first word
+	 * in PIO transactions to be garbage.  Since packed DMA transactions
+	 * require transfers to be 4 byte aligned we need a bounce buffer
+	 * for the generic case.
+	 */
+	struct tegra_dma_req	rx_dma_req;
+	struct tegra_dma_channel *rx_dma;
+	u32			*rx_bb;
+	dma_addr_t		rx_bb_phys;
+};
+
+
+static inline unsigned long spi_tegra_readl(struct spi_tegra_data *tspi,
+					    unsigned long reg)
+{
+	return readl(tspi->base + reg);
+}
+
+static inline void spi_tegra_writel(struct spi_tegra_data *tspi,
+				    unsigned long val,
+				    unsigned long reg)
+{
+	writel(val, tspi->base + reg);
+}
+
+static void spi_tegra_go(struct spi_tegra_data *tspi)
+{
+	unsigned long val;
+
+	wmb();
+
+	val = spi_tegra_readl(tspi, SLINK_DMA_CTL);
+	val &= ~SLINK_DMA_BLOCK_SIZE(~0) & ~SLINK_DMA_EN;
+	val |= SLINK_DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
+	spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
+
+	tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
+
+	val |= SLINK_DMA_EN;
+	spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
+}
+
+static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
+			   tspi->cur_bytes_per_word);
+	u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	val &= ~SLINK_WORD_SIZE(~0);
+	val |= SLINK_WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = 0;
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			val |= tx_buf[i + j] << j * 8;
+
+		spi_tegra_writel(tspi, val, SLINK_TX_FIFO);
+	}
+
+	tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
+
+	return len;
+}
+
+static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
+				  struct spi_transfer *t)
+{
+	unsigned len = tspi->cur_len;
+	u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
+	int i, j;
+	unsigned long val;
+
+	for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
+		val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
+		for (j = 0; j < tspi->cur_bytes_per_word; j++)
+			rx_buf[i + j] = (val >> (j * 8)) & 0xff;
+	}
+
+	return len;
+}
+
+static void spi_tegra_start_transfer(struct spi_device *spi,
+				    struct spi_transfer *t)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	u32 speed;
+	u8 bits_per_word;
+	unsigned long val;
+
+	speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
+	bits_per_word = t->bits_per_word ? t->bits_per_word  :
+		spi->bits_per_word;
+
+	tspi->cur_bytes_per_word = (bits_per_word - 1) / 8 + 1;
+
+	if (speed != tspi->cur_speed)
+		clk_set_rate(tspi->clk, speed);
+
+	if (tspi->cur_speed == 0)
+		clk_enable(tspi->clk);
+
+	tspi->cur_speed = speed;
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND2);
+	val &= ~SLINK_SS_EN_CS(~0) | SLINK_RXEN | SLINK_TXEN;
+	if (t->rx_buf)
+		val |= SLINK_RXEN;
+	if (t->tx_buf)
+		val |= SLINK_TXEN;
+	val |= SLINK_SS_EN_CS(spi->chip_select);
+	val |= SLINK_SPIE;
+	spi_tegra_writel(tspi, val, SLINK_COMMAND2);
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	val &= ~SLINK_BIT_LENGTH(~0);
+	val |= SLINK_BIT_LENGTH(bits_per_word - 1);
+
+	/* FIXME: should probably control CS manually so that we can be sure
+	 * it does not go low between transfer and to support delay_usecs
+	 * correctly.
+	 */
+	val &= ~SLINK_IDLE_SCLK_MASK & ~SLINK_CK_SDA & ~SLINK_CS_SW;
+
+	if (spi->mode & SPI_CPHA)
+		val |= SLINK_CK_SDA;
+
+	if (spi->mode & SPI_CPOL)
+		val |= SLINK_IDLE_SCLK_DRIVE_HIGH;
+	else
+		val |= SLINK_IDLE_SCLK_DRIVE_LOW;
+
+	val |= SLINK_M_S;
+
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	spi_tegra_writel(tspi, SLINK_RX_FLUSH | SLINK_TX_FLUSH, SLINK_STATUS);
+
+	tspi->cur = t;
+	tspi->cur_pos = 0;
+	tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
+
+	spi_tegra_go(tspi);
+}
+
+static void spi_tegra_start_message(struct spi_device *spi,
+				    struct spi_message *m)
+{
+	struct spi_transfer *t;
+
+	m->actual_length = 0;
+	m->status = 0;
+
+	t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
+	spi_tegra_start_transfer(spi, t);
+}
+
+static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
+{
+	struct spi_tegra_data *tspi = req->dev;
+	unsigned long flags;
+	struct spi_message *m;
+	struct spi_device *spi;
+	int timeout = 0;
+	unsigned long val;
+
+	/* the SPI controller may come back with both the BSY and RDY bits
+	 * set.  In this case we need to wait for the BSY bit to clear so
+	 * that we are sure the DMA is finished.  1000 reads was empirically
+	 * determined to be long enough.
+	 */
+	while (timeout++ < 1000) {
+		if (!(spi_tegra_readl(tspi, SLINK_STATUS) & SLINK_BSY))
+			break;
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	if (timeout >= 1000)
+		m->status = -EIO;
+
+	val = spi_tegra_readl(tspi, SLINK_STATUS);
+	val |= SLINK_RDY;
+	spi_tegra_writel(tspi, val, SLINK_STATUS);
+
+
+	m = list_first_entry(&tspi->queue, struct spi_message, queue);
+	spi = m->state;
+
+	tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
+	m->actual_length += tspi->cur_pos;
+
+	if (tspi->cur_pos < tspi->cur->len) {
+		tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
+		spi_tegra_go(tspi);
+	} else if (!list_is_last(&tspi->cur->transfer_list,
+				 &m->transfers)) {
+		tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
+					      struct spi_transfer,
+					      transfer_list);
+		spi_tegra_start_transfer(spi, tspi->cur);
+	} else {
+		list_del(&m->queue);
+
+		m->complete(m->context);
+
+		if (!list_empty(&tspi->queue)) {
+			m = list_first_entry(&tspi->queue, struct spi_message,
+					     queue);
+			spi = m->state;
+			spi_tegra_start_message(spi, m);
+		} else {
+			clk_disable(tspi->clk);
+			tspi->cur_speed = 0;
+		}
+	}
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+}
+
+static int spi_tegra_setup(struct spi_device *spi)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	unsigned long cs_bit;
+	unsigned long val;
+	unsigned long flags;
+
+	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);
+
+
+	switch (spi->chip_select) {
+	case 0:
+		cs_bit = SLINK_CS_POLARITY;
+		break;
+
+	case 1:
+		cs_bit = SLINK_CS_POLARITY1;
+		break;
+
+	case 2:
+		cs_bit = SLINK_CS_POLARITY2;
+		break;
+
+	case 4:
+		cs_bit = SLINK_CS_POLARITY3;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&tspi->lock, flags);
+
+	val = spi_tegra_readl(tspi, SLINK_COMMAND);
+	if (spi->mode & SPI_CS_HIGH)
+		val |= cs_bit;
+	else
+		val &= ~cs_bit;
+	spi_tegra_writel(tspi, val, SLINK_COMMAND);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
+{
+	struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
+	struct spi_transfer *t;
+	unsigned long flags;
+	int was_empty;
+
+	if (list_empty(&m->transfers) || !m->complete)
+		return -EINVAL;
+
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		if (t->bits_per_word < 0 || t->bits_per_word > 32)
+			return -EINVAL;
+
+		if (t->len == 0)
+			return -EINVAL;
+
+		if (!t->rx_buf && !t->tx_buf)
+			return -EINVAL;
+	}
+
+	m->state = spi;
+
+	spin_lock_irqsave(&tspi->lock, flags);
+	was_empty = list_empty(&tspi->queue);
+	list_add_tail(&m->queue, &tspi->queue);
+
+	if (was_empty)
+		spi_tegra_start_message(spi, m);
+
+	spin_unlock_irqrestore(&tspi->lock, flags);
+
+	return 0;
+}
+
+static void spi_tegra_cleanup(struct spi_device *spi)
+{
+	dev_dbg(&spi->dev, "cleanup\n");
+}
+
+static int __init spi_tegra_probe(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof *tspi);
+	if (master == NULL) {
+		dev_err(&pdev->dev, "master allocation failed\n");
+		return -ENOMEM;
+	}
+
+	/* the spi->mode bits understood by this driver: */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+
+	if (pdev->id != -1)
+		master->bus_num = pdev->id;
+
+	master->setup = spi_tegra_setup;
+	master->transfer = spi_tegra_transfer;
+	master->cleanup = spi_tegra_cleanup;
+	master->num_chipselect = 4;
+
+	dev_set_drvdata(&pdev->dev, master);
+	tspi = spi_master_get_devdata(master);
+	tspi->master = master;
+	tspi->pdev = pdev;
+	spin_lock_init(&tspi->lock);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		ret = -ENODEV;
+		goto err0;
+	}
+
+	if (!request_mem_region(r->start, (r->end - r->start) + 1,
+				dev_name(&pdev->dev))) {
+		ret = -EBUSY;
+		goto err0;
+	}
+
+	tspi->phys = r->start;
+	tspi->base = ioremap(r->start, r->end - r->start + 1);
+	if (!tspi->base) {
+		dev_err(&pdev->dev, "can't ioremap iomem\n");
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	tspi->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR_OR_NULL(tspi->clk)) {
+		dev_err(&pdev->dev, "can not get clock\n");
+		ret = PTR_ERR(tspi->clk);
+		goto err2;
+	}
+
+	INIT_LIST_HEAD(&tspi->queue);
+
+	tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
+	if (!tspi->rx_dma) {
+		dev_err(&pdev->dev, "can not allocate rx dma channel\n");
+		ret = -ENODEV;
+		goto err3;
+	}
+
+	tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+					 &tspi->rx_bb_phys, GFP_KERNEL);
+	if (!tspi->rx_bb) {
+		dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
+		ret = -ENOMEM;
+		goto err4;
+	}
+
+	tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
+	tspi->rx_dma_req.to_memory = 1;
+	tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
+	tspi->rx_dma_req.dest_bus_width = 32;
+	tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
+	tspi->rx_dma_req.source_bus_width = 32;
+	tspi->rx_dma_req.source_wrap = 4;
+	tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
+	tspi->rx_dma_req.dev = tspi;
+
+	ret = spi_register_master(master);
+
+	if (ret < 0)
+		goto err5;
+
+	return ret;
+
+err5:
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+err4:
+	tegra_dma_free_channel(tspi->rx_dma);
+err3:
+	clk_put(tspi->clk);
+err2:
+	iounmap(tspi->base);
+err1:
+	release_mem_region(r->start, (r->end - r->start) + 1);
+err0:
+	spi_master_put(master);
+	return ret;
+}
+
+static int __devexit spi_tegra_remove(struct platform_device *pdev)
+{
+	struct spi_master	*master;
+	struct spi_tegra_data	*tspi;
+	struct resource		*r;
+
+	master = dev_get_drvdata(&pdev->dev);
+	tspi = spi_master_get_devdata(master);
+
+	tegra_dma_free_channel(tspi->rx_dma);
+
+	dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
+			  tspi->rx_bb, tspi->rx_bb_phys);
+
+	clk_put(tspi->clk);
+	iounmap(tspi->base);
+
+	spi_master_put(master);
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(r->start, (r->end - r->start) + 1);
+
+	return 0;
+}
+
+MODULE_ALIAS("platform:spi_tegra");
+
+static struct platform_driver spi_tegra_driver = {
+	.driver = {
+		.name =		"spi_tegra",
+		.owner =	THIS_MODULE,
+	},
+	.remove =	__devexit_p(spi_tegra_remove),
+};
+
+static int __init spi_tegra_init(void)
+{
+	return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
+}
+module_init(spi_tegra_init);
+
+static void __exit spi_tegra_exit(void)
+{
+	platform_driver_unregister(&spi_tegra_driver);
+}
+module_exit(spi_tegra_exit);
+
+MODULE_LICENSE("GPL");
-- 
1.6.5.6

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

* Re: [PATCH v4] spi: add spi_tegra driver
  2010-09-01 22:16         ` [PATCH v4] " Erik Gilling
@ 2010-09-01 22:18           ` Erik Gilling
  2010-09-02 14:22           ` Grant Likely
  1 sibling, 0 replies; 18+ messages in thread
From: Erik Gilling @ 2010-09-01 22:18 UTC (permalink / raw)
  To: linux-tegra
  Cc: Russell King, Erik Gilling, Thierry Reding, Grant Likely,
	spi-devel-general, linux-arm-kernel

Grant,
    The DMA support is now in the tegra for-next tree.  Please add
this driver to your spi-next tree.

Thanks,
    Erik

On Wed, Sep 1, 2010 at 3:16 PM, Erik Gilling <konkers@android.com> wrote:
> v2 changes:
>  from Thierry Reding:
>    * add "select TEGRA_SYSTEM_DMA" to Kconfig
>  from Grant Likely:
>    * add oneline description to header
>    * inline references to DRIVER_NAME
>    * inline references to BUSY_TIMEOUT
>    * open coded bytes_per_word()
>    * spi_readl/writel -> spi_tegra_readl/writel
>    * move transfer validation to spi_tegra_transfer
>    * don't request_mem_region iomem as platform bus does that for us
>    * __exit -> __devexit
>
> v3 changes:
>  from Russell King:
>    * put request_mem_region back int
>  from Grant Likely:
>    * remove #undef DEBUG
>    * add SLINK_ to register bit defines
>    * remove unused bytes_per_word
>    * make spi_tegra_readl/writel static linine
>    * various refactoring for clarity
>    * mark err if BSY bit is not cleared after 1000 retries
>    * move spinlock to protect setting of RDY bit
>    * subsys_initcall -> module_init
>
> v3 changes:
>  from Grant Likely:
>    * update spi_tegra to use PTR_ERRless dma API
>
> Signed-off-by: Erik Gilling <konkers@android.com>
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Russell King <linux@arm.linux.org.uk>
> ---
>  drivers/spi/Kconfig     |    7 +
>  drivers/spi/Makefile    |    1 +
>  drivers/spi/spi_tegra.c |  625 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 633 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/spi_tegra.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 91c2f4f..9fdb309 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -298,6 +298,13 @@ config SPI_STMP3XXX
>        help
>          SPI driver for Freescale STMP37xx/378x SoC SSP interface
>
> +config SPI_TEGRA
> +       tristate "Nvidia Tegra SPI controller"
> +       depends on ARCH_TEGRA
> +       select TEGRA_SYSTEM_DMA
> +       help
> +         SPI driver for NVidia Tegra SoCs
> +
>  config SPI_TXX9
>        tristate "Toshiba TXx9 SPI controller"
>        depends on GENERIC_GPIO && CPU_TX49XX
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index e9cbd18..b6573d8 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_SPI_PPC4xx)              += spi_ppc4xx.o
>  obj-$(CONFIG_SPI_S3C24XX_GPIO)         += spi_s3c24xx_gpio.o
>  obj-$(CONFIG_SPI_S3C24XX)              += spi_s3c24xx_hw.o
>  obj-$(CONFIG_SPI_S3C64XX)              += spi_s3c64xx.o
> +obj-$(CONFIG_SPI_TEGRA)                        += spi_tegra.o
>  obj-$(CONFIG_SPI_TXX9)                 += spi_txx9.o
>  obj-$(CONFIG_SPI_XILINX)               += xilinx_spi.o
>  obj-$(CONFIG_SPI_XILINX_OF)            += xilinx_spi_of.o
> diff --git a/drivers/spi/spi_tegra.c b/drivers/spi/spi_tegra.c
> new file mode 100644
> index 0000000..b277734
> --- /dev/null
> +++ b/drivers/spi/spi_tegra.c
> @@ -0,0 +1,625 @@
> +/*
> + * Driver for Nvidia TEGRA spi controller.
> + *
> + * Copyright (C) 2010 Google, Inc.
> + *
> + * Author:
> + *     Erik Gilling <konkers@android.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +
> +#include <linux/spi/spi.h>
> +
> +#include <mach/dma.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_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_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) & 0x3f) >> 16)
> +
> +#define SLINK_TX_FIFO          0x100
> +#define SLINK_RX_FIFO          0x180
> +
> +static const unsigned long spi_tegra_req_sels[] = {
> +       TEGRA_DMA_REQ_SEL_SL2B1,
> +       TEGRA_DMA_REQ_SEL_SL2B2,
> +       TEGRA_DMA_REQ_SEL_SL2B3,
> +       TEGRA_DMA_REQ_SEL_SL2B4,
> +};
> +
> +#define BB_LEN                 32
> +
> +struct spi_tegra_data {
> +       struct spi_master       *master;
> +       struct platform_device  *pdev;
> +       spinlock_t              lock;
> +
> +       struct clk              *clk;
> +       void __iomem            *base;
> +       unsigned long           phys;
> +
> +       u32                     cur_speed;
> +
> +       struct list_head        queue;
> +       struct spi_transfer     *cur;
> +       unsigned                cur_pos;
> +       unsigned                cur_len;
> +       unsigned                cur_bytes_per_word;
> +
> +       /* The tegra spi controller has a bug which causes the first word
> +        * in PIO transactions to be garbage.  Since packed DMA transactions
> +        * require transfers to be 4 byte aligned we need a bounce buffer
> +        * for the generic case.
> +        */
> +       struct tegra_dma_req    rx_dma_req;
> +       struct tegra_dma_channel *rx_dma;
> +       u32                     *rx_bb;
> +       dma_addr_t              rx_bb_phys;
> +};
> +
> +
> +static inline unsigned long spi_tegra_readl(struct spi_tegra_data *tspi,
> +                                           unsigned long reg)
> +{
> +       return readl(tspi->base + reg);
> +}
> +
> +static inline void spi_tegra_writel(struct spi_tegra_data *tspi,
> +                                   unsigned long val,
> +                                   unsigned long reg)
> +{
> +       writel(val, tspi->base + reg);
> +}
> +
> +static void spi_tegra_go(struct spi_tegra_data *tspi)
> +{
> +       unsigned long val;
> +
> +       wmb();
> +
> +       val = spi_tegra_readl(tspi, SLINK_DMA_CTL);
> +       val &= ~SLINK_DMA_BLOCK_SIZE(~0) & ~SLINK_DMA_EN;
> +       val |= SLINK_DMA_BLOCK_SIZE(tspi->rx_dma_req.size / 4 - 1);
> +       spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
> +
> +       tegra_dma_enqueue_req(tspi->rx_dma, &tspi->rx_dma_req);
> +
> +       val |= SLINK_DMA_EN;
> +       spi_tegra_writel(tspi, val, SLINK_DMA_CTL);
> +}
> +
> +static unsigned spi_tegra_fill_tx_fifo(struct spi_tegra_data *tspi,
> +                                 struct spi_transfer *t)
> +{
> +       unsigned len = min(t->len - tspi->cur_pos, BB_LEN *
> +                          tspi->cur_bytes_per_word);
> +       u8 *tx_buf = (u8 *)t->tx_buf + tspi->cur_pos;
> +       int i, j;
> +       unsigned long val;
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND);
> +       val &= ~SLINK_WORD_SIZE(~0);
> +       val |= SLINK_WORD_SIZE(len / tspi->cur_bytes_per_word - 1);
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);
> +
> +       for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
> +               val = 0;
> +               for (j = 0; j < tspi->cur_bytes_per_word; j++)
> +                       val |= tx_buf[i + j] << j * 8;
> +
> +               spi_tegra_writel(tspi, val, SLINK_TX_FIFO);
> +       }
> +
> +       tspi->rx_dma_req.size = len / tspi->cur_bytes_per_word * 4;
> +
> +       return len;
> +}
> +
> +static unsigned spi_tegra_drain_rx_fifo(struct spi_tegra_data *tspi,
> +                                 struct spi_transfer *t)
> +{
> +       unsigned len = tspi->cur_len;
> +       u8 *rx_buf = (u8 *)t->rx_buf + tspi->cur_pos;
> +       int i, j;
> +       unsigned long val;
> +
> +       for (i = 0; i < len; i += tspi->cur_bytes_per_word) {
> +               val = tspi->rx_bb[i / tspi->cur_bytes_per_word];
> +               for (j = 0; j < tspi->cur_bytes_per_word; j++)
> +                       rx_buf[i + j] = (val >> (j * 8)) & 0xff;
> +       }
> +
> +       return len;
> +}
> +
> +static void spi_tegra_start_transfer(struct spi_device *spi,
> +                                   struct spi_transfer *t)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       u32 speed;
> +       u8 bits_per_word;
> +       unsigned long val;
> +
> +       speed = t->speed_hz ? t->speed_hz : spi->max_speed_hz;
> +       bits_per_word = t->bits_per_word ? t->bits_per_word  :
> +               spi->bits_per_word;
> +
> +       tspi->cur_bytes_per_word = (bits_per_word - 1) / 8 + 1;
> +
> +       if (speed != tspi->cur_speed)
> +               clk_set_rate(tspi->clk, speed);
> +
> +       if (tspi->cur_speed == 0)
> +               clk_enable(tspi->clk);
> +
> +       tspi->cur_speed = speed;
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND2);
> +       val &= ~SLINK_SS_EN_CS(~0) | SLINK_RXEN | SLINK_TXEN;
> +       if (t->rx_buf)
> +               val |= SLINK_RXEN;
> +       if (t->tx_buf)
> +               val |= SLINK_TXEN;
> +       val |= SLINK_SS_EN_CS(spi->chip_select);
> +       val |= SLINK_SPIE;
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND2);
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND);
> +       val &= ~SLINK_BIT_LENGTH(~0);
> +       val |= SLINK_BIT_LENGTH(bits_per_word - 1);
> +
> +       /* FIXME: should probably control CS manually so that we can be sure
> +        * it does not go low between transfer and to support delay_usecs
> +        * correctly.
> +        */
> +       val &= ~SLINK_IDLE_SCLK_MASK & ~SLINK_CK_SDA & ~SLINK_CS_SW;
> +
> +       if (spi->mode & SPI_CPHA)
> +               val |= SLINK_CK_SDA;
> +
> +       if (spi->mode & SPI_CPOL)
> +               val |= SLINK_IDLE_SCLK_DRIVE_HIGH;
> +       else
> +               val |= SLINK_IDLE_SCLK_DRIVE_LOW;
> +
> +       val |= SLINK_M_S;
> +
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);
> +
> +       spi_tegra_writel(tspi, SLINK_RX_FLUSH | SLINK_TX_FLUSH, SLINK_STATUS);
> +
> +       tspi->cur = t;
> +       tspi->cur_pos = 0;
> +       tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, t);
> +
> +       spi_tegra_go(tspi);
> +}
> +
> +static void spi_tegra_start_message(struct spi_device *spi,
> +                                   struct spi_message *m)
> +{
> +       struct spi_transfer *t;
> +
> +       m->actual_length = 0;
> +       m->status = 0;
> +
> +       t = list_first_entry(&m->transfers, struct spi_transfer, transfer_list);
> +       spi_tegra_start_transfer(spi, t);
> +}
> +
> +static void tegra_spi_rx_dma_complete(struct tegra_dma_req *req)
> +{
> +       struct spi_tegra_data *tspi = req->dev;
> +       unsigned long flags;
> +       struct spi_message *m;
> +       struct spi_device *spi;
> +       int timeout = 0;
> +       unsigned long val;
> +
> +       /* the SPI controller may come back with both the BSY and RDY bits
> +        * set.  In this case we need to wait for the BSY bit to clear so
> +        * that we are sure the DMA is finished.  1000 reads was empirically
> +        * determined to be long enough.
> +        */
> +       while (timeout++ < 1000) {
> +               if (!(spi_tegra_readl(tspi, SLINK_STATUS) & SLINK_BSY))
> +                       break;
> +       }
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +
> +       if (timeout >= 1000)
> +               m->status = -EIO;
> +
> +       val = spi_tegra_readl(tspi, SLINK_STATUS);
> +       val |= SLINK_RDY;
> +       spi_tegra_writel(tspi, val, SLINK_STATUS);
> +
> +
> +       m = list_first_entry(&tspi->queue, struct spi_message, queue);
> +       spi = m->state;
> +
> +       tspi->cur_pos += spi_tegra_drain_rx_fifo(tspi, tspi->cur);
> +       m->actual_length += tspi->cur_pos;
> +
> +       if (tspi->cur_pos < tspi->cur->len) {
> +               tspi->cur_len = spi_tegra_fill_tx_fifo(tspi, tspi->cur);
> +               spi_tegra_go(tspi);
> +       } else if (!list_is_last(&tspi->cur->transfer_list,
> +                                &m->transfers)) {
> +               tspi->cur =  list_first_entry(&tspi->cur->transfer_list,
> +                                             struct spi_transfer,
> +                                             transfer_list);
> +               spi_tegra_start_transfer(spi, tspi->cur);
> +       } else {
> +               list_del(&m->queue);
> +
> +               m->complete(m->context);
> +
> +               if (!list_empty(&tspi->queue)) {
> +                       m = list_first_entry(&tspi->queue, struct spi_message,
> +                                            queue);
> +                       spi = m->state;
> +                       spi_tegra_start_message(spi, m);
> +               } else {
> +                       clk_disable(tspi->clk);
> +                       tspi->cur_speed = 0;
> +               }
> +       }
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +}
> +
> +static int spi_tegra_setup(struct spi_device *spi)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       unsigned long cs_bit;
> +       unsigned long val;
> +       unsigned long flags;
> +
> +       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);
> +
> +
> +       switch (spi->chip_select) {
> +       case 0:
> +               cs_bit = SLINK_CS_POLARITY;
> +               break;
> +
> +       case 1:
> +               cs_bit = SLINK_CS_POLARITY1;
> +               break;
> +
> +       case 2:
> +               cs_bit = SLINK_CS_POLARITY2;
> +               break;
> +
> +       case 4:
> +               cs_bit = SLINK_CS_POLARITY3;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +
> +       val = spi_tegra_readl(tspi, SLINK_COMMAND);
> +       if (spi->mode & SPI_CS_HIGH)
> +               val |= cs_bit;
> +       else
> +               val &= ~cs_bit;
> +       spi_tegra_writel(tspi, val, SLINK_COMMAND);
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int spi_tegra_transfer(struct spi_device *spi, struct spi_message *m)
> +{
> +       struct spi_tegra_data *tspi = spi_master_get_devdata(spi->master);
> +       struct spi_transfer *t;
> +       unsigned long flags;
> +       int was_empty;
> +
> +       if (list_empty(&m->transfers) || !m->complete)
> +               return -EINVAL;
> +
> +       list_for_each_entry(t, &m->transfers, transfer_list) {
> +               if (t->bits_per_word < 0 || t->bits_per_word > 32)
> +                       return -EINVAL;
> +
> +               if (t->len == 0)
> +                       return -EINVAL;
> +
> +               if (!t->rx_buf && !t->tx_buf)
> +                       return -EINVAL;
> +       }
> +
> +       m->state = spi;
> +
> +       spin_lock_irqsave(&tspi->lock, flags);
> +       was_empty = list_empty(&tspi->queue);
> +       list_add_tail(&m->queue, &tspi->queue);
> +
> +       if (was_empty)
> +               spi_tegra_start_message(spi, m);
> +
> +       spin_unlock_irqrestore(&tspi->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void spi_tegra_cleanup(struct spi_device *spi)
> +{
> +       dev_dbg(&spi->dev, "cleanup\n");
> +}
> +
> +static int __init spi_tegra_probe(struct platform_device *pdev)
> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +       struct resource         *r;
> +       int ret;
> +
> +       master = spi_alloc_master(&pdev->dev, sizeof *tspi);
> +       if (master == NULL) {
> +               dev_err(&pdev->dev, "master allocation failed\n");
> +               return -ENOMEM;
> +       }
> +
> +       /* the spi->mode bits understood by this driver: */
> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> +
> +       if (pdev->id != -1)
> +               master->bus_num = pdev->id;
> +
> +       master->setup = spi_tegra_setup;
> +       master->transfer = spi_tegra_transfer;
> +       master->cleanup = spi_tegra_cleanup;
> +       master->num_chipselect = 4;
> +
> +       dev_set_drvdata(&pdev->dev, master);
> +       tspi = spi_master_get_devdata(master);
> +       tspi->master = master;
> +       tspi->pdev = pdev;
> +       spin_lock_init(&tspi->lock);
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (r == NULL) {
> +               ret = -ENODEV;
> +               goto err0;
> +       }
> +
> +       if (!request_mem_region(r->start, (r->end - r->start) + 1,
> +                               dev_name(&pdev->dev))) {
> +               ret = -EBUSY;
> +               goto err0;
> +       }
> +
> +       tspi->phys = r->start;
> +       tspi->base = ioremap(r->start, r->end - r->start + 1);
> +       if (!tspi->base) {
> +               dev_err(&pdev->dev, "can't ioremap iomem\n");
> +               ret = -ENOMEM;
> +               goto err1;
> +       }
> +
> +       tspi->clk = clk_get(&pdev->dev, NULL);
> +       if (IS_ERR_OR_NULL(tspi->clk)) {
> +               dev_err(&pdev->dev, "can not get clock\n");
> +               ret = PTR_ERR(tspi->clk);
> +               goto err2;
> +       }
> +
> +       INIT_LIST_HEAD(&tspi->queue);
> +
> +       tspi->rx_dma = tegra_dma_allocate_channel(TEGRA_DMA_MODE_ONESHOT);
> +       if (!tspi->rx_dma) {
> +               dev_err(&pdev->dev, "can not allocate rx dma channel\n");
> +               ret = -ENODEV;
> +               goto err3;
> +       }
> +
> +       tspi->rx_bb = dma_alloc_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                                        &tspi->rx_bb_phys, GFP_KERNEL);
> +       if (!tspi->rx_bb) {
> +               dev_err(&pdev->dev, "can not allocate rx bounce buffer\n");
> +               ret = -ENOMEM;
> +               goto err4;
> +       }
> +
> +       tspi->rx_dma_req.complete = tegra_spi_rx_dma_complete;
> +       tspi->rx_dma_req.to_memory = 1;
> +       tspi->rx_dma_req.dest_addr = tspi->rx_bb_phys;
> +       tspi->rx_dma_req.dest_bus_width = 32;
> +       tspi->rx_dma_req.source_addr = tspi->phys + SLINK_RX_FIFO;
> +       tspi->rx_dma_req.source_bus_width = 32;
> +       tspi->rx_dma_req.source_wrap = 4;
> +       tspi->rx_dma_req.req_sel = spi_tegra_req_sels[pdev->id];
> +       tspi->rx_dma_req.dev = tspi;
> +
> +       ret = spi_register_master(master);
> +
> +       if (ret < 0)
> +               goto err5;
> +
> +       return ret;
> +
> +err5:
> +       dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                         tspi->rx_bb, tspi->rx_bb_phys);
> +err4:
> +       tegra_dma_free_channel(tspi->rx_dma);
> +err3:
> +       clk_put(tspi->clk);
> +err2:
> +       iounmap(tspi->base);
> +err1:
> +       release_mem_region(r->start, (r->end - r->start) + 1);
> +err0:
> +       spi_master_put(master);
> +       return ret;
> +}
> +
> +static int __devexit spi_tegra_remove(struct platform_device *pdev)
> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +       struct resource         *r;
> +
> +       master = dev_get_drvdata(&pdev->dev);
> +       tspi = spi_master_get_devdata(master);
> +
> +       tegra_dma_free_channel(tspi->rx_dma);
> +
> +       dma_free_coherent(&pdev->dev, sizeof(u32) * BB_LEN,
> +                         tspi->rx_bb, tspi->rx_bb_phys);
> +
> +       clk_put(tspi->clk);
> +       iounmap(tspi->base);
> +
> +       spi_master_put(master);
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       release_mem_region(r->start, (r->end - r->start) + 1);
> +
> +       return 0;
> +}
> +
> +MODULE_ALIAS("platform:spi_tegra");
> +
> +static struct platform_driver spi_tegra_driver = {
> +       .driver = {
> +               .name =         "spi_tegra",
> +               .owner =        THIS_MODULE,
> +       },
> +       .remove =       __devexit_p(spi_tegra_remove),
> +};
> +
> +static int __init spi_tegra_init(void)
> +{
> +       return platform_driver_probe(&spi_tegra_driver, spi_tegra_probe);
> +}
> +module_init(spi_tegra_init);
> +
> +static void __exit spi_tegra_exit(void)
> +{
> +       platform_driver_unregister(&spi_tegra_driver);
> +}
> +module_exit(spi_tegra_exit);
> +
> +MODULE_LICENSE("GPL");
> --
> 1.6.5.6
>
>

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

* Re: [PATCH v4] spi: add spi_tegra driver
  2010-09-01 22:16         ` [PATCH v4] " Erik Gilling
  2010-09-01 22:18           ` Erik Gilling
@ 2010-09-02 14:22           ` Grant Likely
       [not found]             ` <AANLkTimonLa1mUVSHR=QnR48HPtAZ5BW3BsVsg7MWALy-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 18+ messages in thread
From: Grant Likely @ 2010-09-02 14:22 UTC (permalink / raw)
  To: Erik Gilling
  Cc: linux-tegra, spi-devel-general, Thierry Reding, Russell King,
	linux-arm-kernel

On Wed, Sep 1, 2010 at 4:16 PM, Erik Gilling <konkers@android.com> wrote:
> v2 changes:
>  from Thierry Reding:
>    * add "select TEGRA_SYSTEM_DMA" to Kconfig
>  from Grant Likely:
>    * add oneline description to header
>    * inline references to DRIVER_NAME
>    * inline references to BUSY_TIMEOUT
>    * open coded bytes_per_word()
>    * spi_readl/writel -> spi_tegra_readl/writel
>    * move transfer validation to spi_tegra_transfer
>    * don't request_mem_region iomem as platform bus does that for us
>    * __exit -> __devexit
>
> v3 changes:
>  from Russell King:
>    * put request_mem_region back int
>  from Grant Likely:
>    * remove #undef DEBUG
>    * add SLINK_ to register bit defines
>    * remove unused bytes_per_word
>    * make spi_tegra_readl/writel static linine
>    * various refactoring for clarity
>    * mark err if BSY bit is not cleared after 1000 retries
>    * move spinlock to protect setting of RDY bit
>    * subsys_initcall -> module_init
>
> v3 changes:
>  from Grant Likely:
>    * update spi_tegra to use PTR_ERRless dma API
>
> Signed-off-by: Erik Gilling <konkers@android.com>
> Cc: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Russell King <linux@arm.linux.org.uk>

Hi Erik,

Go ahead and add my Acked-by line and merge this via the tegra tree to
avoid ordering issues.  I also have a few more minor comments below.

g.

> +       /* FIXME: should probably control CS manually so that we can be sure
> +        * it does not go low between transfer and to support delay_usecs
> +        * correctly.
> +        */

Yes, you'll probably want to revisit this.  A lot of SPI hardware
doesn't understand that the actual transfer may be longer that the
data it immediately knows about.  Also, it is common to use GPIOs as
chip selects.

> +static void spi_tegra_cleanup(struct spi_device *spi)
> +{
> +       dev_dbg(&spi->dev, "cleanup\n");
> +}

Remove the empty hook

> +static int __init spi_tegra_probe(struct platform_device *pdev)
> +{
> +       struct spi_master       *master;
> +       struct spi_tegra_data   *tspi;
> +       struct resource         *r;
> +       int ret;
> +
> +       master = spi_alloc_master(&pdev->dev, sizeof *tspi);
> +       if (master == NULL) {
> +               dev_err(&pdev->dev, "master allocation failed\n");
> +               return -ENOMEM;
> +       }
> +
> +       /* the spi->mode bits understood by this driver: */
> +       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> +
> +       if (pdev->id != -1)
> +               master->bus_num = pdev->id;

even if pdev->id is -1, you probably want to set master->bus_num to is
so that a spi bus number can be dynamically assigned.

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

* Re: [PATCH v4] spi: add spi_tegra driver
       [not found]             ` <AANLkTimonLa1mUVSHR=QnR48HPtAZ5BW3BsVsg7MWALy-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-10-10  6:31               ` Grant Likely
       [not found]                 ` <AANLkTini_PA3J+R4gEP4DzFpcAxHhdmDbcXxOtj2h81O-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Grant Likely @ 2010-10-10  6:31 UTC (permalink / raw)
  To: Erik Gilling
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Russell King,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Thu, Sep 2, 2010 at 8:22 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Wed, Sep 1, 2010 at 4:16 PM, Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> wrote:
>> v2 changes:
>>  from Thierry Reding:
>>    * add "select TEGRA_SYSTEM_DMA" to Kconfig
>>  from Grant Likely:
>>    * add oneline description to header
>>    * inline references to DRIVER_NAME
>>    * inline references to BUSY_TIMEOUT
>>    * open coded bytes_per_word()
>>    * spi_readl/writel -> spi_tegra_readl/writel
>>    * move transfer validation to spi_tegra_transfer
>>    * don't request_mem_region iomem as platform bus does that for us
>>    * __exit -> __devexit
>>
>> v3 changes:
>>  from Russell King:
>>    * put request_mem_region back int
>>  from Grant Likely:
>>    * remove #undef DEBUG
>>    * add SLINK_ to register bit defines
>>    * remove unused bytes_per_word
>>    * make spi_tegra_readl/writel static linine
>>    * various refactoring for clarity
>>    * mark err if BSY bit is not cleared after 1000 retries
>>    * move spinlock to protect setting of RDY bit
>>    * subsys_initcall -> module_init
>>
>> v3 changes:
>>  from Grant Likely:
>>    * update spi_tegra to use PTR_ERRless dma API
>>
>> Signed-off-by: Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>> Cc: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
>> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
>
> Hi Erik,
>
> Go ahead and add my Acked-by line and merge this via the tegra tree to
> avoid ordering issues.  I also have a few more minor comments below.

Hi Erik,

What's the status on this one?  I've not seen it show up in the
for-next branch of the android tegra tree.

g.

------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb

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

* Re: [PATCH v4] spi: add spi_tegra driver
       [not found]                 ` <AANLkTini_PA3J+R4gEP4DzFpcAxHhdmDbcXxOtj2h81O-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-10-11 21:01                   ` Erik Gilling
  0 siblings, 0 replies; 18+ messages in thread
From: Erik Gilling @ 2010-10-11 21:01 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Russell King,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Grant,
    Sorry.  Fell off my radar.   I'll push it this week.

Cheers,
    Erik

On Sat, Oct 9, 2010 at 11:31 PM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Thu, Sep 2, 2010 at 8:22 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Wed, Sep 1, 2010 at 4:16 PM, Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> wrote:
>>> v2 changes:
>>>  from Thierry Reding:
>>>    * add "select TEGRA_SYSTEM_DMA" to Kconfig
>>>  from Grant Likely:
>>>    * add oneline description to header
>>>    * inline references to DRIVER_NAME
>>>    * inline references to BUSY_TIMEOUT
>>>    * open coded bytes_per_word()
>>>    * spi_readl/writel -> spi_tegra_readl/writel
>>>    * move transfer validation to spi_tegra_transfer
>>>    * don't request_mem_region iomem as platform bus does that for us
>>>    * __exit -> __devexit
>>>
>>> v3 changes:
>>>  from Russell King:
>>>    * put request_mem_region back int
>>>  from Grant Likely:
>>>    * remove #undef DEBUG
>>>    * add SLINK_ to register bit defines
>>>    * remove unused bytes_per_word
>>>    * make spi_tegra_readl/writel static linine
>>>    * various refactoring for clarity
>>>    * mark err if BSY bit is not cleared after 1000 retries
>>>    * move spinlock to protect setting of RDY bit
>>>    * subsys_initcall -> module_init
>>>
>>> v3 changes:
>>>  from Grant Likely:
>>>    * update spi_tegra to use PTR_ERRless dma API
>>>
>>> Signed-off-by: Erik Gilling <konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
>>> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>>> Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
>>
>> Hi Erik,
>>
>> Go ahead and add my Acked-by line and merge this via the tegra tree to
>> avoid ordering issues.  I also have a few more minor comments below.
>
> Hi Erik,
>
> What's the status on this one?  I've not seen it show up in the
> for-next branch of the android tegra tree.
>
> g.
>

------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb

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

end of thread, other threads:[~2010-10-11 21:01 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-30  0:37 [PATCH] spi: add spi_tegra driver Colin Cross
2010-07-30  7:54 ` Thierry Reding
     [not found]   ` <20100730075443.GA17814-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
2010-08-02 18:48     ` Erik Gilling
2010-08-01  6:21 ` Grant Likely
     [not found]   ` <AANLkTimUm_wj+gyxTa=X+845kLj1sZ9GF+jsmsBwxnLN-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-08-12  0:31     ` Erik Gilling
2010-08-12 15:24       ` Russell King - ARM Linux
     [not found]         ` <20100812152424.GB31982-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2010-08-12 18:44           ` Erik Gilling
     [not found] ` <1280450224-25118-1-git-send-email-ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2010-08-12  0:47   ` [PATCH v2] " Erik Gilling
     [not found]     ` <1281574023-25160-1-git-send-email-konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
2010-08-12 19:54       ` Grant Likely
     [not found]         ` <AANLkTim4nL+Re=qc55CGS+ksq+ACtGe=vqOeGQ3HbQWM-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-08-25 21:22           ` Erik Gilling
     [not found]             ` <AANLkTincgapUK5ncs_t3ax=1Kt_56GdR_RVwRqdCm5ge-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-08-27 19:26               ` Erik Gilling
     [not found]                 ` <AANLkTin4HxVLt=4VH-NQWhB-p_2EpX_yoTcwEOTJP3m_-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-08-27 20:13                   ` Grant Likely
2010-08-25 21:23       ` [PATCH v3] " Erik Gilling
2010-09-01 22:16         ` [PATCH v4] " Erik Gilling
2010-09-01 22:18           ` Erik Gilling
2010-09-02 14:22           ` Grant Likely
     [not found]             ` <AANLkTimonLa1mUVSHR=QnR48HPtAZ5BW3BsVsg7MWALy-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-10-10  6:31               ` Grant Likely
     [not found]                 ` <AANLkTini_PA3J+R4gEP4DzFpcAxHhdmDbcXxOtj2h81O-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-10-11 21:01                   ` Erik Gilling

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).