[PATCH] spi: SuperH MSIOF SPI Master driver

[PATCH] spi: SuperH MSIOF SPI Master driver

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

This patch adds SPI Master support for the SuperH MSIOF
hardware block. Full duplex, spi mode 0-3, active high cs,
3-wire and lsb first should all be supported, but the driver
has so far only been tested with "mmc_spi".

The MSIOF hardware comes with 32-bit FIFOs for receive and
transmit, and this driver simply breaks the SPI messages
into FIFO-sized chunks. The MSIOF hardware manages the pins
for clock, receive and transmit (sck/miso/mosi), but the chip
select pin is managed by software and must be configured as
a regular GPIO pin by the board code.

Performance wise there is still room for improvement, but
on a Ecovec board with the built-in sh7724 MSIOF0 this driver
gets Mini-sd read speeds of about half a megabyte per second.

Future work include better clock setup and merging of 8-bit
transfers into 32-bit words to reduce interrupt load and
improve throughput.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 drivers/spi/Kconfig          |    7 
 drivers/spi/Makefile         |    1 
 drivers/spi/spi.c            |    1 
 drivers/spi/spi_sh_msiof.c   |  675 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/sh_msiof.h |   10 
 5 files changed, 693 insertions(+), 1 deletion(-)

--- 0001/drivers/spi/Kconfig
+++ work/drivers/spi/Kconfig	2009-11-24 20:39:48.000000000 +0900
@@ -215,6 +215,13 @@ config SPI_S3C24XX_GPIO
 	  the inbuilt hardware cannot provide the transfer mode, or
 	  where the board is using non hardware connected pins.
 
+config SPI_SH_MSIOF
+	tristate "SuperH MSIOF SPI controller"
+	depends on SUPERH && HAVE_CLK
+	select SPI_BITBANG
+	help
+	  SPI driver for SuperH MSIOF blocks.
+
 config SPI_SH_SCI
 	tristate "SuperH SCI SPI controller"
 	depends on SUPERH
--- 0001/drivers/spi/Makefile
+++ work/drivers/spi/Makefile	2009-11-24 20:39:48.000000000 +0900
@@ -32,6 +32,7 @@ obj-$(CONFIG_SPI_S3C24XX)		+= spi_s3c24x
 obj-$(CONFIG_SPI_TXX9)			+= spi_txx9.o
 obj-$(CONFIG_SPI_XILINX)		+= xilinx_spi.o
 obj-$(CONFIG_SPI_SH_SCI)		+= spi_sh_sci.o
+obj-$(CONFIG_SPI_SH_MSIOF)		+= spi_sh_msiof.o
 obj-$(CONFIG_SPI_STMP3XXX)		+= spi_stmp.o
 # 	... add above this line ...
 
--- 0001/drivers/spi/spi.c
+++ work/drivers/spi/spi.c	2009-11-24 20:39:48.000000000 +0900
@@ -17,7 +17,6 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
-
 #include <linux/kernel.h>
 #include <linux/device.h>
 #include <linux/init.h>
--- /dev/null
+++ work/drivers/spi/spi_sh_msiof.c	2009-11-24 20:39:49.000000000 +0900
@@ -0,0 +1,675 @@
+/*
+ * SuperH MSIOF SPI Master Interface
+ *
+ * Copyright (c) 2009 Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/completion.h>
+#include <linux/pm_runtime.h>
+#include <linux/gpio.h>
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#include <linux/spi/spi.h>
+#include <linux/spi/spi_bitbang.h>
+#include <linux/spi/sh_msiof.h>
+
+#include <asm/spi.h>
+#include <asm/unaligned.h>
+
+struct sh_msiof_spi_priv {
+	struct spi_bitbang bitbang; /* must be first for spi_bitbang.c */
+	void __iomem *mapbase;
+	struct clk *clk;
+	struct platform_device *pdev;
+	struct sh_msiof_spi_info *info;
+	struct completion done;
+	unsigned long flags;
+	int tx_fifo_size;
+	int rx_fifo_size;
+};
+
+#define TMDR1	0x00
+#define TMDR2	0x04
+#define TMDR3	0x08
+#define RMDR1	0x10
+#define RMDR2	0x14
+#define RMDR3	0x18
+#define TSCR	0x20
+#define RSCR	0x22
+#define CTR	0x28
+#define FCTR	0x30
+#define STR	0x40
+#define IER	0x44
+#define TDR1	0x48
+#define TDR2	0x4c
+#define TFDR	0x50
+#define RDR1	0x58
+#define RDR2	0x5c
+#define RFDR	0x60
+
+#define CTR_TSCKE (1 << 15)
+#define CTR_TFSE  (1 << 14)
+#define CTR_TXE   (1 << 9)
+#define CTR_RXE   (1 << 8)
+
+#define STR_TEOF  (1 << 23)
+#define STR_REOF  (1 << 7)
+
+static unsigned long sh_msiof_read(struct sh_msiof_spi_priv *p, int reg_offs)
+{
+	switch (reg_offs) {
+	case TSCR:
+	case RSCR:
+		return ioread16(p->mapbase + reg_offs);
+	default:
+		return ioread32(p->mapbase + reg_offs);
+	}
+}
+
+static void sh_msiof_write(struct sh_msiof_spi_priv *p, int reg_offs,
+			   unsigned long value)
+{
+	switch (reg_offs) {
+	case TSCR:
+	case RSCR:
+		iowrite16(value, p->mapbase + reg_offs);
+		break;
+	default:
+		iowrite32(value, p->mapbase + reg_offs);
+		break;
+	}
+}
+
+static void sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
+				     unsigned long clr, unsigned long set)
+{
+	unsigned long mask = clr | set;
+	unsigned long data;
+
+	data = sh_msiof_read(p, CTR);
+	data &= ~clr;
+	data |= set;
+	sh_msiof_write(p, CTR, data);
+
+	while ((sh_msiof_read(p, CTR) & mask) != set)
+		;
+}
+
+static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
+{
+	struct sh_msiof_spi_priv *p = data;
+
+	/* just disable the interrupt and wake up */
+	sh_msiof_write(p, IER, 0);
+
+	complete(&p->done);
+
+	return IRQ_HANDLED;
+}
+
+static struct {
+	unsigned short div;
+	unsigned short scr;
+} sh_msiof_spi_clk_table[] = {
+	{ 1, 0x0007 },
+	{ 2, 0x0000 },
+	{ 4, 0x0001 },
+	{ 8, 0x0002 },
+	{ 16, 0x0003 },
+	{ 32, 0x0004 },
+	{ 64, 0x1f00 },
+	{ 128, 0x1f01 },
+	{ 256, 0x1f02 },
+	{ 512, 0x1f03 },
+	{ 1024, 0x1f04 },
+};
+
+static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
+				      unsigned long parent_rate,
+				      unsigned long spi_hz)
+{
+	unsigned long div = 1024;
+	int k;
+
+	if (!spi_hz || !parent_rate)
+		WARN_ON(1);
+	else
+		div = parent_rate / spi_hz;
+
+	/* TODO: make more fine grained */
+
+	for (k = 0; k < ARRAY_SIZE(sh_msiof_spi_clk_table); k++) {
+		if (sh_msiof_spi_clk_table[k].div >= div)
+			break;
+	}
+
+	k = min_t(int, k, ARRAY_SIZE(sh_msiof_spi_clk_table) - 1);
+
+	sh_msiof_write(p, TSCR, sh_msiof_spi_clk_table[k].scr);
+	sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
+}
+
+static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
+				      int cpol, int cpha,
+				      int tx_hi_z, int lsb_first)
+{
+	unsigned long tmp;
+	int edge;
+
+	/*
+	 * CPOL CPHA     TSCKIZ RSCKIZ TEDG REDG(!)
+	 *    0    0         10     10    1    0
+	 *    0    1         10     10    0    1
+	 *    1    0         11     11    0    1
+	 *    1    1         11     11    1    0
+	 *
+	 * (!) Note: REDG is inverted recommended data sheet setting
+	 */
+
+	sh_msiof_write(p, FCTR, 0);
+	sh_msiof_write(p, TMDR1, 0xe2000005 | (lsb_first << 24));
+	sh_msiof_write(p, RMDR1, 0x22000005 | (lsb_first << 24));
+
+	tmp = 0xa0000000;
+	tmp |= cpol << 30; /* TSCKIZ */
+	tmp |= cpol << 28; /* RSCKIZ */
+
+	edge = cpol ? cpha : !cpha;
+
+	tmp |= edge << 27; /* TEDG */
+	tmp |= !edge << 26; /* REDG */
+	tmp |= (tx_hi_z ? 2 : 0) << 22; /* TXDIZ */
+	sh_msiof_write(p, CTR, tmp);
+}
+
+static void sh_msiof_spi_set_mode_regs(struct sh_msiof_spi_priv *p,
+				       const void *tx_buf, void *rx_buf,
+				       int bits, int words)
+{
+	unsigned long dr2;
+
+	dr2 = ((bits - 1) << 24) | ((words - 1) << 16);
+
+	if (tx_buf)
+		sh_msiof_write(p, TMDR2, dr2);
+	else
+		sh_msiof_write(p, TMDR2, dr2 | 1);
+
+	if (rx_buf)
+		sh_msiof_write(p, RMDR2, dr2);
+
+	sh_msiof_write(p, IER, STR_TEOF | STR_REOF);
+}
+
+static void sh_msiof_reset_str(struct sh_msiof_spi_priv *p)
+{
+	sh_msiof_write(p, STR, sh_msiof_read(p, STR));
+}
+
+static void sh_msiof_spi_write_fifo_8(struct sh_msiof_spi_priv *p,
+				      const void *tx_buf, int words, int fs)
+{
+	const unsigned char *buf_8 = tx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		sh_msiof_write(p, TFDR, buf_8[k] << fs);
+}
+
+static void sh_msiof_spi_write_fifo_16(struct sh_msiof_spi_priv *p,
+				       const void *tx_buf, int words, int fs)
+{
+	const unsigned short *buf_16 = tx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		sh_msiof_write(p, TFDR, buf_16[k] << fs);
+}
+
+static void sh_msiof_spi_write_fifo_16u(struct sh_msiof_spi_priv *p,
+					const void *tx_buf, int words, int fs)
+{
+	const unsigned short *buf_16 = tx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		sh_msiof_write(p, TFDR, get_unaligned(&buf_16[k]) << fs);
+}
+
+static void sh_msiof_spi_write_fifo_32(struct sh_msiof_spi_priv *p,
+				       const void *tx_buf, int words, int fs)
+{
+	const unsigned int *buf_32 = tx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		sh_msiof_write(p, TFDR, buf_32[k] << fs);
+}
+
+static void sh_msiof_spi_write_fifo_32u(struct sh_msiof_spi_priv *p,
+					const void *tx_buf, int words, int fs)
+{
+	const unsigned int *buf_32 = tx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		sh_msiof_write(p, TFDR, get_unaligned(&buf_32[k]) << fs);
+}
+
+static void sh_msiof_spi_read_fifo_8(struct sh_msiof_spi_priv *p,
+				     void *rx_buf, int words, int fs)
+{
+	unsigned char *buf_8 = rx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		buf_8[k] = sh_msiof_read(p, RFDR) >> fs;
+}
+
+static void sh_msiof_spi_read_fifo_16(struct sh_msiof_spi_priv *p,
+				      void *rx_buf, int words, int fs)
+{
+	unsigned short *buf_16 = rx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		buf_16[k] = sh_msiof_read(p, RFDR) >> fs;
+}
+
+static void sh_msiof_spi_read_fifo_16u(struct sh_msiof_spi_priv *p,
+				       void *rx_buf, int words, int fs)
+{
+	unsigned short *buf_16 = rx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		put_unaligned(sh_msiof_read(p, RFDR) >> fs, &buf_16[k]);
+}
+
+static void sh_msiof_spi_read_fifo_32(struct sh_msiof_spi_priv *p,
+				      void *rx_buf, int words, int fs)
+{
+	unsigned int *buf_32 = rx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		buf_32[k] = sh_msiof_read(p, RFDR) >> fs;
+}
+
+static void sh_msiof_spi_read_fifo_32u(struct sh_msiof_spi_priv *p,
+				       void *rx_buf, int words, int fs)
+{
+	unsigned int *buf_32 = rx_buf;
+	int k;
+
+	for (k = 0; k < words; k++)
+		put_unaligned(sh_msiof_read(p, RFDR) >> fs, &buf_32[k]);
+}
+
+static int sh_msiof_spi_bits(struct spi_device *spi, struct spi_transfer *t)
+{
+	int bits;
+
+	bits = t ? t->bits_per_word : 0;
+	bits = bits ? bits : spi->bits_per_word;
+
+	return bits;
+}
+
+static unsigned long sh_msiof_spi_hz(struct spi_device *spi,
+				     struct spi_transfer *t)
+{
+	unsigned long hz;
+
+	hz = t ? t->speed_hz : 0;
+	hz = hz ? hz : spi->max_speed_hz;
+
+	return hz;
+}
+
+static int sh_msiof_spi_setup_transfer(struct spi_device *spi,
+				       struct spi_transfer *t)
+{
+	int bits;
+
+	bits = sh_msiof_spi_bits(spi, t);
+
+	if (bits < 8)
+		return -EINVAL;
+
+	if (bits > 32)
+		return -EINVAL;
+
+	/* noting to check hz values against since parent clock is disabled */
+
+	return 0;
+}
+
+static void sh_msiof_spi_chipselect(struct spi_device *spi, int is_on)
+{
+	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
+	int value;
+
+	/* chip select ia active low unless SPI_CS_HIGH is set */
+	if (spi->mode & SPI_CS_HIGH)
+		value = (is_on == BITBANG_CS_ACTIVE) ? 1 : 0;
+	else
+		value = (is_on == BITBANG_CS_ACTIVE) ? 0 : 1;
+
+	if (is_on == BITBANG_CS_ACTIVE) {
+		if (!test_and_set_bit(0, &p->flags)) {
+			pm_runtime_get_sync(&p->pdev->dev);
+			clk_enable(p->clk);
+		}
+
+		/* Configure pins before asserting CS */
+		sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
+					  !!(spi->mode & SPI_CPHA),
+					  !!(spi->mode & SPI_3WIRE),
+					  !!(spi->mode & SPI_LSB_FIRST));
+	}
+
+	/* use spi->controller data for CS (same as spi_gpio) */
+	gpio_set_value((unsigned)spi->controller_data, value);
+
+	if (is_on == BITBANG_CS_INACTIVE) {
+		if (test_and_clear_bit(0, &p->flags)) {
+			clk_disable(p->clk);
+			pm_runtime_put(&p->pdev->dev);
+		}
+	}
+}
+
+static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
+				  void (*tx_fifo)(struct sh_msiof_spi_priv *,
+						  const void *, int, int),
+				  void (*rx_fifo)(struct sh_msiof_spi_priv *,
+						  void *, int, int),
+				  const void *tx_buf, void *rx_buf,
+				  int words, int bits)
+{
+	int fifo_shift;
+
+	/* limit maximum word transfer to rx/tx fifo size */
+	if (tx_buf)
+		words = min_t(int, words, p->tx_fifo_size);
+	if (rx_buf)
+		words = min_t(int, words, p->rx_fifo_size);
+
+	/* the fifo contents need shifting */
+	fifo_shift = 32 - bits;
+
+	/* setup msiof transfer mode registers */
+	sh_msiof_spi_set_mode_regs(p, tx_buf, rx_buf, bits, words);
+
+	/* write tx fifo */
+	if (tx_buf)
+		tx_fifo(p, tx_buf, words, fifo_shift);
+
+	/* setup clock and rx/tx signals */
+	sh_msiof_modify_ctr_wait(p, 0, CTR_TSCKE);
+	if (rx_buf)
+		sh_msiof_modify_ctr_wait(p, 0, CTR_RXE);
+	sh_msiof_modify_ctr_wait(p, 0, CTR_TXE);
+
+	/* start by setting frame bit */
+	INIT_COMPLETION(p->done);
+	sh_msiof_modify_ctr_wait(p, 0, CTR_TFSE);
+
+	/* wait for tx fifo to be emptied / rx fifo to be filled */
+	wait_for_completion(&p->done);
+
+	/* read rx fifo */
+	if (rx_buf)
+		rx_fifo(p, rx_buf, words, fifo_shift);
+
+	/* clear status bits */
+	sh_msiof_reset_str(p);
+
+	/* shut down frame, tx/tx and clock signals */
+	sh_msiof_modify_ctr_wait(p, CTR_TFSE, 0);
+	sh_msiof_modify_ctr_wait(p, CTR_TXE, 0);
+	if (rx_buf)
+		sh_msiof_modify_ctr_wait(p, CTR_RXE, 0);
+	sh_msiof_modify_ctr_wait(p, CTR_TSCKE, 0);
+
+	return words;
+}
+
+static int sh_msiof_spi_txrx(struct spi_device *spi, struct spi_transfer *t)
+{
+	struct sh_msiof_spi_priv *p = spi_master_get_devdata(spi->master);
+	void (*tx_fifo)(struct sh_msiof_spi_priv *, const void *, int, int);
+	void (*rx_fifo)(struct sh_msiof_spi_priv *, void *, int, int);
+	int bits;
+	int bytes_per_word;
+	int bytes_done;
+	int words;
+	int n;
+
+	bits = sh_msiof_spi_bits(spi, t);
+
+	/* setup bytes per word and fifo read/write functions */
+	if (bits <= 8) {
+		bytes_per_word = 1;
+		tx_fifo = sh_msiof_spi_write_fifo_8;
+		rx_fifo = sh_msiof_spi_read_fifo_8;
+	} else if (bits <= 16) {
+		bytes_per_word = 2;
+		if ((unsigned long)t->tx_buf & 0x01)
+			tx_fifo = sh_msiof_spi_write_fifo_16u;
+		else
+			tx_fifo = sh_msiof_spi_write_fifo_16;
+
+		if ((unsigned long)t->rx_buf & 0x01)
+			rx_fifo = sh_msiof_spi_read_fifo_16u;
+		else
+			rx_fifo = sh_msiof_spi_read_fifo_16;
+	} else {
+		bytes_per_word = 4;
+		if ((unsigned long)t->tx_buf & 0x03)
+			tx_fifo = sh_msiof_spi_write_fifo_32u;
+		else
+			tx_fifo = sh_msiof_spi_write_fifo_32;
+
+		if ((unsigned long)t->rx_buf & 0x03)
+			rx_fifo = sh_msiof_spi_read_fifo_32u;
+		else
+			rx_fifo = sh_msiof_spi_read_fifo_32;
+	}
+
+	/* setup clocks (clock already enabled in chipselect()) */
+	sh_msiof_spi_set_clk_regs(p, clk_get_rate(p->clk),
+				  sh_msiof_spi_hz(spi, t));
+
+	/* transfer in fifo sized chunks */
+	words = t->len / bytes_per_word;
+	bytes_done = 0;
+
+	while (bytes_done < t->len) {
+		n = sh_msiof_spi_txrx_once(p, tx_fifo, rx_fifo,
+					   t->tx_buf + bytes_done,
+					   t->rx_buf + bytes_done,
+					   words, bits);
+		bytes_done += n * bytes_per_word;
+		words -= n;
+	}
+
+	return bytes_done;
+}
+
+static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
+				  u32 word, u8 bits)
+{
+	BUG_ON(1); /* unused but needed by bitbang code */
+	return 0;
+}
+
+static int sh_msiof_spi_probe(struct platform_device *pdev)
+{
+	struct resource	*r;
+	struct spi_master *master;
+	struct sh_msiof_spi_priv *p;
+	char clk_name[16];
+	int i;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(struct sh_msiof_spi_priv));
+	if (master == NULL) {
+		dev_err(&pdev->dev, "failed to allocate spi master\n");
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	p = spi_master_get_devdata(master);
+
+	platform_set_drvdata(pdev, p);
+	p->info = pdev->dev.platform_data;
+	init_completion(&p->done);
+
+	snprintf(clk_name, sizeof(clk_name), "msiof%d", pdev->id);
+	p->clk = clk_get(&pdev->dev, clk_name);
+	if (IS_ERR(p->clk)) {
+		dev_err(&pdev->dev, "cannot get clock \"%s\"\n", clk_name);
+		ret = PTR_ERR(p->clk);
+		goto err1;
+	}
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	i = platform_get_irq(pdev, 0);
+	if (!r || i < 0) {
+		dev_err(&pdev->dev, "cannot get platform resources\n");
+		ret = -ENOENT;
+		goto err2;
+	}
+	p->mapbase = ioremap_nocache(r->start, resource_size(r));
+	if (!p->mapbase) {
+		dev_err(&pdev->dev, "unable to ioremap\n");
+		ret = -ENXIO;
+		goto err2;
+	}
+
+	ret = request_irq(i, sh_msiof_spi_irq, IRQF_DISABLED,
+			  dev_name(&pdev->dev), p);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to request irq\n");
+		goto err3;
+	}
+
+	p->pdev = pdev;
+	pm_runtime_enable(&pdev->dev);
+
+	/* The standard version of MSIOF use 64 word FIFOs */
+	p->tx_fifo_size = 64;
+	p->rx_fifo_size = 64;
+
+	/* Platform data may override FIFO sizes */
+	if (p->info->tx_fifo_override)
+		p->tx_fifo_size = p->info->tx_fifo_override;
+	if (p->info->rx_fifo_override)
+		p->rx_fifo_size = p->info->rx_fifo_override;
+
+	/* init master and bitbang code */
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	master->mode_bits |= SPI_LSB_FIRST | SPI_3WIRE;
+	master->flags = 0;
+	master->bus_num = pdev->id;
+	master->num_chipselect = p->info->num_chipselect;
+	master->setup = spi_bitbang_setup;
+	master->cleanup = spi_bitbang_cleanup;
+
+	p->bitbang.master = master;
+	p->bitbang.chipselect = sh_msiof_spi_chipselect;
+	p->bitbang.setup_transfer = sh_msiof_spi_setup_transfer;
+	p->bitbang.txrx_bufs = sh_msiof_spi_txrx;
+	p->bitbang.txrx_word[SPI_MODE_0] = sh_msiof_spi_txrx_word;
+	p->bitbang.txrx_word[SPI_MODE_1] = sh_msiof_spi_txrx_word;
+	p->bitbang.txrx_word[SPI_MODE_2] = sh_msiof_spi_txrx_word;
+	p->bitbang.txrx_word[SPI_MODE_3] = sh_msiof_spi_txrx_word;
+
+	ret = spi_bitbang_start(&p->bitbang);
+	if (ret == 0)
+		return 0;
+
+	pm_runtime_disable(&pdev->dev);
+ err3:
+	iounmap(p->mapbase);
+ err2:
+	clk_put(p->clk);
+ err1:
+	spi_master_put(master);
+ err0:
+	return ret;
+}
+
+static int sh_msiof_spi_remove(struct platform_device *pdev)
+{
+	struct sh_msiof_spi_priv *p = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = spi_bitbang_stop(&p->bitbang);
+	if (!ret) {
+		pm_runtime_disable(&pdev->dev);
+		free_irq(platform_get_irq(pdev, 0), sh_msiof_spi_irq);
+		iounmap(p->mapbase);
+		clk_put(p->clk);
+		spi_master_put(p->bitbang.master);
+	}
+	return ret;
+}
+
+static int sh_msiof_spi_runtime_nop(struct device *dev)
+{
+	/* Runtime PM callback shared between ->runtime_suspend()
+	 * and ->runtime_resume(). Simply returns success.
+	 *
+	 * This driver re-initializes all registers after
+	 * pm_runtime_get_sync() anyway so there is no need
+	 * to save and restore registers here.
+	 */
+	return 0;
+}
+
+static struct dev_pm_ops sh_msiof_spi_dev_pm_ops = {
+	.runtime_suspend = sh_msiof_spi_runtime_nop,
+	.runtime_resume = sh_msiof_spi_runtime_nop,
+};
+
+static struct platform_driver sh_msiof_spi_drv = {
+	.probe		= sh_msiof_spi_probe,
+	.remove		= sh_msiof_spi_remove,
+	.driver		= {
+		.name	= "spi_sh_msiof",
+		.owner		= THIS_MODULE,
+		.pm		= &sh_msiof_spi_dev_pm_ops,
+	},
+};
+
+static int __init sh_msiof_spi_init(void)
+{
+	return platform_driver_register(&sh_msiof_spi_drv);
+}
+module_init(sh_msiof_spi_init);
+
+static void __exit sh_msiof_spi_exit(void)
+{
+	platform_driver_unregister(&sh_msiof_spi_drv);
+}
+module_exit(sh_msiof_spi_exit);
+
+MODULE_DESCRIPTION("SuperH MSIOF SPI Master Interface Driver");
+MODULE_AUTHOR("Magnus Damm");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:spi_sh_msiof");
--- /dev/null
+++ work/include/linux/spi/sh_msiof.h	2009-11-24 20:39:49.000000000 +0900
@@ -0,0 +1,10 @@
+#ifndef __SPI_SH_MSIOF_H__
+#define __SPI_SH_MSIOF_H__
+
+struct sh_msiof_spi_info {
+	int tx_fifo_override;
+	int rx_fifo_override;
+	u16 num_chipselect;
+};
+
+#endif /* __SPI_SH_MSIOF_H__ */

Re: [PATCH] spi: SuperH MSIOF SPI Master driver

[Andrew Morton]

On Tue, 24 Nov 2009 21:55:31 +0900
Magnus Damm <magnus.damm@gmail.com> wrote:

> From: Magnus Damm <damm@opensource.se>
> 
> This patch adds SPI Master support for the SuperH MSIOF
> hardware block. Full duplex, spi mode 0-3, active high cs,
> 3-wire and lsb first should all be supported, but the driver
> has so far only been tested with "mmc_spi".
> 
> The MSIOF hardware comes with 32-bit FIFOs for receive and
> transmit, and this driver simply breaks the SPI messages
> into FIFO-sized chunks. The MSIOF hardware manages the pins
> for clock, receive and transmit (sck/miso/mosi), but the chip
> select pin is managed by software and must be configured as
> a regular GPIO pin by the board code.
> 
> Performance wise there is still room for improvement, but
> on a Ecovec board with the built-in sh7724 MSIOF0 this driver
> gets Mini-sd read speeds of about half a megabyte per second.
> 
> Future work include better clock setup and merging of 8-bit
> transfers into 32-bit words to reduce interrupt load and
> improve throughput.
> 
>
> ...
>
> --- 0001/drivers/spi/spi.c
> +++ work/drivers/spi/spi.c	2009-11-24 20:39:48.000000000 +0900
> @@ -17,7 +17,6 @@
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
> -
>  #include <linux/kernel.h>
>  #include <linux/device.h>
>  #include <linux/init.h>

whoops?

> --- /dev/null
> +++ work/drivers/spi/spi_sh_msiof.c	2009-11-24 20:39:49.000000000 +0900
> @@ -0,0 +1,675 @@
> +/*
> + * SuperH MSIOF SPI Master Interface
> + *
> + * Copyright (c) 2009 Magnus Damm
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/kernel.h>

But not consistently.

> +#include <linux/init.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/completion.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/gpio.h>
> +#include <linux/bitmap.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +#include <linux/spi/spi.h>
> +#include <linux/spi/spi_bitbang.h>
> +#include <linux/spi/sh_msiof.h>
> +
> +#include <asm/spi.h>
> +#include <asm/unaligned.h>
> +
> +struct sh_msiof_spi_priv {
> +	struct spi_bitbang bitbang; /* must be first for spi_bitbang.c */

Well if that's the case then spi_bitbang.c needs smacking.  What causes
this requirement?

> +	void __iomem *mapbase;
> +	struct clk *clk;
> +	struct platform_device *pdev;
> +	struct sh_msiof_spi_info *info;
> +	struct completion done;
> +	unsigned long flags;
> +	int tx_fifo_size;
> +	int rx_fifo_size;
> +};
> +
>
> ...
>
> +static void sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
> +				     unsigned long clr, unsigned long set)
> +{
> +	unsigned long mask = clr | set;
> +	unsigned long data;
> +
> +	data = sh_msiof_read(p, CTR);
> +	data &= ~clr;
> +	data |= set;
> +	sh_msiof_write(p, CTR, data);
> +
> +	while ((sh_msiof_read(p, CTR) & mask) != set)
> +		;

hm, confidence.  No timeout needed here?

> +}
> +
> +static irqreturn_t sh_msiof_spi_irq(int irq, void *data)
> +{
> +	struct sh_msiof_spi_priv *p = data;
> +
> +	/* just disable the interrupt and wake up */
> +	sh_msiof_write(p, IER, 0);
> +
> +	complete(&p->done);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct {
> +	unsigned short div;
> +	unsigned short scr;
> +} sh_msiof_spi_clk_table[] = {
> +	{ 1, 0x0007 },
> +	{ 2, 0x0000 },
> +	{ 4, 0x0001 },
> +	{ 8, 0x0002 },
> +	{ 16, 0x0003 },
> +	{ 32, 0x0004 },
> +	{ 64, 0x1f00 },
> +	{ 128, 0x1f01 },
> +	{ 256, 0x1f02 },
> +	{ 512, 0x1f03 },
> +	{ 1024, 0x1f04 },
> +};

Could be const (to save some .data) btu I think the compiler does that
itself nowadays.

> +static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
> +				      unsigned long parent_rate,
> +				      unsigned long spi_hz)
> +{
> +	unsigned long div = 1024;
> +	int k;
> +
> +	if (!spi_hz || !parent_rate)
> +		WARN_ON(1);
> +	else
> +		div = parent_rate / spi_hz;

This could be more neatly coded as

	if (!WARN_ON(!spi_hz || !parent_rate))
		div = parent_rate / spi_hz;

also, if this warning ever triggers, you won't know whether it was due
to !spi_hx or to !parent_rate, which might make you sad.

Can spi_hz and parent_rate ever be zero here anyway?  If not, zap it. 
If so, why?  Programming bug?

> +	/* TODO: make more fine grained */
> +
> +	for (k = 0; k < ARRAY_SIZE(sh_msiof_spi_clk_table); k++) {
> +		if (sh_msiof_spi_clk_table[k].div >= div)
> +			break;
> +	}
> +
> +	k = min_t(int, k, ARRAY_SIZE(sh_msiof_spi_clk_table) - 1);

actually it's pretty obvious from all the above that k should have been
size_t.

> +	sh_msiof_write(p, TSCR, sh_msiof_spi_clk_table[k].scr);
> +	sh_msiof_write(p, RSCR, sh_msiof_spi_clk_table[k].scr);
> +}
> +
> +static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p,
> +				      int cpol, int cpha,
> +				      int tx_hi_z, int lsb_first)
> +{
> +	unsigned long tmp;
> +	int edge;
> +
> +	/*
> +	 * CPOL CPHA     TSCKIZ RSCKIZ TEDG REDG(!)
> +	 *    0    0         10     10    1    0
> +	 *    0    1         10     10    0    1
> +	 *    1    0         11     11    0    1
> +	 *    1    1         11     11    1    0
> +	 *
> +	 * (!) Note: REDG is inverted recommended data sheet setting
> +	 */
> +
> +	sh_msiof_write(p, FCTR, 0);
> +	sh_msiof_write(p, TMDR1, 0xe2000005 | (lsb_first << 24));
> +	sh_msiof_write(p, RMDR1, 0x22000005 | (lsb_first << 24));
> +
> +	tmp = 0xa0000000;
> +	tmp |= cpol << 30; /* TSCKIZ */
> +	tmp |= cpol << 28; /* RSCKIZ */
> +
> +	edge = cpol ? cpha : !cpha;
> +
> +	tmp |= edge << 27; /* TEDG */
> +	tmp |= !edge << 26; /* REDG */

um, OK.  This mixture of logical-not with bit-operations is a bit woozy
but I don't see any actual bugs there.

> +	tmp |= (tx_hi_z ? 2 : 0) << 22; /* TXDIZ */
> +	sh_msiof_write(p, CTR, tmp);
> +}
> +
>
> ...
>
> +static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
> +				  u32 word, u8 bits)
> +{
> +	BUG_ON(1); /* unused but needed by bitbang code */

	BUG();

> +	return 0;
> +}
> +
>
> ...
>

Re: [PATCH] spi: SuperH MSIOF SPI Master driver

[Magnus Damm]

Hi Andrew,

On Thu, Nov 26, 2009 at 5:11 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 24 Nov 2009 21:55:31 +0900
> Magnus Damm <magnus.damm@gmail.com> wrote:
>> This patch adds SPI Master support for the SuperH MSIOF
>> --- 0001/drivers/spi/spi.c
>> +++ work/drivers/spi/spi.c    2009-11-24 20:39:48.000000000 +0900
>> @@ -17,7 +17,6 @@
>>   * along with this program; if not, write to the Free Software
>>   * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>>   */
>> -
>>  #include <linux/kernel.h>
>>  #include <linux/device.h>
>>  #include <linux/init.h>
>
> whoops?

Yeah, i totally missed that one.

>> --- /dev/null
>> +++ work/drivers/spi/spi_sh_msiof.c   2009-11-24 20:39:49.000000000 +0900
>> @@ -0,0 +1,675 @@
>> +/*
>> + * SuperH MSIOF SPI Master Interface
>> + *
>> + * Copyright (c) 2009 Magnus Damm
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>
> But not consistently.

Never. =)

>> +#include <linux/init.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/completion.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/gpio.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +
>> +#include <linux/spi/spi.h>
>> +#include <linux/spi/spi_bitbang.h>
>> +#include <linux/spi/sh_msiof.h>
>> +
>> +#include <asm/spi.h>
>> +#include <asm/unaligned.h>
>> +
>> +struct sh_msiof_spi_priv {
>> +     struct spi_bitbang bitbang; /* must be first for spi_bitbang.c */
>
> Well if that's the case then spi_bitbang.c needs smacking.  What causes
> this requirement?

The spi_bitbang.c functions spi_bitbang_setup() and
spi_bitbang_transfer() use spi_master_get_devdata() to get the bitbang
data structure. Most drivers want to be able to get a pointer to
private data using spi_master_get_devdata(), so the
bitbang-structure-as-first-member trick is used so the driver code and
the bitbang functions can coexist.

A couple of in-tree drivers do the same thing and store the bitbang
structure as the first member. For instance, have a look at
omap_uwire.c and spi_gpio.c.

Maybe adding a spi_alloc_bitbang_master() function to spi_bitbang.c
that does the bitbang structure allocation behind the scenes would be
a good thing. The bitbang drivers can then use that function instead
of spi_alloc_master(). The bitbang code itself can retrieve the
bitbang structure by doing offset calculations from the master
pointer. That would free up the private pointer.

There may be better ways to do this though...

>> +static void sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
>> +                                  unsigned long clr, unsigned long set)
>> +{
>> +     unsigned long mask = clr | set;
>> +     unsigned long data;
>> +
>> +     data = sh_msiof_read(p, CTR);
>> +     data &= ~clr;
>> +     data |= set;
>> +     sh_msiof_write(p, CTR, data);
>> +
>> +     while ((sh_msiof_read(p, CTR) & mask) != set)
>> +             ;
>
> hm, confidence.  No timeout needed here?

Good plan, I'll add some timeout handling code.

>> +static struct {
>> +     unsigned short div;
>> +     unsigned short scr;
>> +} sh_msiof_spi_clk_table[] = {
>> +     { 1, 0x0007 },
>> +     { 2, 0x0000 },
>> +     { 4, 0x0001 },
>> +     { 8, 0x0002 },
>> +     { 16, 0x0003 },
>> +     { 32, 0x0004 },
>> +     { 64, 0x1f00 },
>> +     { 128, 0x1f01 },
>> +     { 256, 0x1f02 },
>> +     { 512, 0x1f03 },
>> +     { 1024, 0x1f04 },
>> +};
>
> Could be const (to save some .data) btu I think the compiler does that
> itself nowadays.

I'll fix that up.

>> +static void sh_msiof_spi_set_clk_regs(struct sh_msiof_spi_priv *p,
>> +                                   unsigned long parent_rate,
>> +                                   unsigned long spi_hz)
>> +{
>> +     unsigned long div = 1024;
>> +     int k;
>> +
>> +     if (!spi_hz || !parent_rate)
>> +             WARN_ON(1);
>> +     else
>> +             div = parent_rate / spi_hz;
>
> This could be more neatly coded as
>
>        if (!WARN_ON(!spi_hz || !parent_rate))
>                div = parent_rate / spi_hz;
>
> also, if this warning ever triggers, you won't know whether it was due
> to !spi_hx or to !parent_rate, which might make you sad.
>
> Can spi_hz and parent_rate ever be zero here anyway?  If not, zap it.
> If so, why?  Programming bug?

I wanted to fail gracefully if the spi layer or the clock framework
handed us zeroes. They are most likely not supposed to do so, but it
probably doesn't hurt to keep the check. This to catch bugs that may
be around - the spi setup code flow is pretty complex and the clock
framework implementation may vary from processor to processor.

>> +     /* TODO: make more fine grained */
>> +
>> +     for (k = 0; k < ARRAY_SIZE(sh_msiof_spi_clk_table); k++) {
>> +             if (sh_msiof_spi_clk_table[k].div >= div)
>> +                     break;
>> +     }
>> +
>> +     k = min_t(int, k, ARRAY_SIZE(sh_msiof_spi_clk_table) - 1);
>
> actually it's pretty obvious from all the above that k should have been
> size_t.

Cool, will fix.

>> +static u32 sh_msiof_spi_txrx_word(struct spi_device *spi, unsigned nsecs,
>> +                               u32 word, u8 bits)
>> +{
>> +     BUG_ON(1); /* unused but needed by bitbang code */
>
>        BUG();

That makes sense. =)

Thanks for your review!

/ magnus

Re: [PATCH] spi: SuperH MSIOF SPI Master driver

[Paul Mundt]

On Wed, Nov 25, 2009 at 12:11:52PM -0800, Andrew Morton wrote:
> On Tue, 24 Nov 2009 21:55:31 +0900
> Magnus Damm <magnus.damm@gmail.com> wrote:
> > +struct sh_msiof_spi_priv {
> > +	struct spi_bitbang bitbang; /* must be first for spi_bitbang.c */
> 
> Well if that's the case then spi_bitbang.c needs smacking.  What causes
> this requirement?
> 
spi_bitbang causes this requirement by being lazy with regards to
private data stashing. Both the drivers and the bitbang code use
spi_master_get_devdata() for getting their private data, which wraps
in to a dev_get_drvdata().

This needs to be reworked so that struct spi_master has its own private
data pointer which helper code like spi_bitbang can use, while freeing up
the struct device private data pointer so it can be freely used by
drivers as normal.

This is the same sort of private data through casting whimsy that the
framebuffer drivers used to employ, only fortunately that behaviour never
made it out of 2.3.x kernels -- until now!

> > +	void __iomem *mapbase;
> > +	struct clk *clk;
> > +	struct platform_device *pdev;
> > +	struct sh_msiof_spi_info *info;
> > +	struct completion done;
> > +	unsigned long flags;
> > +	int tx_fifo_size;
> > +	int rx_fifo_size;
> > +};
> > +
> >
> > ...
> >
> > +static void sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
> > +				     unsigned long clr, unsigned long set)
> > +{
> > +	unsigned long mask = clr | set;
> > +	unsigned long data;
> > +
> > +	data = sh_msiof_read(p, CTR);
> > +	data &= ~clr;
> > +	data |= set;
> > +	sh_msiof_write(p, CTR, data);
> > +
> > +	while ((sh_msiof_read(p, CTR) & mask) != set)
> > +		;
> 
> hm, confidence.  No timeout needed here?
> 
This definitely needs a timeout, nothing involving SPI inspires
confidence. A cpu_relax() to prevent the compiler from optimizing the
loop out would help, too.

Re: [PATCH] spi: SuperH MSIOF SPI Master driver

[Andrew Morton]

On Thu, 26 Nov 2009 15:43:16 +0900 Paul Mundt <lethal-M7jkjyW5wf5g9hUCZPvPmw@public.gmane.org> wrote:

> > > +static void sh_msiof_modify_ctr_wait(struct sh_msiof_spi_priv *p,
> > > +				     unsigned long clr, unsigned long set)
> > > +{
> > > +	unsigned long mask = clr | set;
> > > +	unsigned long data;
> > > +
> > > +	data = sh_msiof_read(p, CTR);
> > > +	data &= ~clr;
> > > +	data |= set;
> > > +	sh_msiof_write(p, CTR, data);
> > > +
> > > +	while ((sh_msiof_read(p, CTR) & mask) != set)
> > > +		;
> > 
> > hm, confidence.  No timeout needed here?
> > 
> This definitely needs a timeout, nothing involving SPI inspires
> confidence. A cpu_relax() to prevent the compiler from optimizing the
> loop out would help, too.

We generally don't bother with the relax in an IO polling loop
like this.  It involves an IO read/write which the compiler cannot fiddle
with and I believe that CPUs will generally take the opportunity to have
a little snooze while the slow IO operation is happening.

Can't hurt though ;)

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july