From 708f742ea556b306c74ebdd9b8786c784873052f Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Thu, 8 Nov 2018 08:06:10 +0100 Subject: spi: bcm2835: Fix race on DMA termination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit e82b0b3828451c1cd331d9f304c6078fcd43b62e upstream. If a DMA transfer finishes orderly right when spi_transfer_one_message() determines that it has timed out, the callbacks bcm2835_spi_dma_done() and bcm2835_spi_handle_err() race to call dmaengine_terminate_all(), potentially leading to double termination. Prevent by atomically changing the dma_pending flag before calling dmaengine_terminate_all(). Signed-off-by: Lukas Wunner Fixes: 3ecd37edaa2a ("spi: bcm2835: enable dma modes for transfers meeting certain conditions") Cc: stable@vger.kernel.org # v4.2+ Cc: Mathias Duckeck Cc: Frank Pavlic Cc: Martin Sperl Cc: Noralf Trønnes Signed-off-by: Mark Brown Signed-off-by: Greg Kroah-Hartman --- drivers/spi/spi-bcm2835.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/spi/spi-bcm2835.c') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index f35cc10772f6..feb921005382 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -233,10 +233,9 @@ static void bcm2835_spi_dma_done(void *data) * is called the tx-dma must have finished - can't get to this * situation otherwise... */ - dmaengine_terminate_all(master->dma_tx); - - /* mark as no longer pending */ - bs->dma_pending = 0; + if (cmpxchg(&bs->dma_pending, true, false)) { + dmaengine_terminate_all(master->dma_tx); + } /* and mark as completed */; complete(&master->xfer_completion); @@ -617,10 +616,9 @@ static void bcm2835_spi_handle_err(struct spi_master *master, struct bcm2835_spi *bs = spi_master_get_devdata(master); /* if an error occurred and we have an active dma, then terminate */ - if (bs->dma_pending) { + if (cmpxchg(&bs->dma_pending, true, false)) { dmaengine_terminate_all(master->dma_tx); dmaengine_terminate_all(master->dma_rx); - bs->dma_pending = 0; } /* and reset */ bcm2835_spi_reset_hw(master); -- cgit v1.2.3 From a0049c05f7c1c7b4f678e0eae650bb8eaa9a5161 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Thu, 8 Nov 2018 08:06:10 +0100 Subject: spi: bcm2835: Fix book-keeping of DMA termination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit dbc944115eed48af110646992893dc43321368d8 upstream. If submission of a DMA TX transfer succeeds but submission of the corresponding RX transfer does not, the BCM2835 SPI driver terminates the TX transfer but neglects to reset the dma_pending flag to false. Thus, if the next transfer uses interrupt mode (because it is shorter than BCM2835_SPI_DMA_MIN_LENGTH) and runs into a timeout, dmaengine_terminate_all() will be called both for TX (once more) and for RX (which was never started in the first place). Fix it. Signed-off-by: Lukas Wunner Fixes: 3ecd37edaa2a ("spi: bcm2835: enable dma modes for transfers meeting certain conditions") Cc: stable@vger.kernel.org # v4.2+ Cc: Mathias Duckeck Cc: Frank Pavlic Cc: Martin Sperl Cc: Noralf Trønnes Signed-off-by: Mark Brown Signed-off-by: Greg Kroah-Hartman --- drivers/spi/spi-bcm2835.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/spi/spi-bcm2835.c') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index feb921005382..a011189b698c 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -341,6 +341,7 @@ static int bcm2835_spi_transfer_one_dma(struct spi_master *master, if (ret) { /* need to reset on errors */ dmaengine_terminate_all(master->dma_tx); + bs->dma_pending = false; bcm2835_spi_reset_hw(master); return ret; } -- cgit v1.2.3 From 31115f50aa7a09e827c96678e0bab370a8cbda21 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Thu, 8 Nov 2018 08:06:10 +0100 Subject: spi: bcm2835: Avoid finishing transfer prematurely in IRQ mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 56c1723426d3cfd4723bfbfce531d7b38bae6266 upstream. The IRQ handler bcm2835_spi_interrupt() first reads as much as possible from the RX FIFO, then writes as much as possible to the TX FIFO. Afterwards it decides whether the transfer is finished by checking if the TX FIFO is empty. If very few bytes were written to the TX FIFO, they may already have been transmitted by the time the FIFO's emptiness is checked. As a result, the transfer will be declared finished and the chip will be reset without reading the corresponding received bytes from the RX FIFO. The odds of this happening increase with a high clock frequency (such that the TX FIFO drains quickly) and either passing "threadirqs" on the command line or enabling CONFIG_PREEMPT_RT_BASE (such that the IRQ handler may be preempted between filling the TX FIFO and checking its emptiness). Fix by instead checking whether rx_len has reached zero, which means that the transfer has been received in full. This is also more efficient as it avoids one bus read access per interrupt. Note that bcm2835_spi_transfer_one_poll() likewise uses rx_len to determine whether the transfer has finished. Signed-off-by: Lukas Wunner Fixes: e34ff011c70e ("spi: bcm2835: move to the transfer_one driver model") Cc: stable@vger.kernel.org # v4.1+ Cc: Mathias Duckeck Cc: Frank Pavlic Cc: Martin Sperl Cc: Noralf Trønnes Signed-off-by: Mark Brown Signed-off-by: Greg Kroah-Hartman --- drivers/spi/spi-bcm2835.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/spi/spi-bcm2835.c') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index a011189b698c..774161bbcb2e 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -155,8 +155,7 @@ static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id) /* Write as many bytes as possible to FIFO */ bcm2835_wr_fifo(bs); - /* based on flags decide if we can finish the transfer */ - if (bcm2835_rd(bs, BCM2835_SPI_CS) & BCM2835_SPI_CS_DONE) { + if (!bs->rx_len) { /* Transfer complete - reset SPI HW */ bcm2835_spi_reset_hw(master); /* wake up the framework */ -- cgit v1.2.3 From 0b7a78688a4c51c2de363960de1afaa9d0f081e0 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Thu, 29 Nov 2018 15:14:49 +0100 Subject: spi: bcm2835: Unbreak the build of esoteric configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 29bdedfd9cf40e59456110ca417a8cb672ac9b92 upstream. Commit e82b0b382845 ("spi: bcm2835: Fix race on DMA termination") broke the build with COMPILE_TEST=y on arches whose cmpxchg() requires 32-bit operands (xtensa, older arm ISAs). Fix by changing the dma_pending flag's type from bool to unsigned int. Fixes: e82b0b382845 ("spi: bcm2835: Fix race on DMA termination") Signed-off-by: Lukas Wunner Signed-off-by: Mark Brown Cc: Frank Pavlic Cc: Martin Sperl Cc: Noralf Trønnes Cc: Sudip Mukherjee Signed-off-by: Greg Kroah-Hartman --- drivers/spi/spi-bcm2835.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/spi/spi-bcm2835.c') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 774161bbcb2e..25abf2d1732a 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -88,7 +88,7 @@ struct bcm2835_spi { u8 *rx_buf; int tx_len; int rx_len; - bool dma_pending; + unsigned int dma_pending; }; static inline u32 bcm2835_rd(struct bcm2835_spi *bs, unsigned reg) -- cgit v1.2.3