[PATCH v2 0/4] patch serie for dw_spi driver

[PATCH v2 0/4] patch serie for dw_spi driver

[Feng Tang]

Hi all,

Alek Du made some optimization for the dw_spi driver, the original driver
will wait for non-busy state after every work of read/write. With these
patches, driver will read/write as much as possible data from/to the HW
FIFO. The DMA mode transfer is not affected at all. There is also some
minor code cleanup.

Pls help to review, thanks!

v2 change:
	* fix a typo about bits_per_word checking 
	* re-generate the patches against v2.6.39-rc1

Thanks,
Feng
-------------------------

Alek Du (3):
  spi/dw_spi: remove the un-necessary flush()
  spi/dw_spi: change poll mode transfer from byte ops to batch ops
  spi/dw_spi: improve the interrupt mode with the batch ops

Feng Tang (1):
  spi/dw_spi: unify the low level read/write routines

 drivers/spi/dw_spi.c |  202 ++++++++++++++++---------------------------------
 drivers/spi/dw_spi.h |    2 -
 2 files changed, 66 insertions(+), 138 deletions(-)


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

[PATCH v2 1/4] spi/dw_spi: unify the low level read/write routines

[Feng Tang]

The original version has many duplicated codes for null/u8/u16 case,
so unify them to make it cleaner

Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/dw_spi.c |  103 +++++++++++++-------------------------------------
 drivers/spi/dw_spi.h |    2 -
 2 files changed, 26 insertions(+), 79 deletions(-)

diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 9a61964..c4fca3d 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -58,8 +58,6 @@ struct chip_data {
 	u8 bits_per_word;
 	u16 clk_div;		/* baud rate divider */
 	u32 speed_hz;		/* baud rate */
-	int (*write)(struct dw_spi *dws);
-	int (*read)(struct dw_spi *dws);
 	void (*cs_control)(u32 command);
 };
 
@@ -185,80 +183,45 @@ static void flush(struct dw_spi *dws)
 	wait_till_not_busy(dws);
 }
 
-static int null_writer(struct dw_spi *dws)
-{
-	u8 n_bytes = dws->n_bytes;
-
-	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
-		|| (dws->tx == dws->tx_end))
-		return 0;
-	dw_writew(dws, dr, 0);
-	dws->tx += n_bytes;
 
-	wait_till_not_busy(dws);
-	return 1;
-}
-
-static int null_reader(struct dw_spi *dws)
+static int dw_writer(struct dw_spi *dws)
 {
-	u8 n_bytes = dws->n_bytes;
+	u16 txw = 0;
 
-	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
-		&& (dws->rx < dws->rx_end)) {
-		dw_readw(dws, dr);
-		dws->rx += n_bytes;
-	}
-	wait_till_not_busy(dws);
-	return dws->rx == dws->rx_end;
-}
-
-static int u8_writer(struct dw_spi *dws)
-{
 	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
 		|| (dws->tx == dws->tx_end))
 		return 0;
 
-	dw_writew(dws, dr, *(u8 *)(dws->tx));
-	++dws->tx;
-
-	wait_till_not_busy(dws);
-	return 1;
-}
-
-static int u8_reader(struct dw_spi *dws)
-{
-	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
-		&& (dws->rx < dws->rx_end)) {
-		*(u8 *)(dws->rx) = dw_readw(dws, dr);
-		++dws->rx;
+	/* Set the tx word if the transfer's original "tx" is not null */
+	if (dws->tx_end - dws->len) {
+		if (dws->n_bytes == 1)
+			txw = *(u8 *)(dws->tx);
+		else
+			txw = *(u16 *)(dws->tx);
 	}
 
-	wait_till_not_busy(dws);
-	return dws->rx == dws->rx_end;
-}
-
-static int u16_writer(struct dw_spi *dws)
-{
-	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
-		|| (dws->tx == dws->tx_end))
-		return 0;
-
-	dw_writew(dws, dr, *(u16 *)(dws->tx));
-	dws->tx += 2;
+	dw_writew(dws, dr, txw);
+	dws->tx += dws->n_bytes;
 
 	wait_till_not_busy(dws);
 	return 1;
 }
 
-static int u16_reader(struct dw_spi *dws)
+static int dw_reader(struct dw_spi *dws)
 {
-	u16 temp;
+	u16 rxw;
 
 	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
 		&& (dws->rx < dws->rx_end)) {
-		temp = dw_readw(dws, dr);
-		*(u16 *)(dws->rx) = temp;
-		dws->rx += 2;
+		rxw = dw_readw(dws, dr);
+		/* Care rx only if the transfer's original "rx" is not null */
+		if (dws->rx_end - dws->len) {
+			if (dws->n_bytes == 1)
+				*(u8 *)(dws->rx) = rxw;
+			else
+				*(u16 *)(dws->rx) = rxw;
+		}
+		dws->rx += dws->n_bytes;
 	}
 
 	wait_till_not_busy(dws);
@@ -383,8 +346,8 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
 		left = (left > int_level) ? int_level : left;
 
 		while (left--)
-			dws->write(dws);
-		dws->read(dws);
+			dw_writer(dws);
+		dw_reader(dws);
 
 		/* Re-enable the IRQ if there is still data left to tx */
 		if (dws->tx_end > dws->tx)
@@ -417,13 +380,13 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
 /* Must be called inside pump_transfers() */
 static void poll_transfer(struct dw_spi *dws)
 {
-	while (dws->write(dws))
-		dws->read(dws);
+	while (dw_writer(dws))
+		dw_reader(dws);
 	/*
 	 * There is a possibility that the last word of a transaction
 	 * will be lost if data is not ready. Re-read to solve this issue.
 	 */
-	dws->read(dws);
+	dw_reader(dws);
 
 	dw_spi_xfer_done(dws);
 }
@@ -483,8 +446,6 @@ static void pump_transfers(unsigned long data)
 	dws->tx_end = dws->tx + transfer->len;
 	dws->rx = transfer->rx_buf;
 	dws->rx_end = dws->rx + transfer->len;
-	dws->write = dws->tx ? chip->write : null_writer;
-	dws->read = dws->rx ? chip->read : null_reader;
 	dws->cs_change = transfer->cs_change;
 	dws->len = dws->cur_transfer->len;
 	if (chip != dws->prev_chip)
@@ -520,18 +481,10 @@ static void pump_transfers(unsigned long data)
 		case 8:
 			dws->n_bytes = 1;
 			dws->dma_width = 1;
-			dws->read = (dws->read != null_reader) ?
-					u8_reader : null_reader;
-			dws->write = (dws->write != null_writer) ?
-					u8_writer : null_writer;
 			break;
 		case 16:
 			dws->n_bytes = 2;
 			dws->dma_width = 2;
-			dws->read = (dws->read != null_reader) ?
-					u16_reader : null_reader;
-			dws->write = (dws->write != null_writer) ?
-					u16_writer : null_writer;
 			break;
 		default:
 			printk(KERN_ERR "MRST SPI0: unsupported bits:"
@@ -733,13 +686,9 @@ static int dw_spi_setup(struct spi_device *spi)
 	if (spi->bits_per_word <= 8) {
 		chip->n_bytes = 1;
 		chip->dma_width = 1;
-		chip->read = u8_reader;
-		chip->write = u8_writer;
 	} else if (spi->bits_per_word <= 16) {
 		chip->n_bytes = 2;
 		chip->dma_width = 2;
-		chip->read = u16_reader;
-		chip->write = u16_writer;
 	} else {
 		/* Never take >16b case for MRST SPIC */
 		dev_err(&spi->dev, "invalid wordsize\n");
diff --git a/drivers/spi/dw_spi.h b/drivers/spi/dw_spi.h
index fb0bce5..d8aac1f 100644
--- a/drivers/spi/dw_spi.h
+++ b/drivers/spi/dw_spi.h
@@ -137,8 +137,6 @@ struct dw_spi {
 	u8			max_bits_per_word;	/* maxim is 16b */
 	u32			dma_width;
 	int			cs_change;
-	int			(*write)(struct dw_spi *dws);
-	int			(*read)(struct dw_spi *dws);
 	irqreturn_t		(*transfer_handler)(struct dw_spi *dws);
 	void			(*cs_control)(u32 command);
 
-- 
1.7.0.4


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

[PATCH v2 2/4] spi/dw_spi: remove the un-necessary flush()

[Feng Tang]

From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

The flush() is used to drain all the left data in rx fifo, currently
is is always called together with disabling hw. But from spec, disabling
hw will also reset all the fifo, so flush() is not needed.

Signed-off-by: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/dw_spi.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index c4fca3d..d3aaf8d 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -173,17 +173,6 @@ static void wait_till_not_busy(struct dw_spi *dws)
 		"DW SPI: Status keeps busy for 5000us after a read/write!\n");
 }
 
-static void flush(struct dw_spi *dws)
-{
-	while (dw_readw(dws, sr) & SR_RF_NOT_EMPT) {
-		dw_readw(dws, dr);
-		cpu_relax();
-	}
-
-	wait_till_not_busy(dws);
-}
-
-
 static int dw_writer(struct dw_spi *dws)
 {
 	u16 txw = 0;
@@ -297,8 +286,7 @@ static void giveback(struct dw_spi *dws)
 
 static void int_error_stop(struct dw_spi *dws, const char *msg)
 {
-	/* Stop and reset hw */
-	flush(dws);
+	/* Stop the hw */
 	spi_enable_chip(dws, 0);
 
 	dev_err(&dws->master->dev, "%s\n", msg);
@@ -800,7 +788,6 @@ static void spi_hw_init(struct dw_spi *dws)
 	spi_enable_chip(dws, 0);
 	spi_mask_intr(dws, 0xff);
 	spi_enable_chip(dws, 1);
-	flush(dws);
 
 	/*
 	 * Try to detect the FIFO depth if not set by interface driver,
-- 
1.7.0.4


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

[PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Feng Tang]

From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Current poll transfer will read/write one word, then wait till the
hw is non-busy, it's not efficient. This patch will try to read/write
as many words as permitted by hardware FIFO depth.

Signed-off-by: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/dw_spi.c |   71 +++++++++++++++++++++++++++++++++----------------
 1 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index d3aaf8d..7a2a722 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -160,6 +160,37 @@ static inline void mrst_spi_debugfs_remove(struct dw_spi *dws)
 }
 #endif /* CONFIG_DEBUG_FS */
 
+/* Return the max entries we can fill into tx fifo */
+static inline u32 tx_max(struct dw_spi *dws)
+{
+	u32 tx_left, tx_room, rxtx_gap;
+
+	tx_left = (dws->tx_end - dws->tx) / dws->n_bytes;
+	tx_room = dws->fifo_len - dw_readw(dws, txflr);
+
+	/*
+	 * Another concern is about the tx/rx mismatch, we
+	 * though to use (dws->fifo_len - rxflr - txflr) as
+	 * one maximum value for tx, but it doesn't cover the
+	 * data which is out of tx/rx fifo and inside the
+	 * shift registers. So a control from sw point of
+	 * view is taken.
+	 */
+	rxtx_gap =  ((dws->rx_end - dws->rx) - (dws->tx_end - dws->tx))
+			/ dws->n_bytes;
+
+	return min3(tx_left, tx_room, (u32) (dws->fifo_len - rxtx_gap));
+}
+
+/* Return the max entries we should read out of rx fifo */
+static inline u32 rx_max(struct dw_spi *dws)
+{
+	u32 rx_left = (dws->rx_end - dws->rx) / dws->n_bytes;
+
+	return min(rx_left, (u32)dw_readw(dws, rxflr));
+}
+
+
 static void wait_till_not_busy(struct dw_spi *dws)
 {
 	unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
@@ -175,33 +206,30 @@ static void wait_till_not_busy(struct dw_spi *dws)
 
 static int dw_writer(struct dw_spi *dws)
 {
+	u32 max = tx_max(dws);
 	u16 txw = 0;
 
-	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
-		|| (dws->tx == dws->tx_end))
-		return 0;
-
-	/* Set the tx word if the transfer's original "tx" is not null */
-	if (dws->tx_end - dws->len) {
-		if (dws->n_bytes == 1)
-			txw = *(u8 *)(dws->tx);
-		else
-			txw = *(u16 *)(dws->tx);
+	while (max--) {
+		/* Set the tx word if the transfer's original "tx" is not null */
+		if (dws->tx_end - dws->len) {
+			if (dws->n_bytes == 1)
+				txw = *(u8 *)(dws->tx);
+			else
+				txw = *(u16 *)(dws->tx);
+		}
+		dw_writew(dws, dr, txw);
+		dws->tx += dws->n_bytes;
 	}
 
-	dw_writew(dws, dr, txw);
-	dws->tx += dws->n_bytes;
-
-	wait_till_not_busy(dws);
 	return 1;
 }
 
 static int dw_reader(struct dw_spi *dws)
 {
+	u32 max = rx_max(dws);
 	u16 rxw;
 
-	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
-		&& (dws->rx < dws->rx_end)) {
+	while (max--) {
 		rxw = dw_readw(dws, dr);
 		/* Care rx only if the transfer's original "rx" is not null */
 		if (dws->rx_end - dws->len) {
@@ -213,7 +241,6 @@ static int dw_reader(struct dw_spi *dws)
 		dws->rx += dws->n_bytes;
 	}
 
-	wait_till_not_busy(dws);
 	return dws->rx == dws->rx_end;
 }
 
@@ -368,13 +395,11 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
 /* Must be called inside pump_transfers() */
 static void poll_transfer(struct dw_spi *dws)
 {
-	while (dw_writer(dws))
+	do {
+		dw_writer(dws);
 		dw_reader(dws);
-	/*
-	 * There is a possibility that the last word of a transaction
-	 * will be lost if data is not ready. Re-read to solve this issue.
-	 */
-	dw_reader(dws);
+		cpu_relax();
+	} while (dws->rx_end > dws->rx);
 
 	dw_spi_xfer_done(dws);
 }
-- 
1.7.0.4


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

[PATCH v2 4/4] spi/dw_spi: improve the interrupt mode with the batch ops

[Feng Tang]

From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

leverage the performance gain by change in low level
read/write batch operations

Signed-off-by: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/dw_spi.c |   63 ++++++++++++-------------------------------------
 1 files changed, 16 insertions(+), 47 deletions(-)

diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 7a2a722..855ac4a 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -190,21 +190,7 @@ static inline u32 rx_max(struct dw_spi *dws)
 	return min(rx_left, (u32)dw_readw(dws, rxflr));
 }
 
-
-static void wait_till_not_busy(struct dw_spi *dws)
-{
-	unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
-
-	while (time_before(jiffies, end)) {
-		if (!(dw_readw(dws, sr) & SR_BUSY))
-			return;
-		cpu_relax();
-	}
-	dev_err(&dws->master->dev,
-		"DW SPI: Status keeps busy for 5000us after a read/write!\n");
-}
-
-static int dw_writer(struct dw_spi *dws)
+static void dw_writer(struct dw_spi *dws)
 {
 	u32 max = tx_max(dws);
 	u16 txw = 0;
@@ -220,11 +206,9 @@ static int dw_writer(struct dw_spi *dws)
 		dw_writew(dws, dr, txw);
 		dws->tx += dws->n_bytes;
 	}
-
-	return 1;
 }
 
-static int dw_reader(struct dw_spi *dws)
+static void dw_reader(struct dw_spi *dws)
 {
 	u32 max = rx_max(dws);
 	u16 rxw;
@@ -240,8 +224,6 @@ static int dw_reader(struct dw_spi *dws)
 		}
 		dws->rx += dws->n_bytes;
 	}
-
-	return dws->rx == dws->rx_end;
 }
 
 static void *next_transfer(struct dw_spi *dws)
@@ -340,35 +322,28 @@ EXPORT_SYMBOL_GPL(dw_spi_xfer_done);
 
 static irqreturn_t interrupt_transfer(struct dw_spi *dws)
 {
-	u16 irq_status, irq_mask = 0x3f;
-	u32 int_level = dws->fifo_len / 2;
-	u32 left;
+	u16 irq_status = dw_readw(dws, isr);
 
-	irq_status = dw_readw(dws, isr) & irq_mask;
 	/* Error handling */
 	if (irq_status & (SPI_INT_TXOI | SPI_INT_RXOI | SPI_INT_RXUI)) {
 		dw_readw(dws, txoicr);
 		dw_readw(dws, rxoicr);
 		dw_readw(dws, rxuicr);
-		int_error_stop(dws, "interrupt_transfer: fifo overrun");
+		int_error_stop(dws, "interrupt_transfer: fifo overrun/underrun");
 		return IRQ_HANDLED;
 	}
 
+	dw_reader(dws);
+	if (dws->rx_end == dws->rx) {
+		spi_mask_intr(dws, SPI_INT_TXEI);
+		dw_spi_xfer_done(dws);
+		return IRQ_HANDLED;
+	}
 	if (irq_status & SPI_INT_TXEI) {
 		spi_mask_intr(dws, SPI_INT_TXEI);
-
-		left = (dws->tx_end - dws->tx) / dws->n_bytes;
-		left = (left > int_level) ? int_level : left;
-
-		while (left--)
-			dw_writer(dws);
-		dw_reader(dws);
-
-		/* Re-enable the IRQ if there is still data left to tx */
-		if (dws->tx_end > dws->tx)
-			spi_umask_intr(dws, SPI_INT_TXEI);
-		else
-			dw_spi_xfer_done(dws);
+		dw_writer(dws);
+		/* Enable TX irq always, it will be disabled when RX finished */
+		spi_umask_intr(dws, SPI_INT_TXEI);
 	}
 
 	return IRQ_HANDLED;
@@ -377,15 +352,13 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
 static irqreturn_t dw_spi_irq(int irq, void *dev_id)
 {
 	struct dw_spi *dws = dev_id;
-	u16 irq_status, irq_mask = 0x3f;
+	u16 irq_status = dw_readw(dws, isr) & 0x3f;
 
-	irq_status = dw_readw(dws, isr) & irq_mask;
 	if (!irq_status)
 		return IRQ_NONE;
 
 	if (!dws->cur_msg) {
 		spi_mask_intr(dws, SPI_INT_TXEI);
-		/* Never fail */
 		return IRQ_HANDLED;
 	}
 
@@ -492,12 +465,8 @@ static void pump_transfers(unsigned long data)
 
 		switch (bits) {
 		case 8:
-			dws->n_bytes = 1;
-			dws->dma_width = 1;
-			break;
 		case 16:
-			dws->n_bytes = 2;
-			dws->dma_width = 2;
+			dws->n_bytes = dws->dma_width = bits >> 3;
 			break;
 		default:
 			printk(KERN_ERR "MRST SPI0: unsupported bits:"
@@ -541,7 +510,7 @@ static void pump_transfers(unsigned long data)
 		txint_level = dws->fifo_len / 2;
 		txint_level = (templen > txint_level) ? txint_level : templen;
 
-		imask |= SPI_INT_TXEI;
+		imask |= SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI;
 		dws->transfer_handler = interrupt_transfer;
 	}
 
-- 
1.7.0.4


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 1/4] spi/dw_spi: unify the low level read/write routines

[Grant Likely]

On Wed, Mar 30, 2011 at 11:09:52PM +0800, Feng Tang wrote:
> The original version has many duplicated codes for null/u8/u16 case,
> so unify them to make it cleaner
> 
> Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/dw_spi.c |  103 +++++++++++++-------------------------------------
>  drivers/spi/dw_spi.h |    2 -
>  2 files changed, 26 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index 9a61964..c4fca3d 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -58,8 +58,6 @@ struct chip_data {
>  	u8 bits_per_word;
>  	u16 clk_div;		/* baud rate divider */
>  	u32 speed_hz;		/* baud rate */
> -	int (*write)(struct dw_spi *dws);
> -	int (*read)(struct dw_spi *dws);
>  	void (*cs_control)(u32 command);
>  };
>  
> @@ -185,80 +183,45 @@ static void flush(struct dw_spi *dws)
>  	wait_till_not_busy(dws);
>  }
>  
> -static int null_writer(struct dw_spi *dws)
> -{
> -	u8 n_bytes = dws->n_bytes;
> -
> -	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
> -		|| (dws->tx == dws->tx_end))
> -		return 0;
> -	dw_writew(dws, dr, 0);
> -	dws->tx += n_bytes;
>  
> -	wait_till_not_busy(dws);
> -	return 1;
> -}
> -
> -static int null_reader(struct dw_spi *dws)
> +static int dw_writer(struct dw_spi *dws)
>  {
> -	u8 n_bytes = dws->n_bytes;
> +	u16 txw = 0;
>  
> -	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
> -		&& (dws->rx < dws->rx_end)) {
> -		dw_readw(dws, dr);
> -		dws->rx += n_bytes;
> -	}
> -	wait_till_not_busy(dws);
> -	return dws->rx == dws->rx_end;
> -}
> -
> -static int u8_writer(struct dw_spi *dws)
> -{
>  	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
>  		|| (dws->tx == dws->tx_end))
>  		return 0;
>  
> -	dw_writew(dws, dr, *(u8 *)(dws->tx));
> -	++dws->tx;
> -
> -	wait_till_not_busy(dws);
> -	return 1;
> -}
> -
> -static int u8_reader(struct dw_spi *dws)
> -{
> -	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
> -		&& (dws->rx < dws->rx_end)) {
> -		*(u8 *)(dws->rx) = dw_readw(dws, dr);
> -		++dws->rx;
> +	/* Set the tx word if the transfer's original "tx" is not null */
> +	if (dws->tx_end - dws->len) {
> +		if (dws->n_bytes == 1)
> +			txw = *(u8 *)(dws->tx);
> +		else
> +			txw = *(u16 *)(dws->tx);
>  	}
>  
> -	wait_till_not_busy(dws);
> -	return dws->rx == dws->rx_end;
> -}
> -
> -static int u16_writer(struct dw_spi *dws)
> -{
> -	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
> -		|| (dws->tx == dws->tx_end))
> -		return 0;
> -
> -	dw_writew(dws, dr, *(u16 *)(dws->tx));
> -	dws->tx += 2;
> +	dw_writew(dws, dr, txw);
> +	dws->tx += dws->n_bytes;
>  
>  	wait_till_not_busy(dws);
>  	return 1;
>  }
>  
> -static int u16_reader(struct dw_spi *dws)
> +static int dw_reader(struct dw_spi *dws)
>  {
> -	u16 temp;
> +	u16 rxw;
>  
>  	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
>  		&& (dws->rx < dws->rx_end)) {
> -		temp = dw_readw(dws, dr);
> -		*(u16 *)(dws->rx) = temp;
> -		dws->rx += 2;
> +		rxw = dw_readw(dws, dr);
> +		/* Care rx only if the transfer's original "rx" is not null */
> +		if (dws->rx_end - dws->len) {
> +			if (dws->n_bytes == 1)
> +				*(u8 *)(dws->rx) = rxw;
> +			else
> +				*(u16 *)(dws->rx) = rxw;
> +		}
> +		dws->rx += dws->n_bytes;
>  	}
>  
>  	wait_till_not_busy(dws);
> @@ -383,8 +346,8 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
>  		left = (left > int_level) ? int_level : left;
>  
>  		while (left--)
> -			dws->write(dws);
> -		dws->read(dws);
> +			dw_writer(dws);
> +		dw_reader(dws);
>  
>  		/* Re-enable the IRQ if there is still data left to tx */
>  		if (dws->tx_end > dws->tx)
> @@ -417,13 +380,13 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
>  /* Must be called inside pump_transfers() */
>  static void poll_transfer(struct dw_spi *dws)
>  {
> -	while (dws->write(dws))
> -		dws->read(dws);
> +	while (dw_writer(dws))
> +		dw_reader(dws);
>  	/*
>  	 * There is a possibility that the last word of a transaction
>  	 * will be lost if data is not ready. Re-read to solve this issue.
>  	 */
> -	dws->read(dws);
> +	dw_reader(dws);
>  
>  	dw_spi_xfer_done(dws);
>  }
> @@ -483,8 +446,6 @@ static void pump_transfers(unsigned long data)
>  	dws->tx_end = dws->tx + transfer->len;
>  	dws->rx = transfer->rx_buf;
>  	dws->rx_end = dws->rx + transfer->len;
> -	dws->write = dws->tx ? chip->write : null_writer;
> -	dws->read = dws->rx ? chip->read : null_reader;
>  	dws->cs_change = transfer->cs_change;
>  	dws->len = dws->cur_transfer->len;
>  	if (chip != dws->prev_chip)
> @@ -520,18 +481,10 @@ static void pump_transfers(unsigned long data)
>  		case 8:
>  			dws->n_bytes = 1;
>  			dws->dma_width = 1;
> -			dws->read = (dws->read != null_reader) ?
> -					u8_reader : null_reader;
> -			dws->write = (dws->write != null_writer) ?
> -					u8_writer : null_writer;
>  			break;
>  		case 16:
>  			dws->n_bytes = 2;
>  			dws->dma_width = 2;
> -			dws->read = (dws->read != null_reader) ?
> -					u16_reader : null_reader;
> -			dws->write = (dws->write != null_writer) ?
> -					u16_writer : null_writer;
>  			break;
>  		default:
>  			printk(KERN_ERR "MRST SPI0: unsupported bits:"
> @@ -733,13 +686,9 @@ static int dw_spi_setup(struct spi_device *spi)
>  	if (spi->bits_per_word <= 8) {
>  		chip->n_bytes = 1;
>  		chip->dma_width = 1;
> -		chip->read = u8_reader;
> -		chip->write = u8_writer;
>  	} else if (spi->bits_per_word <= 16) {
>  		chip->n_bytes = 2;
>  		chip->dma_width = 2;
> -		chip->read = u16_reader;
> -		chip->write = u16_writer;
>  	} else {
>  		/* Never take >16b case for MRST SPIC */
>  		dev_err(&spi->dev, "invalid wordsize\n");
> diff --git a/drivers/spi/dw_spi.h b/drivers/spi/dw_spi.h
> index fb0bce5..d8aac1f 100644
> --- a/drivers/spi/dw_spi.h
> +++ b/drivers/spi/dw_spi.h
> @@ -137,8 +137,6 @@ struct dw_spi {
>  	u8			max_bits_per_word;	/* maxim is 16b */
>  	u32			dma_width;
>  	int			cs_change;
> -	int			(*write)(struct dw_spi *dws);
> -	int			(*read)(struct dw_spi *dws);
>  	irqreturn_t		(*transfer_handler)(struct dw_spi *dws);
>  	void			(*cs_control)(u32 command);
>  
> -- 
> 1.7.0.4
> 

------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 2/4] spi/dw_spi: remove the un-necessary flush()

[Grant Likely]

On Wed, Mar 30, 2011 at 11:09:53PM +0800, Feng Tang wrote:
> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> The flush() is used to drain all the left data in rx fifo, currently
> is is always called together with disabling hw. But from spec, disabling
> hw will also reset all the fifo, so flush() is not needed.
> 
> Signed-off-by: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/dw_spi.c |   15 +--------------
>  1 files changed, 1 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index c4fca3d..d3aaf8d 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -173,17 +173,6 @@ static void wait_till_not_busy(struct dw_spi *dws)
>  		"DW SPI: Status keeps busy for 5000us after a read/write!\n");
>  }
>  
> -static void flush(struct dw_spi *dws)
> -{
> -	while (dw_readw(dws, sr) & SR_RF_NOT_EMPT) {
> -		dw_readw(dws, dr);
> -		cpu_relax();
> -	}
> -
> -	wait_till_not_busy(dws);
> -}
> -
> -
>  static int dw_writer(struct dw_spi *dws)
>  {
>  	u16 txw = 0;
> @@ -297,8 +286,7 @@ static void giveback(struct dw_spi *dws)
>  
>  static void int_error_stop(struct dw_spi *dws, const char *msg)
>  {
> -	/* Stop and reset hw */
> -	flush(dws);
> +	/* Stop the hw */
>  	spi_enable_chip(dws, 0);
>  
>  	dev_err(&dws->master->dev, "%s\n", msg);
> @@ -800,7 +788,6 @@ static void spi_hw_init(struct dw_spi *dws)
>  	spi_enable_chip(dws, 0);
>  	spi_mask_intr(dws, 0xff);
>  	spi_enable_chip(dws, 1);
> -	flush(dws);
>  
>  	/*
>  	 * Try to detect the FIFO depth if not set by interface driver,
> -- 
> 1.7.0.4
> 

------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Grant Likely]

On Wed, Mar 30, 2011 at 11:09:54PM +0800, Feng Tang wrote:
> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Current poll transfer will read/write one word, then wait till the
> hw is non-busy, it's not efficient. This patch will try to read/write
> as many words as permitted by hardware FIFO depth.
> 
> Signed-off-by: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/dw_spi.c |   71 +++++++++++++++++++++++++++++++++----------------
>  1 files changed, 48 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index d3aaf8d..7a2a722 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -160,6 +160,37 @@ static inline void mrst_spi_debugfs_remove(struct dw_spi *dws)
>  }
>  #endif /* CONFIG_DEBUG_FS */
>  
> +/* Return the max entries we can fill into tx fifo */
> +static inline u32 tx_max(struct dw_spi *dws)
> +{
> +	u32 tx_left, tx_room, rxtx_gap;
> +
> +	tx_left = (dws->tx_end - dws->tx) / dws->n_bytes;
> +	tx_room = dws->fifo_len - dw_readw(dws, txflr);
> +
> +	/*
> +	 * Another concern is about the tx/rx mismatch, we
> +	 * though to use (dws->fifo_len - rxflr - txflr) as
> +	 * one maximum value for tx, but it doesn't cover the
> +	 * data which is out of tx/rx fifo and inside the
> +	 * shift registers. So a control from sw point of
> +	 * view is taken.
> +	 */
> +	rxtx_gap =  ((dws->rx_end - dws->rx) - (dws->tx_end - dws->tx))
> +			/ dws->n_bytes;
> +
> +	return min3(tx_left, tx_room, (u32) (dws->fifo_len - rxtx_gap));
> +}
> +
> +/* Return the max entries we should read out of rx fifo */
> +static inline u32 rx_max(struct dw_spi *dws)
> +{
> +	u32 rx_left = (dws->rx_end - dws->rx) / dws->n_bytes;
> +
> +	return min(rx_left, (u32)dw_readw(dws, rxflr));
> +}
> +
> +
>  static void wait_till_not_busy(struct dw_spi *dws)
>  {
>  	unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
> @@ -175,33 +206,30 @@ static void wait_till_not_busy(struct dw_spi *dws)
>  
>  static int dw_writer(struct dw_spi *dws)
>  {
> +	u32 max = tx_max(dws);
>  	u16 txw = 0;
>  
> -	if (!(dw_readw(dws, sr) & SR_TF_NOT_FULL)
> -		|| (dws->tx == dws->tx_end))
> -		return 0;
> -
> -	/* Set the tx word if the transfer's original "tx" is not null */
> -	if (dws->tx_end - dws->len) {
> -		if (dws->n_bytes == 1)
> -			txw = *(u8 *)(dws->tx);
> -		else
> -			txw = *(u16 *)(dws->tx);
> +	while (max--) {
> +		/* Set the tx word if the transfer's original "tx" is not null */
> +		if (dws->tx_end - dws->len) {
> +			if (dws->n_bytes == 1)
> +				txw = *(u8 *)(dws->tx);
> +			else
> +				txw = *(u16 *)(dws->tx);
> +		}
> +		dw_writew(dws, dr, txw);
> +		dws->tx += dws->n_bytes;
>  	}
>  
> -	dw_writew(dws, dr, txw);
> -	dws->tx += dws->n_bytes;
> -
> -	wait_till_not_busy(dws);
>  	return 1;
>  }
>  
>  static int dw_reader(struct dw_spi *dws)
>  {
> +	u32 max = rx_max(dws);
>  	u16 rxw;
>  
> -	while ((dw_readw(dws, sr) & SR_RF_NOT_EMPT)
> -		&& (dws->rx < dws->rx_end)) {
> +	while (max--) {
>  		rxw = dw_readw(dws, dr);
>  		/* Care rx only if the transfer's original "rx" is not null */
>  		if (dws->rx_end - dws->len) {
> @@ -213,7 +241,6 @@ static int dw_reader(struct dw_spi *dws)
>  		dws->rx += dws->n_bytes;
>  	}
>  
> -	wait_till_not_busy(dws);
>  	return dws->rx == dws->rx_end;
>  }
>  
> @@ -368,13 +395,11 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
>  /* Must be called inside pump_transfers() */
>  static void poll_transfer(struct dw_spi *dws)
>  {
> -	while (dw_writer(dws))
> +	do {
> +		dw_writer(dws);
>  		dw_reader(dws);
> -	/*
> -	 * There is a possibility that the last word of a transaction
> -	 * will be lost if data is not ready. Re-read to solve this issue.
> -	 */
> -	dw_reader(dws);
> +		cpu_relax();
> +	} while (dws->rx_end > dws->rx);
>  
>  	dw_spi_xfer_done(dws);
>  }
> -- 
> 1.7.0.4
> 

------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 4/4] spi/dw_spi: improve the interrupt mode with the batch ops

[Grant Likely]

On Wed, Mar 30, 2011 at 11:09:55PM +0800, Feng Tang wrote:
> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> leverage the performance gain by change in low level
> read/write batch operations
> 
> Signed-off-by: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/dw_spi.c |   63 ++++++++++++-------------------------------------
>  1 files changed, 16 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index 7a2a722..855ac4a 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -190,21 +190,7 @@ static inline u32 rx_max(struct dw_spi *dws)
>  	return min(rx_left, (u32)dw_readw(dws, rxflr));
>  }
>  
> -
> -static void wait_till_not_busy(struct dw_spi *dws)
> -{
> -	unsigned long end = jiffies + 1 + usecs_to_jiffies(5000);
> -
> -	while (time_before(jiffies, end)) {
> -		if (!(dw_readw(dws, sr) & SR_BUSY))
> -			return;
> -		cpu_relax();
> -	}
> -	dev_err(&dws->master->dev,
> -		"DW SPI: Status keeps busy for 5000us after a read/write!\n");
> -}
> -
> -static int dw_writer(struct dw_spi *dws)
> +static void dw_writer(struct dw_spi *dws)
>  {
>  	u32 max = tx_max(dws);
>  	u16 txw = 0;
> @@ -220,11 +206,9 @@ static int dw_writer(struct dw_spi *dws)
>  		dw_writew(dws, dr, txw);
>  		dws->tx += dws->n_bytes;
>  	}
> -
> -	return 1;
>  }
>  
> -static int dw_reader(struct dw_spi *dws)
> +static void dw_reader(struct dw_spi *dws)
>  {
>  	u32 max = rx_max(dws);
>  	u16 rxw;
> @@ -240,8 +224,6 @@ static int dw_reader(struct dw_spi *dws)
>  		}
>  		dws->rx += dws->n_bytes;
>  	}
> -
> -	return dws->rx == dws->rx_end;
>  }
>  
>  static void *next_transfer(struct dw_spi *dws)
> @@ -340,35 +322,28 @@ EXPORT_SYMBOL_GPL(dw_spi_xfer_done);
>  
>  static irqreturn_t interrupt_transfer(struct dw_spi *dws)
>  {
> -	u16 irq_status, irq_mask = 0x3f;
> -	u32 int_level = dws->fifo_len / 2;
> -	u32 left;
> +	u16 irq_status = dw_readw(dws, isr);
>  
> -	irq_status = dw_readw(dws, isr) & irq_mask;
>  	/* Error handling */
>  	if (irq_status & (SPI_INT_TXOI | SPI_INT_RXOI | SPI_INT_RXUI)) {
>  		dw_readw(dws, txoicr);
>  		dw_readw(dws, rxoicr);
>  		dw_readw(dws, rxuicr);
> -		int_error_stop(dws, "interrupt_transfer: fifo overrun");
> +		int_error_stop(dws, "interrupt_transfer: fifo overrun/underrun");
>  		return IRQ_HANDLED;
>  	}
>  
> +	dw_reader(dws);
> +	if (dws->rx_end == dws->rx) {
> +		spi_mask_intr(dws, SPI_INT_TXEI);
> +		dw_spi_xfer_done(dws);
> +		return IRQ_HANDLED;
> +	}
>  	if (irq_status & SPI_INT_TXEI) {
>  		spi_mask_intr(dws, SPI_INT_TXEI);
> -
> -		left = (dws->tx_end - dws->tx) / dws->n_bytes;
> -		left = (left > int_level) ? int_level : left;
> -
> -		while (left--)
> -			dw_writer(dws);
> -		dw_reader(dws);
> -
> -		/* Re-enable the IRQ if there is still data left to tx */
> -		if (dws->tx_end > dws->tx)
> -			spi_umask_intr(dws, SPI_INT_TXEI);
> -		else
> -			dw_spi_xfer_done(dws);
> +		dw_writer(dws);
> +		/* Enable TX irq always, it will be disabled when RX finished */
> +		spi_umask_intr(dws, SPI_INT_TXEI);
>  	}
>  
>  	return IRQ_HANDLED;
> @@ -377,15 +352,13 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
>  static irqreturn_t dw_spi_irq(int irq, void *dev_id)
>  {
>  	struct dw_spi *dws = dev_id;
> -	u16 irq_status, irq_mask = 0x3f;
> +	u16 irq_status = dw_readw(dws, isr) & 0x3f;
>  
> -	irq_status = dw_readw(dws, isr) & irq_mask;
>  	if (!irq_status)
>  		return IRQ_NONE;
>  
>  	if (!dws->cur_msg) {
>  		spi_mask_intr(dws, SPI_INT_TXEI);
> -		/* Never fail */
>  		return IRQ_HANDLED;
>  	}
>  
> @@ -492,12 +465,8 @@ static void pump_transfers(unsigned long data)
>  
>  		switch (bits) {
>  		case 8:
> -			dws->n_bytes = 1;
> -			dws->dma_width = 1;
> -			break;
>  		case 16:
> -			dws->n_bytes = 2;
> -			dws->dma_width = 2;
> +			dws->n_bytes = dws->dma_width = bits >> 3;
>  			break;
>  		default:
>  			printk(KERN_ERR "MRST SPI0: unsupported bits:"
> @@ -541,7 +510,7 @@ static void pump_transfers(unsigned long data)
>  		txint_level = dws->fifo_len / 2;
>  		txint_level = (templen > txint_level) ? txint_level : templen;
>  
> -		imask |= SPI_INT_TXEI;
> +		imask |= SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI;
>  		dws->transfer_handler = interrupt_transfer;
>  	}
>  
> -- 
> 1.7.0.4
> 

------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Alan Cox]

On Wed, 30 Mar 2011 23:09:54 +0800
Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Current poll transfer will read/write one word, then wait till the
> hw is non-busy, it's not efficient. This patch will try to read/write
> as many words as permitted by hardware FIFO depth.

This is reported to cause a regression on internal testing. It's one of
that set of changes which seems to be breaking the touchscreen.

Should we get that resolved before these go upstream ?

------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Du, Alek]

On Thu, 31 Mar 2011 17:54:48 +0800
Alan Cox <alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:

> On Wed, 30 Mar 2011 23:09:54 +0800
> Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> 
> > From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > 
> > Current poll transfer will read/write one word, then wait till the
> > hw is non-busy, it's not efficient. This patch will try to read/write
> > as many words as permitted by hardware FIFO depth.
> 
> This is reported to cause a regression on internal testing. It's one of
> that set of changes which seems to be breaking the touchscreen.
> 

What's kind of touchscreen? Just do not understand how they are related.
You can see this patch only try to fill the TX FIFO more efficiently.


> Should we get that resolved before these go upstream ?


------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Grant Likely]

On Thu, Mar 31, 2011 at 3:54 AM, Alan Cox <alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Wed, 30 Mar 2011 23:09:54 +0800
> Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
>> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>
>> Current poll transfer will read/write one word, then wait till the
>> hw is non-busy, it's not efficient. This patch will try to read/write
>> as many words as permitted by hardware FIFO depth.
>
> This is reported to cause a regression on internal testing. It's one of
> that set of changes which seems to be breaking the touchscreen.
>
> Should we get that resolved before these go upstream ?

If this has a known/suspected regression, then I would say yes.  I've
got it queued for 2.6.40, but I can easily drop it.

g.

------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself; 
WebMatrix provides all the features you need to develop and 
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf

Re: [PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Feng Tang]

Hi Grant,

On Thu, 31 Mar 2011 21:13:05 +0800
Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:

> On Thu, Mar 31, 2011 at 3:54 AM, Alan Cox <alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> wrote:
> > On Wed, 30 Mar 2011 23:09:54 +0800
> > Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> >
> >> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >>
> >> Current poll transfer will read/write one word, then wait till the
> >> hw is non-busy, it's not efficient. This patch will try to
> >> read/write as many words as permitted by hardware FIFO depth.
> >
> > This is reported to cause a regression on internal testing. It's
> > one of that set of changes which seems to be breaking the
> > touchscreen.
> >
> > Should we get that resolved before these go upstream ?
> 
> If this has a known/suspected regression, then I would say yes.  I've
> got it queued for 2.6.40, but I can easily drop it.

I did some investigation about the regression, it's about a spi touch
screen, which needs some delay between every word of read/write, it works
with old dw_spi driver, whose basic PIO working model is waiting till hw
is not busy for every word operation.

With this optimization, the dw spi driver will try to feed the FIFO as
much as possible. If we enforce the delay in that touch screen driver,
it works again.

So I would say, this patch should be ok to get merged.

Thanks,
Feng

> 
> g.

------------------------------------------------------------------------------
Fulfilling the Lean Software Promise
Lean software platforms are now widely adopted and the benefits have been 
demonstrated beyond question. Learn why your peers are replacing JEE 
containers with lightweight application servers - and what you can gain 
from the move. http://p.sf.net/sfu/vmware-sfemails

Re: [PATCH v2 3/4] spi/dw_spi: change poll mode transfer from byte ops to batch ops

[Du, Alek]

On Mon, 25 Apr 2011 15:46:07 +0800
"Tang, Feng" <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> Hi Grant,
> 
> On Thu, 31 Mar 2011 21:13:05 +0800
> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> 
> > On Thu, Mar 31, 2011 at 3:54 AM, Alan Cox <alan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > wrote:
> > > On Wed, 30 Mar 2011 23:09:54 +0800
> > > Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> > >
> > >> From: Alek Du <alek.du-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > >>
> > >> Current poll transfer will read/write one word, then wait till the
> > >> hw is non-busy, it's not efficient. This patch will try to
> > >> read/write as many words as permitted by hardware FIFO depth.
> > >
> > > This is reported to cause a regression on internal testing. It's
> > > one of that set of changes which seems to be breaking the
> > > touchscreen.
> > >
> > > Should we get that resolved before these go upstream ?
> > 
> > If this has a known/suspected regression, then I would say yes.  I've
> > got it queued for 2.6.40, but I can easily drop it.
> 
> I did some investigation about the regression, it's about a spi touch
> screen, which needs some delay between every word of read/write, it works
> with old dw_spi driver, whose basic PIO working model is waiting till hw
> is not busy for every word operation.
> 
> With this optimization, the dw spi driver will try to feed the FIFO as
> much as possible. If we enforce the delay in that touch screen driver,
> it works again.
> 
> So I would say, this patch should be ok to get merged.

Thanks Feng.

I agree this issue is due to the poor timing control in this touch screen driver.
The SPI layer should do transfer as fast as possible.

Alek
> 
> Thanks,
> Feng
> 
> > 
> > g.


------------------------------------------------------------------------------
Fulfilling the Lean Software Promise
Lean software platforms are now widely adopted and the benefits have been 
demonstrated beyond question. Learn why your peers are replacing JEE 
containers with lightweight application servers - and what you can gain 
from the move. http://p.sf.net/sfu/vmware-sfemails