diff mbox

[05/10] dmaengine: bcm2835-dma: Use vchan_terminate_vdesc() instead of desc_free

Message ID 20171114143212.8311-6-peter.ujfalusi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Ujfalusi Nov. 14, 2017, 2:32 p.m. UTC
To avoid race with vchan_complete, use the race free way to terminate
running transfer.

Implement the device_synchronize callback to make sure that the terminated
descriptor is freed.

CC: Martin Sperl <kernel@martin.sperl.org>
CC: Eric Anholt <eric@anholt.net>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/bcm2835-dma.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Eric Anholt Nov. 15, 2017, 6:53 p.m. UTC | #1
Peter Ujfalusi <peter.ujfalusi@ti.com> writes:

> To avoid race with vchan_complete, use the race free way to terminate
> running transfer.
>
> Implement the device_synchronize callback to make sure that the terminated
> descriptor is freed.
>
> CC: Martin Sperl <kernel@martin.sperl.org>
> CC: Eric Anholt <eric@anholt.net>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

I haven't fully followed the series, but thanks for porting your fix to
other platforms!

Acked-by: Eric Anholt <eric@anholt.net>
Peter Ujfalusi Nov. 21, 2017, 11:42 a.m. UTC | #2
Eric,

On 11/15/2017 08:53 PM, Eric Anholt wrote:
> Peter Ujfalusi <peter.ujfalusi@ti.com> writes:
> 
>> To avoid race with vchan_complete, use the race free way to terminate
>> running transfer.
>>
>> Implement the device_synchronize callback to make sure that the terminated
>> descriptor is freed.
>>
>> CC: Martin Sperl <kernel@martin.sperl.org>
>> CC: Eric Anholt <eric@anholt.net>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> I haven't fully followed the series, but thanks for porting your fix to
> other platforms!
I have seen similar patterns in these drivers and it was the right thing to do
imho.
Unfortunately I can not test on other platforms than eDMA and sDMA, but I
firmly believe that based on the usage it should be fine as it survives my
torture tests.

It would be great to see some Tested-by from others to have more coverage.

> Acked-by: Eric Anholt <eric@anholt.net>

Thank you!
diff mbox

Patch

diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
index 6204cc32d09c..847f84a41a69 100644
--- a/drivers/dma/bcm2835-dma.c
+++ b/drivers/dma/bcm2835-dma.c
@@ -812,7 +812,7 @@  static int bcm2835_dma_terminate_all(struct dma_chan *chan)
 	 * c->desc is NULL and exit.)
 	 */
 	if (c->desc) {
-		bcm2835_dma_desc_free(&c->desc->vd);
+		vchan_terminate_vdesc(&c->desc->vd);
 		c->desc = NULL;
 		bcm2835_dma_abort(c->chan_base);
 
@@ -836,6 +836,13 @@  static int bcm2835_dma_terminate_all(struct dma_chan *chan)
 	return 0;
 }
 
+static void bcm2835_dma_synchronize(struct dma_chan *chan)
+{
+	struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
+
+	vchan_synchronize(&c->vc);
+}
+
 static int bcm2835_dma_chan_init(struct bcm2835_dmadev *d, int chan_id,
 				 int irq, unsigned int irq_flags)
 {
@@ -942,6 +949,7 @@  static int bcm2835_dma_probe(struct platform_device *pdev)
 	od->ddev.device_prep_dma_memcpy = bcm2835_dma_prep_dma_memcpy;
 	od->ddev.device_config = bcm2835_dma_slave_config;
 	od->ddev.device_terminate_all = bcm2835_dma_terminate_all;
+	od->ddev.device_synchronize = bcm2835_dma_synchronize;
 	od->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
 	od->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
 	od->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV) |