Hi Laurent Thank you for your patch > The DMAC is a general purpose multi-channel DMA controller that supports > both slave and memcpy transfers. > > The driver currently supports the DMAC found in the r8a7790 and r8a7791 > SoCs. Support for compatible DMA controllers (such as the audio DMAC) > will be added later. > > Feature-wise, automatic hardware handling of descriptors chains isn't > supported yet. LPAE support is implemented. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > --- (snip) > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > new file mode 100644 > index 0000000..9cb1dd3 > --- /dev/null > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -0,0 +1,1514 @@ > +/* > + * Renesas R-Car Gen2 DMA Controller Driver We can reuse this driver in Gen3 too ? "Renesas R-Car" only is very enough. > +static void rcar_dmac_chan_start_xfer(struct rcar_dmac_chan *chan) > +{ > + struct rcar_dmac_desc *desc = chan->desc.running; > + struct rcar_dmac_hw_desc *hwdesc = desc->running; > + > + dev_dbg(chan->chan.device->dev, > + "chan%u: queue hwdesc %p: %u@%pad -> %pad\n", > + chan->index, hwdesc, hwdesc->size, &hwdesc->src_addr, > + &hwdesc->dst_addr); > + > + WARN_ON_ONCE(rcar_dmac_chan_is_busy(chan)); > + > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT > + rcar_dmac_chan_write(chan, RCAR_DMAFIXSAR, hwdesc->src_addr >> 32); > + rcar_dmac_chan_write(chan, RCAR_DMAFIXDAR, hwdesc->dst_addr >> 32); > +#endif > + rcar_dmac_chan_write(chan, RCAR_DMASAR, hwdesc->src_addr & 0xffffffff); > + rcar_dmac_chan_write(chan, RCAR_DMADAR, hwdesc->dst_addr & 0xffffffff); > + > + if (chan->mid_rid >= 0) > + rcar_dmac_chan_write(chan, RCAR_DMARS, chan->mid_rid); I guess (chan->mid_rid < 0) case should be error > + > + rcar_dmac_chan_write(chan, RCAR_DMATCR, > + hwdesc->size >> desc->xfer_shift); > + > + rcar_dmac_chan_write(chan, RCAR_DMACHCR, desc->chcr | RCAR_DMACHCR_DE | > + RCAR_DMACHCR_IE); > +} (snip) > +static dma_cookie_t rcar_dmac_tx_submit(struct dma_async_tx_descriptor *tx) > +{ > + struct rcar_dmac_chan *chan = to_rcar_dmac_chan(tx->chan); > + struct rcar_dmac_desc *desc = to_rcar_dmac_desc(tx); > + unsigned long flags; > + dma_cookie_t cookie; > + bool resume; > + > + mutex_lock(&chan->power_lock); > + > + spin_lock_irqsave(&chan->lock, flags); > + > + cookie = dma_cookie_assign(tx); > + > + dev_dbg(chan->chan.device->dev, "chan%u: submit #%d@%p\n", > + chan->index, tx->cookie, desc); > + > + list_add_tail(&desc->node, &chan->desc.pending); > + desc->running = list_first_entry(&desc->hwdescs, > + struct rcar_dmac_hw_desc, node); > + > + /* Resume the device when submitting the first descriptor. */ > + resume = chan->desc.submitted++ == 0; > + > + spin_unlock_irqrestore(&chan->lock, flags); > + > + if (resume) > + pm_runtime_get_sync(chan->chan.device->dev); pm_runtime_xxx itself has inside counter ? > + > + mutex_unlock(&chan->power_lock); > + > + return cookie; > +} (snip) > +static int rcar_dmac_hw_desc_alloc(struct rcar_dmac_chan *chan) > +{ > + struct rcar_dmac_desc_page *page; > + LIST_HEAD(list); > + unsigned int i; > + > + page = (void *)get_zeroed_page(GFP_KERNEL); > + if (page == NULL) > + return -ENOMEM; I like "if (!page)" > + > + for (i = 0; i < RCAR_DMAC_HWDESCS_PER_PAGE; ++i) { > + struct rcar_dmac_hw_desc *hwdesc = &page->hwdescs[i]; > + > + list_add_tail(&hwdesc->node, &list); > + } > + > + spin_lock_irq(&chan->lock); > + list_splice_tail(&list, &chan->desc.hw_free); > + list_add_tail(&page->node, &chan->desc.pages); > + spin_unlock_irq(&chan->lock); > + > + return 0; > +} (snip) > +static void rcar_dmac_chan_configure_desc(struct rcar_dmac_chan *chan, > + struct rcar_dmac_desc *desc) > +{ > + static const u32 chcr_ts[] = { > + RCAR_DMACHCR_TS_1B, RCAR_DMACHCR_TS_2B, > + RCAR_DMACHCR_TS_4B, RCAR_DMACHCR_TS_8B, > + RCAR_DMACHCR_TS_16B, RCAR_DMACHCR_TS_32B, > + RCAR_DMACHCR_TS_64B, > + }; > + > + unsigned int xfer_size; > + u32 chcr; > + > + switch (desc->direction) { > + case DMA_DEV_TO_MEM: > + chcr = RCAR_DMACHCR_DM_INC | RCAR_DMACHCR_SM_FIXED > + | RCAR_DMACHCR_RS_DMARS; > + xfer_size = chan->xfer_size; > + break; > + > + case DMA_MEM_TO_DEV: > + chcr = RCAR_DMACHCR_DM_FIXED | RCAR_DMACHCR_SM_INC > + | RCAR_DMACHCR_RS_DMARS; > + xfer_size = chan->xfer_size; > + break; > + > + case DMA_MEM_TO_MEM: > + default: > + chcr = RCAR_DMACHCR_DM_INC | RCAR_DMACHCR_SM_INC > + | RCAR_DMACHCR_RS_AUTO; > + xfer_size = RCAR_DMAC_MEMCPY_XFER_SIZE; > + break; > + } > + > + desc->xfer_shift = ilog2(xfer_size); > + desc->chcr = chcr | chcr_ts[desc->xfer_shift]; > +} Here and rcar_dmac_slave_config()'s chcr method is very nice for me !! > +/* > + * rcar_dmac_chan_prep_sg - prepare transfer descriptors from an SG list > + * > + * Common routine for public (MEMCPY) and slave DMA. The MEMCPY case is also > + * converted to scatter-gather to guarantee consistent locking and a correct > + * list manipulation. For slave DMA direction carries the usual meaning, and, > + * logically, the SG list is RAM and the addr variable contains slave address, > + * e.g., the FIFO I/O register. For MEMCPY direction equals DMA_MEM_TO_MEM > + * and the SG list contains only one element and points at the source buffer. > + */ > +static struct dma_async_tx_descriptor * > +rcar_dmac_chan_prep_sg(struct rcar_dmac_chan *chan, struct scatterlist *sgl, > + unsigned int sg_len, dma_addr_t dev_addr, > + enum dma_transfer_direction dir, unsigned long dma_flags, > + bool cyclic) > +{ > + struct rcar_dmac_hw_desc *hwdesc; > + struct rcar_dmac_desc *desc; > + struct scatterlist *sg = sgl; > + size_t full_size = 0; > + unsigned int i; > + > + desc = rcar_dmac_desc_get(chan); > + if (desc == NULL) > + return NULL; ditto > +static struct dma_async_tx_descriptor * > +rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, > + size_t buf_len, size_t period_len, > + enum dma_transfer_direction dir, unsigned long flags, > + void *context) > +{ > + struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan); > + struct dma_async_tx_descriptor *desc; > + struct scatterlist *sgl; > + unsigned int sg_len; > + unsigned int i; > + > + /* Someone calling slave DMA on a generic channel? */ > + if (rchan->mid_rid < 0 || buf_len < period_len) { > + dev_warn(chan->device->dev, > + "%s: bad parameter: buf_len=%zu, period_len=%zu, id=%d\n", > + __func__, buf_len, period_len, rchan->mid_rid); > + return NULL; > + } > + > + sg_len = buf_len / period_len; > + if (sg_len > RCAR_DMAC_MAX_SG_LEN) { > + dev_err(chan->device->dev, > + "chan%u: sg length %d exceds limit %d", > + rchan->index, sg_len, RCAR_DMAC_MAX_SG_LEN); > + return NULL; > + } > + > + sgl = kcalloc(RCAR_DMAC_MAX_SG_LEN, sizeof(*sgl), GFP_NOWAIT); > + if (!sgl) > + return NULL; This kcalloc() is using fixed size. we can't use this ? struct scatterlist sgl[RCAR_DMAC_MAX_SG_LEN]; > + > + sg_init_table(sgl, sg_len); > + > + for (i = 0; i < sg_len; ++i) { > + dma_addr_t src = buf_addr + (period_len * i); > + > + sg_set_page(&sgl[i], pfn_to_page(PFN_DOWN(src)), period_len, > + offset_in_page(src)); > + sg_dma_address(&sgl[i]) = src; > + sg_dma_len(&sgl[i]) = period_len; > + } > + > + desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, rchan->slave_addr, > + dir, flags, true); > + > + kfree(sgl); > + return desc; > +} (snip) > +static int rcar_dmac_probe(struct platform_device *pdev) > +{ > + struct dma_device *engine; > + struct rcar_dmac *dmac; > + struct resource *mem; > + unsigned int i; > + int irq; > + int ret; > + > + dmac = devm_kzalloc(&pdev->dev, sizeof(*dmac), GFP_KERNEL); > + if (!dmac) > + return -ENOMEM; > + > + dmac->dev = &pdev->dev; > + platform_set_drvdata(pdev, dmac); > + > + ret = rcar_dmac_parse_of(&pdev->dev, dmac); > + if (ret < 0) > + return ret; > + > + dmac->channels = devm_kcalloc(&pdev->dev, dmac->n_channels, > + sizeof(*dmac->channels), GFP_KERNEL); > + if (!dmac->channels) > + return -ENOMEM; > + > + /* Request resources. */ > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + dmac->iomem = devm_ioremap_resource(&pdev->dev, mem); > + if (IS_ERR(dmac->iomem)) > + return PTR_ERR(dmac->iomem); > + > + irq = platform_get_irq_byname(pdev, "error"); > + if (irq < 0) { > + dev_err(&pdev->dev, "no error IRQ specified\n"); > + return -ENODEV; > + } > + > + ret = devm_request_irq(&pdev->dev, irq, rcar_dmac_isr_error, 0, > + "DMAC Address Error", dmac); > + if (ret) { > + dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n", > + irq, ret); > + return ret; > + } > + > + dmac->clk = clk_get(&pdev->dev, "fck"); > + if (IS_ERR(dmac->clk)) { > + dev_err(&pdev->dev, "unable to get fck clock\n"); > + return PTR_ERR(dmac->clk); > + } Maybe this is my misunderstanding, but, which clock are you expecting to "fck" clock ? Is it not same as pm_runtime_xxx()'s clock ? > + /* Initialize the channels. */ > + INIT_LIST_HEAD(&dmac->engine.channels); > + > + for (i = 0; i < dmac->n_channels; ++i) { > + ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], i); > + if (ret < 0) > + return ret; > + } > + > + /* Enable runtime PM and initialize the device. */ > + pm_runtime_enable(&pdev->dev); > + ret = pm_runtime_get_sync(&pdev->dev); > + if (ret < 0) { > + dev_err(&pdev->dev, "runtime PM get sync failed (%d)\n", ret); > + return ret; > + } > + > + ret = rcar_dmac_init(dmac); > + pm_runtime_put(&pdev->dev); power ON -> initialize -> power OFF here, and shdmac.c has similar method. And it works, but, it is a littile bit strange for me :P Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html