Hi Robin, Am Mittwoch, den 12.09.2018, 09:47 +0000 schrieb Robin Gong: > My testcase is simple loopback test as attached. Are you sure your serial port running in DMA mode? > By default, DMA not enabled on i.mx7d, you can enable it in uart device node of dts as below: > + dmas = <&sdma 32 4 0>, <&sdma 33 4 0>; > + dma-names = "rx", "tx" > Please flow below command(ttymxc5 on i.mx7d-sdb board) to run loopback test: > ./mxc_uart_stress_test.out /dev/ttymxc5 2000000 D L 100 100 L Thanks. With this testcase I was able to reproduce the issue on i.MX6. The v2 patchset I just sent out fixed this, so should hopefully be good enough to go in now. Regards, Lucas > > -----Original Message----- > > > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > Sent: 2018年9月12日 17:22 > > > > > > To: Robin Gong <yibin.gong@xxxxxxx>; Vinod Koul <vkoul@xxxxxxxxxx> > > > > > > Cc: dmaengine@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; > > kernel@xxxxxxxxxxxxxx; patchwork-lst@xxxxxxxxxxxxxx > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel > > termination via worker > > > > Hi Robin, > > > > Am Dienstag, den 11.09.2018, 08:18 +0000 schrieb Robin Gong: > > > Hi Lucas, > > > I have quick test for UART, seems this patch set broke uart function > > > on i.mx7d, I believe it should be same on other i.mx6 family. Could you double > > > > check it? > > > > Can you describe your testcase and the issue you've seen in more detail? I'm > > running this patch series on a system that does a lot of communication with an > > UART attached Co-processor and I'm not seeing any issues. > > > > Regards, > > Lucas > > > > > > -----Original Message----- > > > > > > > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > > > Sent: 2018年9月10日 17:59 > > > > > > > > To: Robin Gong <yibin.gong@xxxxxxx>; Vinod Koul <vkoul@xxxxxxxxxx> > > > > > > > > > > > > Cc: dmaengine@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; > > > > kernel@xxxxxxxxxxxxxx; patchwork-lst@xxxxxxxxxxxxxx > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel > > > > termination via worker > > > > > > > > Am Dienstag, den 04.09.2018, 02:36 +0000 schrieb Robin Gong: > > > > > > -----Original Message----- > > > > > > > > > > > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > > > > > Sent: 2018年9月3日 21:12 > > > > > > > > > > > > To: Robin Gong <yibin.gong@xxxxxxx>; Vinod Koul <vkoul@kernel.o > > > > > > rg> > > > > > > > > > > > > Cc: dmaengine@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > > > > > > > > > > > > ; kernel@xxxxxxxxxxxxxx; patchwork-lst@xxxxxxxxxxxxxx > > > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel > > > > > > termination via worker > > > > > > > > > > > > Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong: > > > > > > > > -----Original Message----- > > > > > > > > > > > > > > > > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > > > > > > > Sent: 2018年9月3日 16:41 > > > > > > > > > > > > > > > > To: Robin Gong <yibin.gong@xxxxxxx>; Vinod Koul <vkoul@kern > > > > > > > > el.o > > > > > > > > rg> > > > > > > > > > > > > > > > > Cc: dmaengine@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@nxp. > > > > > > > > com> > > > > > > > > > > > > > > > > ; kernel@xxxxxxxxxxxxxx; patchwork-lst@xxxxxxxxxxxxxx > > > > > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement > > > > > > > > channel termination via worker > > > > > > > > > > > > > > > > Hi Robin, > > > > > > > > > > > > > > > > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong: > > > > > > > > > Hi Lucas, > > > > > > > > > Seems I miss your previous mail. Thanks for your patch, > > > > > > > > > but if move most jobs of sdma_disable_channel_with_delay() > > > > > > > > > into worker, that will bring another race condition that > > > > > > > > > upper driver such as Audio terminate channel and free > > > > > > > > > resource of dma channel without really channel stop, if > > > > > > > > > dma transfer done interrupt come after that, oops or > > > > > > > > > kernel cash may be caught. > > > > > > > > > Leave 'sdmac->desc = NULL' in the > > > > > > > > > > > > > > > > sdma_disable_channel_with_delay() may fix such potential > > > > > > > > issue. > > > > > > > > > > > > > > > > No, there is no such issue. The audio channel terminate will > > > > > > > > call dmaengine_terminate_sync(), which internally calls > > > > > > > > dmaengine_terminate_async() and then does a > > > > > > > > dmaengine_synchronize(). > > > > > > > > As this patchset implements the device_synchronize function > > > > > > > > in the sdma driver, this will wait for the worker to finish > > > > > > > > its execution, so there is no race condition to worry about > > > > > > > > here. > > > > > > > > > > > > > > > > Regards, > > > > > > > > Lucas > > > > > > > > > > > > > > Yes, but how about other drivers which not call > > > > > > > dmaengine_terminate_sync()? > > > > > > > > > > > > Please read the dmaengine documentation. device_terminate_all > > > > > > has no requirement that the transfer is actually canceled when > > > > > > the call returns. If the caller needs a guarantee that the > > > > > > channel is stopped it _must_ call device_synchronize. > > > > > > > > > > I know that, but the fact is some driver still use > > > > > dmaengine_terminate_all() such as > > > > > Spi/uart driver. My concern is how to avoid to break their > > > > > function. > > > > > > > > They should simply be fixed to not use a deprecated function. Both > > > > of those are only using device_terminate_all in error or shutdown > > > > paths, so the risk of races is pretty minimal even with the current > > > > code. And I think the SPI driver is trivial to fix, as we can just > > > > use the terminate_sync variant there. The UART driver is a bit more > > > > tricky. > > > > > > > > Regards, > > > > Lucas