Hi, Tony Can we merge this patch in? Thanks. -Hu Tao > -----Original Message----- > From: svenkatr@xxxxxxxxx [mailto:svenkatr@xxxxxxxxx] On > Behalf Of Venkatraman S > Sent: Friday, November 06, 2009 8:23 PM > To: HU TAO-TGHK48 > Cc: linux-omap@xxxxxxxxxxxxxxx; Tony Lindgren; Yang > Fei-AFY095; jon-hunter@xxxxxx; 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@xxxxxxxxxxxx> wrote: > > > > Any new comments? > > > > -Hu Tao > > > > > I am happy with this version. > Regards, > > Venkatraman. > > > > >> -----Original Message----- > >> From: linux-omap-owner@xxxxxxxxxxxxxxx > >> [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of HU > TAO-TGHK48 > >> Sent: Saturday, October 31, 2009 5:33 AM > >> To: Venkatraman S > >> Cc: linux-omap@xxxxxxxxxxxxxxx; Tony Lindgren; Yang Fei-AFY095; > >> jon-hunter@xxxxxx; 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@xxxxxxxxxxxx> > >> 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@xxxxxxxxxxxx> > >> Signed-off-by: Tao Hu <taohu@xxxxxxxxxxxx> > >> --- > >> 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@xxxxxxxxx] > >> > Sent: Saturday, October 31, 2009 1:22 AM > >> > To: HU TAO-TGHK48 > >> > Cc: linux-omap@xxxxxxxxxxxxxxx; Tony Lindgren; Yang Fei-AFY095; > >> > jon-hunter@xxxxxx; 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@xxxxxxxxxxxx> > >> > wrote: > >> > > From a36dac7ee6140ffa23f0adc024964aaf3e266e5f Mon Sep 17 > >> > 00:00:00 2001 > >> > > From: Tao Hu <taohu@xxxxxxxxxxxx> > >> > > 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@xxxxxxxxxxxx> > >> > > Signed-off-by: Tao Hu <taohu@xxxxxxxxxxxx> > >> > > --- > >> > > 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
