linux-kernel - RE: [PATCH v2 3/3] dma: Add Freescale eDMA engine driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <B56CDBE15CE27145A4B77D2D24263E851FC086@039-SN1MPN1-004.039d.mgd.msft.net>
Date:	Mon, 5 Aug 2013 09:44:55 +0000
From:	Lu Jingchang-B35083 <B35083@...escale.com>
To:	Lothar Waßmann <LW@...O-electronics.de>
CC:	"vinod.koul@...el.com" <vinod.koul@...el.com>,
	Li Xiaochun-B41219 <B41219@...escale.com>,
	Wang Huan-B18965 <B18965@...escale.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"djbw@...com" <djbw@...com>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH v2 3/3] dma: Add Freescale eDMA engine driver support

> -----Original Message-----
> From: Lothar Waßmann [mailto:LW@...O-electronics.de]
> Sent: Monday, August 05, 2013 3:54 PM
> To: Lu Jingchang-B35083
> Cc: vinod.koul@...el.com; Li Xiaochun-B41219; Wang Huan-B18965; linux-
> kernel@...r.kernel.org; djbw@...com; shawn.guo@...aro.org; linux-arm-
> kernel@...ts.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