[PATCH] Fix race condition in omap_request_dma()

[PATCH] Fix race condition in omap_request_dma()

[HU TAO-TGHK48]

>From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17 00:00:00 2001
From: Tao Hu <taohu@motorola.com>
Date: Thu, 29 Oct 2009 17:17:21 -0500
Subject: [PATCH] Fix race condition in omap_request_dma()

The bug could cause irq enable bit of one DMA channel is
cleared unexpectedly when 2 (or more) drivers are calling
omap_request_dma() simultaneously

Signed-off-by: Fei Yang <AFY095@motorola.com>
Signed-off-by: Tao Hu  <taohu@motorola.com>
---
 arch/arm/plat-omap/dma.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index cd53b28..6895484 100755
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, const char
*dev_name,
 	}
 
 	if (cpu_class_is_omap2()) {
+		spin_lock_irqsave(&dma_chan_lock, flags);
 		omap2_enable_irq_lch(free_ch);
 		omap_enable_channel_irq(free_ch);
 		/* Clear the CSR register and IRQ status register */
 		dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(free_ch));
 		dma_write(1 << free_ch, IRQSTATUS_L0);
+		spin_unlock_irqrestore(&dma_chan_lock, flags);
 	}
 
 	*dma_ch_out = free_ch;
-- 
1.5.4.3

Re: [PATCH] Fix race condition in omap_request_dma()

[Venkatraman S]

On Fri, Oct 30, 2009 at 9:00 AM, HU TAO-TGHK48 <taohu@motorola.com> wrote:
> From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17 00:00:00 2001
> From: Tao Hu <taohu@motorola.com>
> Date: Thu, 29 Oct 2009 17:17:21 -0500
> Subject: [PATCH] Fix race condition in omap_request_dma()
>
> The bug could cause irq enable bit of one DMA channel is
> cleared unexpectedly when 2 (or more) drivers are calling
> omap_request_dma() simultaneously
>
> Signed-off-by: Fei Yang <AFY095@motorola.com>
> Signed-off-by: Tao Hu  <taohu@motorola.com>
> ---
>  arch/arm/plat-omap/dma.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index cd53b28..6895484 100755
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, const char
> *dev_name,
>        }
>
>        if (cpu_class_is_omap2()) {
> +               spin_lock_irqsave(&dma_chan_lock, flags);
>                omap2_enable_irq_lch(free_ch);
>                omap_enable_channel_irq(free_ch);
>                /* Clear the CSR register and IRQ status register */
>                dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(free_ch));
>                dma_write(1 << free_ch, IRQSTATUS_L0);
> +               spin_unlock_irqrestore(&dma_chan_lock, flags);
>        }

Nice catch. I think the lock needs to be applied only for the global
registers which are changed using the read - update - write method.
The dma_write to per channel configuration registers need not be
locked.
Hence the lock is better placed "within" omap2_enable_irq_lch
function. That way, if it's reused, the locking would also be in
place.
Thank you !
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Fix race condition in omap_request_dma()

[HU TAO-TGHK48]

Hi, Venkatraman 

Agree with you. And just found omap_free_dma() need to be fixed too.
New patch attached.

From 7aa22ece591365c2dcd73c799d93781497322ff1 Mon Sep 17 00:00:00 2001
From: Tao Hu <taohu@motorola.com>
Date: Fri, 30 Oct 2009 16:24:25 -0500
Subject: [PATCH] Fix race condition in omap dma driver

The bug could cause irq enable bit of one DMA channel is
cleared/set unexpectedly when 2 (or more) drivers are calling
omap_request_dma()/omap_free_dma() simultaneously

Signed-off-by: Fei Yang <AFY095@motorola.com>
Signed-off-by: Tao Hu  <taohu@motorola.com>
---
 arch/arm/plat-omap/dma.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index cd53b28..5767899 100755
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -673,13 +673,16 @@ static inline void disable_lnk(int lch)
 static inline void omap2_enable_irq_lch(int lch)
 {
 	u32 val;
+	unsigned long flags;
 
 	if (!cpu_class_is_omap2())
 		return;
 
+	spin_lock_irqsave(&dma_chan_lock, flags);
 	val = dma_read(IRQENABLE_L0);
 	val |= 1 << lch;
 	dma_write(val, IRQENABLE_L0);
+	spin_unlock_irqrestore(&dma_chan_lock, flags);
 }
 
 int omap_request_dma(int dev_id, const char *dev_name,
@@ -788,10 +791,13 @@ void omap_free_dma(int lch)
 
 	if (cpu_class_is_omap2()) {
 		u32 val;
+
+		spin_lock_irqsave(&dma_chan_lock, flags);
 		/* Disable interrupts */
 		val = dma_read(IRQENABLE_L0);
 		val &= ~(1 << lch);
 		dma_write(val, IRQENABLE_L0);
+		spin_unlock_irqrestore(&dma_chan_lock, flags);
 
 		/* Clear the CSR register and IRQ status register */
 		dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(lch));
-- 
1.5.4.3
       
 

> -----Original Message-----
> From: Venkatraman S [mailto:svenkatr@gmail.com] 
> Sent: Saturday, October 31, 2009 1:22 AM
> To: HU TAO-TGHK48
> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang 
> Fei-AFY095; jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> Subject: Re: [PATCH] Fix race condition in omap_request_dma()
> 
> On Fri, Oct 30, 2009 at 9:00 AM, HU TAO-TGHK48 
> <taohu@motorola.com> wrote:
> > From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17 
> 00:00:00 2001
> > From: Tao Hu <taohu@motorola.com>
> > Date: Thu, 29 Oct 2009 17:17:21 -0500
> > Subject: [PATCH] Fix race condition in omap_request_dma()
> >
> > The bug could cause irq enable bit of one DMA channel is cleared 
> > unexpectedly when 2 (or more) drivers are calling
> > omap_request_dma() simultaneously
> >
> > Signed-off-by: Fei Yang <AFY095@motorola.com>
> > Signed-off-by: Tao Hu  <taohu@motorola.com>
> > ---
> >  arch/arm/plat-omap/dma.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/dma.c 
> b/arch/arm/plat-omap/dma.c index 
> > cd53b28..6895484 100755
> > --- a/arch/arm/plat-omap/dma.c
> > +++ b/arch/arm/plat-omap/dma.c
> > @@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, const char 
> > *dev_name,
> >        }
> >
> >        if (cpu_class_is_omap2()) {
> > +               spin_lock_irqsave(&dma_chan_lock, flags);
> >                omap2_enable_irq_lch(free_ch);
> >                omap_enable_channel_irq(free_ch);
> >                /* Clear the CSR register and IRQ status register */
> >                dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(free_ch));
> >                dma_write(1 << free_ch, IRQSTATUS_L0);
> > +               spin_unlock_irqrestore(&dma_chan_lock, flags);
> >        }
> 
> Nice catch. I think the lock needs to be applied only for the 
> global registers which are changed using the read - update - 
> write method.
> The dma_write to per channel configuration registers need not 
> be locked.
> Hence the lock is better placed "within" omap2_enable_irq_lch 
> function. That way, if it's reused, the locking would also be 
> in place.
> Thank you !
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Fix race condition in omap_request_dma()

[HU TAO-TGHK48]

Any new comments?

-Hu Tao
      
 

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of HU TAO-TGHK48
> Sent: Saturday, October 31, 2009 5:33 AM
> To: Venkatraman S
> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang 
> Fei-AFY095; jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> Subject: RE: [PATCH] Fix race condition in omap_request_dma()
> 
> Hi, Venkatraman 
> 
> Agree with you. And just found omap_free_dma() need to be fixed too.
> New patch attached.
> 
> From 7aa22ece591365c2dcd73c799d93781497322ff1 Mon Sep 17 00:00:00 2001
> From: Tao Hu <taohu@motorola.com>
> Date: Fri, 30 Oct 2009 16:24:25 -0500
> Subject: [PATCH] Fix race condition in omap dma driver
> 
> The bug could cause irq enable bit of one DMA channel is 
> cleared/set unexpectedly when 2 (or more) drivers are calling
> omap_request_dma()/omap_free_dma() simultaneously
> 
> Signed-off-by: Fei Yang <AFY095@motorola.com>
> Signed-off-by: Tao Hu  <taohu@motorola.com>
> ---
>  arch/arm/plat-omap/dma.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/dma.c 
> b/arch/arm/plat-omap/dma.c index cd53b28..5767899 100755
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -673,13 +673,16 @@ static inline void disable_lnk(int lch) 
>  static inline void omap2_enable_irq_lch(int lch)  {
>  	u32 val;
> +	unsigned long flags;
>  
>  	if (!cpu_class_is_omap2())
>  		return;
>  
> +	spin_lock_irqsave(&dma_chan_lock, flags);
>  	val = dma_read(IRQENABLE_L0);
>  	val |= 1 << lch;
>  	dma_write(val, IRQENABLE_L0);
> +	spin_unlock_irqrestore(&dma_chan_lock, flags);
>  }
>  
>  int omap_request_dma(int dev_id, const char *dev_name, @@ 
> -788,10 +791,13 @@ void omap_free_dma(int lch)
>  
>  	if (cpu_class_is_omap2()) {
>  		u32 val;
> +
> +		spin_lock_irqsave(&dma_chan_lock, flags);
>  		/* Disable interrupts */
>  		val = dma_read(IRQENABLE_L0);
>  		val &= ~(1 << lch);
>  		dma_write(val, IRQENABLE_L0);
> +		spin_unlock_irqrestore(&dma_chan_lock, flags);
>  
>  		/* Clear the CSR register and IRQ status register */
>  		dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(lch));
> --
> 1.5.4.3
>        
>  
> 
> > -----Original Message-----
> > From: Venkatraman S [mailto:svenkatr@gmail.com]
> > Sent: Saturday, October 31, 2009 1:22 AM
> > To: HU TAO-TGHK48
> > Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang Fei-AFY095; 
> > jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> > Subject: Re: [PATCH] Fix race condition in omap_request_dma()
> > 
> > On Fri, Oct 30, 2009 at 9:00 AM, HU TAO-TGHK48 <taohu@motorola.com> 
> > wrote:
> > > From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17
> > 00:00:00 2001
> > > From: Tao Hu <taohu@motorola.com>
> > > Date: Thu, 29 Oct 2009 17:17:21 -0500
> > > Subject: [PATCH] Fix race condition in omap_request_dma()
> > >
> > > The bug could cause irq enable bit of one DMA channel is cleared 
> > > unexpectedly when 2 (or more) drivers are calling
> > > omap_request_dma() simultaneously
> > >
> > > Signed-off-by: Fei Yang <AFY095@motorola.com>
> > > Signed-off-by: Tao Hu  <taohu@motorola.com>
> > > ---
> > >  arch/arm/plat-omap/dma.c |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/arm/plat-omap/dma.c
> > b/arch/arm/plat-omap/dma.c index
> > > cd53b28..6895484 100755
> > > --- a/arch/arm/plat-omap/dma.c
> > > +++ b/arch/arm/plat-omap/dma.c
> > > @@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, const char 
> > > *dev_name,
> > >        }
> > >
> > >        if (cpu_class_is_omap2()) {
> > > +               spin_lock_irqsave(&dma_chan_lock, flags);
> > >                omap2_enable_irq_lch(free_ch);
> > >                omap_enable_channel_irq(free_ch);
> > >                /* Clear the CSR register and IRQ status 
> register */
> > >                dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(free_ch));
> > >                dma_write(1 << free_ch, IRQSTATUS_L0);
> > > +               spin_unlock_irqrestore(&dma_chan_lock, flags);
> > >        }
> > 
> > Nice catch. I think the lock needs to be applied only for 
> the global 
> > registers which are changed using the read - update - write method.
> > The dma_write to per channel configuration registers need not be 
> > locked.
> > Hence the lock is better placed "within" omap2_enable_irq_lch 
> > function. That way, if it's reused, the locking would also be in 
> > place.
> > Thank you !
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" 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 linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fix race condition in omap_request_dma()

[Venkatraman S]

On Fri, Nov 6, 2009 at 12:45 AM, HU TAO-TGHK48 <taohu@motorola.com> wrote:
>
> Any new comments?
>
> -Hu Tao
>
>
I am happy with this version.
Regards,

Venkatraman.

>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org
>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of HU TAO-TGHK48
>> Sent: Saturday, October 31, 2009 5:33 AM
>> To: Venkatraman S
>> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang
>> Fei-AFY095; jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
>> Subject: RE: [PATCH] Fix race condition in omap_request_dma()
>>
>> Hi, Venkatraman
>>
>> Agree with you. And just found omap_free_dma() need to be fixed too.
>> New patch attached.
>>
>> From 7aa22ece591365c2dcd73c799d93781497322ff1 Mon Sep 17 00:00:00 2001
>> From: Tao Hu <taohu@motorola.com>
>> Date: Fri, 30 Oct 2009 16:24:25 -0500
>> Subject: [PATCH] Fix race condition in omap dma driver
>>
>> The bug could cause irq enable bit of one DMA channel is
>> cleared/set unexpectedly when 2 (or more) drivers are calling
>> omap_request_dma()/omap_free_dma() simultaneously
>>
>> Signed-off-by: Fei Yang <AFY095@motorola.com>
>> Signed-off-by: Tao Hu  <taohu@motorola.com>
>> ---
>>  arch/arm/plat-omap/dma.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/dma.c
>> b/arch/arm/plat-omap/dma.c index cd53b28..5767899 100755
>> --- a/arch/arm/plat-omap/dma.c
>> +++ b/arch/arm/plat-omap/dma.c
>> @@ -673,13 +673,16 @@ static inline void disable_lnk(int lch)
>>  static inline void omap2_enable_irq_lch(int lch)  {
>>       u32 val;
>> +     unsigned long flags;
>>
>>       if (!cpu_class_is_omap2())
>>               return;
>>
>> +     spin_lock_irqsave(&dma_chan_lock, flags);
>>       val = dma_read(IRQENABLE_L0);
>>       val |= 1 << lch;
>>       dma_write(val, IRQENABLE_L0);
>> +     spin_unlock_irqrestore(&dma_chan_lock, flags);
>>  }
>>
>>  int omap_request_dma(int dev_id, const char *dev_name, @@
>> -788,10 +791,13 @@ void omap_free_dma(int lch)
>>
>>       if (cpu_class_is_omap2()) {
>>               u32 val;
>> +
>> +             spin_lock_irqsave(&dma_chan_lock, flags);
>>               /* Disable interrupts */
>>               val = dma_read(IRQENABLE_L0);
>>               val &= ~(1 << lch);
>>               dma_write(val, IRQENABLE_L0);
>> +             spin_unlock_irqrestore(&dma_chan_lock, flags);
>>
>>               /* Clear the CSR register and IRQ status register */
>>               dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(lch));
>> --
>> 1.5.4.3
>>
>>
>>
>> > -----Original Message-----
>> > From: Venkatraman S [mailto:svenkatr@gmail.com]
>> > Sent: Saturday, October 31, 2009 1:22 AM
>> > To: HU TAO-TGHK48
>> > Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang Fei-AFY095;
>> > jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
>> > Subject: Re: [PATCH] Fix race condition in omap_request_dma()
>> >
>> > On Fri, Oct 30, 2009 at 9:00 AM, HU TAO-TGHK48 <taohu@motorola.com>
>> > wrote:
>> > > From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17
>> > 00:00:00 2001
>> > > From: Tao Hu <taohu@motorola.com>
>> > > Date: Thu, 29 Oct 2009 17:17:21 -0500
>> > > Subject: [PATCH] Fix race condition in omap_request_dma()
>> > >
>> > > The bug could cause irq enable bit of one DMA channel is cleared
>> > > unexpectedly when 2 (or more) drivers are calling
>> > > omap_request_dma() simultaneously
>> > >
>> > > Signed-off-by: Fei Yang <AFY095@motorola.com>
>> > > Signed-off-by: Tao Hu  <taohu@motorola.com>
>> > > ---
>> > >  arch/arm/plat-omap/dma.c |    2 ++
>> > >  1 files changed, 2 insertions(+), 0 deletions(-)
>> > >
>> > > diff --git a/arch/arm/plat-omap/dma.c
>> > b/arch/arm/plat-omap/dma.c index
>> > > cd53b28..6895484 100755
>> > > --- a/arch/arm/plat-omap/dma.c
>> > > +++ b/arch/arm/plat-omap/dma.c
>> > > @@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, const char
>> > > *dev_name,
>> > >        }
>> > >
>> > >        if (cpu_class_is_omap2()) {
>> > > +               spin_lock_irqsave(&dma_chan_lock, flags);
>> > >                omap2_enable_irq_lch(free_ch);
>> > >                omap_enable_channel_irq(free_ch);
>> > >                /* Clear the CSR register and IRQ status
>> register */
>> > >                dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(free_ch));
>> > >                dma_write(1 << free_ch, IRQSTATUS_L0);
>> > > +               spin_unlock_irqrestore(&dma_chan_lock, flags);
>> > >        }
>> >
>> > Nice catch. I think the lock needs to be applied only for
>> the global
>> > registers which are changed using the read - update - write method.
>> > The dma_write to per channel configuration registers need not be
>> > locked.
>> > Hence the lock is better placed "within" omap2_enable_irq_lch
>> > function. That way, if it's reused, the locking would also be in
>> > place.
>> > Thank you !
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-omap" 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 linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Fix race condition in omap_request_dma()

[HU TAO-TGHK48]

Hi, Tony

Can we merge this patch in?
Thanks.
 
-Hu Tao

> -----Original Message-----
> From: svenkatr@gmail.com [mailto:svenkatr@gmail.com] On 
> Behalf Of Venkatraman S
> Sent: Friday, November 06, 2009 8:23 PM
> To: HU TAO-TGHK48
> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang 
> Fei-AFY095; jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> Subject: Re: [PATCH] Fix race condition in omap_request_dma()
> 
> On Fri, Nov 6, 2009 at 12:45 AM, HU TAO-TGHK48 
> <taohu@motorola.com> wrote:
> >
> > Any new comments?
> >
> > -Hu Tao
> >
> >
> I am happy with this version.
> Regards,
> 
> Venkatraman.
> 
> >
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org 
> >> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of HU 
> TAO-TGHK48
> >> Sent: Saturday, October 31, 2009 5:33 AM
> >> To: Venkatraman S
> >> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang Fei-AFY095; 
> >> jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> >> Subject: RE: [PATCH] Fix race condition in omap_request_dma()
> >>
> >> Hi, Venkatraman
> >>
> >> Agree with you. And just found omap_free_dma() need to be 
> fixed too.
> >> New patch attached.
> >>
> >> From 7aa22ece591365c2dcd73c799d93781497322ff1 Mon Sep 17 00:00:00 
> >> 2001
> >> From: Tao Hu <taohu@motorola.com>
> >> Date: Fri, 30 Oct 2009 16:24:25 -0500
> >> Subject: [PATCH] Fix race condition in omap dma driver
> >>
> >> The bug could cause irq enable bit of one DMA channel is 
> cleared/set 
> >> unexpectedly when 2 (or more) drivers are calling
> >> omap_request_dma()/omap_free_dma() simultaneously
> >>
> >> Signed-off-by: Fei Yang <AFY095@motorola.com>
> >> Signed-off-by: Tao Hu  <taohu@motorola.com>
> >> ---
> >>  arch/arm/plat-omap/dma.c |    6 ++++++
> >>  1 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c 
> >> index cd53b28..5767899 100755
> >> --- a/arch/arm/plat-omap/dma.c
> >> +++ b/arch/arm/plat-omap/dma.c
> >> @@ -673,13 +673,16 @@ static inline void disable_lnk(int lch)
> >>  static inline void omap2_enable_irq_lch(int lch)  {
> >>       u32 val;
> >> +     unsigned long flags;
> >>
> >>       if (!cpu_class_is_omap2())
> >>               return;
> >>
> >> +     spin_lock_irqsave(&dma_chan_lock, flags);
> >>       val = dma_read(IRQENABLE_L0);
> >>       val |= 1 << lch;
> >>       dma_write(val, IRQENABLE_L0);
> >> +     spin_unlock_irqrestore(&dma_chan_lock, flags);
> >>  }
> >>
> >>  int omap_request_dma(int dev_id, const char *dev_name, @@ -788,10 
> >> +791,13 @@ void omap_free_dma(int lch)
> >>
> >>       if (cpu_class_is_omap2()) {
> >>               u32 val;
> >> +
> >> +             spin_lock_irqsave(&dma_chan_lock, flags);
> >>               /* Disable interrupts */
> >>               val = dma_read(IRQENABLE_L0);
> >>               val &= ~(1 << lch);
> >>               dma_write(val, IRQENABLE_L0);
> >> +             spin_unlock_irqrestore(&dma_chan_lock, flags);
> >>
> >>               /* Clear the CSR register and IRQ status register */
> >>               dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(lch));
> >> --
> >> 1.5.4.3
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: Venkatraman S [mailto:svenkatr@gmail.com]
> >> > Sent: Saturday, October 31, 2009 1:22 AM
> >> > To: HU TAO-TGHK48
> >> > Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang Fei-AFY095; 
> >> > jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> >> > Subject: Re: [PATCH] Fix race condition in omap_request_dma()
> >> >
> >> > On Fri, Oct 30, 2009 at 9:00 AM, HU TAO-TGHK48 
> <taohu@motorola.com>
> >> > wrote:
> >> > > From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17
> >> > 00:00:00 2001
> >> > > From: Tao Hu <taohu@motorola.com>
> >> > > Date: Thu, 29 Oct 2009 17:17:21 -0500
> >> > > Subject: [PATCH] Fix race condition in omap_request_dma()
> >> > >
> >> > > The bug could cause irq enable bit of one DMA channel 
> is cleared 
> >> > > unexpectedly when 2 (or more) drivers are calling
> >> > > omap_request_dma() simultaneously
> >> > >
> >> > > Signed-off-by: Fei Yang <AFY095@motorola.com>
> >> > > Signed-off-by: Tao Hu  <taohu@motorola.com>
> >> > > ---
> >> > >  arch/arm/plat-omap/dma.c |    2 ++
> >> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> >> > >
> >> > > diff --git a/arch/arm/plat-omap/dma.c
> >> > b/arch/arm/plat-omap/dma.c index
> >> > > cd53b28..6895484 100755
> >> > > --- a/arch/arm/plat-omap/dma.c
> >> > > +++ b/arch/arm/plat-omap/dma.c
> >> > > @@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, 
> const char 
> >> > > *dev_name,
> >> > >        }
> >> > >
> >> > >        if (cpu_class_is_omap2()) {
> >> > > +               spin_lock_irqsave(&dma_chan_lock, flags);
> >> > >                omap2_enable_irq_lch(free_ch);
> >> > >                omap_enable_channel_irq(free_ch);
> >> > >                /* Clear the CSR register and IRQ status
> >> register */
> >> > >                dma_write(OMAP2_DMA_CSR_CLEAR_MASK, 
> CSR(free_ch));
> >> > >                dma_write(1 << free_ch, IRQSTATUS_L0);
> >> > > +               spin_unlock_irqrestore(&dma_chan_lock, flags);
> >> > >        }
> >> >
> >> > Nice catch. I think the lock needs to be applied only for
> >> the global
> >> > registers which are changed using the read - update - 
> write method.
> >> > The dma_write to per channel configuration registers need not be 
> >> > locked.
> >> > Hence the lock is better placed "within" omap2_enable_irq_lch 
> >> > function. That way, if it's reused, the locking would also be in 
> >> > place.
> >> > Thank you !
> >> >
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" 
> >> 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 linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[APPLIED] >> > > [PATCH] Fix race condition in omap_request_dma()

[Tony Lindgren]

This patch has been applied to the linux-omap
by youw fwiendly patch wobot.

Branch in linux-omap: omap-fixes

Initial commit ID (Likely to change): 1f2e5af0fc0de168e55662a7cc928af037c0a4ad

PatchWorks
http://patchwork.kernel.org/patch/56703/

Git (Likely to change, and takes a while to get mirrored)
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=1f2e5af0fc0de168e55662a7cc928af037c0a4ad

Re: [PATCH] Fix race condition in omap_request_dma()

[Tony Lindgren]

* HU TAO-TGHK48 <taohu@motorola.com> [091106 08:45]:
> Hi, Tony
> 
> Can we merge this patch in?

Yeah, let's try to get that in as a fix.

Tony


> Thanks.
>  
> -Hu Tao
> 
> > -----Original Message-----
> > From: svenkatr@gmail.com [mailto:svenkatr@gmail.com] On 
> > Behalf Of Venkatraman S
> > Sent: Friday, November 06, 2009 8:23 PM
> > To: HU TAO-TGHK48
> > Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang 
> > Fei-AFY095; jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> > Subject: Re: [PATCH] Fix race condition in omap_request_dma()
> > 
> > On Fri, Nov 6, 2009 at 12:45 AM, HU TAO-TGHK48 
> > <taohu@motorola.com> wrote:
> > >
> > > Any new comments?
> > >
> > > -Hu Tao
> > >
> > >
> > I am happy with this version.
> > Regards,
> > 
> > Venkatraman.
> > 
> > >
> > >> -----Original Message-----
> > >> From: linux-omap-owner@vger.kernel.org 
> > >> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of HU 
> > TAO-TGHK48
> > >> Sent: Saturday, October 31, 2009 5:33 AM
> > >> To: Venkatraman S
> > >> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang Fei-AFY095; 
> > >> jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> > >> Subject: RE: [PATCH] Fix race condition in omap_request_dma()
> > >>
> > >> Hi, Venkatraman
> > >>
> > >> Agree with you. And just found omap_free_dma() need to be 
> > fixed too.
> > >> New patch attached.
> > >>
> > >> From 7aa22ece591365c2dcd73c799d93781497322ff1 Mon Sep 17 00:00:00 
> > >> 2001
> > >> From: Tao Hu <taohu@motorola.com>
> > >> Date: Fri, 30 Oct 2009 16:24:25 -0500
> > >> Subject: [PATCH] Fix race condition in omap dma driver
> > >>
> > >> The bug could cause irq enable bit of one DMA channel is 
> > cleared/set 
> > >> unexpectedly when 2 (or more) drivers are calling
> > >> omap_request_dma()/omap_free_dma() simultaneously
> > >>
> > >> Signed-off-by: Fei Yang <AFY095@motorola.com>
> > >> Signed-off-by: Tao Hu  <taohu@motorola.com>
> > >> ---
> > >>  arch/arm/plat-omap/dma.c |    6 ++++++
> > >>  1 files changed, 6 insertions(+), 0 deletions(-)
> > >>
> > >> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c 
> > >> index cd53b28..5767899 100755
> > >> --- a/arch/arm/plat-omap/dma.c
> > >> +++ b/arch/arm/plat-omap/dma.c
> > >> @@ -673,13 +673,16 @@ static inline void disable_lnk(int lch)
> > >>  static inline void omap2_enable_irq_lch(int lch)  {
> > >>       u32 val;
> > >> +     unsigned long flags;
> > >>
> > >>       if (!cpu_class_is_omap2())
> > >>               return;
> > >>
> > >> +     spin_lock_irqsave(&dma_chan_lock, flags);
> > >>       val = dma_read(IRQENABLE_L0);
> > >>       val |= 1 << lch;
> > >>       dma_write(val, IRQENABLE_L0);
> > >> +     spin_unlock_irqrestore(&dma_chan_lock, flags);
> > >>  }
> > >>
> > >>  int omap_request_dma(int dev_id, const char *dev_name, @@ -788,10 
> > >> +791,13 @@ void omap_free_dma(int lch)
> > >>
> > >>       if (cpu_class_is_omap2()) {
> > >>               u32 val;
> > >> +
> > >> +             spin_lock_irqsave(&dma_chan_lock, flags);
> > >>               /* Disable interrupts */
> > >>               val = dma_read(IRQENABLE_L0);
> > >>               val &= ~(1 << lch);
> > >>               dma_write(val, IRQENABLE_L0);
> > >> +             spin_unlock_irqrestore(&dma_chan_lock, flags);
> > >>
> > >>               /* Clear the CSR register and IRQ status register */
> > >>               dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(lch));
> > >> --
> > >> 1.5.4.3
> > >>
> > >>
> > >>
> > >> > -----Original Message-----
> > >> > From: Venkatraman S [mailto:svenkatr@gmail.com]
> > >> > Sent: Saturday, October 31, 2009 1:22 AM
> > >> > To: HU TAO-TGHK48
> > >> > Cc: linux-omap@vger.kernel.org; Tony Lindgren; Yang Fei-AFY095; 
> > >> > jon-hunter@ti.com; Zhou Ming-a17711; Ye Yuan.Bo-A22116
> > >> > Subject: Re: [PATCH] Fix race condition in omap_request_dma()
> > >> >
> > >> > On Fri, Oct 30, 2009 at 9:00 AM, HU TAO-TGHK48 
> > <taohu@motorola.com>
> > >> > wrote:
> > >> > > From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17
> > >> > 00:00:00 2001
> > >> > > From: Tao Hu <taohu@motorola.com>
> > >> > > Date: Thu, 29 Oct 2009 17:17:21 -0500
> > >> > > Subject: [PATCH] Fix race condition in omap_request_dma()
> > >> > >
> > >> > > The bug could cause irq enable bit of one DMA channel 
> > is cleared 
> > >> > > unexpectedly when 2 (or more) drivers are calling
> > >> > > omap_request_dma() simultaneously
> > >> > >
> > >> > > Signed-off-by: Fei Yang <AFY095@motorola.com>
> > >> > > Signed-off-by: Tao Hu  <taohu@motorola.com>
> > >> > > ---
> > >> > >  arch/arm/plat-omap/dma.c |    2 ++
> > >> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > >> > >
> > >> > > diff --git a/arch/arm/plat-omap/dma.c
> > >> > b/arch/arm/plat-omap/dma.c index
> > >> > > cd53b28..6895484 100755
> > >> > > --- a/arch/arm/plat-omap/dma.c
> > >> > > +++ b/arch/arm/plat-omap/dma.c
> > >> > > @@ -749,11 +749,13 @@ int omap_request_dma(int dev_id, 
> > const char 
> > >> > > *dev_name,
> > >> > >        }
> > >> > >
> > >> > >        if (cpu_class_is_omap2()) {
> > >> > > +               spin_lock_irqsave(&dma_chan_lock, flags);
> > >> > >                omap2_enable_irq_lch(free_ch);
> > >> > >                omap_enable_channel_irq(free_ch);
> > >> > >                /* Clear the CSR register and IRQ status
> > >> register */
> > >> > >                dma_write(OMAP2_DMA_CSR_CLEAR_MASK, 
> > CSR(free_ch));
> > >> > >                dma_write(1 << free_ch, IRQSTATUS_L0);
> > >> > > +               spin_unlock_irqrestore(&dma_chan_lock, flags);
> > >> > >        }
> > >> >
> > >> > Nice catch. I think the lock needs to be applied only for
> > >> the global
> > >> > registers which are changed using the read - update - 
> > write method.
> > >> > The dma_write to per channel configuration registers need not be 
> > >> > locked.
> > >> > Hence the lock is better placed "within" omap2_enable_irq_lch 
> > >> > function. That way, if it's reused, the locking would also be in 
> > >> > place.
> > >> > Thank you !
> > >> >
> > >> --
> > >> To unsubscribe from this list: send the line "unsubscribe 
> > linux-omap" 
> > >> 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 linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html