| Message ID | 20190919104526.29851-2-philipp.puschmann@emlix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix UART DMA freezes for i.MX SOCs | expand |
Hi Philipp, On Thu, Sep 19, 2019 at 7:45 AM Philipp Puschmann <philipp.puschmann@emlix.com> wrote: > > 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. > > Fixes: broken since start The Fixes tag requires a commit ID like this: Fixes: 1ec1e82f2510 ("dmaengine: Add Freescale i.MX SDMA support") Same applies to the other patch.
Hi Fabio, Am 19.09.19 um 13:37 schrieb Fabio Estevam: > Hi Philipp, > > On Thu, Sep 19, 2019 at 7:45 AM Philipp Puschmann > <philipp.puschmann@emlix.com> wrote: >> >> 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. >> >> Fixes: broken since start > > The Fixes tag requires a commit ID like this: > > Fixes: 1ec1e82f2510 ("dmaengine: Add Freescale i.MX SDMA support") > > Same applies to the other patch. > oh okay, thanks. I will fix this with v4. Regards, Philipp
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(struct sdma_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(struct sdma_channel *sdmac) dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL); spin_lock(&sdmac->vc.lock); + dma_wmb(); + bd->mode.status |= BD_DONE; + if (error) sdmac->status = old_status; }
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. Fixes: broken since start Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com> --- Changelog v3: - use correct dma_wmb() instead of dma_wb() - add fixes tag Changelog v2: - add dma_wb() drivers/dma/imx-sdma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)