RE: [PATCH 3/4] dmaengine: imx-sdma: implement channel termination via worker

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> -----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

Attachment: mxc_uart_stress_test.out
Description: mxc_uart_stress_test.out


[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux