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

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

 



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



[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