* [PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation
@ 2016-11-22 7:01 Sanchayan Maity
2016-11-22 7:01 ` [PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup Sanchayan Maity
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sanchayan Maity @ 2016-11-22 7:01 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: stefan-XLVq0VzYD2Y,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA, Sanchayan Maity
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/linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/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
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup 2016-11-22 7:01 [PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation Sanchayan Maity @ 2016-11-22 7:01 ` Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers Sanchayan Maity 2 siblings, 0 replies; 6+ messages in thread From: Sanchayan Maity @ 2016-11-22 7:01 UTC (permalink / raw) To: broonie Cc: stefan, linux-arm-kernel, linux-kernel, linux-spi, Sanchayan Maity 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 <maitysanchayan@gmail.com> Reviewed-by: Stefan Agner <stefan@agner.ch> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format 2016-11-22 7:01 [PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup Sanchayan Maity @ 2016-11-22 7:01 ` Sanchayan Maity 2016-11-22 17:26 ` Applied "spi: spi-fsl-dspi: Fix continuous selection format" to the spi tree Mark Brown 2016-11-22 7:01 ` [PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers Sanchayan Maity 2 siblings, 1 reply; 6+ messages in thread From: Sanchayan Maity @ 2016-11-22 7:01 UTC (permalink / raw) To: broonie Cc: stefan, linux-arm-kernel, linux-kernel, linux-spi, Sanchayan Maity 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 <maitysanchayan@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Applied "spi: spi-fsl-dspi: Fix continuous selection format" to the spi tree 2016-11-22 7:01 ` [PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format Sanchayan Maity @ 2016-11-22 17:26 ` Mark Brown 0 siblings, 0 replies; 6+ messages in thread From: Mark Brown @ 2016-11-22 17:26 UTC (permalink / raw) To: Sanchayan Maity Cc: Mark Brown, broonie, stefan, linux-arm-kernel, linux-kernel, linux-spi 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 <maitysanchayan@gmail.com> 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 <maitysanchayan@gmail.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers 2016-11-22 7:01 [PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format Sanchayan Maity @ 2016-11-22 7:01 ` Sanchayan Maity 2016-11-22 17:26 ` Applied "spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers" to the spi tree Mark Brown 2 siblings, 1 reply; 6+ messages in thread From: Sanchayan Maity @ 2016-11-22 7:01 UTC (permalink / raw) To: broonie Cc: stefan, linux-arm-kernel, linux-kernel, linux-spi, Sanchayan Maity 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 <maitysanchayan@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Applied "spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers" to the spi tree 2016-11-22 7:01 ` [PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers Sanchayan Maity @ 2016-11-22 17:26 ` Mark Brown 0 siblings, 0 replies; 6+ messages in thread From: Mark Brown @ 2016-11-22 17:26 UTC (permalink / raw) To: Sanchayan Maity Cc: Mark Brown, broonie, stefan, linux-arm-kernel, linux-kernel, linux-spi 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 <maitysanchayan@gmail.com> 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 <maitysanchayan@gmail.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-22 17:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-22 7:01 [PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup Sanchayan Maity 2016-11-22 7:01 ` [PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format Sanchayan Maity 2016-11-22 17:26 ` Applied "spi: spi-fsl-dspi: Fix continuous selection format" to the spi tree Mark Brown 2016-11-22 7:01 ` [PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers Sanchayan Maity 2016-11-22 17:26 ` Applied "spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers" to the spi tree Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).