Re: [PATCH v2 1/3] dmaengine: imx-sdma: fix buffer ownership
From: Lucas Stach <l.stach@pengutronix.de>
Date: 2019-09-19 10:28:01
Also in:
dmaengine, lkml
Hi Philipp, On Do, 2019-09-19 at 12:23 +0200, Philipp Puschmann wrote:
quoted hunk ↗ jump to hunk
BD_DONE flag marks ownership of the buffer. When 1 SDMA owns the buffer, when 0 ARM owns it. When processing the buffers in sdma_update_channel_loop the ownership of the currently processed buffer was set to SDMA again before running the callback function of the buffer and while the sdma script may be running in parallel. So there was the possibility to get the buffer overwritten by SDMA before it has been processed by kernel leading to kind of random errors in the upper layers, e.g. bluetooth. Signed-off-by: Philipp Puschmann <redacted> --- Changelog v2: - add dma_wb() drivers/dma/imx-sdma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 9ba74ab7e912..e029a2443cfc 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c@@ -802,7 +802,6 @@ static void sdma_update_channel_loop(structsdma_channel *sdmac) */ desc->chn_real_count = bd->mode.count; - bd->mode.status |= BD_DONE; bd->mode.count = desc->period_len; desc->buf_ptail = desc->buf_tail; desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;@@ -817,6 +816,9 @@ static void sdma_update_channel_loop(structsdma_channel *sdmac) dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL); spin_lock(&sdmac->vc.lock); + dma_wb();
Has this change been tested? The function you want here is called dma_wmb(). Regards, Lucas
+ bd->mode.status |= BD_DONE; + if (error) sdmac->status = old_status; }
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel