[PATCH 1/5] spi: img-spfi: Limit bit clock to 1/4th of input clock

[PATCH 1/5] spi: img-spfi: Limit bit clock to 1/4th of input clock

[Andrew Bresticker]

Although the SPFI BITCLK divider supports a value of up to 255, only
values up to 128 are usable.  This results in a maximum possible bit
clock rate of 1/4th the input clock rate.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/spi/spi-img-spfi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index e649bc7..7d18bec 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -405,10 +405,10 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 
 	/*
 	 * output = spfi_clk * (BITCLK / 512), where BITCLK must be a
-	 * power of 2 up to 256 (where 255 == 256 since BITCLK is 8 bits)
+	 * power of 2 up to 128
 	 */
-	div = DIV_ROUND_UP(master->max_speed_hz, xfer->speed_hz);
-	div = clamp(512 / (1 << get_count_order(div)), 1, 255);
+	div = DIV_ROUND_UP(clk_get_rate(spfi->spfi_clk), xfer->speed_hz);
+	div = clamp(512 / (1 << get_count_order(div)), 1, 128);
 
 	val = spfi_readl(spfi, SPFI_DEVICE_PARAMETER(spi->chip_select));
 	val &= ~(SPFI_DEVICE_PARAMETER_BITCLK_MASK <<
@@ -594,8 +594,8 @@ static int img_spfi_probe(struct platform_device *pdev)
 	master->num_chipselect = 5;
 	master->dev.of_node = pdev->dev.of_node;
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(8);
-	master->max_speed_hz = clk_get_rate(spfi->spfi_clk);
-	master->min_speed_hz = master->max_speed_hz / 512;
+	master->max_speed_hz = clk_get_rate(spfi->spfi_clk) / 4;
+	master->min_speed_hz = clk_get_rate(spfi->spfi_clk) / 512;
 
 	master->set_cs = img_spfi_set_cs;
 	master->transfer_one = img_spfi_transfer_one;
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 1/5] spi: img-spfi: Limit bit clock to 1/4th of input clock

[Andrew Bresticker]

Although the SPFI BITCLK divider supports a value of up to 255, only
values up to 128 are usable.  This results in a maximum possible bit
clock rate of 1/4th the input clock rate.

Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/spi/spi-img-spfi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index e649bc7..7d18bec 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -405,10 +405,10 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 
 	/*
 	 * output = spfi_clk * (BITCLK / 512), where BITCLK must be a
-	 * power of 2 up to 256 (where 255 == 256 since BITCLK is 8 bits)
+	 * power of 2 up to 128
 	 */
-	div = DIV_ROUND_UP(master->max_speed_hz, xfer->speed_hz);
-	div = clamp(512 / (1 << get_count_order(div)), 1, 255);
+	div = DIV_ROUND_UP(clk_get_rate(spfi->spfi_clk), xfer->speed_hz);
+	div = clamp(512 / (1 << get_count_order(div)), 1, 128);
 
 	val = spfi_readl(spfi, SPFI_DEVICE_PARAMETER(spi->chip_select));
 	val &= ~(SPFI_DEVICE_PARAMETER_BITCLK_MASK <<
@@ -594,8 +594,8 @@ static int img_spfi_probe(struct platform_device *pdev)
 	master->num_chipselect = 5;
 	master->dev.of_node = pdev->dev.of_node;
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(8);
-	master->max_speed_hz = clk_get_rate(spfi->spfi_clk);
-	master->min_speed_hz = master->max_speed_hz / 512;
+	master->max_speed_hz = clk_get_rate(spfi->spfi_clk) / 4;
+	master->min_speed_hz = clk_get_rate(spfi->spfi_clk) / 512;
 
 	master->set_cs = img_spfi_set_cs;
 	master->transfer_one = img_spfi_transfer_one;
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] spi: img-spfi: Implement a prepare_message() callback

[Andrew Bresticker]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

In preparation for switching to using the SPI core's CS GPIO handling,
move setup of the PORT_STATE register, which must be configured before
CS is asserted, to a prepare_message() callback.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/spi/spi-img-spfi.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 7d18bec..b1c0165 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -397,6 +397,25 @@ stop_dma:
 	return -EIO;
 }
 
+static int img_spfi_prepare(struct spi_master *master, struct spi_message *msg)
+{
+	struct img_spfi *spfi = spi_master_get_devdata(master);
+	u32 val;
+
+	val = spfi_readl(spfi, SPFI_PORT_STATE);
+	if (msg->spi->mode & SPI_CPHA)
+		val |= SPFI_PORT_STATE_CK_PHASE(msg->spi->chip_select);
+	else
+		val &= ~SPFI_PORT_STATE_CK_PHASE(msg->spi->chip_select);
+	if (msg->spi->mode & SPI_CPOL)
+		val |= SPFI_PORT_STATE_CK_POL(msg->spi->chip_select);
+	else
+		val &= ~SPFI_PORT_STATE_CK_POL(msg->spi->chip_select);
+	spfi_writel(spfi, val, SPFI_PORT_STATE);
+
+	return 0;
+}
+
 static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 			    struct spi_transfer *xfer)
 {
@@ -434,18 +453,6 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 					      &master->cur_msg->transfers))
 		val |= SPFI_CONTROL_CONTINUE;
 	spfi_writel(spfi, val, SPFI_CONTROL);
-
-	val = spfi_readl(spfi, SPFI_PORT_STATE);
-	if (spi->mode & SPI_CPHA)
-		val |= SPFI_PORT_STATE_CK_PHASE(spi->chip_select);
-	else
-		val &= ~SPFI_PORT_STATE_CK_PHASE(spi->chip_select);
-	if (spi->mode & SPI_CPOL)
-		val |= SPFI_PORT_STATE_CK_POL(spi->chip_select);
-	else
-		val &= ~SPFI_PORT_STATE_CK_POL(spi->chip_select);
-	spfi_writel(spfi, val, SPFI_PORT_STATE);
-
 	spfi_writel(spfi, xfer->len << SPFI_TRANSACTION_TSIZE_SHIFT,
 		    SPFI_TRANSACTION);
 }
@@ -599,6 +606,7 @@ static int img_spfi_probe(struct platform_device *pdev)
 
 	master->set_cs = img_spfi_set_cs;
 	master->transfer_one = img_spfi_transfer_one;
+	master->prepare_message = img_spfi_prepare;
 
 	spfi->tx_ch = dma_request_slave_channel(spfi->dev, "tx");
 	spfi->rx_ch = dma_request_slave_channel(spfi->dev, "rx");
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 3/5] spi: img-spfi: Implement an unprepare_message() callback

[Andrew Bresticker]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

The driver can be greatly simplified by moving the transfer
finishing and timeout handling to an unprepare_message() callback.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/spi/spi-img-spfi.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index b1c0165..30c89f9 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -271,7 +271,6 @@ static int img_spfi_start_pio(struct spi_master *master,
 
 	if (rx_bytes > 0 || tx_bytes > 0) {
 		dev_err(spfi->dev, "PIO transfer timed out\n");
-		spfi_reset(spfi);
 		return -ETIMEDOUT;
 	}
 
@@ -397,6 +396,30 @@ stop_dma:
 	return -EIO;
 }
 
+static int img_spfi_unprepare(struct spi_master *master, struct spi_message *m)
+{
+	struct img_spfi *spfi = spi_master_get_devdata(master);
+	unsigned long flags;
+
+	/*
+	 * Stop all DMA and reset the controller if the previous transaction
+	 * timed-out and never completed it's DMA.
+	 */
+	spin_lock_irqsave(&spfi->lock, flags);
+	if (spfi->tx_dma_busy || spfi->rx_dma_busy) {
+		spfi->tx_dma_busy = false;
+		spfi->rx_dma_busy = false;
+
+		dmaengine_terminate_all(spfi->tx_ch);
+		dmaengine_terminate_all(spfi->rx_ch);
+	}
+	spin_unlock_irqrestore(&spfi->lock, flags);
+
+	spfi_reset(spfi);
+
+	return 0;
+}
+
 static int img_spfi_prepare(struct spi_master *master, struct spi_message *msg)
 {
 	struct img_spfi *spfi = spi_master_get_devdata(master);
@@ -462,8 +485,6 @@ static int img_spfi_transfer_one(struct spi_master *master,
 				 struct spi_transfer *xfer)
 {
 	struct img_spfi *spfi = spi_master_get_devdata(spi->master);
-	bool dma_reset = false;
-	unsigned long flags;
 	int ret;
 
 	if (xfer->len > SPFI_TRANSACTION_TSIZE_MASK) {
@@ -473,23 +494,6 @@ static int img_spfi_transfer_one(struct spi_master *master,
 		return -EINVAL;
 	}
 
-	/*
-	 * Stop all DMA and reset the controller if the previous transaction
-	 * timed-out and never completed it's DMA.
-	 */
-	spin_lock_irqsave(&spfi->lock, flags);
-	if (spfi->tx_dma_busy || spfi->rx_dma_busy) {
-		dev_err(spfi->dev, "SPI DMA still busy\n");
-		dma_reset = true;
-	}
-	spin_unlock_irqrestore(&spfi->lock, flags);
-
-	if (dma_reset) {
-		dmaengine_terminate_all(spfi->tx_ch);
-		dmaengine_terminate_all(spfi->rx_ch);
-		spfi_reset(spfi);
-	}
-
 	img_spfi_config(master, spi, xfer);
 	if (master->can_dma && master->can_dma(master, spi, xfer))
 		ret = img_spfi_start_dma(master, spi, xfer);
@@ -607,6 +611,7 @@ static int img_spfi_probe(struct platform_device *pdev)
 	master->set_cs = img_spfi_set_cs;
 	master->transfer_one = img_spfi_transfer_one;
 	master->prepare_message = img_spfi_prepare;
+	master->unprepare_message = img_spfi_unprepare;
 
 	spfi->tx_ch = dma_request_slave_channel(spfi->dev, "tx");
 	spfi->rx_ch = dma_request_slave_channel(spfi->dev, "rx");
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 4/5] spi: img-spfi: Setup TRANSACTION register before CONTROL register

[Andrew Bresticker]

From: Sifan Naeem <sifan.naeem@imgtec.com>

Setting the transfer length in the TRANSACTION register after the
CONTROL register is programmed causes intermittent timeout issues in
SPFI transfers when using the SPI framework to control the CS GPIO
lines.  To avoid this issue, set transfer length before programming
the CONTROL register.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/spi/spi-img-spfi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 30c89f9..adeaa2f 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -458,6 +458,9 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 	val |= div << SPFI_DEVICE_PARAMETER_BITCLK_SHIFT;
 	spfi_writel(spfi, val, SPFI_DEVICE_PARAMETER(spi->chip_select));
 
+	spfi_writel(spfi, xfer->len << SPFI_TRANSACTION_TSIZE_SHIFT,
+		    SPFI_TRANSACTION);
+
 	val = spfi_readl(spfi, SPFI_CONTROL);
 	val &= ~(SPFI_CONTROL_SEND_DMA | SPFI_CONTROL_GET_DMA);
 	if (xfer->tx_buf)
@@ -476,8 +479,6 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 					      &master->cur_msg->transfers))
 		val |= SPFI_CONTROL_CONTINUE;
 	spfi_writel(spfi, val, SPFI_CONTROL);
-	spfi_writel(spfi, xfer->len << SPFI_TRANSACTION_TSIZE_SHIFT,
-		    SPFI_TRANSACTION);
 }
 
 static int img_spfi_transfer_one(struct spi_master *master,
-- 
2.2.0.rc0.207.ga3a616c

[PATCH 4/5] spi: img-spfi: Setup TRANSACTION register before CONTROL register

[Andrew Bresticker]

From: Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Setting the transfer length in the TRANSACTION register after the
CONTROL register is programmed causes intermittent timeout issues in
SPFI transfers when using the SPI framework to control the CS GPIO
lines.  To avoid this issue, set transfer length before programming
the CONTROL register.

Signed-off-by: Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/spi/spi-img-spfi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 30c89f9..adeaa2f 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -458,6 +458,9 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 	val |= div << SPFI_DEVICE_PARAMETER_BITCLK_SHIFT;
 	spfi_writel(spfi, val, SPFI_DEVICE_PARAMETER(spi->chip_select));
 
+	spfi_writel(spfi, xfer->len << SPFI_TRANSACTION_TSIZE_SHIFT,
+		    SPFI_TRANSACTION);
+
 	val = spfi_readl(spfi, SPFI_CONTROL);
 	val &= ~(SPFI_CONTROL_SEND_DMA | SPFI_CONTROL_GET_DMA);
 	if (xfer->tx_buf)
@@ -476,8 +479,6 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 					      &master->cur_msg->transfers))
 		val |= SPFI_CONTROL_CONTINUE;
 	spfi_writel(spfi, val, SPFI_CONTROL);
-	spfi_writel(spfi, xfer->len << SPFI_TRANSACTION_TSIZE_SHIFT,
-		    SPFI_TRANSACTION);
 }
 
 static int img_spfi_transfer_one(struct spi_master *master,
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] spi: img-spfi: Control CS lines with GPIO

[Andrew Bresticker]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

When the CONTINUE bit is set, the interrupt status we are polling to
identify if a transaction has finished can be sporadic.  Even though
the transfer has finished, the interrupt status may erroneously
indicate that there is still data in the FIFO.  This behaviour causes
random timeouts in large PIO transfers.

Instead of using the CONTINUE bit to control the CS lines, use the SPI
core's CS GPIO handling.  Also, now that the CONTINUE bit is not being
used, we can poll for the ALLDONE interrupt to indicate transfer
completion.

Signed-off-by: Sifan Naeem <sifan.naeem@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
This breaks device-tree backwards compatibility, but all existing
device-trees using this binding have been updated.
---
 .../devicetree/bindings/spi/spi-img-spfi.txt       |  1 +
 drivers/spi/spi-img-spfi.c                         | 92 +++++++++++-----------
 2 files changed, 45 insertions(+), 48 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-img-spfi.txt b/Documentation/devicetree/bindings/spi/spi-img-spfi.txt
index c7dd50f..e02fbf1 100644
--- a/Documentation/devicetree/bindings/spi/spi-img-spfi.txt
+++ b/Documentation/devicetree/bindings/spi/spi-img-spfi.txt
@@ -14,6 +14,7 @@ Required properties:
 - dma-names: Must include the following entries:
   - rx
   - tx
+- cs-gpios: Must specify the GPIOs used for chipselect lines.
 - #address-cells: Must be 1.
 - #size-cells: Must be 0.
 
diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index adeaa2f..a834ca8 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>
+#include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/irq.h>
@@ -122,15 +123,6 @@ static inline void spfi_start(struct img_spfi *spfi)
 	spfi_writel(spfi, val, SPFI_CONTROL);
 }
 
-static inline void spfi_stop(struct img_spfi *spfi)
-{
-	u32 val;
-
-	val = spfi_readl(spfi, SPFI_CONTROL);
-	val &= ~SPFI_CONTROL_SPFI_EN;
-	spfi_writel(spfi, val, SPFI_CONTROL);
-}
-
 static inline void spfi_reset(struct img_spfi *spfi)
 {
 	spfi_writel(spfi, SPFI_CONTROL_SOFT_RESET, SPFI_CONTROL);
@@ -138,20 +130,25 @@ static inline void spfi_reset(struct img_spfi *spfi)
 	spfi_writel(spfi, 0, SPFI_CONTROL);
 }
 
-static void spfi_flush_tx_fifo(struct img_spfi *spfi)
+static int spfi_wait_all_done(struct img_spfi *spfi)
 {
-	unsigned long timeout = jiffies + msecs_to_jiffies(10);
+	unsigned long timeout = jiffies + msecs_to_jiffies(50);
 
-	spfi_writel(spfi, SPFI_INTERRUPT_SDE, SPFI_INTERRUPT_CLEAR);
 	while (time_before(jiffies, timeout)) {
-		if (spfi_readl(spfi, SPFI_INTERRUPT_STATUS) &
-		    SPFI_INTERRUPT_SDE)
-			return;
+		u32 status = spfi_readl(spfi, SPFI_INTERRUPT_STATUS);
+
+		if (status & SPFI_INTERRUPT_ALLDONETRIG) {
+			spfi_writel(spfi, SPFI_INTERRUPT_ALLDONETRIG,
+				    SPFI_INTERRUPT_CLEAR);
+			return 0;
+		}
 		cpu_relax();
 	}
 
-	dev_err(spfi->dev, "Timed out waiting for FIFO to drain\n");
+	dev_err(spfi->dev, "Timed out waiting for transaction to complete\n");
 	spfi_reset(spfi);
+
+	return -ETIMEDOUT;
 }
 
 static unsigned int spfi_pio_write32(struct img_spfi *spfi, const u32 *buf,
@@ -237,6 +234,7 @@ static int img_spfi_start_pio(struct spi_master *master,
 	const void *tx_buf = xfer->tx_buf;
 	void *rx_buf = xfer->rx_buf;
 	unsigned long timeout;
+	int ret;
 
 	if (tx_buf)
 		tx_bytes = xfer->len;
@@ -269,15 +267,15 @@ static int img_spfi_start_pio(struct spi_master *master,
 		cpu_relax();
 	}
 
+	ret = spfi_wait_all_done(spfi);
+	if (ret < 0)
+		return ret;
+
 	if (rx_bytes > 0 || tx_bytes > 0) {
 		dev_err(spfi->dev, "PIO transfer timed out\n");
 		return -ETIMEDOUT;
 	}
 
-	if (tx_buf)
-		spfi_flush_tx_fifo(spfi);
-	spfi_stop(spfi);
-
 	return 0;
 }
 
@@ -286,14 +284,12 @@ static void img_spfi_dma_rx_cb(void *data)
 	struct img_spfi *spfi = data;
 	unsigned long flags;
 
-	spin_lock_irqsave(&spfi->lock, flags);
+	spfi_wait_all_done(spfi);
 
+	spin_lock_irqsave(&spfi->lock, flags);
 	spfi->rx_dma_busy = false;
-	if (!spfi->tx_dma_busy) {
-		spfi_stop(spfi);
+	if (!spfi->tx_dma_busy)
 		spi_finalize_current_transfer(spfi->master);
-	}
-
 	spin_unlock_irqrestore(&spfi->lock, flags);
 }
 
@@ -302,16 +298,12 @@ static void img_spfi_dma_tx_cb(void *data)
 	struct img_spfi *spfi = data;
 	unsigned long flags;
 
-	spfi_flush_tx_fifo(spfi);
+	spfi_wait_all_done(spfi);
 
 	spin_lock_irqsave(&spfi->lock, flags);
-
 	spfi->tx_dma_busy = false;
-	if (!spfi->rx_dma_busy) {
-		spfi_stop(spfi);
+	if (!spfi->rx_dma_busy)
 		spi_finalize_current_transfer(spfi->master);
-	}
-
 	spin_unlock_irqrestore(&spfi->lock, flags);
 }
 
@@ -439,6 +431,25 @@ static int img_spfi_prepare(struct spi_master *master, struct spi_message *msg)
 	return 0;
 }
 
+static int img_spfi_setup(struct spi_device *spi)
+{
+	int ret;
+
+	ret = gpio_request_one(spi->cs_gpio, (spi->mode & SPI_CS_HIGH) ?
+			       GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH,
+			       dev_name(&spi->dev));
+	if (ret)
+		dev_err(&spi->dev, "can't request chipselect gpio %d\n",
+				spi->cs_gpio);
+
+	return ret;
+}
+
+static void img_spfi_cleanup(struct spi_device *spi)
+{
+	gpio_free(spi->cs_gpio);
+}
+
 static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 			    struct spi_transfer *xfer)
 {
@@ -474,10 +485,6 @@ static void img_spfi_config(struct spi_master *master, struct spi_device *spi,
 	else if (xfer->tx_nbits == SPI_NBITS_QUAD &&
 		 xfer->rx_nbits == SPI_NBITS_QUAD)
 		val |= SPFI_CONTROL_TMODE_QUAD << SPFI_CONTROL_TMODE_SHIFT;
-	val &= ~SPFI_CONTROL_CONTINUE;
-	if (!xfer->cs_change && !list_is_last(&xfer->transfer_list,
-					      &master->cur_msg->transfers))
-		val |= SPFI_CONTROL_CONTINUE;
 	spfi_writel(spfi, val, SPFI_CONTROL);
 }
 
@@ -504,17 +511,6 @@ static int img_spfi_transfer_one(struct spi_master *master,
 	return ret;
 }
 
-static void img_spfi_set_cs(struct spi_device *spi, bool enable)
-{
-	struct img_spfi *spfi = spi_master_get_devdata(spi->master);
-	u32 val;
-
-	val = spfi_readl(spfi, SPFI_PORT_STATE);
-	val &= ~(SPFI_PORT_STATE_DEV_SEL_MASK << SPFI_PORT_STATE_DEV_SEL_SHIFT);
-	val |= spi->chip_select << SPFI_PORT_STATE_DEV_SEL_SHIFT;
-	spfi_writel(spfi, val, SPFI_PORT_STATE);
-}
-
 static bool img_spfi_can_dma(struct spi_master *master, struct spi_device *spi,
 			     struct spi_transfer *xfer)
 {
@@ -603,13 +599,13 @@ static int img_spfi_probe(struct platform_device *pdev)
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_TX_DUAL | SPI_RX_DUAL;
 	if (of_property_read_bool(spfi->dev->of_node, "img,supports-quad-mode"))
 		master->mode_bits |= SPI_TX_QUAD | SPI_RX_QUAD;
-	master->num_chipselect = 5;
 	master->dev.of_node = pdev->dev.of_node;
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(8);
 	master->max_speed_hz = clk_get_rate(spfi->spfi_clk) / 4;
 	master->min_speed_hz = clk_get_rate(spfi->spfi_clk) / 512;
 
-	master->set_cs = img_spfi_set_cs;
+	master->setup = img_spfi_setup;
+	master->cleanup = img_spfi_cleanup;
 	master->transfer_one = img_spfi_transfer_one;
 	master->prepare_message = img_spfi_prepare;
 	master->unprepare_message = img_spfi_unprepare;
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH 3/5] spi: img-spfi: Implement an unprepare_message() callback

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:05PM -0700, Andrew Bresticker wrote:

> The driver can be greatly simplified by moving the transfer
> finishing and timeout handling to an unprepare_message() callback.

The timeout handling would actually be better moved to the handle_err()
callback which is provided for that purpose.

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

Re: [PATCH 3/5] spi: img-spfi: Implement an unprepare_message() callback

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:05PM -0700, Andrew Bresticker wrote:

> The driver can be greatly simplified by moving the transfer
> finishing and timeout handling to an unprepare_message() callback.

The timeout handling would actually be better moved to the handle_err()
callback which is provided for that purpose.

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

Re: [PATCH 2/5] spi: img-spfi: Implement a prepare_message() callback

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:04PM -0700, Andrew Bresticker wrote:
> From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> 
> In preparation for switching to using the SPI core's CS GPIO handling,
> move setup of the PORT_STATE register, which must be configured before
> CS is asserted, to a prepare_message() callback.

Applied, thanks.

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

Re: [PATCH 2/5] spi: img-spfi: Implement a prepare_message() callback

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:04PM -0700, Andrew Bresticker wrote:
> From: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> 
> In preparation for switching to using the SPI core's CS GPIO handling,
> move setup of the PORT_STATE register, which must be configured before
> CS is asserted, to a prepare_message() callback.

Applied, thanks.

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

Re: [PATCH 1/5] spi: img-spfi: Limit bit clock to 1/4th of input clock

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:03PM -0700, Andrew Bresticker wrote:
> Although the SPFI BITCLK divider supports a value of up to 255, only
> values up to 128 are usable.  This results in a maximum possible bit
> clock rate of 1/4th the input clock rate.

Applied, thanks.

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

Re: [PATCH 1/5] spi: img-spfi: Limit bit clock to 1/4th of input clock

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:03PM -0700, Andrew Bresticker wrote:
> Although the SPFI BITCLK divider supports a value of up to 255, only
> values up to 128 are usable.  This results in a maximum possible bit
> clock rate of 1/4th the input clock rate.

Applied, thanks.

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

Re: [PATCH 4/5] spi: img-spfi: Setup TRANSACTION register before CONTROL register

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
> From: Sifan Naeem <sifan.naeem@imgtec.com>
> 
> Setting the transfer length in the TRANSACTION register after the
> CONTROL register is programmed causes intermittent timeout issues in
> SPFI transfers when using the SPI framework to control the CS GPIO
> lines.  To avoid this issue, set transfer length before programming
> the CONTROL register.

This is fine but it appears to be a bug fix and therefore should have
been at the start of the series so it could be applied as such and sent
to Linus.  As it is it depends on the refactoring for prepare() which
prevents that, please regenerate against Linus' tree so it can be sent
as a fix.

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

Re: [PATCH 4/5] spi: img-spfi: Setup TRANSACTION register before CONTROL register

[Andrew Bresticker]

On Tue, Apr 7, 2015 at 4:23 AM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
>> From: Sifan Naeem <sifan.naeem@imgtec.com>
>>
>> Setting the transfer length in the TRANSACTION register after the
>> CONTROL register is programmed causes intermittent timeout issues in
>> SPFI transfers when using the SPI framework to control the CS GPIO
>> lines.  To avoid this issue, set transfer length before programming
>> the CONTROL register.
>
> This is fine but it appears to be a bug fix and therefore should have
> been at the start of the series so it could be applied as such and sent
> to Linus.  As it is it depends on the refactoring for prepare() which
> prevents that, please regenerate against Linus' tree so it can be sent
> as a fix.

The bug this patch fixes is only exposed once the driver is converted
to use CS GPIOs (patch 5/5 in this series), so it's not needed unless
that patch is taken.  Really the entire series could be considered
fixes (with patches 2, 3, and 4 being preparatory work for patch 5)
since the switch to using CS GPIOs is to work around the hardware's
buggy CS handling.  That said, support for the first SoC using this
controller (IMG Pistachio) is slated to be merged for 4.1, so I'm not
sure that pulling these fixes into 4.0 is totally necessary.

Thanks,
Andrew

Re: [PATCH 4/5] spi: img-spfi: Setup TRANSACTION register before CONTROL register

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
> From: Sifan Naeem <sifan.naeem@imgtec.com>
> 
> Setting the transfer length in the TRANSACTION register after the
> CONTROL register is programmed causes intermittent timeout issues in
> SPFI transfers when using the SPI framework to control the CS GPIO
> lines.  To avoid this issue, set transfer length before programming
> the CONTROL register.

Applied, thanks.

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

Re: [PATCH 4/5] spi: img-spfi: Setup TRANSACTION register before CONTROL register

[Mark Brown]

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

On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
> From: Sifan Naeem <sifan.naeem-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> 
> Setting the transfer length in the TRANSACTION register after the
> CONTROL register is programmed causes intermittent timeout issues in
> SPFI transfers when using the SPI framework to control the CS GPIO
> lines.  To avoid this issue, set transfer length before programming
> the CONTROL register.

Applied, thanks.

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