Messages in this thread | | | From | Lu Jingchang-B35083 <> | Subject | RE: [PATCH v2 3/3] dma: Add Freescale eDMA engine driver support | Date | Mon, 5 Aug 2013 09:44:55 +0000 |
| |
> -----Original Message----- > From: Lothar Waßmann [mailto:LW@KARO-electronics.de] > Sent: Monday, August 05, 2013 3:54 PM > To: Lu Jingchang-B35083 > Cc: vinod.koul@intel.com; Li Xiaochun-B41219; Wang Huan-B18965; linux- > kernel@vger.kernel.org; djbw@fb.com; shawn.guo@linaro.org; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v2 3/3] dma: Add Freescale eDMA engine driver support
> [...] > > + fsl_desc->n_tcds = sg_len; > > + for (i = 0; i < sg_len; i++) { > > + fsl_desc->tcd[i].vtcd = dma_pool_alloc(fsl_chan->tcd_pool, > > + GFP_ATOMIC, &fsl_desc->tcd[i].ptcd); > > > Why is this called with GFP_ATOMIC while fsl_desc above is being > allocated with GFP_KERNEL? [Lu Jingchang-B35083] I will make these consistently. Thanks. > > > + for (ch = 0; ch < 32; ch++) > > + if (intr & (0x1 << ch)) > > + break; > > + > What, if IRQs for multiple channels are pending at the same time? > You could handle them all in one go without extra calls of the IRQ > handler. [Lu Jingchang-B35083] Yes, It will be more efficiently to handle all the irqs once. Thanks. > > > + writeb(EDMA_CINT_CINT(ch), base_addr + EDMA_CINT); > > + > > + fsl_chan = &fsl_edma->chans[ch]; > > + > > + if (!fsl_chan->edesc->iscyclic) { > > + list_del(&fsl_chan->edesc->vdesc.node); > > > Ain't there any protection needed for the list operation? [Lu Jingchang-B35083] Protection is needed indeed, I will fix this, thanks. > > > + clk_prepare_enable(fsl_edmamux->clk); > > > What, if this fails? [Lu Jingchang-B35083] I will add code to check the return value. Thanks. >
Best Regards, Jingchang
| |