* [PATCH RFC v8 1/5] dma: mpc512x: reorder mpc8308 specific instructions
2014-02-24 11:09 [PATCH RFC v8 0/5] MPC512x DMA slave s/g support, OF DMA lookup Alexander Popov
@ 2014-02-24 11:09 ` Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers Alexander Popov
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Alexander Popov @ 2014-02-24 11:09 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Alexander Popov, linuxppc-dev,
dmaengine
Concentrate the specific code for MPC8308 in the 'if' branch
and handle MPC512x in the 'else' branch.
This modification only reorders instructions but doesn't change behaviour.
Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
Acked-by: Anatolij Gustschin <agust@denx.de>
Acked-by: Gerhard Sittig <gsi@denx.de>
---
drivers/dma/mpc512x_dma.c | 42 +++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 448750d..2ce248b 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -52,9 +52,17 @@
#define MPC_DMA_DESCRIPTORS 64
/* Macro definitions */
-#define MPC_DMA_CHANNELS 64
#define MPC_DMA_TCD_OFFSET 0x1000
+/*
+ * Maximum channel counts for individual hardware variants
+ * and the maximum channel count over all supported controllers,
+ * used for data structure size
+ */
+#define MPC8308_DMACHAN_MAX 16
+#define MPC512x_DMACHAN_MAX 64
+#define MPC_DMA_CHANNELS 64
+
/* Arbitration mode of group and channel */
#define MPC_DMA_DMACR_EDCG (1 << 31)
#define MPC_DMA_DMACR_ERGA (1 << 3)
@@ -710,10 +718,10 @@ static int mpc_dma_probe(struct platform_device *op)
dma = &mdma->dma;
dma->dev = dev;
- if (!mdma->is_mpc8308)
- dma->chancnt = MPC_DMA_CHANNELS;
+ if (mdma->is_mpc8308)
+ dma->chancnt = MPC8308_DMACHAN_MAX;
else
- dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
+ dma->chancnt = MPC512x_DMACHAN_MAX;
dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
dma->device_free_chan_resources = mpc_dma_free_chan_resources;
dma->device_issue_pending = mpc_dma_issue_pending;
@@ -747,7 +755,19 @@ static int mpc_dma_probe(struct platform_device *op)
* - Round-robin group arbitration,
* - Round-robin channel arbitration.
*/
- if (!mdma->is_mpc8308) {
+ if (mdma->is_mpc8308) {
+ /* MPC8308 has 16 channels and lacks some registers */
+ out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
+
+ /* enable snooping */
+ out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
+ /* Disable error interrupts */
+ out_be32(&mdma->regs->dmaeeil, 0);
+
+ /* Clear interrupts status */
+ out_be32(&mdma->regs->dmaintl, 0xFFFF);
+ out_be32(&mdma->regs->dmaerrl, 0xFFFF);
+ } else {
out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
@@ -768,18 +788,6 @@ static int mpc_dma_probe(struct platform_device *op)
/* Route interrupts to IPIC */
out_be32(&mdma->regs->dmaihsa, 0);
out_be32(&mdma->regs->dmailsa, 0);
- } else {
- /* MPC8308 has 16 channels and lacks some registers */
- out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
-
- /* enable snooping */
- out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
- /* Disable error interrupts */
- out_be32(&mdma->regs->dmaeeil, 0);
-
- /* Clear interrupts status */
- out_be32(&mdma->regs->dmaintl, 0xFFFF);
- out_be32(&mdma->regs->dmaerrl, 0xFFFF);
}
/* Register DMA engine */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers
2014-02-24 11:09 [PATCH RFC v8 0/5] MPC512x DMA slave s/g support, OF DMA lookup Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 1/5] dma: mpc512x: reorder mpc8308 specific instructions Alexander Popov
@ 2014-02-24 11:09 ` Alexander Popov
2014-02-24 13:03 ` Andy Shevchenko
2014-02-24 11:09 ` [PATCH RFC v8 3/5] dma: of: Add common xlate function for matching by channel id Alexander Popov
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Alexander Popov @ 2014-02-24 11:09 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Alexander Popov, linuxppc-dev,
dmaengine
Introduce support for slave s/g transfer preparation and the associated
device control callback in the MPC512x DMA controller driver, which adds
support for data transfers between memory and peripheral I/O to the
previously supported mem-to-mem transfers.
Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
drivers/dma/mpc512x_dma.c | 235 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 230 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2ce248b..8f504cb 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -2,6 +2,7 @@
* Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
* Copyright (C) Semihalf 2009
* Copyright (C) Ilya Yanok, Emcraft Systems 2010
+ * Copyright (C) Alexander Popov, Promcontroller 2013
*
* Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
* (defines, structures and comments) was taken from MPC5121 DMA driver
@@ -29,8 +30,17 @@
*/
/*
- * This is initial version of MPC5121 DMA driver. Only memory to memory
- * transfers are supported (tested using dmatest module).
+ * MPC512x and MPC8308 DMA driver. It supports
+ * memory to memory data transfers (tested using dmatest module) and
+ * data transfers between memory and peripheral I/O memory
+ * by means of slave s/g with these limitations:
+ * - chunked transfers (transfers with more than one part) are refused
+ * as long as proper support for scatter/gather is missing;
+ * - transfers on MPC8308 always start from software as this SoC appears
+ * not to have external request lines for peripheral flow control;
+ * - minimal memory <-> I/O memory transfer chunk is 4 bytes and consequently
+ * source and destination addresses must be 4-byte aligned
+ * and transfer size must be aligned on (4 * maxburst) boundary;
*/
#include <linux/module.h>
@@ -189,6 +199,7 @@ struct mpc_dma_desc {
dma_addr_t tcd_paddr;
int error;
struct list_head node;
+ int will_access_peripheral;
};
struct mpc_dma_chan {
@@ -201,6 +212,10 @@ struct mpc_dma_chan {
struct mpc_dma_tcd *tcd;
dma_addr_t tcd_paddr;
+ /* Settings for access to peripheral FIFO */
+ dma_addr_t per_paddr; /* FIFO address */
+ u32 tcd_nunits;
+
/* Lock for this structure */
spinlock_t lock;
};
@@ -251,8 +266,23 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
struct mpc_dma_desc *mdesc;
int cid = mchan->chan.chan_id;
- /* Move all queued descriptors to active list */
- list_splice_tail_init(&mchan->queued, &mchan->active);
+ while (!list_empty(&mchan->queued)) {
+ mdesc = list_first_entry(&mchan->queued,
+ struct mpc_dma_desc, node);
+ /*
+ * Grab either several mem-to-mem transfer descriptors
+ * or one peripheral transfer descriptor,
+ * don't mix mem-to-mem and peripheral transfer descriptors
+ * within the same 'active' list.
+ */
+ if (mdesc->will_access_peripheral) {
+ if (list_empty(&mchan->active))
+ list_move_tail(&mdesc->node, &mchan->active);
+ break;
+ } else {
+ list_move_tail(&mdesc->node, &mchan->active);
+ }
+ }
/* Chain descriptors into one transaction */
list_for_each_entry(mdesc, &mchan->active, node) {
@@ -278,7 +308,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
if (first != prev)
mdma->tcd[cid].e_sg = 1;
- out_8(&mdma->regs->dmassrt, cid);
+
+ if (mdma->is_mpc8308) {
+ /* MPC8308, no request lines, software initiated start */
+ out_8(&mdma->regs->dmassrt, cid);
+ } else if (first->will_access_peripheral) {
+ /* peripherals involved, start by external request signal */
+ out_8(&mdma->regs->dmaserq, cid);
+ } else {
+ /* memory to memory transfer, software initiated start */
+ out_8(&mdma->regs->dmassrt, cid);
+ }
}
/* Handle interrupt on one half of DMA controller (32 channels) */
@@ -596,6 +636,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
}
mdesc->error = 0;
+ mdesc->will_access_peripheral = 0;
tcd = mdesc->tcd;
/* Prepare Transfer Control Descriptor for this transaction */
@@ -643,6 +684,187 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
return &mdesc->desc;
}
+static struct dma_async_tx_descriptor *
+mpc_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 mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
+ struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
+ struct mpc_dma_desc *mdesc = NULL;
+ dma_addr_t per_paddr;
+ u32 tcd_nunits;
+ struct mpc_dma_tcd *tcd;
+ unsigned long iflags;
+ struct scatterlist *sg;
+ size_t len;
+ int iter, i;
+
+ /* currently there is no proper support for scatter/gather */
+ if (sg_len != 1)
+ return NULL;
+
+ for_each_sg(sgl, sg, sg_len, i) {
+ spin_lock_irqsave(&mchan->lock, iflags);
+
+ mdesc = list_first_entry(&mchan->free,
+ struct mpc_dma_desc, node);
+ if (!mdesc) {
+ spin_unlock_irqrestore(&mchan->lock, iflags);
+ /* try to free completed descriptors */
+ mpc_dma_process_completed(mdma);
+ return NULL;
+ }
+
+ list_del(&mdesc->node);
+
+ per_paddr = mchan->per_paddr;
+ tcd_nunits = mchan->tcd_nunits;
+
+ spin_unlock_irqrestore(&mchan->lock, iflags);
+
+ if (per_paddr == 0 || tcd_nunits == 0)
+ goto err_prep;
+
+ mdesc->error = 0;
+ mdesc->will_access_peripheral = 1;
+ tcd = mdesc->tcd;
+
+ /* Prepare Transfer Control Descriptor for this transaction */
+
+ memset(tcd, 0, sizeof(struct mpc_dma_tcd));
+
+ if (!IS_ALIGNED(sg_dma_address(sg), 4))
+ goto err_prep;
+
+ if (direction == DMA_DEV_TO_MEM) {
+ tcd->saddr = per_paddr;
+ tcd->daddr = sg_dma_address(sg);
+ tcd->soff = 0;
+ tcd->doff = 4;
+ } else if (direction == DMA_MEM_TO_DEV) {
+ tcd->saddr = sg_dma_address(sg);
+ tcd->daddr = per_paddr;
+ tcd->soff = 4;
+ tcd->doff = 0;
+ } else
+ goto err_prep;
+
+ tcd->ssize = MPC_DMA_TSIZE_4;
+ tcd->dsize = MPC_DMA_TSIZE_4;
+
+ len = sg_dma_len(sg);
+ tcd->nbytes = tcd_nunits * 4;
+ if (!IS_ALIGNED(len, tcd->nbytes))
+ goto err_prep;
+
+ iter = len / tcd->nbytes;
+ if (iter >= 1 << 15) {
+ /* len is too big */
+ goto err_prep;
+ } else {
+ /* citer_linkch contains the high bits of iter */
+ tcd->biter = iter & 0x1ff;
+ tcd->biter_linkch = iter >> 9;
+ tcd->citer = tcd->biter;
+ tcd->citer_linkch = tcd->biter_linkch;
+ }
+
+ tcd->e_sg = 0;
+ tcd->d_req = 1;
+
+ /* Place descriptor in prepared list */
+ spin_lock_irqsave(&mchan->lock, iflags);
+ list_add_tail(&mdesc->node, &mchan->prepared);
+ spin_unlock_irqrestore(&mchan->lock, iflags);
+ }
+
+ return &mdesc->desc;
+
+err_prep:
+ /* Put the descriptor back */
+ spin_lock_irqsave(&mchan->lock, iflags);
+ list_add_tail(&mdesc->node, &mchan->free);
+ spin_unlock_irqrestore(&mchan->lock, iflags);
+
+ return NULL;
+}
+
+static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
+ unsigned long arg)
+{
+ struct mpc_dma_chan *mchan;
+ struct mpc_dma *mdma;
+ struct dma_slave_config *cfg;
+ unsigned long flags;
+
+ mchan = dma_chan_to_mpc_dma_chan(chan);
+ switch (cmd) {
+ case DMA_TERMINATE_ALL:
+ /* disable channel requests */
+ mdma = dma_chan_to_mpc_dma(chan);
+
+ spin_lock_irqsave(&mchan->lock, flags);
+
+ out_8(&mdma->regs->dmacerq, chan->chan_id);
+ list_splice_tail_init(&mchan->prepared, &mchan->free);
+ list_splice_tail_init(&mchan->queued, &mchan->free);
+ list_splice_tail_init(&mchan->active, &mchan->free);
+
+ spin_unlock_irqrestore(&mchan->lock, flags);
+
+ return 0;
+ case DMA_SLAVE_CONFIG:
+ /* Constraints:
+ * - only transfers between a peripheral device and
+ * memory are supported;
+ * - minimal transfer chunk is 4 bytes and consequently
+ * source and destination addresses must be 4-byte aligned
+ * and transfer size must be aligned on (4 * maxburst)
+ * boundary;
+ * - during the transfer RAM address is being incremented by
+ * the size of minimal transfer chunk;
+ * - peripheral port's address is constant during the transfer.
+ */
+
+ cfg = (void *)arg;
+
+ if (cfg->direction != DMA_DEV_TO_MEM &&
+ cfg->direction != DMA_MEM_TO_DEV)
+ return -EINVAL;
+
+ if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
+ cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES)
+ return -EINVAL;
+
+ spin_lock_irqsave(&mchan->lock, flags);
+
+ if (cfg->direction == DMA_DEV_TO_MEM) {
+ mchan->per_paddr = cfg->src_addr;
+ mchan->tcd_nunits = cfg->src_maxburst;
+ } else {
+ mchan->per_paddr = cfg->dst_addr;
+ mchan->tcd_nunits = cfg->dst_maxburst;
+ }
+
+ if (!IS_ALIGNED(mchan->per_paddr, 4)) {
+ spin_unlock_irqrestore(&mchan->lock, flags);
+ return -EINVAL;
+ }
+
+ if (mchan->tcd_nunits == 0)
+ mchan->tcd_nunits = 1; /* apply default */
+
+ spin_unlock_irqrestore(&mchan->lock, flags);
+
+ return 0;
+ default:
+ return -ENOSYS;
+ }
+
+ return -EINVAL;
+}
+
static int mpc_dma_probe(struct platform_device *op)
{
struct device_node *dn = op->dev.of_node;
@@ -727,9 +949,12 @@ static int mpc_dma_probe(struct platform_device *op)
dma->device_issue_pending = mpc_dma_issue_pending;
dma->device_tx_status = mpc_dma_tx_status;
dma->device_prep_dma_memcpy = mpc_dma_prep_memcpy;
+ dma->device_prep_slave_sg = mpc_dma_prep_slave_sg;
+ dma->device_control = mpc_dma_device_control;
INIT_LIST_HEAD(&dma->channels);
dma_cap_set(DMA_MEMCPY, dma->cap_mask);
+ dma_cap_set(DMA_SLAVE, dma->cap_mask);
for (i = 0; i < dma->chancnt; i++) {
mchan = &mdma->channels[i];
--
1.8.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers
2014-02-24 11:09 ` [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers Alexander Popov
@ 2014-02-24 13:03 ` Andy Shevchenko
2014-03-01 9:19 ` Alexander Popov
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2014-02-24 13:03 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig,
dmaengine, Dan Williams, Anatolij Gustschin, linuxppc-dev
On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote:
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.
>
Few comments below.
> Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
> ---
> drivers/dma/mpc512x_dma.c | 235 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 230 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 2ce248b..8f504cb 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -2,6 +2,7 @@
> * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
> * Copyright (C) Semihalf 2009
> * Copyright (C) Ilya Yanok, Emcraft Systems 2010
> + * Copyright (C) Alexander Popov, Promcontroller 2013
> *
> * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description
> * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -29,8 +30,17 @@
> */
>
> /*
> - * This is initial version of MPC5121 DMA driver. Only memory to memory
> - * transfers are supported (tested using dmatest module).
> + * MPC512x and MPC8308 DMA driver. It supports
> + * memory to memory data transfers (tested using dmatest module) and
> + * data transfers between memory and peripheral I/O memory
> + * by means of slave s/g with these limitations:
> + * - chunked transfers (transfers with more than one part) are refused
> + * as long as proper support for scatter/gather is missing;
> + * - transfers on MPC8308 always start from software as this SoC appears
> + * not to have external request lines for peripheral flow control;
> + * - minimal memory <-> I/O memory transfer chunk is 4 bytes and consequently
> + * source and destination addresses must be 4-byte aligned
> + * and transfer size must be aligned on (4 * maxburst) boundary;
> */
>
> #include <linux/module.h>
> @@ -189,6 +199,7 @@ struct mpc_dma_desc {
> dma_addr_t tcd_paddr;
> int error;
> struct list_head node;
> + int will_access_peripheral;
> };
>
> struct mpc_dma_chan {
> @@ -201,6 +212,10 @@ struct mpc_dma_chan {
> struct mpc_dma_tcd *tcd;
> dma_addr_t tcd_paddr;
>
> + /* Settings for access to peripheral FIFO */
> + dma_addr_t per_paddr; /* FIFO address */
> + u32 tcd_nunits;
> +
> /* Lock for this structure */
> spinlock_t lock;
> };
> @@ -251,8 +266,23 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
> struct mpc_dma_desc *mdesc;
> int cid = mchan->chan.chan_id;
>
> - /* Move all queued descriptors to active list */
> - list_splice_tail_init(&mchan->queued, &mchan->active);
> + while (!list_empty(&mchan->queued)) {
> + mdesc = list_first_entry(&mchan->queued,
> + struct mpc_dma_desc, node);
> + /*
> + * Grab either several mem-to-mem transfer descriptors
> + * or one peripheral transfer descriptor,
> + * don't mix mem-to-mem and peripheral transfer descriptors
> + * within the same 'active' list.
> + */
> + if (mdesc->will_access_peripheral) {
> + if (list_empty(&mchan->active))
> + list_move_tail(&mdesc->node, &mchan->active);
> + break;
> + } else {
> + list_move_tail(&mdesc->node, &mchan->active);
> + }
> + }
>
> /* Chain descriptors into one transaction */
> list_for_each_entry(mdesc, &mchan->active, node) {
> @@ -278,7 +308,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>
> if (first != prev)
> mdma->tcd[cid].e_sg = 1;
> - out_8(&mdma->regs->dmassrt, cid);
> +
> + if (mdma->is_mpc8308) {
> + /* MPC8308, no request lines, software initiated start */
> + out_8(&mdma->regs->dmassrt, cid);
> + } else if (first->will_access_peripheral) {
> + /* peripherals involved, start by external request signal */
Probably you have to keep style of all comments in the code. For
example, let's start sentences from a capital letter.
> + out_8(&mdma->regs->dmaserq, cid);
> + } else {
> + /* memory to memory transfer, software initiated start */
> + out_8(&mdma->regs->dmassrt, cid);
> + }
> }
>
> /* Handle interrupt on one half of DMA controller (32 channels) */
> @@ -596,6 +636,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
> }
>
> mdesc->error = 0;
> + mdesc->will_access_peripheral = 0;
> tcd = mdesc->tcd;
>
> /* Prepare Transfer Control Descriptor for this transaction */
> @@ -643,6 +684,187 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
> return &mdesc->desc;
> }
>
> +static struct dma_async_tx_descriptor *
> +mpc_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 mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
> + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
> + struct mpc_dma_desc *mdesc = NULL;
> + dma_addr_t per_paddr;
> + u32 tcd_nunits;
> + struct mpc_dma_tcd *tcd;
> + unsigned long iflags;
> + struct scatterlist *sg;
> + size_t len;
> + int iter, i;
> +
> + /* currently there is no proper support for scatter/gather */
> + if (sg_len != 1)
> + return NULL;
You may check direction right there using is_slave_direction().
> +
> + for_each_sg(sgl, sg, sg_len, i) {
> + spin_lock_irqsave(&mchan->lock, iflags);
> +
> + mdesc = list_first_entry(&mchan->free,
> + struct mpc_dma_desc, node);
> + if (!mdesc) {
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> + /* try to free completed descriptors */
> + mpc_dma_process_completed(mdma);
> + return NULL;
> + }
> +
> + list_del(&mdesc->node);
> +
> + per_paddr = mchan->per_paddr;
> + tcd_nunits = mchan->tcd_nunits;
> +
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> +
> + if (per_paddr == 0 || tcd_nunits == 0)
> + goto err_prep;
> +
> + mdesc->error = 0;
> + mdesc->will_access_peripheral = 1;
> + tcd = mdesc->tcd;
> +
> + /* Prepare Transfer Control Descriptor for this transaction */
> +
Maybe instead of empty line you can move tcd assignment here.
> + memset(tcd, 0, sizeof(struct mpc_dma_tcd));
> +
> + if (!IS_ALIGNED(sg_dma_address(sg), 4))
> + goto err_prep;
> +
> + if (direction == DMA_DEV_TO_MEM) {
> + tcd->saddr = per_paddr;
> + tcd->daddr = sg_dma_address(sg);
> + tcd->soff = 0;
> + tcd->doff = 4;
> + } else if (direction == DMA_MEM_TO_DEV) {
> + tcd->saddr = sg_dma_address(sg);
> + tcd->daddr = per_paddr;
> + tcd->soff = 4;
> + tcd->doff = 0;
> + } else
> + goto err_prep;
First, keep style of conditionals, second, you may remove this branch
and previous if in case of checking direction outside of loop.
> +
> + tcd->ssize = MPC_DMA_TSIZE_4;
> + tcd->dsize = MPC_DMA_TSIZE_4;
> +
> + len = sg_dma_len(sg);
> + tcd->nbytes = tcd_nunits * 4;
> + if (!IS_ALIGNED(len, tcd->nbytes))
> + goto err_prep;
> +
> + iter = len / tcd->nbytes;
> + if (iter >= 1 << 15) {
> + /* len is too big */
> + goto err_prep;
> + } else {
Redundant else branch.
> + /* citer_linkch contains the high bits of iter */
> + tcd->biter = iter & 0x1ff;
> + tcd->biter_linkch = iter >> 9;
> + tcd->citer = tcd->biter;
> + tcd->citer_linkch = tcd->biter_linkch;
> + }
> +
> + tcd->e_sg = 0;
> + tcd->d_req = 1;
> +
> + /* Place descriptor in prepared list */
> + spin_lock_irqsave(&mchan->lock, iflags);
> + list_add_tail(&mdesc->node, &mchan->prepared);
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> + }
> +
> + return &mdesc->desc;
> +
> +err_prep:
> + /* Put the descriptor back */
> + spin_lock_irqsave(&mchan->lock, iflags);
> + list_add_tail(&mdesc->node, &mchan->free);
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> +
> + return NULL;
> +}
> +
> +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> + unsigned long arg)
> +{
> + struct mpc_dma_chan *mchan;
> + struct mpc_dma *mdma;
> + struct dma_slave_config *cfg;
> + unsigned long flags;
> +
> + mchan = dma_chan_to_mpc_dma_chan(chan);
> + switch (cmd) {
> + case DMA_TERMINATE_ALL:
> + /* disable channel requests */
> + mdma = dma_chan_to_mpc_dma(chan);
> +
> + spin_lock_irqsave(&mchan->lock, flags);
> +
> + out_8(&mdma->regs->dmacerq, chan->chan_id);
> + list_splice_tail_init(&mchan->prepared, &mchan->free);
> + list_splice_tail_init(&mchan->queued, &mchan->free);
> + list_splice_tail_init(&mchan->active, &mchan->free);
> +
> + spin_unlock_irqrestore(&mchan->lock, flags);
> +
> + return 0;
> + case DMA_SLAVE_CONFIG:
> + /* Constraints:
> + * - only transfers between a peripheral device and
> + * memory are supported;
> + * - minimal transfer chunk is 4 bytes and consequently
> + * source and destination addresses must be 4-byte aligned
> + * and transfer size must be aligned on (4 * maxburst)
> + * boundary;
> + * - during the transfer RAM address is being incremented by
> + * the size of minimal transfer chunk;
> + * - peripheral port's address is constant during the transfer.
> + */
> +
> + cfg = (void *)arg;
> +
> + if (cfg->direction != DMA_DEV_TO_MEM &&
> + cfg->direction != DMA_MEM_TO_DEV)
> + return -EINVAL;
is_slave_direction()
> +
> + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
> + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&mchan->lock, flags);
> +
> + if (cfg->direction == DMA_DEV_TO_MEM) {
> + mchan->per_paddr = cfg->src_addr;
> + mchan->tcd_nunits = cfg->src_maxburst;
> + } else {
> + mchan->per_paddr = cfg->dst_addr;
> + mchan->tcd_nunits = cfg->dst_maxburst;
> + }
> +
> + if (!IS_ALIGNED(mchan->per_paddr, 4)) {
> + spin_unlock_irqrestore(&mchan->lock, flags);
> + return -EINVAL;
> + }
> +
> + if (mchan->tcd_nunits == 0)
> + mchan->tcd_nunits = 1; /* apply default */
> +
> + spin_unlock_irqrestore(&mchan->lock, flags);
> +
> + return 0;
> + default:
> + return -ENOSYS;
Use break here.
> + }
> +
> + return -EINVAL;
> +}
> +
> static int mpc_dma_probe(struct platform_device *op)
> {
> struct device_node *dn = op->dev.of_node;
> @@ -727,9 +949,12 @@ static int mpc_dma_probe(struct platform_device *op)
> dma->device_issue_pending = mpc_dma_issue_pending;
> dma->device_tx_status = mpc_dma_tx_status;
> dma->device_prep_dma_memcpy = mpc_dma_prep_memcpy;
> + dma->device_prep_slave_sg = mpc_dma_prep_slave_sg;
> + dma->device_control = mpc_dma_device_control;
>
> INIT_LIST_HEAD(&dma->channels);
> dma_cap_set(DMA_MEMCPY, dma->cap_mask);
> + dma_cap_set(DMA_SLAVE, dma->cap_mask);
>
> for (i = 0; i < dma->chancnt; i++) {
> mchan = &mdma->channels[i];
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers
2014-02-24 13:03 ` Andy Shevchenko
@ 2014-03-01 9:19 ` Alexander Popov
0 siblings, 0 replies; 10+ messages in thread
From: Alexander Popov @ 2014-03-01 9:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig,
Alexander Popov, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
Hello Andy.
2014-02-24 17:03 GMT+04:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote:
>> Introduce support for slave s/g transfer preparation and the associated
>> device control callback in the MPC512x DMA controller driver, which adds
>> support for data transfers between memory and peripheral I/O to the
>> previously supported mem-to-mem transfers.
>>
>
> Few comments below.
Thanks for your feedback. I agree with your points and will fix my code.
Best regards,
Alexander
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH RFC v8 3/5] dma: of: Add common xlate function for matching by channel id
2014-02-24 11:09 [PATCH RFC v8 0/5] MPC512x DMA slave s/g support, OF DMA lookup Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 1/5] dma: mpc512x: reorder mpc8308 specific instructions Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers Alexander Popov
@ 2014-02-24 11:09 ` Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 4/5] dma: mpc512x: add device tree binding document Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 5/5] dma: mpc512x: register for device tree channel lookup Alexander Popov
4 siblings, 0 replies; 10+ messages in thread
From: Alexander Popov @ 2014-02-24 11:09 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Alexander Popov, linuxppc-dev,
dmaengine
Cc: devicetree
This patch adds a new common OF dma xlate callback function which will match a
channel by it's id. The binding expects one integer argument which it will use to
lookup the channel by the id.
Unlike of_dma_simple_xlate this function is able to handle a system with
multiple DMA controllers. When registering the of dma provider with
of_dma_controller_register a pointer to the dma_device struct which is
associated with the dt node needs to passed as the data parameter.
New function will use this pointer to match only channels which belong to the
specified DMA controller.
Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
drivers/dma/of-dma.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/of_dma.h | 4 ++++
2 files changed, 39 insertions(+)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index e8fe9dc..d5fbeaa 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -218,3 +218,38 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
&dma_spec->args[0]);
}
EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
+
+/**
+ * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id
+ * @dma_spec: pointer to DMA specifier as found in the device tree
+ * @of_dma: pointer to DMA controller data
+ *
+ * This function can be used as the of xlate callback for DMA driver which wants
+ * to match the channel based on the channel id. When using this xlate function
+ * the #dma-cells propety of the DMA controller dt node needs to be set to 1.
+ * The data parameter of of_dma_controller_register must be a pointer to the
+ * dma_device struct the function should match upon.
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+ struct of_dma *ofdma)
+{
+ struct dma_device *dev = ofdma->of_dma_data;
+ struct dma_chan *chan, *candidate = NULL;
+
+ if (!dev || dma_spec->args_count != 1)
+ return NULL;
+
+ list_for_each_entry(chan, &dev->channels, device_node)
+ if (chan->chan_id == dma_spec->args[0]) {
+ candidate = chan;
+ break;
+ }
+
+ if (!candidate)
+ return NULL;
+
+ return dma_get_slave_channel(candidate);
+}
+EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index ae36298..56bc026 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -41,6 +41,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
const char *name);
extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma);
+extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+ struct of_dma *ofdma);
#else
static inline int of_dma_controller_register(struct device_node *np,
struct dma_chan *(*of_dma_xlate)
@@ -66,6 +68,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s
return NULL;
}
+#define of_dma_xlate_by_chan_id NULL
+
#endif
#endif /* __LINUX_OF_DMA_H */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC v8 4/5] dma: mpc512x: add device tree binding document
2014-02-24 11:09 [PATCH RFC v8 0/5] MPC512x DMA slave s/g support, OF DMA lookup Alexander Popov
` (2 preceding siblings ...)
2014-02-24 11:09 ` [PATCH RFC v8 3/5] dma: of: Add common xlate function for matching by channel id Alexander Popov
@ 2014-02-24 11:09 ` Alexander Popov
2014-02-24 11:09 ` [PATCH RFC v8 5/5] dma: mpc512x: register for device tree channel lookup Alexander Popov
4 siblings, 0 replies; 10+ messages in thread
From: Alexander Popov @ 2014-02-24 11:09 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Alexander Popov, linuxppc-dev,
dmaengine
Cc: devicetree
From: Gerhard Sittig <gsi@denx.de>
introduce a device tree binding document for the MPC512x DMA controller
Signed-off-by: Gerhard Sittig <gsi@denx.de>
[ a13xp0p0v88@gmail.com: turn this into a separate patch ]
---
.../devicetree/bindings/dma/mpc512x-dma.txt | 55 ++++++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
new file mode 100644
index 0000000..a4867d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,55 @@
+* Freescale MPC512x DMA Controller
+
+The DMA controller in the Freescale MPC512x SoC can move blocks of
+memory contents between memory and peripherals or memory to memory.
+
+Refer to the "Generic DMA Controller and DMA request bindings" description
+in the dma.txt file for a more detailled discussion of the binding. The
+MPC512x DMA engine binding follows the common scheme, but doesn't provide
+support for the optional channels and requests counters (those values are
+derived from the detected hardware features) and has a fixed client
+specifier length of 1 integer cell (the value is the DMA channel, since
+the DMA controller uses a fixed assignment of request lines per channel).
+
+
+DMA controller node properties:
+
+Required properties:
+- compatible: should be "fsl,mpc5121-dma"
+- reg: address and size of the DMA controller's register set
+- interrupts: interrupt spec for the DMA controller
+
+Optional properties:
+- #dma-cells: must be <1>, describes the number of integer cells
+ needed to specify the 'dmas' property in client nodes,
+ strongly recommended since common client helper code
+ uses this property
+
+Example:
+
+ dma0: dma@14000 {
+ compatible = "fsl,mpc5121-dma";
+ reg = <0x14000 0x1800>;
+ interrupts = <65 0x8>;
+ #dma-cells = <1>;
+ };
+
+
+Client node properties:
+
+Required properties:
+- dmas: list of DMA specifiers, consisting each of a handle
+ for the DMA controller and integer cells to specify
+ the channel used within the DMA controller
+- dma-names: list of identifier strings for the DMA specifiers,
+ client device driver code uses these strings to
+ have DMA channels looked up at the controller
+
+Example:
+
+ sdhc@1500 {
+ compatible = "fsl,mpc5121-sdhc";
+ /* ... */
+ dmas = <&dma0 30>;
+ dma-names = "rx-tx";
+ };
--
1.8.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC v8 5/5] dma: mpc512x: register for device tree channel lookup
2014-02-24 11:09 [PATCH RFC v8 0/5] MPC512x DMA slave s/g support, OF DMA lookup Alexander Popov
` (3 preceding siblings ...)
2014-02-24 11:09 ` [PATCH RFC v8 4/5] dma: mpc512x: add device tree binding document Alexander Popov
@ 2014-02-24 11:09 ` Alexander Popov
2014-02-24 13:08 ` Andy Shevchenko
4 siblings, 1 reply; 10+ messages in thread
From: Alexander Popov @ 2014-02-24 11:09 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Alexander Popov, linuxppc-dev,
dmaengine
Cc: devicetree
From: Gerhard Sittig <gsi@denx.de>
register the controller for device tree based lookup of DMA channels
(non-fatal for backwards compatibility with older device trees) and
provide the '#dma-cells' property in the shared mpc5121.dtsi file
Signed-off-by: Gerhard Sittig <gsi@denx.de>
[ a13xp0p0v88@gmail.com: resolve little patch conflict and put
MPC512x DMA controller bindings document to a separate patch ]
---
arch/powerpc/boot/dts/mpc5121.dtsi | 1 +
drivers/dma/mpc512x_dma.c | 21 ++++++++++++++++++---
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
index 2c0e155..7f9d14f 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -498,6 +498,7 @@
compatible = "fsl,mpc5121-dma";
reg = <0x14000 0x1800>;
interrupts = <65 0x8>;
+ #dma-cells = <1>;
};
};
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 8f504cb..d9f8740 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -52,6 +52,7 @@
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_irq.h>
+#include <linux/of_dma.h>
#include <linux/of_platform.h>
#include <linux/random.h>
@@ -1018,11 +1019,23 @@ static int mpc_dma_probe(struct platform_device *op)
/* Register DMA engine */
dev_set_drvdata(dev, mdma);
retval = dma_async_device_register(dma);
- if (retval) {
- devm_free_irq(dev, mdma->irq, mdma);
- irq_dispose_mapping(mdma->irq);
+ if (retval)
+ goto out_irq;
+
+ /* register with OF helpers for DMA lookups (nonfatal) */
+ if (dev->of_node) {
+ retval = of_dma_controller_register(dev->of_node,
+ of_dma_xlate_by_chan_id,
+ mdma);
+ if (retval)
+ dev_warn(dev, "could not register for OF lookup\n");
}
+ return 0;
+
+out_irq:
+ devm_free_irq(dev, mdma->irq, mdma);
+ irq_dispose_mapping(mdma->irq);
return retval;
}
@@ -1031,6 +1044,8 @@ static int mpc_dma_remove(struct platform_device *op)
struct device *dev = &op->dev;
struct mpc_dma *mdma = dev_get_drvdata(dev);
+ if (dev->of_node)
+ of_dma_controller_free(dev->of_node);
dma_async_device_unregister(&mdma->dma);
devm_free_irq(dev, mdma->irq, mdma);
irq_dispose_mapping(mdma->irq);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v8 5/5] dma: mpc512x: register for device tree channel lookup
2014-02-24 11:09 ` [PATCH RFC v8 5/5] dma: mpc512x: register for device tree channel lookup Alexander Popov
@ 2014-02-24 13:08 ` Andy Shevchenko
2014-03-06 14:28 ` Alexander Popov
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2014-02-24 13:08 UTC (permalink / raw)
To: Alexander Popov
Cc: devicetree, Lars-Peter Clausen, Arnd Bergmann, Vinod Koul,
Gerhard Sittig, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote:
> From: Gerhard Sittig <gsi@denx.de>
>
> register the controller for device tree based lookup of DMA channels
> (non-fatal for backwards compatibility with older device trees) and
> provide the '#dma-cells' property in the shared mpc5121.dtsi file
>
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> [ a13xp0p0v88@gmail.com: resolve little patch conflict and put
> MPC512x DMA controller bindings document to a separate patch ]
> ---
> arch/powerpc/boot/dts/mpc5121.dtsi | 1 +
> drivers/dma/mpc512x_dma.c | 21 ++++++++++++++++++---
> 2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
> index 2c0e155..7f9d14f 100644
> --- a/arch/powerpc/boot/dts/mpc5121.dtsi
> +++ b/arch/powerpc/boot/dts/mpc5121.dtsi
> @@ -498,6 +498,7 @@
> compatible = "fsl,mpc5121-dma";
> reg = <0x14000 0x1800>;
> interrupts = <65 0x8>;
> + #dma-cells = <1>;
> };
> };
>
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 8f504cb..d9f8740 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -52,6 +52,7 @@
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> #include <linux/of_irq.h>
> +#include <linux/of_dma.h>
> #include <linux/of_platform.h>
>
> #include <linux/random.h>
> @@ -1018,11 +1019,23 @@ static int mpc_dma_probe(struct platform_device *op)
> /* Register DMA engine */
> dev_set_drvdata(dev, mdma);
> retval = dma_async_device_register(dma);
> - if (retval) {
> - devm_free_irq(dev, mdma->irq, mdma);
> - irq_dispose_mapping(mdma->irq);
> + if (retval)
> + goto out_irq;
> +
> + /* register with OF helpers for DMA lookups (nonfatal) */
> + if (dev->of_node) {
> + retval = of_dma_controller_register(dev->of_node,
> + of_dma_xlate_by_chan_id,
> + mdma);
> + if (retval)
> + dev_warn(dev, "could not register for OF lookup\n");
> }
>
> + return 0;
> +
> +out_irq:
> + devm_free_irq(dev, mdma->irq, mdma);
Something wrong either with devm_request_irq() or you don't need to call
devm_free_irq() explicitly. Once we already try to discuss this earlier
in this mailing list with Lars-Peter(?), though there were no solution
how to keep devm_*_irq usability.
> + irq_dispose_mapping(mdma->irq);
> return retval;
> }
>
> @@ -1031,6 +1044,8 @@ static int mpc_dma_remove(struct platform_device *op)
> struct device *dev = &op->dev;
> struct mpc_dma *mdma = dev_get_drvdata(dev);
>
> + if (dev->of_node)
> + of_dma_controller_free(dev->of_node);
> dma_async_device_unregister(&mdma->dma);
> devm_free_irq(dev, mdma->irq, mdma);
> irq_dispose_mapping(mdma->irq);
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v8 5/5] dma: mpc512x: register for device tree channel lookup
2014-02-24 13:08 ` Andy Shevchenko
@ 2014-03-06 14:28 ` Alexander Popov
0 siblings, 0 replies; 10+ messages in thread
From: Alexander Popov @ 2014-03-06 14:28 UTC (permalink / raw)
To: Andy Shevchenko
Cc: devicetree, Lars-Peter Clausen, Arnd Bergmann, Vinod Koul,
Gerhard Sittig, Alexander Popov, dmaengine, Dan Williams,
Anatolij Gustschin, linuxppc-dev
Hello Andy.
2014-02-24 17:08 GMT+04:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote:
>> @@ -1018,11 +1019,23 @@ static int mpc_dma_probe(struct platform_device *op)
>> /* Register DMA engine */
>> dev_set_drvdata(dev, mdma);
>> retval = dma_async_device_register(dma);
>> - if (retval) {
>> - devm_free_irq(dev, mdma->irq, mdma);
>> - irq_dispose_mapping(mdma->irq);
>> + if (retval)
>> + goto out_irq;
>> +
>> + /* register with OF helpers for DMA lookups (nonfatal) */
>> + if (dev->of_node) {
>> + retval = of_dma_controller_register(dev->of_node,
>> + of_dma_xlate_by_chan_id,
>> + mdma);
>> + if (retval)
>> + dev_warn(dev, "could not register for OF lookup\n");
>> }
>>
>> + return 0;
>> +
>> +out_irq:
>> + devm_free_irq(dev, mdma->irq, mdma);
>
> Something wrong either with devm_request_irq() or you don't need to call
> devm_free_irq() explicitly. Once we already try to discuss this earlier
> in this mailing list with Lars-Peter(?), though there were no solution
> how to keep devm_*_irq usability.
Thanks, I've read this discussion. It seems that the current code doesn't do
anything bad, though devm_request_irq() and devm_free_irq() can be changed
to request_irq() and free_irq() accordingly. Do you think it's worth being done
in a separate patch in this series?
>> + irq_dispose_mapping(mdma->irq);
>> return retval;
>> }
>>
>> @@ -1031,6 +1044,8 @@ static int mpc_dma_remove(struct platform_device *op)
>> struct device *dev = &op->dev;
>> struct mpc_dma *mdma = dev_get_drvdata(dev);
>>
>> + if (dev->of_node)
>> + of_dma_controller_free(dev->of_node);
>> dma_async_device_unregister(&mdma->dma);
>> devm_free_irq(dev, mdma->irq, mdma);
>> irq_dispose_mapping(mdma->irq);
Best regards,
Alexander
^ permalink raw reply [flat|nested] 10+ messages in thread