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
| ||
|
Date: Wed, 16 Jan 2013 15:18:42 +0530 From: Viresh Kumar <viresh.kumar@...aro.org> To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com> Cc: Vinod Koul <vinod.koul@...el.com>, linux-kernel@...r.kernel.org, spear-devel <spear-devel@...t.st.com> Subject: Re: [PATCH] dw_dmac: switch to use dma_pool API On Wed, Jan 16, 2013 at 2:40 PM, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote: > It makes the code cleaner and allows developer not to worry about cache > coherency issues. The price for this is a bit slower access to the descriptor > structures. A good patch to have. But i would write subject as: DMAENGINE: dw_dmac: Allocate dma descriptors from DMA_COHERENT memory Currently descriptors are allocated from normal cacheable memory and that slows down filling the descriptors, as we need to call cache_coherency routines afterwards. It would be better to allocate memory for these descriptors from DMA_COHERENT memory. This would make code much cleaner too. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > @@ -777,24 +753,17 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src, > prev->lli.llp = 0; > - dma_sync_single_for_device(chan2parent(chan), > - prev->txd.phys, sizeof(prev->lli), > - DMA_TO_DEVICE); > delete this blank line too. > first->txd.flags = flags; > first->len = len; > @@ -968,9 +929,6 @@ slave_sg_fromdev_fill_desc: > prev->lli.ctllo |= DWC_CTLL_INT_EN; > > prev->lli.llp = 0; > - dma_sync_single_for_device(chan2parent(chan), > - prev->txd.phys, sizeof(prev->lli), > - DMA_TO_DEVICE); > ditto > first->len = total_len; > > @@ -1125,7 +1083,6 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan) > + memset(desc, 0, sizeof(struct dw_desc)); > + why do we need this? We always fill all fields of this structure, isn't it? > @@ -1171,14 +1128,10 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan) > spin_unlock_irqrestore(&dwc->lock, flags); > > dev_dbg(chan2dev(chan), "%s: allocated %d descriptors\n", __func__, i); > - leave this poor blank line as is :) > return i; > > err_desc_alloc: > - kfree(desc); > - > dev_info(chan2dev(chan), "only allocated %d descriptors\n", i); > - ditto > return i; > } > Acked-by: Viresh Kumar <viresh.kumar@...aro.org> -- 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/
Powered by blists - more mailing lists