Re: [PATCH] imx-sdma: fix pagefault when channel is disabled during interrupt

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

 



Thanks Vinod,

I had posted a V2 meanwhile.
I just posted a V3 that takes your comments into account

Best Regards,
Thierry

Le 27/02/2018 à 16:01, Vinod Koul a écrit :
On Tue, Feb 13, 2018 at 09:47:23AM +0100, Thierry Bultel wrote:
Add a spinlock and a 'enabled' boolean on channel descriptor, to avoid
using buffer descriptors in the interrupt context,
sdma_disable_channel is called meanwhile.
---
  drivers/dma/imx-sdma.c | 21 +++++++++++++++++++++
  1 file changed, 21 insertions(+)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d1651a5..7eab42b 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -330,6 +330,7 @@ struct sdma_channel {
  	unsigned int			chn_real_count;
  	struct tasklet_struct		tasklet;
  	struct imx_dma_data		data;
+	unsigned int 			enabled;

description says bool but this is int?

  };
#define IMX_DMA_SG_LOOP BIT(0)
@@ -588,7 +589,13 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
  {
+	unsigned long flags;
+	struct sdma_channel *sdmac = &sdma->channel[channel];

Empty line here please

  	writel(BIT(channel), sdma->regs + SDMA_H_START);
+
+	spin_lock_irqsave(&sdmac->lock, flags);
+	sdmac->enabled = 1;
+	spin_unlock_irqrestore(&sdmac->lock, flags);
  }
/*
@@ -677,6 +684,15 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
  	struct sdma_buffer_descriptor *bd;
  	int error = 0;
  	enum dma_status	old_status = sdmac->status;
+	unsigned long flags;
+

this empty line is bogus

+	int enabled;

and is required here. Empty line needs to be added to a fn after defining
variables and before code statements

+	spin_lock_irqsave(&sdmac->lock, flags);
+	enabled = sdmac->enabled;
+	spin_unlock_irqrestore(&sdmac->lock, flags);
+
+	if (!enabled)
+		return;
/*
  	 * loop mode. Iterate over descriptors, re-setup them and
@@ -930,10 +946,15 @@ static int sdma_disable_channel(struct dma_chan *chan)
  	struct sdma_channel *sdmac = to_sdma_chan(chan);
  	struct sdma_engine *sdma = sdmac->sdma;
  	int channel = sdmac->channel;
+	unsigned long flags;
writel_relaxed(BIT(channel), sdma->regs + SDMA_H_STATSTOP);
  	sdmac->status = DMA_ERROR;
+ spin_lock_irqsave(&sdmac->lock, flags);
+	sdmac->enabled = 0;
+	spin_unlock_irqrestore(&sdmac->lock, flags);
+
  	return 0;
  }
--
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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