* [PATCH 0/3] dmaengine: add support mmp-pdma
@ 2012-07-20 5:46 Zhangfei Gao
2012-07-20 5:46 ` [PATCH 1/3] dmaengine: mmp-pdma support Zhangfei Gao
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Zhangfei Gao @ 2012-07-20 5:46 UTC (permalink / raw)
To: linux-arm-kernel
mmp-pdma is added under dmaengine framework.
The final purpose is replacing arch/arm/plat-pxa/dma.c
Test on pxa910 with pxa3xx-nand and dmatest.ko
Zhangfei Gao (3):
dmaengine: mmp-pdma support
dmaengine: mmp_tdma: add dt support
dmatest: add dmaengine_slave_config for DMA_MEMCPY
Documentation/devicetree/bindings/dma/mmp-dma.txt | 56 ++
drivers/dma/Kconfig | 7 +
drivers/dma/Makefile | 1 +
drivers/dma/dmatest.c | 4 +
drivers/dma/mmp_pdma.c | 877 +++++++++++++++++++++
drivers/dma/mmp_tdma.c | 51 +-
include/linux/platform_data/mmp_dma.h | 25 +
7 files changed, 1001 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/dma/mmp-dma.txt
create mode 100644 drivers/dma/mmp_pdma.c
create mode 100644 include/linux/platform_data/mmp_dma.h
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-20 5:46 [PATCH 0/3] dmaengine: add support mmp-pdma Zhangfei Gao @ 2012-07-20 5:46 ` Zhangfei Gao 2012-07-24 15:42 ` Arnd Bergmann 2012-07-26 6:51 ` Vinod Koul 2012-07-20 5:46 ` [PATCH 2/3] dmaengine: mmp_tdma: add dt support Zhangfei Gao 2012-07-20 5:46 ` [PATCH 3/3] dmatest: add dmaengine_slave_config for DMA_MEMCPY Zhangfei Gao 2 siblings, 2 replies; 15+ messages in thread From: Zhangfei Gao @ 2012-07-20 5:46 UTC (permalink / raw) To: linux-arm-kernel 1. virtual channel vs. physical channel Virtual channel is managed by dmaengine Physical channel handling resource, such as irq Physical channel is alloced dynamically as descending priority, freed immediately when irq done. The availble highest priority physically channel will alwayes be alloced Issue pending list -> alloc highest dma physically channel available -> dma done -> free physically channel 2. list: running list & pending list submit: desc list -> pending list issue_pending_list: if (IDLE) pending list -> running list; free pending list (RUN) irq: free running list (IDLE) check pendlist -> pending list -> running list; free pending list (RUN) 3. irq: Each list generate one irq, calling callback One list may contain several desc chain, in such case, make sure only the last desc list generate irq. 4. async Submit will add desc chain to pending list, which can be multi-called If multi desc chain is submitted, only the last desc would generate irq -> call back If IDLE, issue_pending_list start pending_list, transforming pendlist to running list If RUN, irq will start pending list 5. test 5.1 pxa3xx_nand on pxa910 5.2 insmod dmatest.ko (threads_per_chan=y) By default drivers/dma/dmatest.c test every channel and test memcpy with 1 threads per channel Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com> --- Documentation/devicetree/bindings/dma/mmp-dma.txt | 30 + drivers/dma/Kconfig | 7 + drivers/dma/Makefile | 1 + drivers/dma/mmp_pdma.c | 877 +++++++++++++++++++++ include/linux/platform_data/mmp_dma.h | 25 + 5 files changed, 940 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mmp-dma.txt create mode 100644 drivers/dma/mmp_pdma.c create mode 100644 include/linux/platform_data/mmp_dma.h diff --git a/Documentation/devicetree/bindings/dma/mmp-dma.txt b/Documentation/devicetree/bindings/dma/mmp-dma.txt new file mode 100644 index 0000000..42cd134 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/mmp-dma.txt @@ -0,0 +1,30 @@ +* MARVELL MMP DMA controller + +For driver/dma/mmp-pdma.c + +Required properties: +- compatible: Should be "mrvl,mmp-pdma" +- reg: Should contain DMA registers location and length. +- interrupts: Either contain all of the per-channel DMA interrupts + or one irq for pdma device +- chancnt: Should be channel number in the soc + +Examples: + +/* each channel has specific irq */ +pdma: dma-controller at d4000000 { + compatible = "mrvl,mmp-pdma"; + reg = <0xd4000000 0x10000>; + interrupts = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15>; + interrupt-parent = <&intcmux32>; + chancnt = <16>; + }; + +/* One irq for all channels */ +pdma: dma-controller at d4000000 { + compatible = "mrvl,mmp-pdma"; + reg = <0xd4000000 0x10000>; + interrupts = <47>; + chancnt = <16>; + }; + diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 8904bb6..663e26b 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -292,6 +292,13 @@ config MMP_TDMA Say Y here if you enabled MMP ADMA, otherwise say N. +config MMP_PDMA + bool "MMP PDMA support" + depends on (ARCH_MMP || ARCH_PXA) + select DMA_ENGINE + help + Support the MMP PDMA engine for PXA and MMP platfrom. + config DMA_ENGINE bool diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index eabe13c..27bd86e 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -32,3 +32,4 @@ obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o obj-$(CONFIG_DMA_SA11X0) += sa11x0-dma.o obj-$(CONFIG_DMA_OMAP) += omap-dma.o obj-$(CONFIG_MMP_TDMA) += mmp_tdma.o +obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c new file mode 100644 index 0000000..a97d4e8 --- /dev/null +++ b/drivers/dma/mmp_pdma.c @@ -0,0 +1,877 @@ +/* + * Copyright 2012 Marvell International Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include <linux/module.h> +#include <linux/init.h> +#include <linux/types.h> +#include <linux/interrupt.h> +#include <linux/dma-mapping.h> +#include <linux/slab.h> +#include <linux/dmaengine.h> +#include <linux/platform_device.h> +#include <linux/device.h> +#include <linux/platform_data/mmp_dma.h> +#include <linux/dmapool.h> +#include <linux/of_device.h> +#include <linux/of.h> + +#include "dmaengine.h" + +#define DCSR 0x0000 +#define DALGN 0x00a0 /* DMA Alignment Register */ +#define DINT 0x00f0 /* DMA Interrupt Register */ +#define DDADR 0x0200 +#define DSADR 0x0204 +#define DTADR 0x0208 +#define DCMD 0x020c + +#define DCSR_RUN (1 << 31) /* Run Bit (read / write) */ +#define DCSR_NODESC (1 << 30) /* No-Descriptor Fetch (read / write) */ +#define DCSR_STOPIRQEN (1 << 29) /* Stop Interrupt Enable (read / write) */ +#define DCSR_REQPEND (1 << 8) /* Request Pending (read-only) */ +#define DCSR_STOPSTATE (1 << 3) /* Stop State (read-only) */ +#define DCSR_ENDINTR (1 << 2) /* End Interrupt (read / write) */ +#define DCSR_STARTINTR (1 << 1) /* Start Interrupt (read / write) */ +#define DCSR_BUSERR (1 << 0) /* Bus Error Interrupt (read / write) */ + +#define DCSR_EORIRQEN (1 << 28) /* End of Receive Interrupt Enable (R/W) */ +#define DCSR_EORJMPEN (1 << 27) /* Jump to next descriptor on EOR */ +#define DCSR_EORSTOPEN (1 << 26) /* STOP on an EOR */ +#define DCSR_SETCMPST (1 << 25) /* Set Descriptor Compare Status */ +#define DCSR_CLRCMPST (1 << 24) /* Clear Descriptor Compare Status */ +#define DCSR_CMPST (1 << 10) /* The Descriptor Compare Status */ +#define DCSR_EORINTR (1 << 9) /* The end of Receive */ + +#define DRCMR_MAPVLD (1 << 7) /* Map Valid (read / write) */ +#define DRCMR_CHLNUM 0x1f /* mask for Channel Number (read / write) */ + +#define DDADR_DESCADDR 0xfffffff0 /* Address of next descriptor (mask) */ +#define DDADR_STOP (1 << 0) /* Stop (read / write) */ + +#define DCMD_INCSRCADDR (1 << 31) /* Source Address Increment Setting. */ +#define DCMD_INCTRGADDR (1 << 30) /* Target Address Increment Setting. */ +#define DCMD_FLOWSRC (1 << 29) /* Flow Control by the source. */ +#define DCMD_FLOWTRG (1 << 28) /* Flow Control by the target. */ +#define DCMD_STARTIRQEN (1 << 22) /* Start Interrupt Enable */ +#define DCMD_ENDIRQEN (1 << 21) /* End Interrupt Enable */ +#define DCMD_ENDIAN (1 << 18) /* Device Endian-ness. */ +#define DCMD_BURST8 (1 << 16) /* 8 byte burst */ +#define DCMD_BURST16 (2 << 16) /* 16 byte burst */ +#define DCMD_BURST32 (3 << 16) /* 32 byte burst */ +#define DCMD_WIDTH1 (1 << 14) /* 1 byte width */ +#define DCMD_WIDTH2 (2 << 14) /* 2 byte width (HalfWord) */ +#define DCMD_WIDTH4 (3 << 14) /* 4 byte width (Word) */ +#define DCMD_LENGTH 0x01fff /* length mask (max = 8K - 1) */ + +#define PDMA_ALIGNMENT 3 +#define PDMA_MAX_DESC_BYTES 0x1000 +enum mmp_pdma_type { + MMP_PDMA = 0, +}; + +struct mmp_pdma_desc_hw { + u32 ddadr; /* Points to the next descriptor + flags */ + u32 dsadr; /* DSADR value for the current transfer */ + u32 dtadr; /* DTADR value for the current transfer */ + u32 dcmd; /* DCMD value for the current transfer */ +} __aligned(32); + +struct mmp_pdma_desc_sw { + struct mmp_pdma_desc_hw desc; + struct list_head node; + struct list_head tx_list; + struct dma_async_tx_descriptor async_tx; +}; + +struct mmp_pdma_phy; + +struct mmp_pdma_chan { + struct device *dev; /* Channel device */ + struct dma_chan chan; /* DMA common channel */ + struct dma_async_tx_descriptor desc; + struct mmp_pdma_phy *phy; + enum dma_transfer_direction dir; + + /* channel's basic info */ + enum mmp_pdma_type type; + struct tasklet_struct tasklet; + u32 dcmd; + u32 drcmr; + u32 dev_addr; + + /* list for desc */ + spinlock_t desc_lock; /* Descriptor list lock */ + struct list_head chain_pending; /* Link descriptors queue for pending */ + struct list_head chain_running; /* Link descriptors queue for running */ + bool idle; /* channel statue machine */ + + struct dma_pool *desc_pool; /* Descriptors pool */ +}; + +struct mmp_pdma_phy { + int idx; + void __iomem *base; + struct mmp_pdma_chan *vchan; +}; + +struct mmp_pdma_device { + int chancnt; + void __iomem *base; + struct device *dev; + struct dma_device device; + struct mmp_pdma_phy *phy; +}; + +#define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx) +#define to_mmp_pdma_desc(lh) container_of(lh, struct mmp_pdma_desc_sw, node) +#define to_mmp_pdma_chan(dchan) container_of(dchan, struct mmp_pdma_chan, chan) +#define to_mmp_pdma_dev(dmadev) container_of(dmadev, struct mmp_pdma_device, device) + +static void set_desc(struct mmp_pdma_phy *phy, dma_addr_t addr) +{ + u32 reg = (phy->idx << 4) + DDADR; + + writel(addr, phy->base + reg); +} + +static void enable_chan(struct mmp_pdma_phy *phy) +{ + u32 reg; + + if (!phy->vchan) + return; + + reg = phy->vchan->drcmr; + reg = (((reg) < 64) ? 0x0100 : 0x1100) + (((reg) & 0x3f) << 2); + writel(DRCMR_MAPVLD | phy->idx, phy->base + reg); + + reg = (phy->idx << 2) + DCSR; + writel(readl(phy->base + reg) | DCSR_RUN, + phy->base + reg); +} + +static void disable_chan(struct mmp_pdma_phy *phy) +{ + u32 reg; + + if (phy) { + reg = (phy->idx << 2) + DCSR; + writel(readl(phy->base + reg) & ~DCSR_RUN, + phy->base + reg); + } +} + +static int clear_chan_irq(struct mmp_pdma_phy *phy) +{ + u32 dcsr; + u32 dint = readl(phy->base + DINT); + u32 reg = (phy->idx << 2) + DCSR; + + if (dint & BIT(phy->idx)) { + /* clear irq */ + dcsr = readl(phy->base + reg); + writel(dcsr, phy->base + reg); + if ((dcsr & DCSR_BUSERR) && (phy->vchan)) + dev_warn(phy->vchan->dev, "DCSR_BUSERR\n"); + return 0; + } + return -EAGAIN; +} + +static irqreturn_t mmp_pdma_chan_handler(int irq, void *dev_id) +{ + struct mmp_pdma_phy *phy = dev_id; + + if (clear_chan_irq(phy) == 0) { + tasklet_schedule(&phy->vchan->tasklet); + return IRQ_HANDLED; + } else + return IRQ_NONE; +} + +static irqreturn_t mmp_pdma_int_handler(int irq, void *dev_id) +{ + struct mmp_pdma_device *pdev = dev_id; + struct mmp_pdma_phy *phy; + u32 dint = readl(pdev->base + DINT); + int i, ret; + int irq_num = 0; + + while (dint) { + i = __ffs(dint); + dint &= (dint - 1); + phy = &pdev->phy[i]; + ret = mmp_pdma_chan_handler(irq, phy); + if (ret == IRQ_HANDLED) + irq_num++; + } + + if (irq_num) + return IRQ_HANDLED; + else + return IRQ_NONE; +} + +/* lookup free phy channel as descending priority */ +static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) +{ + int prio, i; + struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); + struct mmp_pdma_phy *phy; + + /* + * dma channel priorities + * ch 0 - 3, 16 - 19 <--> (0) + * ch 4 - 7, 20 - 23 <--> (1) + * ch 8 - 11, 24 - 27 <--> (2) + * ch 12 - 15, 28 - 31 <--> (3) + */ + for (prio = 0; prio <= (((pdev->chancnt - 1) & 0xf) >> 2) ; prio++) { + for (i = 0; i < pdev->chancnt; i++) { + if (prio != ((i & 0xf) >> 2)) + continue; + phy = &pdev->phy[i]; + if (!phy->vchan) { + phy->vchan = pchan; + return phy; + } + } + } + + return NULL; +} + +/* desc->tx_list ==> pending list */ +static void append_pending_queue(struct mmp_pdma_chan *chan, + struct mmp_pdma_desc_sw *desc) +{ + struct mmp_pdma_desc_sw *tail = + to_mmp_pdma_desc(chan->chain_pending.prev); + + if (list_empty(&chan->chain_pending)) + goto out_splice; + + /* one irq per queue, even appended */ + tail->desc.ddadr = desc->async_tx.phys; + tail->desc.dcmd &= ~DCMD_ENDIRQEN; + + /* softly link to pending list */ +out_splice: + list_splice_tail_init(&desc->tx_list, &chan->chain_pending); +} + +/** + * start_pending_queue - transfer any pending transactions + * pending list ==> running list + */ +static void start_pending_queue(struct mmp_pdma_chan *chan) +{ + struct mmp_pdma_desc_sw *desc; + + /* still in running, irq will start the pending list */ + if (!chan->idle) { + dev_dbg(chan->dev, "DMA controller still busy\n"); + return; + } + + if (list_empty(&chan->chain_pending)) { + /* chance to re-fetch phy channel with higher prio */ + if (chan->phy) { + chan->phy->vchan = NULL; + chan->phy = NULL; + } + dev_dbg(chan->dev, "no pending list\n"); + return; + } + + if (!chan->phy) { + chan->phy = lookup_phy(chan); + if (!chan->phy) { + dev_dbg(chan->dev, "no free dma channel\n"); + return; + } + } + + /* + * pending -> running + * reintilize pending list + */ + desc = list_first_entry(&chan->chain_pending, + struct mmp_pdma_desc_sw, node); + list_splice_tail_init(&chan->chain_pending, &chan->chain_running); + + /* + * Program the descriptor's address into the DMA controller, + * then start the DMA transaction + */ + set_desc(chan->phy, desc->async_tx.phys); + enable_chan(chan->phy); + chan->idle = false; +} + + +/* desc->tx_list ==> pending list */ +static dma_cookie_t mmp_pdma_tx_submit(struct dma_async_tx_descriptor *tx) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(tx->chan); + struct mmp_pdma_desc_sw *desc = tx_to_mmp_pdma_desc(tx); + struct mmp_pdma_desc_sw *child; + unsigned long flags; + dma_cookie_t cookie = -EBUSY; + + spin_lock_irqsave(&chan->desc_lock, flags); + + list_for_each_entry(child, &desc->tx_list, node) { + cookie = dma_cookie_assign(&child->async_tx); + } + + append_pending_queue(chan, desc); + + spin_unlock_irqrestore(&chan->desc_lock, flags); + + return cookie; +} + +struct mmp_pdma_desc_sw *mmp_pdma_alloc_descriptor(struct mmp_pdma_chan *chan) +{ + struct mmp_pdma_desc_sw *desc; + dma_addr_t pdesc; + + desc = dma_pool_alloc(chan->desc_pool, GFP_ATOMIC, &pdesc); + if (!desc) { + dev_err(chan->dev, "out of memory for link descriptor\n"); + return NULL; + } + + memset(desc, 0, sizeof(*desc)); + INIT_LIST_HEAD(&desc->tx_list); + dma_async_tx_descriptor_init(&desc->async_tx, &chan->chan); + /* each desc has submit */ + desc->async_tx.tx_submit = mmp_pdma_tx_submit; + desc->async_tx.phys = pdesc; + + return desc; +} + +/** + * mmp_pdma_alloc_chan_resources - Allocate resources for DMA channel. + * + * This function will create a dma pool for descriptor allocation. + * Request irq only when channel is requested + * Return - The number of allocated descriptors. + */ + +static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); + + if (chan->desc_pool) + return 1; + + chan->desc_pool = + dma_pool_create(dev_name(&dchan->dev->device), chan->dev, + sizeof(struct mmp_pdma_desc_sw), + __alignof__(struct mmp_pdma_desc_sw), 0); + if (!chan->desc_pool) { + dev_err(chan->dev, "unable to allocate descriptor pool\n"); + return -ENOMEM; + } + if (chan->phy) { + chan->phy->vchan = NULL; + chan->phy = NULL; + } + chan->idle = true; + chan->dev_addr = 0; + return 1; +} + +static void mmp_pdma_free_desc_list(struct mmp_pdma_chan *chan, + struct list_head *list) +{ + struct mmp_pdma_desc_sw *desc, *_desc; + + list_for_each_entry_safe(desc, _desc, list, node) { + list_del(&desc->node); + dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys); + + } +} + +static void mmp_pdma_free_chan_resources(struct dma_chan *dchan) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); + unsigned long flags; + + spin_lock_irqsave(&chan->desc_lock, flags); + mmp_pdma_free_desc_list(chan, &chan->chain_pending); + mmp_pdma_free_desc_list(chan, &chan->chain_running); + spin_unlock_irqrestore(&chan->desc_lock, flags); + + dma_pool_destroy(chan->desc_pool); + chan->desc_pool = NULL; + chan->idle = true; + chan->dev_addr = 0; + if (chan->phy) { + chan->phy->vchan = NULL; + chan->phy = NULL; + } + return; +} + +static struct dma_async_tx_descriptor * +mmp_pdma_prep_memcpy(struct dma_chan *dchan, + dma_addr_t dma_dst, dma_addr_t dma_src, + size_t len, unsigned long flags) +{ + struct mmp_pdma_chan *chan; + struct mmp_pdma_desc_sw *first = NULL, *prev = NULL, *new; + size_t copy = 0; + + if (!dchan) + return NULL; + + if (!len) + return NULL; + + chan = to_mmp_pdma_chan(dchan); + + do { + /* Allocate the link descriptor from DMA pool */ + new = mmp_pdma_alloc_descriptor(chan); + if (!new) { + dev_err(chan->dev, "no memory for desc\n"); + goto fail; + } + + copy = min_t(size_t, len, PDMA_MAX_DESC_BYTES); + + new->desc.dcmd = chan->dcmd | (DCMD_LENGTH & copy); + new->desc.dsadr = dma_src; + new->desc.dtadr = dma_dst; + + if (!first) + first = new; + else + prev->desc.ddadr = new->async_tx.phys; + + new->async_tx.cookie = 0; + async_tx_ack(&new->async_tx); + + prev = new; + len -= copy; + + if (chan->dir == DMA_MEM_TO_DEV) { + dma_src += copy; + } else if (chan->dir == DMA_DEV_TO_MEM) { + dma_dst += copy; + } else if (chan->dir == DMA_MEM_TO_MEM) { + dma_src += copy; + dma_dst += copy; + } + + /* Insert the link descriptor to the LD ring */ + list_add_tail(&new->node, &first->tx_list); + } while (len); + + first->async_tx.flags = flags; /* client is in control of this ack */ + first->async_tx.cookie = -EBUSY; + + /* last desc and fire IRQ */ + new->desc.ddadr = DDADR_STOP; + new->desc.dcmd |= DCMD_ENDIRQEN; + + return &first->async_tx; + +fail: + if (first) + mmp_pdma_free_desc_list(chan, &first->tx_list); + return NULL; +} + +static struct dma_async_tx_descriptor * +mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl, + unsigned int sg_len, enum dma_transfer_direction dir, + unsigned long flags, void *context) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); + struct mmp_pdma_desc_sw *first = NULL, *prev = NULL, *new = NULL; + size_t len, avail; + struct scatterlist *sg; + dma_addr_t addr; + int i; + + if ((sgl == NULL) || (sg_len == 0)) + return NULL; + + for_each_sg(sgl, sg, sg_len, i) { + addr = sg_dma_address(sg); + avail = sg_dma_len(sgl); + + do { + len = min_t(size_t, avail, PDMA_MAX_DESC_BYTES); + + /* allocate and populate the descriptor */ + new = mmp_pdma_alloc_descriptor(chan); + if (!new) { + dev_err(chan->dev, "no memory for desc\n"); + goto fail; + } + + new->desc.dcmd = chan->dcmd | (DCMD_LENGTH & len); + if (dir == DMA_MEM_TO_DEV) { + new->desc.dsadr = addr; + new->desc.dtadr = chan->dev_addr; + } else { + new->desc.dsadr = chan->dev_addr; + new->desc.dtadr = addr; + } + + if (!first) + first = new; + else + prev->desc.ddadr = new->async_tx.phys; + + new->async_tx.cookie = 0; + async_tx_ack(&new->async_tx); + prev = new; + + /* Insert the link descriptor to the LD ring */ + list_add_tail(&new->node, &first->tx_list); + + /* update metadata */ + addr += len; + avail -= len; + } while (avail); + } + + first->async_tx.cookie = -EBUSY; + first->async_tx.flags = flags; + + /* last desc and fire IRQ */ + new->desc.ddadr = DDADR_STOP; + new->desc.dcmd |= DCMD_ENDIRQEN; + + return &first->async_tx; + +fail: + if (first) + mmp_pdma_free_desc_list(chan, &first->tx_list); + return NULL; +} + +static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, + unsigned long arg) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); + struct dma_slave_config *cfg = (void *)arg; + struct mmp_dma_slave_config *mmp_cfg; + unsigned long flags; + int ret = 0; + u32 maxburst = 0, addr = 0; + enum dma_slave_buswidth width = DMA_SLAVE_BUSWIDTH_UNDEFINED; + + if (!dchan) + return -EINVAL; + mmp_cfg = container_of(cfg, struct mmp_dma_slave_config, slave_config); + + switch (cmd) { + case DMA_TERMINATE_ALL: + disable_chan(chan->phy); + if (chan->phy) { + chan->phy->vchan = NULL; + chan->phy = NULL; + } + spin_lock_irqsave(&chan->desc_lock, flags); + mmp_pdma_free_desc_list(chan, &chan->chain_pending); + mmp_pdma_free_desc_list(chan, &chan->chain_running); + spin_unlock_irqrestore(&chan->desc_lock, flags); + chan->idle = true; + break; + case DMA_SLAVE_CONFIG: + if (cfg->direction == DMA_DEV_TO_MEM) { + chan->dcmd = DCMD_INCTRGADDR | DCMD_FLOWSRC; + maxburst = cfg->src_maxburst; + width = cfg->src_addr_width; + addr = cfg->src_addr; + } else if (cfg->direction == DMA_MEM_TO_DEV) { + chan->dcmd = DCMD_INCSRCADDR | DCMD_FLOWTRG; + maxburst = cfg->dst_maxburst; + width = cfg->dst_addr_width; + addr = cfg->dst_addr; + } else if (cfg->direction == DMA_MEM_TO_MEM) { + chan->dcmd = DCMD_INCTRGADDR | DCMD_INCSRCADDR; + width = DMA_SLAVE_BUSWIDTH_UNDEFINED; + maxburst = 32; + } + + if (width == DMA_SLAVE_BUSWIDTH_1_BYTE) + chan->dcmd |= DCMD_WIDTH1; + else if (width == DMA_SLAVE_BUSWIDTH_2_BYTES) + chan->dcmd |= DCMD_WIDTH2; + else if (width == DMA_SLAVE_BUSWIDTH_4_BYTES) + chan->dcmd |= DCMD_WIDTH4; + + if (maxburst == 8) + chan->dcmd |= DCMD_BURST8; + else if (maxburst == 16) + chan->dcmd |= DCMD_BURST16; + else if (maxburst == 32) + chan->dcmd |= DCMD_BURST32; + + if (cfg) + chan->dir = cfg->direction; + if (mmp_cfg) + chan->drcmr = mmp_cfg->drcmr; + chan->dev_addr = addr; + break; + default: + return -ENOSYS; + } + + return ret; +} + +static enum dma_status mmp_pdma_tx_status(struct dma_chan *dchan, + dma_cookie_t cookie, struct dma_tx_state *txstate) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); + enum dma_status ret; + unsigned long flags; + + spin_lock_irqsave(&chan->desc_lock, flags); + ret = dma_cookie_status(dchan, cookie, txstate); + spin_unlock_irqrestore(&chan->desc_lock, flags); + + return ret; +} + +/** + * mmp_pdma_issue_pending - Issue the DMA start command + * pending list ==> running list + */ +static void mmp_pdma_issue_pending(struct dma_chan *dchan) +{ + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); + unsigned long flags; + + spin_lock_irqsave(&chan->desc_lock, flags); + start_pending_queue(chan); + spin_unlock_irqrestore(&chan->desc_lock, flags); +} + +/* + * dma_do_tasklet + * Do call back + * Start pending list + */ +static void dma_do_tasklet(unsigned long data) +{ + struct mmp_pdma_chan *chan = (struct mmp_pdma_chan *)data; + struct mmp_pdma_desc_sw *desc, *_desc; + LIST_HEAD(chain_cleanup); + unsigned long flags; + + /* submit pending list; callback for each desc; free desc */ + + spin_lock_irqsave(&chan->desc_lock, flags); + + /* update the cookie if we have some descriptors to cleanup */ + if (!list_empty(&chan->chain_running)) { + dma_cookie_t cookie; + + desc = to_mmp_pdma_desc(chan->chain_running.prev); + cookie = desc->async_tx.cookie; + dma_cookie_complete(&desc->async_tx); + + dev_dbg(chan->dev, "completed_cookie=%d\n", cookie); + } + + /* + * move the descriptors to a temporary list so we can drop the lock + * during the entire cleanup operation + */ + list_splice_tail_init(&chan->chain_running, &chain_cleanup); + + /* the hardware is now idle and ready for more */ + chan->idle = true; + + /* Start any pending transactions automatically */ + start_pending_queue(chan); + spin_unlock_irqrestore(&chan->desc_lock, flags); + + /* Run the callback for each descriptor, in order */ + list_for_each_entry_safe(desc, _desc, &chain_cleanup, node) { + struct dma_async_tx_descriptor *txd = &desc->async_tx; + + /* Remove from the list of transactions */ + list_del(&desc->node); + /* Run the link descriptor callback function */ + if (txd->callback) + txd->callback(txd->callback_param); + + dma_pool_free(chan->desc_pool, desc, txd->phys); + } +} + +static int __devexit mmp_pdma_remove(struct platform_device *op) +{ + struct mmp_pdma_device *pdev = platform_get_drvdata(op); + + dma_async_device_unregister(&pdev->device); + return 0; +} + +static int __devinit mmp_pdma_chan_init(struct mmp_pdma_device *pdev, + int idx, int irq) +{ + struct mmp_pdma_phy *phy = &pdev->phy[idx]; + struct mmp_pdma_chan *chan; + int ret; + + chan = devm_kzalloc(pdev->dev, + sizeof(struct mmp_pdma_chan), GFP_KERNEL); + if (chan == NULL) + return -ENOMEM; + + phy->idx = idx; + phy->base = pdev->base; + + if (irq) { + ret = devm_request_irq(pdev->dev, irq, + mmp_pdma_chan_handler, IRQF_DISABLED, "pdma", phy); + if (ret) { + dev_err(pdev->dev, "channel request irq fail!\n"); + return ret; + } + } + + spin_lock_init(&chan->desc_lock); + chan->dev = pdev->dev; + chan->chan.device = &pdev->device; + tasklet_init(&chan->tasklet, dma_do_tasklet, (unsigned long)chan); + INIT_LIST_HEAD(&chan->chain_pending); + INIT_LIST_HEAD(&chan->chain_running); + + /* register virt channel to dma engine */ + list_add_tail(&chan->chan.device_node, + &pdev->device.channels); + + return 0; +} + +static struct of_device_id mmp_pdma_dt_ids[] = { + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, + {} +}; +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); + +static int __devinit mmp_pdma_probe(struct platform_device *op) +{ + struct mmp_pdma_device *pdev; + const struct of_device_id *of_id; + struct mmp_dma_platdata *pdata = dev_get_platdata(&op->dev); + struct resource *iores; + int i, ret, irq = 0; + int chancnt = 0, irq_num = 0; + + pdev = devm_kzalloc(&op->dev, sizeof(*pdev), GFP_KERNEL); + if (!pdev) + return -ENOMEM; + pdev->dev = &op->dev; + + iores = platform_get_resource(op, IORESOURCE_MEM, 0); + if (!iores) + return -EINVAL; + + pdev->base = devm_request_and_ioremap(pdev->dev, iores); + if (!pdev->base) + return -EADDRNOTAVAIL; + + of_id = of_match_device(mmp_pdma_dt_ids, pdev->dev); + if (of_id) + of_property_read_u32(pdev->dev->of_node, "chancnt", &chancnt); + else if (pdata && pdata->chancnt) + chancnt = pdata->chancnt; + else + chancnt = 32; /* default 32 channel */ + pdev->chancnt = chancnt; + for (i = 0; i < chancnt; i++) { + if (platform_get_irq(op, i) > 0) + irq_num++; + } + + pdev->phy = devm_kzalloc(pdev->dev, + chancnt * sizeof(struct mmp_pdma_chan), GFP_KERNEL); + if (pdev->phy == NULL) + return -ENOMEM; + + INIT_LIST_HEAD(&pdev->device.channels); + + if (irq_num != chancnt) { + /* all chan share one irq, demux inside */ + irq = platform_get_irq(op, 0); + ret = devm_request_irq(pdev->dev, irq, + mmp_pdma_int_handler, IRQF_DISABLED, "pdma", pdev); + if (ret) + return ret; + } + + for (i = 0; i < chancnt; i++) { + irq = (irq_num != chancnt) ? 0 : platform_get_irq(op, i); + ret = mmp_pdma_chan_init(pdev, i, irq); + if (ret) + return ret; + } + + dma_cap_set(DMA_SLAVE, pdev->device.cap_mask); + dma_cap_set(DMA_MEMCPY, pdev->device.cap_mask); + dma_cap_set(DMA_SLAVE, pdev->device.cap_mask); + pdev->device.dev = &op->dev; + pdev->device.device_alloc_chan_resources = mmp_pdma_alloc_chan_resources; + pdev->device.device_free_chan_resources = mmp_pdma_free_chan_resources; + pdev->device.device_tx_status = mmp_pdma_tx_status; + pdev->device.device_prep_dma_memcpy = mmp_pdma_prep_memcpy; + pdev->device.device_prep_slave_sg = mmp_pdma_prep_slave_sg; + pdev->device.device_issue_pending = mmp_pdma_issue_pending; + pdev->device.device_control = mmp_pdma_control; + pdev->device.copy_align = PDMA_ALIGNMENT; + + if (pdev->dev->coherent_dma_mask) + dma_set_mask(pdev->dev, pdev->dev->coherent_dma_mask); + else + dma_set_mask(pdev->dev, DMA_BIT_MASK(64)); + + ret = dma_async_device_register(&pdev->device); + if (ret) { + dev_err(pdev->device.dev, "unable to register\n"); + return ret; + } + + return 0; +} + +static const struct platform_device_id mmp_pdma_id_table[] = { + { "mmp-pdma", MMP_PDMA }, + { }, +}; + +static struct platform_driver mmp_pdma_driver = { + .driver = { + .name = "mmp-pdma", + .owner = THIS_MODULE, + .of_match_table = mmp_pdma_dt_ids, + }, + .id_table = mmp_pdma_id_table, + .probe = mmp_pdma_probe, + .remove = __devexit_p(mmp_pdma_remove), +}; + +module_platform_driver(mmp_pdma_driver); + +MODULE_DESCRIPTION("MARVELL MMP Periphera DMA Driver"); +MODULE_AUTHOR("Marvell International Ltd."); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/platform_data/mmp_dma.h b/include/linux/platform_data/mmp_dma.h new file mode 100644 index 0000000..834e05d --- /dev/null +++ b/include/linux/platform_data/mmp_dma.h @@ -0,0 +1,25 @@ +/* + * MMP Platform DMA Management + * + * Copyright (c) 2011 Marvell Semiconductors Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#ifndef MMP_DMA_H +#define MMP_DMA_H +#include <linux/dmaengine.h> + +struct mmp_dma_platdata { + int chancnt; +}; + +struct mmp_dma_slave_config { + struct dma_slave_config slave_config; + u32 drcmr; +}; + +#endif /* MMP_DMA_H */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-20 5:46 ` [PATCH 1/3] dmaengine: mmp-pdma support Zhangfei Gao @ 2012-07-24 15:42 ` Arnd Bergmann 2012-07-25 4:48 ` zhangfei gao 2012-07-26 6:51 ` Vinod Koul 1 sibling, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2012-07-24 15:42 UTC (permalink / raw) To: linux-arm-kernel On Friday 20 July 2012, Zhangfei Gao wrote: > diff --git a/Documentation/devicetree/bindings/dma/mmp-dma.txt b/Documentation/devicetree/bindings/dma/mmp-dma.txt > new file mode 100644 > index 0000000..42cd134 > --- /dev/null > +++ b/Documentation/devicetree/bindings/dma/mmp-dma.txt > @@ -0,0 +1,30 @@ > +* MARVELL MMP DMA controller > + > +For driver/dma/mmp-pdma.c The binding document should normally not refer to a Linux device driver, but rather to a specific hardware, because the binding is OS-independent. Better list here which chips have this specific hardware. > +Required properties: > +- compatible: Should be "mrvl,mmp-pdma" We have recently changed all the Marvell strings to "marvel," instead of "mrvl,". We used to have a mix of both, but it's better to use just one. > +- reg: Should contain DMA registers location and length. > +- interrupts: Either contain all of the per-channel DMA interrupts > + or one irq for pdma device > +- chancnt: Should be channel number in the soc There is ongoing work to specify the generic dma engine bindings. The current proposal is to use - #dma-channels: Number of DMA channels supported by the controller. - #dma-requests: Number of DMA requests signals supported by the controller. instead of the chancnt. You can provide just one or both, depending on what you need. It is not entirely clear what the difference between having one IRQ or lots of them is: > +/* One irq for all channels */ > +pdma: dma-controller at d4000000 { > + compatible = "mrvl,mmp-pdma"; > + reg = <0xd4000000 0x10000>; > + interrupts = <47>; > + chancnt = <16>; > + }; > + Would this be the same as saying interrupts = <47 47 47 47 47 47 47 47 47 47 47 47 47 47 47 47>; with 16 channels? > +static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, > unsigned long arg) > +{ > + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); > + struct dma_slave_config *cfg = (void *)arg; > + struct mmp_dma_slave_config *mmp_cfg; > + unsigned long flags; > + int ret = 0; > + u32 maxburst = 0, addr = 0; > + enum dma_slave_buswidth width = DMA_SLAVE_BUSWIDTH_UNDEFINED; > + > + if (!dchan) > + return -EINVAL; > + mmp_cfg = container_of(cfg, struct mmp_dma_slave_config, slave_config); Encapsulating the slave config like this requires that the slave driver knows what the master is, which is not how this is designed to be used. What is actually the data you encode in there? > +static struct of_device_id mmp_pdma_dt_ids[] = { > + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, > + {} > +}; > +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); You don't actually use the data field afaict, so that can be removed. > + of_id = of_match_device(mmp_pdma_dt_ids, pdev->dev); > + if (of_id) It is already matched here, so just do if (pdev->dev->of_node) > + > +struct mmp_dma_platdata { > + int chancnt; > +}; It this actually necessary? I think it would be better in the long run to only provide the DT based configuration interface here. There are no in-tree users of the platform-data, and I would hope that we can avoid adding them. Any board files that are kept out of tree can of course add support for this back. If you can make a reasonable argument for keeping it, you can probably convince me otherwise in this case, but generally I would prefer new stuff to be DT-only. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-24 15:42 ` Arnd Bergmann @ 2012-07-25 4:48 ` zhangfei gao 2012-07-25 6:01 ` Arnd Bergmann 0 siblings, 1 reply; 15+ messages in thread From: zhangfei gao @ 2012-07-25 4:48 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 24, 2012 at 11:42 PM, Arnd Bergmann <arnd@arndb.de> wrote: Thanks Arnd for taking look at this patch in this busy merge window time :) > On Friday 20 July 2012, Zhangfei Gao wrote: > >> diff --git a/Documentation/devicetree/bindings/dma/mmp-dma.txt b/Documentation/devicetree/bindings/dma/mmp-dma.txt >> new file mode 100644 >> index 0000000..42cd134 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/dma/mmp-dma.txt >> @@ -0,0 +1,30 @@ >> +* MARVELL MMP DMA controller >> + >> +For driver/dma/mmp-pdma.c > > The binding document should normally not refer to a Linux device driver, > but rather to a specific hardware, because the binding is OS-independent. > Better list here which chips have this specific hardware. Got it, will modify accordingly. Besides, we would like to write both pdma and tdma desc to the same mmp-dma.txt > >> +Required properties: >> +- compatible: Should be "mrvl,mmp-pdma" > > We have recently changed all the Marvell strings to "marvel," instead > of "mrvl,". We used to have a mix of both, but it's better to use > just one. Got it, will use "marvel" > >> +- reg: Should contain DMA registers location and length. >> +- interrupts: Either contain all of the per-channel DMA interrupts >> + or one irq for pdma device >> +- chancnt: Should be channel number in the soc > > There is ongoing work to specify the generic dma engine bindings. > The current proposal is to use > > - #dma-channels: Number of DMA channels supported by the controller. > - #dma-requests: Number of DMA requests signals supported by the controller. > > instead of the chancnt. You can provide just one or both, depending > on what you need. mmp-pdma use only dma-channels is fine. Some confusion a. dma-channels & dma-requests only standardize name and dma-engine driver takes responsibility to parse these vectors in probe function like mmp_pdma.c. b. dma-channels & dma-requests not only standardize name but only parse these flags to dmaengine.c, dmaengine.c will parse these flags while dma-engine driver will get these flags from dmaengine.c via some API. While one is correct, my understand is a, mmp_pdma.c probe function will get these flags. > > It is not entirely clear what the difference between having one IRQ > or lots of them is: > >> +/* One irq for all channels */ >> +pdma: dma-controller at d4000000 { >> + compatible = "mrvl,mmp-pdma"; >> + reg = <0xd4000000 0x10000>; >> + interrupts = <47>; >> + chancnt = <16>; >> + }; >> + > > Would this be the same as saying > > interrupts = <47 47 47 47 47 47 47 47 47 47 47 47 47 47 47 47>; > > with 16 channels? Sorry for confusion, they are different, 1. Only one IRQ, then dma engine driver will parse internal register to check channel in irq. For example, arch/arm/plat-pxa/dma.c parse DINT in dma_irq_handler. 2, Each irq for each channel ICU could directly parse which channel is irq directly from ICU register. While DMA controller do not have such register to check which channel irq happens. For example, pxa688 icu register 0x128, bit 0~15 is PDMA channel irq, 18~21 is ADMA irq Using this method, interrupt-parent is required to parse specific channel irq. > > >> +static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, >> unsigned long arg) >> +{ >> + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); >> + struct dma_slave_config *cfg = (void *)arg; >> + struct mmp_dma_slave_config *mmp_cfg; >> + unsigned long flags; >> + int ret = 0; >> + u32 maxburst = 0, addr = 0; >> + enum dma_slave_buswidth width = DMA_SLAVE_BUSWIDTH_UNDEFINED; >> + >> + if (!dchan) >> + return -EINVAL; >> + mmp_cfg = container_of(cfg, struct mmp_dma_slave_config, slave_config); > > Encapsulating the slave config like this requires that the slave driver > knows what the master is, which is not how this is designed to be used. > What is actually the data you encode in there? For peripheral dma, one special requirement of DMA_RCMRx The register is used select which channel is mapped to the peripheral. The channel can be alloc dynamically, but the register is constant to specific peripheral. For example, nand_dma, pxa910 use DRCMR97 (0x1184), what's worse, pxa688 use DRCMR28 (0x170). There is no connection between channel number and drcmr, since channel is dynamically allocated. So we encapsulating the slave config parsing drcmr to dma-engine driver. > >> +static struct of_device_id mmp_pdma_dt_ids[] = { >> + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); > > You don't actually use the data field afaict, so that can be removed. We keep .data as type, for easy extension, in case future soc register may be different. It is fine to remove the data field, as well as mmp_pdma_id_table. > >> + of_id = of_match_device(mmp_pdma_dt_ids, pdev->dev); >> + if (of_id) > > It is already matched here, so just do > > if (pdev->dev->of_node) > >> + >> +struct mmp_dma_platdata { >> + int chancnt; >> +}; > > It this actually necessary? > > I think it would be better in the long run to only provide the DT based > configuration interface here. There are no in-tree users of the > platform-data, and I would hope that we can avoid adding them. Any > board files that are kept out of tree can of course add support for this > back. If you can make a reasonable argument for keeping it, you can > probably convince me otherwise in this case, but generally I would > prefer new stuff to be DT-only. We may have to keep pdata currently. The final purpose is replacing arch/arm/plat-pxa/dma.c, which is widely used pxa3xx system. There are lots of work in replacing pdma on pxa3xx system, which not support dt yet. If pdma only support DT, the replacing work may become impossible. > > Arnd > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-25 4:48 ` zhangfei gao @ 2012-07-25 6:01 ` Arnd Bergmann 2012-07-25 8:28 ` zhangfei gao 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2012-07-25 6:01 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 25 July 2012, zhangfei gao wrote: > On Tue, Jul 24, 2012 at 11:42 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > > The binding document should normally not refer to a Linux device driver, > > but rather to a specific hardware, because the binding is OS-independent. > > Better list here which chips have this specific hardware. > > Got it, will modify accordingly. > Besides, we would like to write both pdma and tdma desc to the same mmp-dma.txt ok, good idea. > Some confusion > a. dma-channels & dma-requests only standardize name and dma-engine driver > takes responsibility to parse these vectors in probe function like mmp_pdma.c. > b. dma-channels & dma-requests not only standardize name but only parse these > flags to dmaengine.c, dmaengine.c will parse these flags while > dma-engine driver will get these flags from dmaengine.c via some API. > > While one is correct, my understand is a, mmp_pdma.c probe function > will get these flags. We have not fully worked out these in the generic binding. Right now, the important part is that you use the standard identifiers so we are free to parse them in the common code if we decide to do that later. > > It is not entirely clear what the difference between having one IRQ > > or lots of them is: > > > >> +/* One irq for all channels */ > >> +pdma: dma-controller at d4000000 { > >> + compatible = "mrvl,mmp-pdma"; > >> + reg = <0xd4000000 0x10000>; > >> + interrupts = <47>; > >> + chancnt = <16>; > >> + }; > >> + > > > > Would this be the same as saying > > > > interrupts = <47 47 47 47 47 47 47 47 47 47 47 47 47 47 47 47>; > > > > with 16 channels? > > Sorry for confusion, they are different, > 1. Only one IRQ, then dma engine driver will parse internal register > to check channel in irq. > For example, arch/arm/plat-pxa/dma.c parse DINT in dma_irq_handler. > > 2, Each irq for each channel > ICU could directly parse which channel is irq directly from ICU register. > While DMA controller do not have such register to check which channel > irq happens. > For example, pxa688 icu register 0x128, bit 0~15 is PDMA channel irq, > 18~21 is ADMA irq > Using this method, interrupt-parent is required to parse specific channel irq. ok, got it. Maybe add this text or a shorter version to the binding. > >> +static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, > >> unsigned long arg) > >> +{ > >> + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); > >> + struct dma_slave_config *cfg = (void *)arg; > >> + struct mmp_dma_slave_config *mmp_cfg; > >> + unsigned long flags; > >> + int ret = 0; > >> + u32 maxburst = 0, addr = 0; > >> + enum dma_slave_buswidth width = DMA_SLAVE_BUSWIDTH_UNDEFINED; > >> + > >> + if (!dchan) > >> + return -EINVAL; > >> + mmp_cfg = container_of(cfg, struct mmp_dma_slave_config, slave_config); > > > > Encapsulating the slave config like this requires that the slave driver > > knows what the master is, which is not how this is designed to be used. > > What is actually the data you encode in there? > > For peripheral dma, one special requirement of DMA_RCMRx > The register is used select which channel is mapped to the peripheral. > The channel can be alloc dynamically, but the register is constant to > specific peripheral. > For example, nand_dma, pxa910 use DRCMR97 (0x1184), what's worse, > pxa688 use DRCMR28 (0x170). > > There is no connection between channel number and drcmr, since channel > is dynamically allocated. > So we encapsulating the slave config parsing drcmr to dma-engine driver. Ah, so it is the request line. Now the current plan for this is to go into the "dmas" property of the child device using the dma engine binding. That way, the driver will end up asking for a channel that matches what is specified for the device, e.g. nand { ... dmas = <&pdma 3 0x1184>; }; I believe this is normally set through the filter function when asking for a channel on other drivers, not through slave_config. > >> +static struct of_device_id mmp_pdma_dt_ids[] = { > >> + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, > >> + {} > >> +}; > >> +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); > > > > You don't actually use the data field afaict, so that can be removed. > > We keep .data as type, for easy extension, in case future soc register > may be different. > It is fine to remove the data field, as well as mmp_pdma_id_table. Ok. Note that in many cases it's actually easier to point the .data field to a data structure rather than an integer, it's valid either way. Unfortunately the type is different between of_device_id and platform_device_id, but a pointer also works for both. If you think that you will see different kinds of pdma that you need to distinguish here, I would recommend that you use a more specific "compatible" identifier, like { .compatible = "marvell,pxa168-pdma", }; > >> + of_id = of_match_device(mmp_pdma_dt_ids, pdev->dev); > >> + if (of_id) > > > > It is already matched here, so just do > > > > if (pdev->dev->of_node) > > > >> + > >> +struct mmp_dma_platdata { > >> + int chancnt; > >> +}; > > > > It this actually necessary? > > > > I think it would be better in the long run to only provide the DT based > > configuration interface here. There are no in-tree users of the > > platform-data, and I would hope that we can avoid adding them. Any > > board files that are kept out of tree can of course add support for this > > back. If you can make a reasonable argument for keeping it, you can > > probably convince me otherwise in this case, but generally I would > > prefer new stuff to be DT-only. > > We may have to keep pdata currently. > The final purpose is replacing arch/arm/plat-pxa/dma.c, which is > widely used pxa3xx system. > There are lots of work in replacing pdma on pxa3xx system, which not > support dt yet. > If pdma only support DT, the replacing work may become impossible. Ok, makes sense. Please keep it for now. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-25 6:01 ` Arnd Bergmann @ 2012-07-25 8:28 ` zhangfei gao 2012-07-25 8:58 ` Arnd Bergmann 0 siblings, 1 reply; 15+ messages in thread From: zhangfei gao @ 2012-07-25 8:28 UTC (permalink / raw) To: linux-arm-kernel >> >> +static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, >> >> unsigned long arg) >> >> +{ >> >> + struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan); >> >> + struct dma_slave_config *cfg = (void *)arg; >> >> + struct mmp_dma_slave_config *mmp_cfg; >> >> + unsigned long flags; >> >> + int ret = 0; >> >> + u32 maxburst = 0, addr = 0; >> >> + enum dma_slave_buswidth width = DMA_SLAVE_BUSWIDTH_UNDEFINED; >> >> + >> >> + if (!dchan) >> >> + return -EINVAL; >> >> + mmp_cfg = container_of(cfg, struct mmp_dma_slave_config, slave_config); >> > >> > Encapsulating the slave config like this requires that the slave driver >> > knows what the master is, which is not how this is designed to be used. >> > What is actually the data you encode in there? >> >> For peripheral dma, one special requirement of DMA_RCMRx >> The register is used select which channel is mapped to the peripheral. >> The channel can be alloc dynamically, but the register is constant to >> specific peripheral. >> For example, nand_dma, pxa910 use DRCMR97 (0x1184), what's worse, >> pxa688 use DRCMR28 (0x170). >> >> There is no connection between channel number and drcmr, since channel >> is dynamically allocated. >> So we encapsulating the slave config parsing drcmr to dma-engine driver. > > Ah, so it is the request line. Now the current plan for this is to go > into the "dmas" property of the child device using the dma engine binding. > > That way, the driver will end up asking for a channel that matches what > is specified for the device, e.g. > > nand { > ... > dmas = <&pdma 3 0x1184>; > }; > > I believe this is normally set through the filter function when asking > for a channel on other drivers, not through slave_config. It is great that the request line is already been considered. Still want to double confirm, it is dmaengine.c will parse out "the request line", Dma-engine driver will get request line from dmaengine.c, right? So there is dependence. Or dmaengine driver parse the request line itself from client driver? Some client may require more than one dma channel, such as camera, YUV three channels are required. camera { ... dmas = <&pdma 1 0x1110 /* Y */ &pdma 1 0x1114 /* U */ &pmaa 1 0x1118>; /* V */ }; Besides, we also have to consider how to parse "request line" if using pdata. > >> >> +static struct of_device_id mmp_pdma_dt_ids[] = { >> >> + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, >> >> + {} >> >> +}; >> >> +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); >> > >> > You don't actually use the data field afaict, so that can be removed. >> >> We keep .data as type, for easy extension, in case future soc register >> may be different. >> It is fine to remove the data field, as well as mmp_pdma_id_table. > > Ok. Note that in many cases it's actually easier to point the .data > field to a data structure rather than an integer, it's valid either > way. Unfortunately the type is different between of_device_id and > platform_device_id, but a pointer also works for both. Yes, also find .data is different between of_device_id and platform_device_id. So using strong conversion for simplification. Will change to data structure if you think it is more professional. > > If you think that you will see different kinds of pdma that you need > to distinguish here, I would recommend that you use a more specific > "compatible" identifier, like > > { .compatible = "marvell,pxa168-pdma", }; > mmp-tdma.c have to take this, since register difference in pxa910 and pxa688. mmp-pdma.c register keeps same since pxa3xx, we will remove the type. > > Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-25 8:28 ` zhangfei gao @ 2012-07-25 8:58 ` Arnd Bergmann 2012-07-26 4:46 ` zhangfei gao 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2012-07-25 8:58 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 25 July 2012, zhangfei gao wrote: > > Ah, so it is the request line. Now the current plan for this is to go > > into the "dmas" property of the child device using the dma engine binding. > > > > That way, the driver will end up asking for a channel that matches what > > is specified for the device, e.g. > > > > nand { > > ... > > dmas = <&pdma 3 0x1184>; > > }; > > > > I believe this is normally set through the filter function when asking > > for a channel on other drivers, not through slave_config. > > It is great that the request line is already been considered. > Still want to double confirm, it is dmaengine.c will parse out "the > request line", > Dma-engine driver will get request line from dmaengine.c, right? > So there is dependence. > Or dmaengine driver parse the request line itself from client driver? > > Some client may require more than one dma channel, > such as camera, YUV three channels are required. > > camera { > ... > dmas = <&pdma 1 0x1110 /* Y */ > &pdma 1 0x1114 /* U */ > &pmaa 1 0x1118>; /* V */ > }; > > Besides, we also have to consider how to parse "request line" if using pdata. The current plan is to allow an arbitrary number of channels in the client, which can all go to the same or different dmaengine drivers. You can also specify multiple channels that can be used as alternatives, by adding a dma-names property and naming them the same. Further, the number of fields required for specifying the request line is determined by the dmac driver. The first two cells of each specifier are parsed by the dmaengine common code and all following ones are parsed handed to the dmac specific driver. A complex example would be dmas = <&pdma 1 0x1110 /* Y on pdma */ &tdma 1 0x100 32 /* Y on tdma */ &pdma 1 0x1114 /* U */ &tdma 1 0x200 32 /* U */ &pmaa 1 0x1118 /* V */ &tdma 1 0x300 32>; /* V */ dma-names = "Y", "Y", "U", "U", "V", "V"; In this case the pdma node would set #dma-cells=<3> and the tdma node would set #dma-cells=<4> in order to pass two separate arguments. Through the dma-names property, the common code can identify which channels belong together and the driver just needs to ask for a channel like chan_y = of_dma_request_named_channel(pdev, "Y"); > >> >> +static struct of_device_id mmp_pdma_dt_ids[] = { > >> >> + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, > >> >> + {} > >> >> +}; > >> >> +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); > >> > > >> > You don't actually use the data field afaict, so that can be removed. > >> > >> We keep .data as type, for easy extension, in case future soc register > >> may be different. > >> It is fine to remove the data field, as well as mmp_pdma_id_table. > > > > Ok. Note that in many cases it's actually easier to point the .data > > field to a data structure rather than an integer, it's valid either > > way. Unfortunately the type is different between of_device_id and > > platform_device_id, but a pointer also works for both. > > Yes, also find .data is different between of_device_id and platform_device_id. > So using strong conversion for simplification. > Will change to data structure if you think it is more professional. Just do whatever your fits your needs, I'm just noting that in a lot of drivers, using a structure ends up being easier. > > If you think that you will see different kinds of pdma that you need > > to distinguish here, I would recommend that you use a more specific > > "compatible" identifier, like > > > > { .compatible = "marvell,pxa168-pdma", }; > > > mmp-tdma.c have to take this, since register difference in pxa910 and pxa688. > mmp-pdma.c register keeps same since pxa3xx, we will remove the type. In the pdma case, I would suggest that you also specify the name of the soc there, but in the device tree list the device as compatible with the older one, e.g. for pxa930: compatible = "marvell,pxa930-pdma", "marvell,pxa300-pdma"; This way the driver needs to know about only one, but is ready for future extensions, e.g. if a future pxa985 is slightly different. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-25 8:58 ` Arnd Bergmann @ 2012-07-26 4:46 ` zhangfei gao 2012-08-06 15:29 ` Arnd Bergmann 0 siblings, 1 reply; 15+ messages in thread From: zhangfei gao @ 2012-07-26 4:46 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jul 25, 2012 at 4:58 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 25 July 2012, zhangfei gao wrote: > >> > Ah, so it is the request line. Now the current plan for this is to go >> > into the "dmas" property of the child device using the dma engine binding. >> > >> > That way, the driver will end up asking for a channel that matches what >> > is specified for the device, e.g. >> > >> > nand { >> > ... >> > dmas = <&pdma 3 0x1184>; >> > }; >> > >> > I believe this is normally set through the filter function when asking >> > for a channel on other drivers, not through slave_config. >> >> It is great that the request line is already been considered. >> Still want to double confirm, it is dmaengine.c will parse out "the >> request line", >> Dma-engine driver will get request line from dmaengine.c, right? >> So there is dependence. >> Or dmaengine driver parse the request line itself from client driver? >> >> Some client may require more than one dma channel, >> such as camera, YUV three channels are required. >> >> camera { >> ... >> dmas = <&pdma 1 0x1110 /* Y */ >> &pdma 1 0x1114 /* U */ >> &pmaa 1 0x1118>; /* V */ >> }; >> >> Besides, we also have to consider how to parse "request line" if using pdata. > > The current plan is to allow an arbitrary number of channels in the client, > which can all go to the same or different dmaengine drivers. You can also > specify multiple channels that can be used as alternatives, by adding a > dma-names property and naming them the same. Further, the number of fields > required for specifying the request line is determined by the dmac driver. > The first two cells of each specifier are parsed by the dmaengine common > code and all following ones are parsed handed to the dmac specific driver. > > A complex example would be > > dmas = <&pdma 1 0x1110 /* Y on pdma */ > &tdma 1 0x100 32 /* Y on tdma */ > &pdma 1 0x1114 /* U */ > &tdma 1 0x200 32 /* U */ > &pmaa 1 0x1118 /* V */ > &tdma 1 0x300 32>; /* V */ > dma-names = "Y", "Y", "U", "U", "V", "V"; > > In this case the pdma node would set #dma-cells=<3> and the tdma node would > set #dma-cells=<4> in order to pass two separate arguments. Through the > dma-names property, the common code can identify which channels belong > together and the driver just needs to ask for a channel like > > chan_y = of_dma_request_named_channel(pdev, "Y"); > Thanks Arnd for patient explain. Still want to double confirm the understand: DMA client (nand, camera) get dmas, since each client has different dmas. DMA client (nand) pass dmas via dmaengine API and request and config channel with dmas. As you said: "The first two cells of dmas are parsed by the dmaengine common code. All following dmas cells are parsed handed to the dmac specific driver." Then dmac get config info like request line, etc. "Further, the number of fields required for specifying the request line is determined by the dmac driver." dmac driver can not get client dmas from dt file but have to get from dmaengine API, right? Besides, still not find of_dma_request_named_channel in latest linux-next.git. So dmac driver have to wait? If request line is must config, how about extend dma_slave_config? struct dma_slave_config { ~ u32 request_line; } Then dmac driver could get "request line" no matter using DT or pdata. >> >> >> +static struct of_device_id mmp_pdma_dt_ids[] = { >> >> >> + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, >> >> >> + {} >> >> >> +}; >> >> >> +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); >> >> > >> >> > You don't actually use the data field afaict, so that can be removed. >> >> >> >> We keep .data as type, for easy extension, in case future soc register >> >> may be different. >> >> It is fine to remove the data field, as well as mmp_pdma_id_table. >> > >> > Ok. Note that in many cases it's actually easier to point the .data >> > field to a data structure rather than an integer, it's valid either >> > way. Unfortunately the type is different between of_device_id and >> > platform_device_id, but a pointer also works for both. >> >> Yes, also find .data is different between of_device_id and platform_device_id. >> So using strong conversion for simplification. >> Will change to data structure if you think it is more professional. > > Just do whatever your fits your needs, I'm just noting that in a lot of > drivers, using a structure ends up being easier. If so, prefer using strong conversion since it is simple :) If using data structure, one structure with single member "type" has to be defined. > >> > If you think that you will see different kinds of pdma that you need >> > to distinguish here, I would recommend that you use a more specific >> > "compatible" identifier, like >> > >> > { .compatible = "marvell,pxa168-pdma", }; >> > >> mmp-tdma.c have to take this, since register difference in pxa910 and pxa688. >> mmp-pdma.c register keeps same since pxa3xx, we will remove the type. > > In the pdma case, I would suggest that you also specify the name of the soc > there, but in the device tree list the device as compatible with the older > one, e.g. for pxa930: > > compatible = "marvell,pxa930-pdma", "marvell,pxa300-pdma"; > > This way the driver needs to know about only one, but is ready for future > extensions, e.g. if a future pxa985 is slightly different. How about using the "marvell,mmp-pdma" on all platforms with same register, including pxa3xx. If future pxa985 is slightly different, we add "marvell,pxa985-pdma" then. If using different name on different platform, they may too many names. > > Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-26 4:46 ` zhangfei gao @ 2012-08-06 15:29 ` Arnd Bergmann 0 siblings, 0 replies; 15+ messages in thread From: Arnd Bergmann @ 2012-08-06 15:29 UTC (permalink / raw) To: linux-arm-kernel On Thursday 26 July 2012, zhangfei gao wrote: > On Wed, Jul 25, 2012 at 4:58 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Wednesday 25 July 2012, zhangfei gao wrote: > > > > The current plan is to allow an arbitrary number of channels in the client, > > which can all go to the same or different dmaengine drivers. You can also > > specify multiple channels that can be used as alternatives, by adding a > > dma-names property and naming them the same. Further, the number of fields > > required for specifying the request line is determined by the dmac driver. > > The first two cells of each specifier are parsed by the dmaengine common > > code and all following ones are parsed handed to the dmac specific driver. > > > > A complex example would be > > > > dmas = <&pdma 1 0x1110 /* Y on pdma */ > > &tdma 1 0x100 32 /* Y on tdma */ > > &pdma 1 0x1114 /* U */ > > &tdma 1 0x200 32 /* U */ > > &pmaa 1 0x1118 /* V */ > > &tdma 1 0x300 32>; /* V */ > > dma-names = "Y", "Y", "U", "U", "V", "V"; > > > > In this case the pdma node would set #dma-cells=<3> and the tdma node would > > set #dma-cells=<4> in order to pass two separate arguments. Through the > > dma-names property, the common code can identify which channels belong > > together and the driver just needs to ask for a channel like > > > > chan_y = of_dma_request_named_channel(pdev, "Y"); > > > > Thanks Arnd for patient explain. > > Still want to double confirm the understand: > DMA client (nand, camera) get dmas, since each client has different dmas. > DMA client (nand) pass dmas via dmaengine API and request and config > channel with dmas. Yes, although the client driver would not actually pass the property, it would just pass the device pointer and then the dmaengine code can look up the property. > As you said: > "The first two cells of dmas are parsed by the dmaengine common code. > All following dmas cells are parsed handed to the dmac specific driver." > Then dmac get config info like request line, etc. > > "Further, the number of fields > required for specifying the request line is determined by the dmac driver." > dmac driver can not get client dmas from dt file but have to get from > dmaengine API, right? The dmac driver registers a callback to the dmaengine subsystem. The client calls into dmaengine, which calls this dmac function. > Besides, still not find of_dma_request_named_channel in latest linux-next.git. > So dmac driver have to wait? Yes, I hope we can get this done in v3.7. > >> >> >> +static struct of_device_id mmp_pdma_dt_ids[] = { > >> >> >> + { .compatible = "mrvl,mmp-pdma", .data = (void *)MMP_PDMA}, > >> >> >> + {} > >> >> >> +}; > >> >> >> +MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); > >> >> > > >> >> > You don't actually use the data field afaict, so that can be removed. > >> >> > >> >> We keep .data as type, for easy extension, in case future soc register > >> >> may be different. > >> >> It is fine to remove the data field, as well as mmp_pdma_id_table. > >> > > >> > Ok. Note that in many cases it's actually easier to point the .data > >> > field to a data structure rather than an integer, it's valid either > >> > way. Unfortunately the type is different between of_device_id and > >> > platform_device_id, but a pointer also works for both. > >> > >> Yes, also find .data is different between of_device_id and platform_device_id. > >> So using strong conversion for simplification. > >> Will change to data structure if you think it is more professional. > > > > Just do whatever your fits your needs, I'm just noting that in a lot of > > drivers, using a structure ends up being easier. > > If so, prefer using strong conversion since it is simple :) > If using data structure, one structure with single member "type" has > to be defined. You misunderstood the purpose of the data structure: The idea is to remove the need for a "type" member but instead convert every decision that is based on a type currently into a lookup of one of the members of the structure. Right now your driver does not seem to look at the type at all, so you can simply remove that member altogether. > >> > If you think that you will see different kinds of pdma that you need > >> > to distinguish here, I would recommend that you use a more specific > >> > "compatible" identifier, like > >> > > >> > { .compatible = "marvell,pxa168-pdma", }; > >> > > >> mmp-tdma.c have to take this, since register difference in pxa910 and pxa688. > >> mmp-pdma.c register keeps same since pxa3xx, we will remove the type. > > > > In the pdma case, I would suggest that you also specify the name of the soc > > there, but in the device tree list the device as compatible with the older > > one, e.g. for pxa930: > > > > compatible = "marvell,pxa930-pdma", "marvell,pxa300-pdma"; > > > > This way the driver needs to know about only one, but is ready for future > > extensions, e.g. if a future pxa985 is slightly different. > > How about using the "marvell,mmp-pdma" on all platforms with same > register, including pxa3xx. > If future pxa985 is slightly different, we add "marvell,pxa985-pdma" then. > > If using different name on different platform, they may too many names. That would also be confusing because then it's not clear why pxa985 does not use a marvell,mmp-pdma. Using the name of the first model that you are compatible with is the more common way, people will understand that. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-20 5:46 ` [PATCH 1/3] dmaengine: mmp-pdma support Zhangfei Gao 2012-07-24 15:42 ` Arnd Bergmann @ 2012-07-26 6:51 ` Vinod Koul 2012-07-26 7:02 ` zhangfei gao 1 sibling, 1 reply; 15+ messages in thread From: Vinod Koul @ 2012-07-26 6:51 UTC (permalink / raw) To: linux-arm-kernel On Fri, 2012-07-20 at 13:46 +0800, Zhangfei Gao wrote: > +struct mmp_dma_platdata { > + int chancnt; > +}; > + > +struct mmp_dma_slave_config { > + struct dma_slave_config slave_config; > + u32 drcmr; > +}; Why do you need these definitions? What is drcmr used for? -- ~Vinod ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-26 6:51 ` Vinod Koul @ 2012-07-26 7:02 ` zhangfei gao 2012-07-26 10:09 ` Vinod Koul 0 siblings, 1 reply; 15+ messages in thread From: zhangfei gao @ 2012-07-26 7:02 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 26, 2012 at 2:51 PM, Vinod Koul <vinod.koul@linux.intel.com> wrote: > On Fri, 2012-07-20 at 13:46 +0800, Zhangfei Gao wrote: >> +struct mmp_dma_platdata { >> + int chancnt; >> +}; >> + >> +struct mmp_dma_slave_config { >> + struct dma_slave_config slave_config; >> + u32 drcmr; >> +}; > Why do you need these definitions? > > What is drcmr used for? Hi, Vinod "drcmr" is request_line, we used to select which channel map to specific peripheral. Is it possible adding request_line to slave_config? Refer mail with Arnd before. > For peripheral dma, one special requirement of DMA_RCMRx > The register is used select which channel is mapped to the peripheral. > The channel can be alloc dynamically, but the register is constant to > specific peripheral. > For example, nand_dma, pxa910 use DRCMR97 (0x1184), what's worse, > pxa688 use DRCMR28 (0x170). > > There is no connection between channel number and drcmr, since channel > is dynamically allocated. > So we encapsulating the slave config parsing drcmr to dma-engine driver. [Arnd] Ah, so it is the request line. Now the current plan for this is to go into the "dmas" property of the child device using the dma engine binding. > > > -- > ~Vinod > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-26 7:02 ` zhangfei gao @ 2012-07-26 10:09 ` Vinod Koul 2012-07-27 2:51 ` zhangfei gao 0 siblings, 1 reply; 15+ messages in thread From: Vinod Koul @ 2012-07-26 10:09 UTC (permalink / raw) To: linux-arm-kernel On Thu, 2012-07-26 at 15:02 +0800, zhangfei gao wrote: > On Thu, Jul 26, 2012 at 2:51 PM, Vinod Koul <vinod.koul@linux.intel.com> wrote: > > On Fri, 2012-07-20 at 13:46 +0800, Zhangfei Gao wrote: > >> +struct mmp_dma_platdata { > >> + int chancnt; > >> +}; > >> + > >> +struct mmp_dma_slave_config { > >> + struct dma_slave_config slave_config; > >> + u32 drcmr; > >> +}; > > Why do you need these definitions? > > > > What is drcmr used for? > > Hi, Vinod > > "drcmr" is request_line, we used to select which channel map to > specific peripheral. > Is it possible adding request_line to slave_config? Can you use the slave_id. > > Refer mail with Arnd before. > > For peripheral dma, one special requirement of DMA_RCMRx > > The register is used select which channel is mapped to the peripheral. > > The channel can be alloc dynamically, but the register is constant to > > specific peripheral. > > For example, nand_dma, pxa910 use DRCMR97 (0x1184), what's worse, > > pxa688 use DRCMR28 (0x170). > > > > There is no connection between channel number and drcmr, since channel > > is dynamically allocated. > > So we encapsulating the slave config parsing drcmr to dma-engine driver. > > [Arnd] > Ah, so it is the request line. Now the current plan for this is to go > into the "dmas" property of the child device using the dma engine binding. > > > > > > > -- > > ~Vinod > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- ~Vinod ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] dmaengine: mmp-pdma support 2012-07-26 10:09 ` Vinod Koul @ 2012-07-27 2:51 ` zhangfei gao 0 siblings, 0 replies; 15+ messages in thread From: zhangfei gao @ 2012-07-27 2:51 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 26, 2012 at 6:09 PM, Vinod Koul <vinod.koul@linux.intel.com> wrote: > On Thu, 2012-07-26 at 15:02 +0800, zhangfei gao wrote: >> On Thu, Jul 26, 2012 at 2:51 PM, Vinod Koul <vinod.koul@linux.intel.com> wrote: >> > On Fri, 2012-07-20 at 13:46 +0800, Zhangfei Gao wrote: >> >> +struct mmp_dma_platdata { >> >> + int chancnt; >> >> +}; >> >> + >> >> +struct mmp_dma_slave_config { >> >> + struct dma_slave_config slave_config; >> >> + u32 drcmr; >> >> +}; >> > Why do you need these definitions? >> > >> > What is drcmr used for? >> >> Hi, Vinod >> >> "drcmr" is request_line, we used to select which channel map to >> specific peripheral. >> Is it possible adding request_line to slave_config? > Can you use the slave_id. > Good suggestion If passing drcmr does not break original design of slave_id, mmp-pdma just need one config member. While some device (nand) use memcpy method instead of slave_sg. * @slave_id: Slave requester id. Only valid for slave channels. The dma * slave peripheral will have unique id as dma requester which need to be * pass as slave config. Will update with slave_id in v1 version. Thanks Vinod. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] dmaengine: mmp_tdma: add dt support 2012-07-20 5:46 [PATCH 0/3] dmaengine: add support mmp-pdma Zhangfei Gao 2012-07-20 5:46 ` [PATCH 1/3] dmaengine: mmp-pdma support Zhangfei Gao @ 2012-07-20 5:46 ` Zhangfei Gao 2012-07-20 5:46 ` [PATCH 3/3] dmatest: add dmaengine_slave_config for DMA_MEMCPY Zhangfei Gao 2 siblings, 0 replies; 15+ messages in thread From: Zhangfei Gao @ 2012-07-20 5:46 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com> --- Documentation/devicetree/bindings/dma/mmp-dma.txt | 26 +++++++++++ drivers/dma/mmp_tdma.c | 51 +++++++++++++-------- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/dma/mmp-dma.txt b/Documentation/devicetree/bindings/dma/mmp-dma.txt index 42cd134..c544af5 100644 --- a/Documentation/devicetree/bindings/dma/mmp-dma.txt +++ b/Documentation/devicetree/bindings/dma/mmp-dma.txt @@ -28,3 +28,29 @@ pdma: dma-controller at d4000000 { chancnt = <16>; }; + +For driver/dma/mmp-tdma.c + +Required properties: +- compatible: Should be "mrvl,mmp-adma" or "mrvl,pxa910-squ" +- reg: Should contain DMA registers location and length. +- interrupts: Either contain all of the per-channel DMA interrupts + or one irq for dma device + +Examples: + +/* each channel has specific irq */ +adma0: dma-controller at d42a0800 { + compatible = "mrvl,mmp-adma"; + reg = <0xd42a0800 0x100>; + interrupts = <18 19>; + interrupt-parent = <&intcmux32>; + }; + +/* One irq for all channels */ +squ: dma-controller at d42a0800 { + compatible = "mrvl,pxa910-squ"; + reg = <0xd42a0800 0x100>; + interrupts = <46>; + }; + diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c index 8a15cf2..cd8d0ee 100644 --- a/drivers/dma/mmp_tdma.c +++ b/drivers/dma/mmp_tdma.c @@ -20,6 +20,7 @@ #include <linux/device.h> #include <mach/regs-icu.h> #include <mach/sram.h> +#include <linux/of_device.h> #include "dmaengine.h" @@ -127,7 +128,6 @@ struct mmp_tdma_device { void __iomem *base; struct dma_device device; struct mmp_tdma_chan *tdmac[TDMA_CHANNEL_NUM]; - int irq; }; #define to_mmp_tdma_chan(dchan) container_of(dchan, struct mmp_tdma_chan, chan) @@ -492,7 +492,7 @@ static int __devinit mmp_tdma_chan_init(struct mmp_tdma_device *tdev, return -ENOMEM; } if (irq) - tdmac->irq = irq + idx; + tdmac->irq = irq; tdmac->dev = tdev->dev; tdmac->chan.device = &tdev->device; tdmac->idx = idx; @@ -505,34 +505,43 @@ static int __devinit mmp_tdma_chan_init(struct mmp_tdma_device *tdev, /* add the channel to tdma_chan list */ list_add_tail(&tdmac->chan.device_node, &tdev->device.channels); - return 0; } +static struct of_device_id mmp_tdma_dt_ids[] = { + { .compatible = "mrvl,mmp-adma", .data = (void *)MMP_AUD_TDMA}, + { .compatible = "mrvl,pxa910-squ", .data = (void *)PXA910_SQU}, + {} +}; +MODULE_DEVICE_TABLE(of, mmp_tdma_dt_ids); + static int __devinit mmp_tdma_probe(struct platform_device *pdev) { - const struct platform_device_id *id = platform_get_device_id(pdev); - enum mmp_tdma_type type = id->driver_data; + enum mmp_tdma_type type; + const struct of_device_id *of_id; struct mmp_tdma_device *tdev; struct resource *iores; int i, ret; - int irq = 0; + int irq = 0, irq_num = 0; int chan_num = TDMA_CHANNEL_NUM; + of_id = of_match_device(mmp_tdma_dt_ids, &pdev->dev); + if (of_id) + type = (enum mmp_tdma_type) of_id->data; + else + type = platform_get_device_id(pdev)->driver_data; + /* always have couple channels */ tdev = devm_kzalloc(&pdev->dev, sizeof(*tdev), GFP_KERNEL); if (!tdev) return -ENOMEM; tdev->dev = &pdev->dev; - iores = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (!iores) - return -EINVAL; - if (resource_size(iores) != chan_num) - tdev->irq = iores->start; - else - irq = iores->start; + for (i = 0; i < chan_num; i++) { + if (platform_get_irq(pdev, i) > 0) + irq_num++; + } iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!iores) @@ -542,25 +551,26 @@ static int __devinit mmp_tdma_probe(struct platform_device *pdev) if (!tdev->base) return -EADDRNOTAVAIL; - if (tdev->irq) { - ret = devm_request_irq(&pdev->dev, tdev->irq, + INIT_LIST_HEAD(&tdev->device.channels); + + if (irq_num != chan_num) { + irq = platform_get_irq(pdev, 0); + ret = devm_request_irq(&pdev->dev, irq, mmp_tdma_int_handler, IRQF_DISABLED, "tdma", tdev); if (ret) return ret; } - dma_cap_set(DMA_SLAVE, tdev->device.cap_mask); - dma_cap_set(DMA_CYCLIC, tdev->device.cap_mask); - - INIT_LIST_HEAD(&tdev->device.channels); - /* initialize channel parameters */ for (i = 0; i < chan_num; i++) { + irq = (irq_num != chan_num) ? 0 : platform_get_irq(pdev, i); ret = mmp_tdma_chan_init(tdev, i, irq, type); if (ret) return ret; } + dma_cap_set(DMA_SLAVE, tdev->device.cap_mask); + dma_cap_set(DMA_CYCLIC, tdev->device.cap_mask); tdev->device.dev = &pdev->dev; tdev->device.device_alloc_chan_resources = mmp_tdma_alloc_chan_resources; @@ -595,6 +605,7 @@ static struct platform_driver mmp_tdma_driver = { .driver = { .name = "mmp-tdma", .owner = THIS_MODULE, + .of_match_table = mmp_tdma_dt_ids, }, .id_table = mmp_tdma_id_table, .probe = mmp_tdma_probe, -- 1.7.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] dmatest: add dmaengine_slave_config for DMA_MEMCPY 2012-07-20 5:46 [PATCH 0/3] dmaengine: add support mmp-pdma Zhangfei Gao 2012-07-20 5:46 ` [PATCH 1/3] dmaengine: mmp-pdma support Zhangfei Gao 2012-07-20 5:46 ` [PATCH 2/3] dmaengine: mmp_tdma: add dt support Zhangfei Gao @ 2012-07-20 5:46 ` Zhangfei Gao 2 siblings, 0 replies; 15+ messages in thread From: Zhangfei Gao @ 2012-07-20 5:46 UTC (permalink / raw) To: linux-arm-kernel Set direction to DMA_MEM_TO_MEM DMA driver may require such configure info Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com> --- drivers/dma/dmatest.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c index 24225f0..73bab68 100644 --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -261,6 +261,9 @@ static int dmatest_func(void *data) int src_cnt; int dst_cnt; int i; + struct dma_slave_config conf = { + .direction = DMA_MEM_TO_MEM, + }; thread_name = current->comm; set_freezable(); @@ -361,6 +364,7 @@ static int dmatest_func(void *data) DMA_BIDIRECTIONAL); } + dmaengine_slave_config(chan, &conf); if (thread->type == DMA_MEMCPY) tx = dev->device_prep_dma_memcpy(chan, -- 1.7.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-08-06 15:29 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-20 5:46 [PATCH 0/3] dmaengine: add support mmp-pdma Zhangfei Gao 2012-07-20 5:46 ` [PATCH 1/3] dmaengine: mmp-pdma support Zhangfei Gao 2012-07-24 15:42 ` Arnd Bergmann 2012-07-25 4:48 ` zhangfei gao 2012-07-25 6:01 ` Arnd Bergmann 2012-07-25 8:28 ` zhangfei gao 2012-07-25 8:58 ` Arnd Bergmann 2012-07-26 4:46 ` zhangfei gao 2012-08-06 15:29 ` Arnd Bergmann 2012-07-26 6:51 ` Vinod Koul 2012-07-26 7:02 ` zhangfei gao 2012-07-26 10:09 ` Vinod Koul 2012-07-27 2:51 ` zhangfei gao 2012-07-20 5:46 ` [PATCH 2/3] dmaengine: mmp_tdma: add dt support Zhangfei Gao 2012-07-20 5:46 ` [PATCH 3/3] dmatest: add dmaengine_slave_config for DMA_MEMCPY Zhangfei Gao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).