On 2020/01/29 Martin Fuzzey <martin.fuzzey@flowbird.group> wrote:
>
> There is a DMA problem with the serial ports on i.MX6.
>
> When the following sequence is performed:
>
> 1) Open a port
> 2) Write some data
> 3) Close the port
> 4) Open a *different* port
> 5) Write some data
> 6) Close the port
>
> The second write sends nothing and the second close hangs.
> If the first close() is omitted it works.
>
> Adding logs to the the UART driver shows that the DMA is being setup but the
> callback is never invoked for the second write.
>
> This used to work in 4.19.
>
> Git bisect leads to:
> ad0d92d: "dmaengine: imx-sdma: refine to load context only once"
>
> This commit adds a "context_loaded" flag used to avoid unnecessary context
> setups.
> However the flag is only reset in sdma_channel_terminate_work(), which is
> only invoked in a worker triggered by sdma_terminate_all() IF there is an active
> descriptor.
>
> So, if no active descriptor remains when the channel is terminated, the flag is
> not reset and, when the channel is later reused the old context is used.
>
> Fix the problem by always resetting the flag in sdma_free_chan_resources().
>
> Fixes: ad0d92d: "dmaengine: imx-sdma: refine to load context only once"
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
>
> ---
>
> The following python script may be used to reproduce the problem:
>
> import re, serial, sys
>
> ports=(0, 4) # Can be any ports not used (no need to connect anything) NOT
> console...
>
> def get_tx_counts():
> pattern = re.compile("(\d+):.*tx:(\d+).*")
> tx_counts = {}
> with open("/proc/tty/driver/IMX-uart", "r") as f:
> for line in f:
> match = pattern.match(line)
> if match:
> tx_counts[int(match.group(1))] =
> int(match.group(2))
> return tx_counts
>
> before = get_tx_counts()
>
> a = serial.Serial("/dev/ttymxc%d" % ports[0])
> a.write("polop")
> a.close()
> b = serial.Serial("/dev/ttymxc%d" % ports[1])
> b.write("test")
>
> after = get_tx_counts()
>
> if (after[ports[0]] - before[ports[0]] > 0) and (after[ports[1]] - before[ports[1]] >
> 0):
> print "PASS"
> sys.exit(0)
> else:
> print "FAIL"
> print "Before: %s" % before
> print "After: %s" % after
> sys.exit(1)
> ---
> drivers/dma/imx-sdma.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> 066b21a..332ca50 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1338,6 +1338,7 @@ static void sdma_free_chan_resources(struct
> dma_chan *chan)
>
> sdmac->event_id0 = 0;
> sdmac->event_id1 = 0;
> + sdmac->context_loaded = false;
Martin, thanks for you patch, sorry for the issue left in kernel for so long, because my below patch set has been pending from last year. I would like revert commit ad0d92d: "dmaengine: imx-sdma: refine to load context only once" since some drivers may change.
context during two transfer like spi did. I would pick up this patch set this week anyway.
https://lore.kernel.org/patchwork/patch/1086454/
>
> sdma_set_channel_priority(sdmac, 0);
>
> --
> 1.9.1
