Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932596AbbDOTdc (ORCPT ); Wed, 15 Apr 2015 15:33:32 -0400 Received: from www.augenpunkt.de ([213.239.207.9]:46493 "EHLO www.augenpunkt.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751480AbbDOTdX (ORCPT ); Wed, 15 Apr 2015 15:33:23 -0400 X-Greylist: delayed 1955 seconds by postgrey-1.27 at vger.kernel.org; Wed, 15 Apr 2015 15:33:23 EDT Message-ID: <552EB54A.3060404@lategoodbye.de> Date: Wed, 15 Apr 2015 21:00:26 +0200 From: Stefan Wahren User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: =?UTF-8?B?Tm9yYWxmIFRyw7hubmVz?= , dmaengine@vger.kernel.org, vinod.koul@intel.com CC: linux-kernel@vger.kernel.org, jonathan@raspberrypi.org, linux-rpi-kernel@lists.infradead.org, dan.j.williams@intel.com Subject: Re: [PATCH] dmaengine: bcm2835: Add slave dma support References: <1429091778-26350-1-git-send-email-noralf@tronnes.org> In-Reply-To: <1429091778-26350-1-git-send-email-noralf@tronnes.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5494 Lines: 196 Hi Noralf, Am 15.04.2015 um 11:56 schrieb Noralf Trønnes: > Add slave transfer capability to BCM2835 dmaengine driver. > This patch is pulled from the bcm2708-dmaengine driver in the > Raspberry Pi repo. The work was done by Gellert Weisz. > > Tested with the bcm2835-mmc driver from the same repo. why not with the upstream kernel? > > Signed-off-by: Noralf Trønnes > --- > > Gellert Weisz has ended his internship with Raspberry Pi Trading and > was not available to sign off this patch. > > Changes from original code: > Remove slave_id use. > SDHCI_BCM_DMA_WAITS changed to BCM2835_DMA_WAIT_CYCLES. > Use SZ_* macros instead of decimal values. > Change MAX_LITE_TRANSFER from 32k to 64K - 1. > Fix several whitespace issues. > > Lee Jones' comments in previous email to Piotr Król: > Remove __func__ from dev_err message. > Cleanup comments. > > > Quick testing: > Enable CONFIG_DMA_BCM2835 > Apply patch: https://gist.github.com/notro/0c9184df1b43a793f006 > > > drivers/dma/bcm2835-dma.c | 200 ++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 186 insertions(+), 14 deletions(-) > > diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c > index c92d6a7..4230aac 100644 > --- a/drivers/dma/bcm2835-dma.c > +++ b/drivers/dma/bcm2835-dma.c > @@ -1,11 +1,10 @@ > [...] > + > +static struct dma_async_tx_descriptor * > +bcm2835_dma_prep_slave_sg(struct dma_chan *chan, > + struct scatterlist *sgl, > + unsigned int sg_len, > + enum dma_transfer_direction direction, > + unsigned long flags, void *context) > +{ > + struct bcm2835_chan *c = to_bcm2835_dma_chan(chan); > + enum dma_slave_buswidth dev_width; > + struct bcm2835_desc *d; > + dma_addr_t dev_addr; > + struct scatterlist *sgent; > + unsigned int es, sync_type; > + unsigned int i, j, splitct, max_size; I think "split_cnt" would be better. > + > + if (!is_slave_direction(direction)) { > + dev_err(chan->device->dev, "direction not supported\n"); > + return NULL; > + } > + > + if (direction == DMA_DEV_TO_MEM) { > + dev_addr = c->cfg.src_addr; > + dev_width = c->cfg.src_addr_width; > + sync_type = BCM2835_DMA_S_DREQ; > + } else { > + dev_addr = c->cfg.dst_addr; > + dev_width = c->cfg.dst_addr_width; > + sync_type = BCM2835_DMA_D_DREQ; > + } > + > + /* Bus width translates to the element size (ES) */ > + switch (dev_width) { > + case DMA_SLAVE_BUSWIDTH_4_BYTES: > + es = BCM2835_DMA_DATA_TYPE_S32; Looks like "es" is never used. > + break; > + default: A dev_err() might be useful here. > + return NULL; > + } > + > + /* Allocate and setup the descriptor. */ > + d = kzalloc(sizeof(*d), GFP_NOWAIT); > + if (!d) > + return NULL; > + > + d->dir = direction; > + > + if (c->ch >= 8) /* LITE channel */ > + max_size = MAX_LITE_TRANSFER; > + else > + max_size = MAX_NORMAL_TRANSFER; > + > + /* > + * Store the length of the SG list in d->frames > + * taking care to account for splitting up transfers > + * too large for a LITE channel > + */ > + d->frames = 0; > + for_each_sg(sgl, sgent, sg_len, i) { > + unsigned int len = sg_dma_len(sgent); > + > + d->frames += 1 + len / max_size; If it's correct this should be more intuitive: d->frames += len / max_size + 1; > + } > + > + /* Allocate memory for control blocks */ > + d->control_block_size = d->frames * sizeof(struct bcm2835_dma_cb); > + d->control_block_base = dma_zalloc_coherent(chan->device->dev, > + d->control_block_size, &d->control_block_base_phys, > + GFP_NOWAIT); > + if (!d->control_block_base) { > + kfree(d); > + return NULL; > + } > + > + /* > + * Iterate over all SG entries, create a control block > + * for each frame and link them together. > + * Count the number of times an SG entry had to be split > + * as a result of using a LITE channel > + */ > + splitct = 0; > + > + for_each_sg(sgl, sgent, sg_len, i) { > + dma_addr_t addr = sg_dma_address(sgent); > + unsigned int len = sg_dma_len(sgent); > + > + for (j = 0; j < len; j += max_size) { It should be possible to move declaration of "j" down here. > + struct bcm2835_dma_cb *control_block = > + &d->control_block_base[i + splitct]; > + > + /* Setup addresses */ > + if (d->dir == DMA_DEV_TO_MEM) { > + control_block->info = BCM2835_DMA_D_INC | > + BCM2835_DMA_D_WIDTH | > + BCM2835_DMA_S_DREQ; > + control_block->src = dev_addr; > + control_block->dst = addr + (dma_addr_t)j; > + } else { > + control_block->info = BCM2835_DMA_S_INC | > + BCM2835_DMA_S_WIDTH | > + BCM2835_DMA_D_DREQ; > + control_block->src = addr + (dma_addr_t)j; > + control_block->dst = dev_addr; > + } > + > + /* Common part */ > + control_block->info |= > + BCM2835_DMA_WAITS(BCM2835_DMA_WAIT_CYCLES); > + control_block->info |= BCM2835_DMA_WAIT_RESP; > + > + /* Enable */ > + if (i == sg_len - 1 && len - j <= max_size) > + control_block->info |= BCM2835_DMA_INT_EN; > + > + /* Setup synchronization */ > + if (sync_type != 0) if (sync_type) ? > + control_block->info |= sync_type; > + > + /* Setup DREQ channel */ > + if (c->dreq) > + control_block->info |= > + BCM2835_DMA_PER_MAP(c->dreq); > + Best regards Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/