| Message ID | 20110629140038.GB25931@S2100-06.ap.freescale.net |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 29, 2011 at 10:00:39PM +0800, Shawn Guo wrote: > On Wed, Jun 29, 2011 at 02:29:42PM +0200, Wolfram Sang wrote: > > > > > > Still, the problem exists: When a second channel GPMI channel is > > > > requested, dmaengine will return -EBUSY, because the DMAIRQ is already > > > > taken. > > > > > > > Yes, we should change the DMA code, it is a DMA bug. > > > I ever submitted a patch about the issue: > > > http://patchwork.ozlabs.org/patch/87145/ > > > > That approach was rejected because it would register the same handler > > n-times where one time would do. Your other approach puts too much > > mach-specific details into the driver IMO and probably won't scale very > > well. Maybe we should add something to the private dma_data (like flags > > indicating SHARED) and then do some refcounting? > > > I would suggest leave this gpmi specific quirk to gpmi driver to sort > out. With the following mxs-dma change, it should work if gpmi driver > can pass the valid gpmi irq number for only one gpmi channel, and -1 > for all others. ...which brings us right into the 'NO_IRQ is 0' discussion :) Other than that, [thinking loud] this will help if all irq-sharing channels are handled by the same driver. If not, we would just add IRQF_SHARED (hopefully this will never be needed). Yup, sounds reasonable to me. Will give it a second thought later, though.
On Wed, Jun 29, 2011 at 04:15:57PM +0200, Wolfram Sang wrote: > On Wed, Jun 29, 2011 at 10:00:39PM +0800, Shawn Guo wrote: > > On Wed, Jun 29, 2011 at 02:29:42PM +0200, Wolfram Sang wrote: > > > > > > > > Still, the problem exists: When a second channel GPMI channel is > > > > > requested, dmaengine will return -EBUSY, because the DMAIRQ is already > > > > > taken. > > > > > > > > > Yes, we should change the DMA code, it is a DMA bug. > > > > I ever submitted a patch about the issue: > > > > http://patchwork.ozlabs.org/patch/87145/ > > > > > > That approach was rejected because it would register the same handler > > > n-times where one time would do. Your other approach puts too much > > > mach-specific details into the driver IMO and probably won't scale very > > > well. Maybe we should add something to the private dma_data (like flags > > > indicating SHARED) and then do some refcounting? > > > > > I would suggest leave this gpmi specific quirk to gpmi driver to sort > > out. With the following mxs-dma change, it should work if gpmi driver > > can pass the valid gpmi irq number for only one gpmi channel, and -1 > > for all others. > > ...which brings us right into the 'NO_IRQ is 0' discussion :) > Though I do not know what it means exactly, number 0 is an valid IRQ on both mx23 and mx28 (see mx23.h and mx28.h). > Other than that, [thinking loud] this will help if all irq-sharing > channels are handled by the same driver. If not, we would just add > IRQF_SHARED (hopefully this will never be needed). Yup, sounds > reasonable to me. Will give it a second thought later, though. > GPMI is the only mxs-dma user that gets irq-sharing. So yes, all irq-sharing channels are handled by the same driver, gpmi-nfc :)
Hi Shawn, > > > I would suggest leave this gpmi specific quirk to gpmi driver to sort > > > out. With the following mxs-dma change, it should work if gpmi driver > > > can pass the valid gpmi irq number for only one gpmi channel, and -1 > > > for all others. > > > > ...which brings us right into the 'NO_IRQ is 0' discussion :) > > > Though I do not know what it means exactly, number 0 is an valid IRQ > on both mx23 and mx28 (see mx23.h and mx28.h). It could be remapped. It is a looong story. Start here if you are interested: http://lkml.org/lkml/2005/11/21/221 http://yarchive.net/comp/linux/no_irq.html Currently, it's a can of worms. Hopefully, irq_desc might help somewhen. > > Other than that, [thinking loud] this will help if all irq-sharing > > channels are handled by the same driver. If not, we would just add > > IRQF_SHARED (hopefully this will never be needed). Yup, sounds > > reasonable to me. Will give it a second thought later, though. > > > GPMI is the only mxs-dma user that gets irq-sharing. So yes, all > irq-sharing channels are handled by the same driver, gpmi-nfc :) Currently, yes. But we have to make it future-proof. Regards, Wolfram
On Thu, Jun 30, 2011 at 06:46:55AM +0200, Wolfram Sang wrote: > Hi Shawn, > > > > > I would suggest leave this gpmi specific quirk to gpmi driver to sort > > > > out. With the following mxs-dma change, it should work if gpmi driver > > > > can pass the valid gpmi irq number for only one gpmi channel, and -1 > > > > for all others. > > > > > > ...which brings us right into the 'NO_IRQ is 0' discussion :) > > > > > Though I do not know what it means exactly, number 0 is an valid IRQ > > on both mx23 and mx28 (see mx23.h and mx28.h). > > It could be remapped. It is a looong story. Start here if you are interested: > > http://lkml.org/lkml/2005/11/21/221 > http://yarchive.net/comp/linux/no_irq.html > > Currently, it's a can of worms. Hopefully, irq_desc might help somewhen. > Thanks for the info. It deserves a good study. With your comment, I would use macro NO_IRQ than -1 for the mxs-dma change. > > > Other than that, [thinking loud] this will help if all irq-sharing > > > channels are handled by the same driver. If not, we would just add > > > IRQF_SHARED (hopefully this will never be needed). Yup, sounds > > > reasonable to me. Will give it a second thought later, though. > > > > > GPMI is the only mxs-dma user that gets irq-sharing. So yes, all > > irq-sharing channels are handled by the same driver, gpmi-nfc :) > > Currently, yes. But we have to make it future-proof. > For the SoCs coming in future, some, e.g. mx6 series, is the same case, and some, like mx50, has separate irq for each gpmi channel, which is even better. So we can survive for quite a long time.
diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c index 88aad4f..ea27002 100644 --- a/drivers/dma/mxs-dma.c +++ b/drivers/dma/mxs-dma.c @@ -327,10 +327,12 @@ static int mxs_dma_alloc_chan_resources(struct dma_chan *chan) memset(mxs_chan->ccw, 0, PAGE_SIZE); - ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler, - 0, "mxs-dma", mxs_dma); - if (ret) - goto err_irq; + if (mxs_chan->chan_irq >= 0) { + ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler, + 0, "mxs-dma", mxs_dma); + if (ret) + goto err_irq; + } ret = clk_enable(mxs_dma->clk); if (ret)