Hello,
This v3 set of patches have fixes for Vybrid SPI DMA
implementation and is rebased on top of latest topic/fsl-dspi.
http://git.kernel.org/cgit/linux/kernel/git/broonie/spi.git/log/?h=topic/fsl-dspi
The patches have been tested on a Toradex Colibri Vybrid VF61 module
and now incoporate feedback from Stefan on version 2 of patchset.
Changes since v2:
1. Drop the patch "Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE"
since it's now applied and rebase the whole patchset
2. Second patch in this series "Fix continuous selection format" now
fixes the issue using an existing function and handling it similar to
existing EOQ mode and also cleaning up nicely the transmit code path.
3. Third patch now just fixes the incorrect freeing of DMA allocated buffers
and drops the minor clean up patch from earlier series completely due to
the clean up from 2 above.
Changes since v1:
1. Place the continuous selection format patch second in order and remove
code duplication
2. Improve the use of curr_xfer_len and instead of converting from bytes
to DMA transfers in every use, do it at a single place. Accordingly change
it's use at other places
3. Code cleanup patch has less to clean with change above
v2:
https://www.spinics.net/lists/arm-kernel/msg543941.html
v1:
http://www.mail-archive.com/[email protected]/msg1274632.html
Thanks & Regards,
Sanchayan.
Sanchayan Maity (3):
spi: spi-fsl-dspi: Fix incorrect DMA setup
spi: spi-fsl-dspi: Fix continuous selection format
spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers
drivers/spi/spi-fsl-dspi.c | 59 +++++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 32 deletions(-)
--
2.10.2
Currently dmaengine_prep_slave_single was being called with length
set to the complete DMA buffer size. This resulted in unwanted bytes
being transferred to the SPI register leading to clock and MOSI lines
having unwanted data even after chip select got deasserted and the
required bytes having been transferred.
While at it also clean up the use of curr_xfer_len which is central
to the DMA setup, from bytes to DMA transfers for every use.
Signed-off-by: Sanchayan Maity <[email protected]>
Reviewed-by: Stefan Agner <[email protected]>
---
drivers/spi/spi-fsl-dspi.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b1ee1f5..911aadb 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -151,6 +151,7 @@ static const struct fsl_dspi_devtype_data ls2085a_data = {
};
struct fsl_dspi_dma {
+ /* Length of transfer in words of DSPI_FIFO_SIZE */
u32 curr_xfer_len;
u32 *tx_dma_buf;
@@ -217,15 +218,13 @@ static void dspi_rx_dma_callback(void *arg)
struct fsl_dspi *dspi = arg;
struct fsl_dspi_dma *dma = dspi->dma;
int rx_word;
- int i, len;
+ int i;
u16 d;
rx_word = is_double_byte_mode(dspi);
- len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
-
if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
- for (i = 0; i < len; i++) {
+ for (i = 0; i < dma->curr_xfer_len; i++) {
d = dspi->dma->rx_dma_buf[i];
rx_word ? (*(u16 *)dspi->rx = d) :
(*(u8 *)dspi->rx = d);
@@ -242,14 +241,12 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
struct device *dev = &dspi->pdev->dev;
int time_left;
int tx_word;
- int i, len;
+ int i;
u16 val;
tx_word = is_double_byte_mode(dspi);
- len = tx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
-
- for (i = 0; i < len - 1; i++) {
+ for (i = 0; i < dma->curr_xfer_len - 1; i++) {
val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
dspi->dma->tx_dma_buf[i] =
SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
@@ -265,7 +262,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
- DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
+ dma->curr_xfer_len *
+ DMA_SLAVE_BUSWIDTH_4_BYTES,
+ DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!dma->tx_desc) {
dev_err(dev, "Not able to get desc for DMA xfer\n");
@@ -281,7 +280,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
dma->rx_dma_phys,
- DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
+ dma->curr_xfer_len *
+ DMA_SLAVE_BUSWIDTH_4_BYTES,
+ DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!dma->rx_desc) {
dev_err(dev, "Not able to get desc for DMA xfer\n");
@@ -328,17 +329,17 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
struct device *dev = &dspi->pdev->dev;
int curr_remaining_bytes;
int bytes_per_buffer;
- int tx_word;
+ int word = 1;
int ret = 0;
- tx_word = is_double_byte_mode(dspi);
+ if (is_double_byte_mode(dspi))
+ word = 2;
curr_remaining_bytes = dspi->len;
+ bytes_per_buffer = DSPI_DMA_BUFSIZE / DSPI_FIFO_SIZE;
while (curr_remaining_bytes) {
/* Check if current transfer fits the DMA buffer */
- dma->curr_xfer_len = curr_remaining_bytes;
- bytes_per_buffer = DSPI_DMA_BUFSIZE /
- (DSPI_FIFO_SIZE / (tx_word ? 2 : 1));
- if (curr_remaining_bytes > bytes_per_buffer)
+ dma->curr_xfer_len = curr_remaining_bytes / word;
+ if (dma->curr_xfer_len > bytes_per_buffer)
dma->curr_xfer_len = bytes_per_buffer;
ret = dspi_next_xfer_dma_submit(dspi);
@@ -347,7 +348,7 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
goto exit;
} else {
- curr_remaining_bytes -= dma->curr_xfer_len;
+ curr_remaining_bytes -= dma->curr_xfer_len * word;
if (curr_remaining_bytes < 0)
curr_remaining_bytes = 0;
dspi->len = curr_remaining_bytes;
--
2.10.2
Buffers allocated with a call to dma_alloc_coherent should be
freed with dma_free_coherent instead of the currently used
devm_kfree.
Signed-off-by: Sanchayan Maity <[email protected]>
---
drivers/spi/spi-fsl-dspi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 8af3151..7ada112 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -420,9 +420,11 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
return 0;
err_slave_config:
- devm_kfree(dev, dma->rx_dma_buf);
+ dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+ dma->rx_dma_buf, dma->rx_dma_phys);
err_rx_dma_buf:
- devm_kfree(dev, dma->tx_dma_buf);
+ dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+ dma->tx_dma_buf, dma->tx_dma_phys);
err_tx_dma_buf:
dma_release_channel(dma->chan_tx);
err_tx_channel:
--
2.10.2
Current DMA implementation was not handling the continuous selection
format viz. SPI chip select would be deasserted even between sequential
serial transfers.
Use existing dspi_data_to_pushr function to restructure the transmit
code path and set or reset the CONT bit on same lines as code path
in EOQ mode does. This correctly implements continuous selection format
while also correcting and cleaning up the transmit code path.
Signed-off-by: Sanchayan Maity <[email protected]>
---
drivers/spi/spi-fsl-dspi.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 911aadb..8af3151 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -196,6 +196,8 @@ struct fsl_dspi {
struct fsl_dspi_dma *dma;
};
+static u32 dspi_data_to_pushr(struct fsl_dspi *dspi, int tx_word);
+
static inline int is_double_byte_mode(struct fsl_dspi *dspi)
{
unsigned int val;
@@ -242,24 +244,15 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
int time_left;
int tx_word;
int i;
- u16 val;
tx_word = is_double_byte_mode(dspi);
- for (i = 0; i < dma->curr_xfer_len - 1; i++) {
- val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
- dspi->dma->tx_dma_buf[i] =
- SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
- SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
- dspi->tx += tx_word + 1;
+ for (i = 0; i < dma->curr_xfer_len; i++) {
+ dspi->dma->tx_dma_buf[i] = dspi_data_to_pushr(dspi, tx_word);
+ if ((dspi->cs_change) && (!dspi->len))
+ dspi->dma->tx_dma_buf[i] &= ~SPI_PUSHR_CONT;
}
- val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
- dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
- SPI_PUSHR_PCS(dspi->cs) |
- SPI_PUSHR_CTAS(0);
- dspi->tx += tx_word + 1;
-
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
dma->curr_xfer_len *
@@ -351,7 +344,6 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
curr_remaining_bytes -= dma->curr_xfer_len * word;
if (curr_remaining_bytes < 0)
curr_remaining_bytes = 0;
- dspi->len = curr_remaining_bytes;
}
}
--
2.10.2
The patch
spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers
has been applied to the spi tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 27d21e9f988e527982a7516fcf411994f498787d Mon Sep 17 00:00:00 2001
From: Sanchayan Maity <[email protected]>
Date: Tue, 22 Nov 2016 12:31:32 +0530
Subject: [PATCH] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated
buffers
Buffers allocated with a call to dma_alloc_coherent should be
freed with dma_free_coherent instead of the currently used
devm_kfree.
Signed-off-by: Sanchayan Maity <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/spi/spi-fsl-dspi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b1ee1f521ba0..22f7ce1279bd 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -427,9 +427,11 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
return 0;
err_slave_config:
- devm_kfree(dev, dma->rx_dma_buf);
+ dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+ dma->rx_dma_buf, dma->rx_dma_phys);
err_rx_dma_buf:
- devm_kfree(dev, dma->tx_dma_buf);
+ dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+ dma->tx_dma_buf, dma->tx_dma_phys);
err_tx_dma_buf:
dma_release_channel(dma->chan_tx);
err_tx_channel:
--
2.10.2
The patch
spi: spi-fsl-dspi: Fix continuous selection format
has been applied to the spi tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From ccf7d8ee3d6eb600338a184e9a192ec1d5aee924 Mon Sep 17 00:00:00 2001
From: Sanchayan Maity <[email protected]>
Date: Tue, 22 Nov 2016 12:31:31 +0530
Subject: [PATCH] spi: spi-fsl-dspi: Fix continuous selection format
Current DMA implementation was not handling the continuous selection
format viz. SPI chip select would be deasserted even between sequential
serial transfers.
Use existing dspi_data_to_pushr function to restructure the transmit
code path and set or reset the CONT bit on same lines as code path
in EOQ mode does. This correctly implements continuous selection format
while also correcting and cleaning up the transmit code path.
Signed-off-by: Sanchayan Maity <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/spi/spi-fsl-dspi.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index cb41c327bd77..7ada112bfd85 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -196,6 +196,8 @@ struct fsl_dspi {
struct fsl_dspi_dma *dma;
};
+static u32 dspi_data_to_pushr(struct fsl_dspi *dspi, int tx_word);
+
static inline int is_double_byte_mode(struct fsl_dspi *dspi)
{
unsigned int val;
@@ -242,24 +244,15 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
int time_left;
int tx_word;
int i;
- u16 val;
tx_word = is_double_byte_mode(dspi);
- for (i = 0; i < dma->curr_xfer_len - 1; i++) {
- val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
- dspi->dma->tx_dma_buf[i] =
- SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
- SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
- dspi->tx += tx_word + 1;
+ for (i = 0; i < dma->curr_xfer_len; i++) {
+ dspi->dma->tx_dma_buf[i] = dspi_data_to_pushr(dspi, tx_word);
+ if ((dspi->cs_change) && (!dspi->len))
+ dspi->dma->tx_dma_buf[i] &= ~SPI_PUSHR_CONT;
}
- val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
- dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
- SPI_PUSHR_PCS(dspi->cs) |
- SPI_PUSHR_CTAS(0);
- dspi->tx += tx_word + 1;
-
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
dma->curr_xfer_len *
@@ -351,7 +344,6 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
curr_remaining_bytes -= dma->curr_xfer_len * word;
if (curr_remaining_bytes < 0)
curr_remaining_bytes = 0;
- dspi->len = curr_remaining_bytes;
}
}
--
2.10.2