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 <[email protected]>
---
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
From: Ezequiel Garcia <[email protected]>
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 <[email protected]>
Signed-off-by: Andrew Bresticker <[email protected]>
---
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
From: Ezequiel Garcia <[email protected]>
The driver can be greatly simplified by moving the transfer
finishing and timeout handling to an unprepare_message() callback.
Signed-off-by: Ezequiel Garcia <[email protected]>
Signed-off-by: Andrew Bresticker <[email protected]>
---
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
From: Sifan Naeem <[email protected]>
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 <[email protected]>
Signed-off-by: Andrew Bresticker <[email protected]>
---
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
From: Ezequiel Garcia <[email protected]>
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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
Signed-off-by: Andrew Bresticker <[email protected]>
---
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
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.
On Mon, Apr 06, 2015 at 02:29:04PM -0700, Andrew Bresticker wrote:
> From: Ezequiel Garcia <[email protected]>
>
> 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.
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.
On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
> From: Sifan Naeem <[email protected]>
>
> 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.
On Tue, Apr 7, 2015 at 4:23 AM, Mark Brown <[email protected]> wrote:
> On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
>> From: Sifan Naeem <[email protected]>
>>
>> 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
On Mon, Apr 06, 2015 at 02:29:06PM -0700, Andrew Bresticker wrote:
> From: Sifan Naeem <[email protected]>
>
> 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.