linux-kernel - Re: [PATCH v3] dmaengine: add CSR SiRFprimaII DMAC driver

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: <1318869759.23438.159.camel@vkoul-udesk3>
Date:	Mon, 17 Oct 2011 22:12:39 +0530
From:	Vinod Koul <vinod.koul@...el.com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Barry Song <21cnbao@...il.com>,
	Jassi Brar <jaswinder.singh@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-kernel@...r.kernel.org, workgroup.linux@....com,
	Rongjun Ying <rongjun.ying@....com>,
	Barry Song <Barry.Song@....com>
Subject: Re: [PATCH v3] dmaengine: add CSR SiRFprimaII DMAC driver

On Mon, 2011-10-17 at 17:11 +0200, Arnd Bergmann wrote:
> On Monday 17 October 2011, Barry Song wrote:
> > >> +
> > >> +     /* Start the DMA transfer */
> > >> +     writel_relaxed(sdesc->width, sdma->base + SIRFSOC_DMA_WIDTH_0 + cid * 4);
> > >> +     writel_relaxed(cid | (schan->mode << SIRFSOC_DMA_MODE_CTRL_BIT) |
> > >> +             (sdesc->dir << SIRFSOC_DMA_DIR_CTRL_BIT),
> > >> +             sdma->base + cid * 0x10 + SIRFSOC_DMA_CH_CTRL);
> > >> +     writel_relaxed(sdesc->xlen, sdma->base + cid * 0x10 + SIRFSOC_DMA_CH_XLEN);
> > >> +     writel_relaxed(sdesc->ylen, sdma->base + cid * 0x10 + SIRFSOC_DMA_CH_YLEN);
> > >> +     writel_relaxed(readl_relaxed(sdma->base + SIRFSOC_DMA_INT_EN) | (1 << cid),
> > >> +             sdma->base + SIRFSOC_DMA_INT_EN);
> > >> +     writel(sdesc->addr >> 2, sdma->base + cid * 0x10 + SIRFSOC_DMA_CH_ADDR);
> > >> +
> > >> +     if (sdesc->cyclic) {
> > >> +             writel((1 << cid) | 1 << (cid + 16) |
> > >> +                     readl_relaxed(sdma->base + SIRFSOC_DMA_CH_LOOP_CTRL),
> > >> +                     sdma->base + SIRFSOC_DMA_CH_LOOP_CTRL);
> > >> +             schan->happened_cyclic = schan->completed_cyclic = 0;
> > >> +     }
> > > any reason why we have mixed use of writel_relaxes and writel?
> > > Shouldn't all the DMA register writes be done only using writel?
> > 
> > Arnd comment this in v2.
> 
> The new version looks good to me, but a comment in the source code would
> be very helpful, especially to prevent people from attempting to "fix" it
> in the future.
> 
> I'm not sure about the writel/readl_relaxed in the end of that function,
> because I don't understand what having a "cyclic" descriptor means.
"cyclic" descriptor represents a dma operation which is cyclic in
nature, i.e would auto restart the DMA operation again, till it is
explicitly aborted.
Typically you need to tell dmac that it has to perform such operation
after the current transfer/list is completed.
In above case looks like you have to do that by performing a writel
which invokes a readl_relaxed as well

-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/