RE: [PATCH] dmaengine: imx-sdma: fix context cache

[Robin Gong]

On 2020/01/29 Martin Fuzzey <martin.fuzzey@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> 
> ---
> 
> 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