* [PATCH] Fix race condition in omap_request_dma() @ 2009-10-30 3:30 HU TAO-TGHK48 2009-10-30 17:22 ` Venkatraman S 0 siblings, 1 reply; 8+ messages in thread From: HU TAO-TGHK48 @ 2009-10-30 3:30 UTC (permalink / raw) To: linux-omap, Tony Lindgren Cc: Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116 >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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix race condition in omap_request_dma() 2009-10-30 3:30 [PATCH] Fix race condition in omap_request_dma() HU TAO-TGHK48 @ 2009-10-30 17:22 ` Venkatraman S 2009-10-30 21:33 ` HU TAO-TGHK48 0 siblings, 1 reply; 8+ messages in thread From: Venkatraman S @ 2009-10-30 17:22 UTC (permalink / raw) To: HU TAO-TGHK48 Cc: linux-omap, Tony Lindgren, Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] Fix race condition in omap_request_dma() 2009-10-30 17:22 ` Venkatraman S @ 2009-10-30 21:33 ` HU TAO-TGHK48 2009-11-05 19:15 ` HU TAO-TGHK48 2009-11-10 0:31 ` [APPLIED] >> > > " Tony Lindgren 0 siblings, 2 replies; 8+ messages in thread From: HU TAO-TGHK48 @ 2009-10-30 21:33 UTC (permalink / raw) To: Venkatraman S Cc: linux-omap, Tony Lindgren, Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH] Fix race condition in omap_request_dma() 2009-10-30 21:33 ` HU TAO-TGHK48 @ 2009-11-05 19:15 ` HU TAO-TGHK48 2009-11-06 12:22 ` Venkatraman S 2009-11-10 0:31 ` [APPLIED] >> > > " Tony Lindgren 1 sibling, 1 reply; 8+ messages in thread From: HU TAO-TGHK48 @ 2009-11-05 19:15 UTC (permalink / raw) To: Venkatraman S Cc: linux-omap, Tony Lindgren, Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix race condition in omap_request_dma() 2009-11-05 19:15 ` HU TAO-TGHK48 @ 2009-11-06 12:22 ` Venkatraman S 2009-11-06 16:45 ` HU TAO-TGHK48 0 siblings, 1 reply; 8+ messages in thread From: Venkatraman S @ 2009-11-06 12:22 UTC (permalink / raw) To: HU TAO-TGHK48 Cc: linux-omap, Tony Lindgren, Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] Fix race condition in omap_request_dma() 2009-11-06 12:22 ` Venkatraman S @ 2009-11-06 16:45 ` HU TAO-TGHK48 2009-11-10 0:40 ` Tony Lindgren 0 siblings, 1 reply; 8+ messages in thread From: HU TAO-TGHK48 @ 2009-11-06 16:45 UTC (permalink / raw) To: Tony Lindgren Cc: linux-omap, Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116, Venkatraman S 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix race condition in omap_request_dma() 2009-11-06 16:45 ` HU TAO-TGHK48 @ 2009-11-10 0:40 ` Tony Lindgren 0 siblings, 0 replies; 8+ messages in thread From: Tony Lindgren @ 2009-11-10 0:40 UTC (permalink / raw) To: HU TAO-TGHK48 Cc: linux-omap, Yang Fei-AFY095, jon-hunter, Zhou Ming-a17711, Ye Yuan.Bo-A22116, Venkatraman S * 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [APPLIED] >> > > [PATCH] Fix race condition in omap_request_dma() 2009-10-30 21:33 ` HU TAO-TGHK48 2009-11-05 19:15 ` HU TAO-TGHK48 @ 2009-11-10 0:31 ` Tony Lindgren 1 sibling, 0 replies; 8+ messages in thread From: Tony Lindgren @ 2009-11-10 0:31 UTC (permalink / raw) To: linux-omap 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-11-10 0:40 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-10-30 3:30 [PATCH] Fix race condition in omap_request_dma() HU TAO-TGHK48 2009-10-30 17:22 ` Venkatraman S 2009-10-30 21:33 ` HU TAO-TGHK48 2009-11-05 19:15 ` HU TAO-TGHK48 2009-11-06 12:22 ` Venkatraman S 2009-11-06 16:45 ` HU TAO-TGHK48 2009-11-10 0:40 ` Tony Lindgren 2009-11-10 0:31 ` [APPLIED] >> > > " Tony Lindgren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox