[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ad87f175496358adb825240f1de609318ed8204c.camel@pengutronix.de>
Date: Thu, 19 Sep 2019 17:19:39 +0200
From: Jan Lübbe <jlu@...gutronix.de>
To: Philipp Puschmann <philipp.puschmann@...ix.com>,
linux-kernel@...r.kernel.org
Cc: fugang.duan@....com, festevam@...il.com, s.hauer@...gutronix.de,
vkoul@...nel.org, linux-imx@....com, kernel@...gutronix.de,
dan.j.williams@...el.com, yibin.gong@....com, shawnguo@...nel.org,
dmaengine@...r.kernel.or, linux-arm-kernel@...ts.infradead.org,
l.stach@...gutronix.de
Subject: Re: [PATCH v4 2/3] dmaengine: imx-sdma: fix dma freezes
Hi Philipp,
see below...
On Thu, 2019-09-19 at 16:29 +0200, Philipp Puschmann wrote:
> For some years and since many kernel versions there are reports that the
> RX UART SDMA channel stops working at some point. The workaround was to
> disable DMA for RX. This commit tries to fix the problem itself.
>
> Due to its license i wasn't able to debug the sdma script itself but it
> somehow leads to blocking the scheduling of the channel script when a
> running sdma script does not find any free descriptor in the ring to put
> its data into.
>
> If we detect such a potential case we manually restart the channel.
>
> As sdmac->desc is constant we can move desc out of the loop.
>
> Fixes: 1ec1e82f2510 ("dmaengine: Add Freescale i.MX SDMA support")
> Signed-off-by: Philipp Puschmann <philipp.puschmann@...ix.com>
> Reviewed-by: Lucas Stach <l.stach@...gutronix.de>
> ---
>
> Changelog v4:
> - fixed the fixes tag
>
> Changelog v3:
> - use correct dma_wmb() instead of dma_wb()
> - add fixes tag
>
> Changelog v2:
> - clarify comment and commit description
>
> drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index e029a2443cfc..a32b5962630e 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -775,21 +775,23 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
> static void sdma_update_channel_loop(struct sdma_channel *sdmac)
> {
> struct sdma_buffer_descriptor *bd;
> - int error = 0;
> - enum dma_status old_status = sdmac->status;
> + struct sdma_desc *desc = sdmac->desc;
> + int error = 0, cnt = 0;
> + enum dma_status old_status = sdmac->status;
>
> /*
> * loop mode. Iterate over descriptors, re-setup them and
> * call callback function.
> */
> - while (sdmac->desc) {
> - struct sdma_desc *desc = sdmac->desc;
> + while (desc) {
>
> bd = &desc->bd[desc->buf_tail];
>
> if (bd->mode.status & BD_DONE)
> break;
>
> + cnt++;
> +
> if (bd->mode.status & BD_RROR) {
> bd->mode.status &= ~BD_RROR;
> sdmac->status = DMA_ERROR;
> @@ -822,6 +824,17 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
> if (error)
> sdmac->status = old_status;
> }
> +
> + /* In some situations it may happen that the sdma does not found any
^ hasn't
> + * usable descriptor in the ring to put data into. The channel is
> + * stopped then. While there is no specific error condition we can
> + * check for, a necessary condition is that all available buffers for
> + * the current channel have been written to by the sdma script. In
> + * this case and after we have made the buffers available again,
> + * we restart the channel.
> + */
Are you sure we can't miss cases where we only had to make some buffers
available again, but the SDMA already ran out of buffers before?
A while ago, I was debugging a similar issue triggered by receiving
data with a wrong baud rate, which leads to all descriptors being
marked with the error flag very quickly (and the SDMA stalling).
I noticed that you can check if the channel is still running by
checking the SDMA_H_STATSTOP register & BIT(sdmac->channel).
I also added a flag for the sdmac->flags field to allow stopping the
channel from the callback (otherwise it would enable the channel
again).
Attached is my current version of that patch for reference.
> + if (cnt >= desc->num_bd)
> + sdma_enable_channel(sdmac->sdma, sdmac->channel);
> }
>
> static void mxc_sdma_handle_channel_normal(struct sdma_channel *data)
View attachment "0001-dmaengine-imx-sdma-restart-stopped-cyclic-transfers.patch" of type "text/x-patch" (2949 bytes)
Powered by blists - more mailing lists