lkml.org 
[lkml]   [2013]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 3/3] dma: Add Freescale eDMA engine driver support
Date
> -----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
\
 
 \ /
  Last update: 2013-08-05 12:01    [W:0.125 / U:0.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site