All of lore.kernel.org
 help / color / mirror / Atom feed
* imx-sdma: fix pagefault when channel is disabled during interrupt
@ 2018-02-13  8:47 Thierry Bultel
  0 siblings, 0 replies; 4+ messages in thread
From: Thierry Bultel @ 2018-02-13  8:47 UTC (permalink / raw)
  To: dmaengine; +Cc: Thierry Bultel

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;
 };
 
 #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];
 	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;
+
+	int enabled;
+	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;
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* imx-sdma: fix pagefault when channel is disabled during interrupt
@ 2018-03-01  8:26 Vinod Koul
  0 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2018-03-01  8:26 UTC (permalink / raw)
  To: thierry bultel; +Cc: dmaengine

On Tue, Feb 27, 2018 at 04:17:18PM +0100, thierry bultel wrote:
> Thanks Vinod,
> 
> I had posted a V2 meanwhile.
> I just posted a V3 that takes your comments into account

Okay thanks, will check v3 in a while..

^ permalink raw reply	[flat|nested] 4+ messages in thread
* imx-sdma: fix pagefault when channel is disabled during interrupt
@ 2018-02-27 15:17 Thierry Bultel
  0 siblings, 0 replies; 4+ messages in thread
From: Thierry Bultel @ 2018-02-27 15:17 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dmaengine

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@vger.kernel.org
>> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread
* imx-sdma: fix pagefault when channel is disabled during interrupt
@ 2018-02-27 15:01 Vinod Koul
  0 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2018-02-27 15:01 UTC (permalink / raw)
  To: Thierry Bultel; +Cc: dmaengine

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-01  8:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-13  8:47 imx-sdma: fix pagefault when channel is disabled during interrupt Thierry Bultel
2018-02-27 15:01 Vinod Koul
2018-02-27 15:17 Thierry Bultel
2018-03-01  8:26 Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.