* [PATCH 1/7] fsl-dma: remove attribute DMA_INTERRUPT of dmaengine
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-07-31 23:45 ` [PATCH 2/7] fsl-dma: minimize locking overhead Ira W. Snyder
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto; +Cc: Vinod Koul, B32616, Qiang Liu, Dan Williams, linuxppc-dev
From: Qiang Liu <qiang.liu@freescale.com>
Delete attribute DMA_INTERRUPT because fsl-dma doesn't support this function,
exception will be thrown if talitos is used to offload xor at the same time.
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Li Yang <leoli@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
Acked-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
drivers/dma/fsldma.c | 31 -------------------------------
1 files changed, 0 insertions(+), 31 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 8f84761..4f2f212 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -543,35 +543,6 @@ static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
}
static struct dma_async_tx_descriptor *
-fsl_dma_prep_interrupt(struct dma_chan *dchan, unsigned long flags)
-{
- struct fsldma_chan *chan;
- struct fsl_desc_sw *new;
-
- if (!dchan)
- return NULL;
-
- chan = to_fsl_chan(dchan);
-
- new = fsl_dma_alloc_descriptor(chan);
- if (!new) {
- chan_err(chan, "%s\n", msg_ld_oom);
- return NULL;
- }
-
- new->async_tx.cookie = -EBUSY;
- new->async_tx.flags = flags;
-
- /* Insert the link descriptor to the LD ring */
- list_add_tail(&new->node, &new->tx_list);
-
- /* Set End-of-link to the last link descriptor of new list */
- set_ld_eol(chan, new);
-
- return &new->async_tx;
-}
-
-static struct dma_async_tx_descriptor *
fsl_dma_prep_memcpy(struct dma_chan *dchan,
dma_addr_t dma_dst, dma_addr_t dma_src,
size_t len, unsigned long flags)
@@ -1352,12 +1323,10 @@ static int __devinit fsldma_of_probe(struct platform_device *op)
fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);
dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
- dma_cap_set(DMA_INTERRUPT, fdev->common.cap_mask);
dma_cap_set(DMA_SG, fdev->common.cap_mask);
dma_cap_set(DMA_SLAVE, fdev->common.cap_mask);
fdev->common.device_alloc_chan_resources = fsl_dma_alloc_chan_resources;
fdev->common.device_free_chan_resources = fsl_dma_free_chan_resources;
- fdev->common.device_prep_dma_interrupt = fsl_dma_prep_interrupt;
fdev->common.device_prep_dma_memcpy = fsl_dma_prep_memcpy;
fdev->common.device_prep_dma_sg = fsl_dma_prep_sg;
fdev->common.device_tx_status = fsl_tx_status;
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/7] fsl-dma: minimize locking overhead
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
2012-07-31 23:45 ` [PATCH 1/7] fsl-dma: remove attribute DMA_INTERRUPT of dmaengine Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-07-31 23:45 ` [PATCH 3/7] fsl-dma: add fsl_dma_free_descriptor() to reduce code duplication Ira W. Snyder
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto; +Cc: B32616, linuxppc-dev, Ira W. Snyder
From: "Ira W. Snyder" <iws@ovro.caltech.edu>
The use of spin_lock_irqsave() was a stronger locking mechanism than was
actually needed in many cases.
As the current code is written, spin_lock_bh() everywhere is sufficient.
The next patch in this series will add some code to hardware interrupt
context. This patch is intended to minimize the differences in the next
patch and make review easier.
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
drivers/dma/fsldma.c | 25 ++++++++++---------------
1 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 4f2f212..8f0505d 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -405,10 +405,9 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
struct fsldma_chan *chan = to_fsl_chan(tx->chan);
struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
struct fsl_desc_sw *child;
- unsigned long flags;
dma_cookie_t cookie;
- spin_lock_irqsave(&chan->desc_lock, flags);
+ spin_lock_irq(&chan->desc_lock);
/*
* assign cookies to all of the software descriptors
@@ -421,7 +420,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
/* put this transaction onto the tail of the pending queue */
append_ld_queue(chan, desc);
- spin_unlock_irqrestore(&chan->desc_lock, flags);
+ spin_unlock_irq(&chan->desc_lock);
return cookie;
}
@@ -530,13 +529,12 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
{
struct fsldma_chan *chan = to_fsl_chan(dchan);
- unsigned long flags;
chan_dbg(chan, "free all channel resources\n");
- spin_lock_irqsave(&chan->desc_lock, flags);
+ spin_lock_irq(&chan->desc_lock);
fsldma_free_desc_list(chan, &chan->ld_pending);
fsldma_free_desc_list(chan, &chan->ld_running);
- spin_unlock_irqrestore(&chan->desc_lock, flags);
+ spin_unlock_irq(&chan->desc_lock);
dma_pool_destroy(chan->desc_pool);
chan->desc_pool = NULL;
@@ -755,7 +753,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
{
struct dma_slave_config *config;
struct fsldma_chan *chan;
- unsigned long flags;
int size;
if (!dchan)
@@ -765,7 +762,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
switch (cmd) {
case DMA_TERMINATE_ALL:
- spin_lock_irqsave(&chan->desc_lock, flags);
+ spin_lock_irq(&chan->desc_lock);
/* Halt the DMA engine */
dma_halt(chan);
@@ -775,7 +772,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
fsldma_free_desc_list(chan, &chan->ld_running);
chan->idle = true;
- spin_unlock_irqrestore(&chan->desc_lock, flags);
+ spin_unlock_irq(&chan->desc_lock);
return 0;
case DMA_SLAVE_CONFIG:
@@ -935,11 +932,10 @@ static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
{
struct fsldma_chan *chan = to_fsl_chan(dchan);
- unsigned long flags;
- spin_lock_irqsave(&chan->desc_lock, flags);
+ spin_lock_irq(&chan->desc_lock);
fsl_chan_xfer_ld_queue(chan);
- spin_unlock_irqrestore(&chan->desc_lock, flags);
+ spin_unlock_irq(&chan->desc_lock);
}
/**
@@ -952,11 +948,10 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
{
struct fsldma_chan *chan = to_fsl_chan(dchan);
enum dma_status ret;
- unsigned long flags;
- spin_lock_irqsave(&chan->desc_lock, flags);
+ spin_lock_irq(&chan->desc_lock);
ret = dma_cookie_status(dchan, cookie, txstate);
- spin_unlock_irqrestore(&chan->desc_lock, flags);
+ spin_unlock_irq(&chan->desc_lock);
return ret;
}
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/7] fsl-dma: add fsl_dma_free_descriptor() to reduce code duplication
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
2012-07-31 23:45 ` [PATCH 1/7] fsl-dma: remove attribute DMA_INTERRUPT of dmaengine Ira W. Snyder
2012-07-31 23:45 ` [PATCH 2/7] fsl-dma: minimize locking overhead Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-07-31 23:45 ` [PATCH 4/7] fsl-dma: move functions to avoid forward declarations Ira W. Snyder
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto; +Cc: B32616, linuxppc-dev, Ira W. Snyder
From: "Ira W. Snyder" <iws@ovro.caltech.edu>
There are several places where descriptors are freed using identical
code. Put this code into a function to reduce code duplication.
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
drivers/dma/fsldma.c | 32 ++++++++++++++------------------
1 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 8f0505d..c34a628 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -400,6 +400,15 @@ out_splice:
list_splice_tail_init(&desc->tx_list, &chan->ld_pending);
}
+static void fsl_dma_free_descriptor(struct fsldma_chan *chan, struct fsl_desc_sw *desc)
+{
+ list_del(&desc->node);
+#ifdef FSL_DMA_LD_DEBUG
+ chan_dbg(chan, "LD %p free\n", desc);
+#endif
+ dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
+}
+
static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
{
struct fsldma_chan *chan = to_fsl_chan(tx->chan);
@@ -499,13 +508,8 @@ static void fsldma_free_desc_list(struct fsldma_chan *chan,
{
struct fsl_desc_sw *desc, *_desc;
- list_for_each_entry_safe(desc, _desc, list, node) {
- list_del(&desc->node);
-#ifdef FSL_DMA_LD_DEBUG
- chan_dbg(chan, "LD %p free\n", desc);
-#endif
- dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
- }
+ list_for_each_entry_safe(desc, _desc, list, node)
+ fsl_dma_free_descriptor(chan, desc);
}
static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
@@ -513,13 +517,8 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
{
struct fsl_desc_sw *desc, *_desc;
- list_for_each_entry_safe_reverse(desc, _desc, list, node) {
- list_del(&desc->node);
-#ifdef FSL_DMA_LD_DEBUG
- chan_dbg(chan, "LD %p free\n", desc);
-#endif
- dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
- }
+ list_for_each_entry_safe_reverse(desc, _desc, list, node)
+ fsl_dma_free_descriptor(chan, desc);
}
/**
@@ -852,10 +851,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
dma_unmap_page(dev, src, len, DMA_TO_DEVICE);
}
-#ifdef FSL_DMA_LD_DEBUG
- chan_dbg(chan, "LD %p free\n", desc);
-#endif
- dma_pool_free(chan->desc_pool, desc, txd->phys);
+ fsl_dma_free_descriptor(chan, desc);
}
/**
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/7] fsl-dma: move functions to avoid forward declarations
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
` (2 preceding siblings ...)
2012-07-31 23:45 ` [PATCH 3/7] fsl-dma: add fsl_dma_free_descriptor() to reduce code duplication Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-07-31 23:45 ` [PATCH 5/7] fsl-dma: fix support for async_tx API Ira W. Snyder
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto; +Cc: B32616, linuxppc-dev, Ira W. Snyder
From: "Ira W. Snyder" <iws@ovro.caltech.edu>
This function will be modified in the next patch in the series. By
moving the function in a patch separate from the changes, it will make
review easier.
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
drivers/dma/fsldma.c | 96 +++++++++++++++++++++++++-------------------------
1 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index c34a628..80edc63 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -409,6 +409,54 @@ static void fsl_dma_free_descriptor(struct fsldma_chan *chan, struct fsl_desc_sw
dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
}
+/**
+ * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
+ * @chan: Freescale DMA channel
+ * @desc: descriptor to cleanup and free
+ *
+ * This function is used on a descriptor which has been executed by the DMA
+ * controller. It will run any callbacks, submit any dependencies, and then
+ * free the descriptor.
+ */
+static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
+ struct fsl_desc_sw *desc)
+{
+ struct dma_async_tx_descriptor *txd = &desc->async_tx;
+ struct device *dev = chan->common.device->dev;
+ dma_addr_t src = get_desc_src(chan, desc);
+ dma_addr_t dst = get_desc_dst(chan, desc);
+ u32 len = get_desc_cnt(chan, desc);
+
+ /* Run the link descriptor callback function */
+ if (txd->callback) {
+#ifdef FSL_DMA_LD_DEBUG
+ chan_dbg(chan, "LD %p callback\n", desc);
+#endif
+ txd->callback(txd->callback_param);
+ }
+
+ /* Run any dependencies */
+ dma_run_dependencies(txd);
+
+ /* Unmap the dst buffer, if requested */
+ if (!(txd->flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
+ if (txd->flags & DMA_COMPL_DEST_UNMAP_SINGLE)
+ dma_unmap_single(dev, dst, len, DMA_FROM_DEVICE);
+ else
+ dma_unmap_page(dev, dst, len, DMA_FROM_DEVICE);
+ }
+
+ /* Unmap the src buffer, if requested */
+ if (!(txd->flags & DMA_COMPL_SKIP_SRC_UNMAP)) {
+ if (txd->flags & DMA_COMPL_SRC_UNMAP_SINGLE)
+ dma_unmap_single(dev, src, len, DMA_TO_DEVICE);
+ else
+ dma_unmap_page(dev, src, len, DMA_TO_DEVICE);
+ }
+
+ fsl_dma_free_descriptor(chan, desc);
+}
+
static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
{
struct fsldma_chan *chan = to_fsl_chan(tx->chan);
@@ -807,54 +855,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
}
/**
- * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
- * @chan: Freescale DMA channel
- * @desc: descriptor to cleanup and free
- *
- * This function is used on a descriptor which has been executed by the DMA
- * controller. It will run any callbacks, submit any dependencies, and then
- * free the descriptor.
- */
-static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
- struct fsl_desc_sw *desc)
-{
- struct dma_async_tx_descriptor *txd = &desc->async_tx;
- struct device *dev = chan->common.device->dev;
- dma_addr_t src = get_desc_src(chan, desc);
- dma_addr_t dst = get_desc_dst(chan, desc);
- u32 len = get_desc_cnt(chan, desc);
-
- /* Run the link descriptor callback function */
- if (txd->callback) {
-#ifdef FSL_DMA_LD_DEBUG
- chan_dbg(chan, "LD %p callback\n", desc);
-#endif
- txd->callback(txd->callback_param);
- }
-
- /* Run any dependencies */
- dma_run_dependencies(txd);
-
- /* Unmap the dst buffer, if requested */
- if (!(txd->flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
- if (txd->flags & DMA_COMPL_DEST_UNMAP_SINGLE)
- dma_unmap_single(dev, dst, len, DMA_FROM_DEVICE);
- else
- dma_unmap_page(dev, dst, len, DMA_FROM_DEVICE);
- }
-
- /* Unmap the src buffer, if requested */
- if (!(txd->flags & DMA_COMPL_SKIP_SRC_UNMAP)) {
- if (txd->flags & DMA_COMPL_SRC_UNMAP_SINGLE)
- dma_unmap_single(dev, src, len, DMA_TO_DEVICE);
- else
- dma_unmap_page(dev, src, len, DMA_TO_DEVICE);
- }
-
- fsl_dma_free_descriptor(chan, desc);
-}
-
-/**
* fsl_chan_xfer_ld_queue - transfer any pending transactions
* @chan : Freescale DMA channel
*
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/7] fsl-dma: fix support for async_tx API
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
` (3 preceding siblings ...)
2012-07-31 23:45 ` [PATCH 4/7] fsl-dma: move functions to avoid forward declarations Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-08-01 6:03 ` Liu Qiang-B32616
2012-07-31 23:45 ` [PATCH 6/7] fsl-dma: fix a warning of unitialized cookie Ira W. Snyder
` (2 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto
Cc: Ira W. Snyder, Vinod Koul, B32616, Qiang Liu, Dan Williams,
linuxppc-dev
From: "Ira W. Snyder" <iws@ovro.caltech.edu>
The current fsldma driver does not support the async_tx API. This is due
to a bug: all descriptors are released when the hardware is finished
with them. The async_tx API requires that the ACK bit is set by software
before descriptors are freed.
This bug is easiest to reproduce by enabling both CONFIG_NET_DMA and
CONFIG_ASYNC_TX, and using the hardware offload support in MD RAID. The
network stack will force operations on shared DMA channels, and will
free descriptors which are being used by the MD RAID code.
The BUG_ON(async_tx_test_ack(depend_tx)) test in async_tx_submit() will
be hit, and panic the machine.
TASK = ee1a94a0[1390] 'md0_raid5' THREAD: ecf40000 CPU: 0
GPR00: 00000001 ecf41ca0 ee44/921a94a0 0000003f 00000001 c00593e4 00000000 00000001
GPR08: 00000000 a7a7a7a7 00000001 045/920000002 42028042 100a38d4 ed576d98 00000000
GPR16: ed5a11b0 00000000 2b162000 00000200 046/920000000 2d555000 ed3015e8 c15a7aa0
GPR24: 00000000 c155fc40 00000000 ecb63220 ecf41d28 e47/92f640bb0 ef640c30 ecf41ca0
NIP [c02b048c] async_tx_submit+0x6c/0x2b4
LR [c02b068c] async_tx_submit+0x26c/0x2b4
Call Trace:
[ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
[ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
[ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
[ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
[ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
[ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
[ecf41f40] [c04329b8] md_thread+0x138/0x16c
[ecf41f90] [c008277c] kthread+0x8c/0x90
[ecf41ff0] [c0011630] kernel_thread+0x4c/0x68
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Li Yang <leoli@freescale.com>
Cc: Qiang Liu <qiang.liu@freescale.com>
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
drivers/dma/fsldma.c | 156 +++++++++++++++++++++++++++++++-------------------
drivers/dma/fsldma.h | 1 +
2 files changed, 97 insertions(+), 60 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 80edc63..380c1b7 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -410,16 +410,15 @@ static void fsl_dma_free_descriptor(struct fsldma_chan *chan, struct fsl_desc_sw
}
/**
- * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
+ * fsldma_run_tx_complete_actions - run callback and unmap descriptor
* @chan: Freescale DMA channel
* @desc: descriptor to cleanup and free
*
* This function is used on a descriptor which has been executed by the DMA
- * controller. It will run any callbacks, submit any dependencies, and then
- * free the descriptor.
+ * controller. It will run the callback and unmap the descriptor, if requested.
*/
-static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
- struct fsl_desc_sw *desc)
+static void fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
+ struct fsl_desc_sw *desc)
{
struct dma_async_tx_descriptor *txd = &desc->async_tx;
struct device *dev = chan->common.device->dev;
@@ -427,6 +426,10 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
dma_addr_t dst = get_desc_dst(chan, desc);
u32 len = get_desc_cnt(chan, desc);
+ /* Cookies with value zero are already cleaned up */
+ if (txd->cookie == 0)
+ return;
+
/* Run the link descriptor callback function */
if (txd->callback) {
#ifdef FSL_DMA_LD_DEBUG
@@ -435,9 +438,6 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
txd->callback(txd->callback_param);
}
- /* Run any dependencies */
- dma_run_dependencies(txd);
-
/* Unmap the dst buffer, if requested */
if (!(txd->flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
if (txd->flags & DMA_COMPL_DEST_UNMAP_SINGLE)
@@ -454,7 +454,68 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
dma_unmap_page(dev, src, len, DMA_TO_DEVICE);
}
- fsl_dma_free_descriptor(chan, desc);
+ /*
+ * A zeroed cookie indicates that cleanup actions have been
+ * run, but the descriptor has not yet been ACKed.
+ */
+ txd->cookie = 0;
+}
+
+/**
+ * fsldma_cleanup_descriptors - cleanup and free link descriptors
+ * @chan: Freescale DMA channel
+ *
+ * This function is used to clean up all descriptors which have been executed
+ * by the DMA controller. It will run any callbacks, submit any dependencies,
+ * and free any descriptors which have been ACKed.
+ *
+ * LOCKING: must NOT hold chan->desc_lock
+ * CONTEXT: any
+ */
+static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
+{
+ struct fsl_desc_sw *desc, *_desc;
+ dma_cookie_t cookie = 0;
+ LIST_HEAD(ld_cleanup);
+ unsigned long flags;
+
+ /*
+ * Move all descriptors onto a temporary list so that hardware
+ * interrupts can be enabled during cleanup
+ */
+ spin_lock_irqsave(&chan->desc_lock, flags);
+ list_splice_tail_init(&chan->ld_completed, &ld_cleanup);
+ spin_unlock_irqrestore(&chan->desc_lock, flags);
+
+ /* Run TX completion actions on completed descriptors */
+ list_for_each_entry_safe(desc, _desc, &ld_cleanup, node) {
+ struct dma_async_tx_descriptor *txd = &desc->async_tx;
+ cookie = txd->cookie;
+
+ /* Clean up this descriptor */
+ fsldma_run_tx_complete_actions(chan, desc);
+
+ /* Run any dependencies */
+ dma_run_dependencies(txd);
+
+ /* Test for ACK and free */
+ if (async_tx_test_ack(txd))
+ fsl_dma_free_descriptor(chan, desc);
+ }
+
+ spin_lock_irqsave(&chan->desc_lock, flags);
+
+ /*
+ * Replace any non-ACKed descriptors on the front of the ld_completed
+ * list so they will be freed in the order they were submitted
+ */
+ list_splice(&ld_cleanup, &chan->ld_completed);
+
+ /* Update the cookie, if it changed */
+ if (cookie > 0)
+ chan->common.completed_cookie = cookie;
+
+ spin_unlock_irqrestore(&chan->desc_lock, flags);
}
static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
@@ -578,9 +639,11 @@ static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
struct fsldma_chan *chan = to_fsl_chan(dchan);
chan_dbg(chan, "free all channel resources\n");
+ fsldma_cleanup_descriptors(chan);
spin_lock_irq(&chan->desc_lock);
fsldma_free_desc_list(chan, &chan->ld_pending);
fsldma_free_desc_list(chan, &chan->ld_running);
+ fsldma_free_desc_list(chan, &chan->ld_completed);
spin_unlock_irq(&chan->desc_lock);
dma_pool_destroy(chan->desc_pool);
@@ -945,11 +1008,13 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
struct fsldma_chan *chan = to_fsl_chan(dchan);
enum dma_status ret;
- spin_lock_irq(&chan->desc_lock);
ret = dma_cookie_status(dchan, cookie, txstate);
- spin_unlock_irq(&chan->desc_lock);
+ if (ret == DMA_SUCCESS)
+ return ret;
- return ret;
+ tasklet_schedule(&chan->tasklet);
+
+ return dma_cookie_status(dchan, cookie, txstate);
}
/*----------------------------------------------------------------------------*/
@@ -1013,10 +1078,24 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
if (stat)
chan_err(chan, "irq: unhandled sr 0x%08x\n", stat);
+ spin_lock(&chan->desc_lock);
+ chan->idle = true;
+
+ /* Move all running descriptors onto the completed list */
+ list_splice_tail_init(&chan->ld_running, &chan->ld_completed);
+
+ /*
+ * Start any pending transactions automatically
+ *
+ * In the ideal case, we keep the DMA controller busy while we go
+ * ahead and free the descriptors in the tasklet.
+ */
+ fsl_chan_xfer_ld_queue(chan);
+ spin_unlock(&chan->desc_lock);
+
/*
- * Schedule the tasklet to handle all cleanup of the current
- * transaction. It will start a new transaction if there is
- * one pending.
+ * Schedule the tasklet to handle all cleanup of the completed
+ * descriptors.
*/
tasklet_schedule(&chan->tasklet);
chan_dbg(chan, "irq: Exit\n");
@@ -1026,53 +1105,9 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
static void dma_do_tasklet(unsigned long data)
{
struct fsldma_chan *chan = (struct fsldma_chan *)data;
- struct fsl_desc_sw *desc, *_desc;
- LIST_HEAD(ld_cleanup);
- unsigned long flags;
chan_dbg(chan, "tasklet entry\n");
-
- spin_lock_irqsave(&chan->desc_lock, flags);
-
- /* update the cookie if we have some descriptors to cleanup */
- if (!list_empty(&chan->ld_running)) {
- dma_cookie_t cookie;
-
- desc = to_fsl_desc(chan->ld_running.prev);
- cookie = desc->async_tx.cookie;
- dma_cookie_complete(&desc->async_tx);
-
- chan_dbg(chan, "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->ld_running, &ld_cleanup);
-
- /* the hardware is now idle and ready for more */
- chan->idle = true;
-
- /*
- * Start any pending transactions automatically
- *
- * In the ideal case, we keep the DMA controller busy while we go
- * ahead and free the descriptors below.
- */
- fsl_chan_xfer_ld_queue(chan);
- spin_unlock_irqrestore(&chan->desc_lock, flags);
-
- /* Run the callback for each descriptor, in order */
- list_for_each_entry_safe(desc, _desc, &ld_cleanup, node) {
-
- /* Remove from the list of transactions */
- list_del(&desc->node);
-
- /* Run all cleanup for this descriptor */
- fsldma_cleanup_descriptor(chan, desc);
- }
-
+ fsldma_cleanup_descriptors(chan);
chan_dbg(chan, "tasklet exit\n");
}
@@ -1253,6 +1288,7 @@ static int __devinit fsl_dma_chan_probe(struct fsldma_device *fdev,
spin_lock_init(&chan->desc_lock);
INIT_LIST_HEAD(&chan->ld_pending);
INIT_LIST_HEAD(&chan->ld_running);
+ INIT_LIST_HEAD(&chan->ld_completed);
chan->idle = true;
chan->common.device = &fdev->common;
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index f5c3879..7ede908 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -140,6 +140,7 @@ struct fsldma_chan {
spinlock_t desc_lock; /* Descriptor operation lock */
struct list_head ld_pending; /* Link descriptors queue */
struct list_head ld_running; /* Link descriptors queue */
+ struct list_head ld_completed; /* Link descriptors queue */
struct dma_chan common; /* DMA common channel */
struct dma_pool *desc_pool; /* Descriptors pool */
struct device *dev; /* Channel device */
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 5/7] fsl-dma: fix support for async_tx API
2012-07-31 23:45 ` [PATCH 5/7] fsl-dma: fix support for async_tx API Ira W. Snyder
@ 2012-08-01 6:03 ` Liu Qiang-B32616
0 siblings, 0 replies; 11+ messages in thread
From: Liu Qiang-B32616 @ 2012-08-01 6:03 UTC (permalink / raw)
To: Ira W. Snyder, linux-crypto@vger.kernel.org
Cc: Vinod Koul, dan.j.williams@gmail.com, Dan Williams,
linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
> -----Original Message-----
> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]
> Sent: Wednesday, August 01, 2012 7:46 AM
> To: linux-crypto@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org; Liu Qiang-B32616; Ira W. Snyder; Dan
> Williams; Vinod Koul; Li Yang-R58472; Liu Qiang-B32616
> Subject: [PATCH 5/7] fsl-dma: fix support for async_tx API
>=20
> From: "Ira W. Snyder" <iws@ovro.caltech.edu>
>=20
> The current fsldma driver does not support the async_tx API. This is due
> to a bug: all descriptors are released when the hardware is finished
> with them. The async_tx API requires that the ACK bit is set by software
> before descriptors are freed.
>=20
> This bug is easiest to reproduce by enabling both CONFIG_NET_DMA and
> CONFIG_ASYNC_TX, and using the hardware offload support in MD RAID. The
> network stack will force operations on shared DMA channels, and will
> free descriptors which are being used by the MD RAID code.
>=20
> The BUG_ON(async_tx_test_ack(depend_tx)) test in async_tx_submit() will
> be hit, and panic the machine.
>=20
> TASK =3D ee1a94a0[1390] 'md0_raid5' THREAD: ecf40000 CPU: 0
> GPR00: 00000001 ecf41ca0 ee44/921a94a0 0000003f 00000001 c00593e4
> 00000000 00000001
> GPR08: 00000000 a7a7a7a7 00000001 045/920000002 42028042 100a38d4
> ed576d98 00000000
> GPR16: ed5a11b0 00000000 2b162000 00000200 046/920000000 2d555000
> ed3015e8 c15a7aa0
> GPR24: 00000000 c155fc40 00000000 ecb63220 ecf41d28 e47/92f640bb0
> ef640c30 ecf41ca0
> NIP [c02b048c] async_tx_submit+0x6c/0x2b4
> LR [c02b068c] async_tx_submit+0x26c/0x2b4
> Call Trace:
> [ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
> [ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
> [ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
> [ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
> [ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
> [ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
> [ecf41f40] [c04329b8] md_thread+0x138/0x16c
> [ecf41f90] [c008277c] kthread+0x8c/0x90
> [ecf41ff0] [c0011630] kernel_thread+0x4c/0x68
>=20
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Li Yang <leoli@freescale.com>
> Cc: Qiang Liu <qiang.liu@freescale.com>
I have no idea where the log is from, if the log is from my patch, please n=
ote it.
> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> ---
> drivers/dma/fsldma.c | 156 +++++++++++++++++++++++++++++++-------------
> ------
> drivers/dma/fsldma.h | 1 +
> 2 files changed, 97 insertions(+), 60 deletions(-)
>=20
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 80edc63..380c1b7 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -410,16 +410,15 @@ static void fsl_dma_free_descriptor(struct
> fsldma_chan *chan, struct fsl_desc_sw
> }
>=20
> /**
> - * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
> + * fsldma_run_tx_complete_actions - run callback and unmap descriptor
> * @chan: Freescale DMA channel
> * @desc: descriptor to cleanup and free
> *
> * This function is used on a descriptor which has been executed by the
> DMA
> - * controller. It will run any callbacks, submit any dependencies, and
> then
> - * free the descriptor.
> + * controller. It will run the callback and unmap the descriptor, if
> requested.
> */
> -static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
> - struct fsl_desc_sw *desc)
> +static void fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
> + struct fsl_desc_sw *desc)
> {
Ah.. I am the original author of this design. Especially some important int=
erfaces,
at least you should add my name or note the idea before you modify it?
> struct dma_async_tx_descriptor *txd =3D &desc->async_tx;
> struct device *dev =3D chan->common.device->dev;
> @@ -427,6 +426,10 @@ static void fsldma_cleanup_descriptor(struct
> fsldma_chan *chan,
> dma_addr_t dst =3D get_desc_dst(chan, desc);
> u32 len =3D get_desc_cnt(chan, desc);
>=20
> + /* Cookies with value zero are already cleaned up */
> + if (txd->cookie =3D=3D 0)
> + return;
> +
> /* Run the link descriptor callback function */
> if (txd->callback) {
> #ifdef FSL_DMA_LD_DEBUG
> @@ -435,9 +438,6 @@ static void fsldma_cleanup_descriptor(struct
> fsldma_chan *chan,
> txd->callback(txd->callback_param);
> }
>=20
> - /* Run any dependencies */
> - dma_run_dependencies(txd);
> -
> /* Unmap the dst buffer, if requested */
> if (!(txd->flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
> if (txd->flags & DMA_COMPL_DEST_UNMAP_SINGLE)
> @@ -454,7 +454,68 @@ static void fsldma_cleanup_descriptor(struct
> fsldma_chan *chan,
> dma_unmap_page(dev, src, len, DMA_TO_DEVICE);
> }
>=20
> - fsl_dma_free_descriptor(chan, desc);
> + /*
> + * A zeroed cookie indicates that cleanup actions have been
> + * run, but the descriptor has not yet been ACKed.
> + */
> + txd->cookie =3D 0;
> +}
> +
> +/**
> + * fsldma_cleanup_descriptors - cleanup and free link descriptors
> + * @chan: Freescale DMA channel
> + *
> + * This function is used to clean up all descriptors which have been
> executed
> + * by the DMA controller. It will run any callbacks, submit any
> dependencies,
> + * and free any descriptors which have been ACKed.
> + *
> + * LOCKING: must NOT hold chan->desc_lock
> + * CONTEXT: any
> + */
> +static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
> +{
> + struct fsl_desc_sw *desc, *_desc;
> + dma_cookie_t cookie =3D 0;
> + LIST_HEAD(ld_cleanup);
> + unsigned long flags;
> +
> + /*
> + * Move all descriptors onto a temporary list so that hardware
> + * interrupts can be enabled during cleanup
> + */
> + spin_lock_irqsave(&chan->desc_lock, flags);
> + list_splice_tail_init(&chan->ld_completed, &ld_cleanup);
NACK.
Here is bug, you cannot assume all descriptors in s/w queue have been compl=
eted,
e.g. client submit some descriptors with zero length, and mixed with normal=
descriptors,
an interrupt is raised, some are finished but some are not completed, but y=
ou will free
all descriptors at all. So, I want to ask what is the standard of completio=
n of a descriptor?
Hardware is very fast is not enough:(
> + spin_unlock_irqrestore(&chan->desc_lock, flags);
> +
> + /* Run TX completion actions on completed descriptors */
> + list_for_each_entry_safe(desc, _desc, &ld_cleanup, node) {
> + struct dma_async_tx_descriptor *txd =3D &desc->async_tx;
> + cookie =3D txd->cookie;
> +
> + /* Clean up this descriptor */
> + fsldma_run_tx_complete_actions(chan, desc);
> +
> + /* Run any dependencies */
> + dma_run_dependencies(txd);
> +
> + /* Test for ACK and free */
> + if (async_tx_test_ack(txd))
> + fsl_dma_free_descriptor(chan, desc);
> + }
> +
> + spin_lock_irqsave(&chan->desc_lock, flags);
> +
> + /*
> + * Replace any non-ACKed descriptors on the front of the
> ld_completed
> + * list so they will be freed in the order they were submitted
> + */
> + list_splice(&ld_cleanup, &chan->ld_completed);
> +
> + /* Update the cookie, if it changed */
> + if (cookie > 0)
> + chan->common.completed_cookie =3D cookie;
> +
> + spin_unlock_irqrestore(&chan->desc_lock, flags);
> }
>=20
> static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx=
)
> @@ -578,9 +639,11 @@ static void fsl_dma_free_chan_resources(struct
> dma_chan *dchan)
> struct fsldma_chan *chan =3D to_fsl_chan(dchan);
>=20
> chan_dbg(chan, "free all channel resources\n");
> + fsldma_cleanup_descriptors(chan);
> spin_lock_irq(&chan->desc_lock);
> fsldma_free_desc_list(chan, &chan->ld_pending);
> fsldma_free_desc_list(chan, &chan->ld_running);
> + fsldma_free_desc_list(chan, &chan->ld_completed);
> spin_unlock_irq(&chan->desc_lock);
>=20
> dma_pool_destroy(chan->desc_pool);
> @@ -945,11 +1008,13 @@ static enum dma_status fsl_tx_status(struct
> dma_chan *dchan,
> struct fsldma_chan *chan =3D to_fsl_chan(dchan);
> enum dma_status ret;
>=20
> - spin_lock_irq(&chan->desc_lock);
> ret =3D dma_cookie_status(dchan, cookie, txstate);
> - spin_unlock_irq(&chan->desc_lock);
> + if (ret =3D=3D DMA_SUCCESS)
> + return ret;
>=20
> - return ret;
> + tasklet_schedule(&chan->tasklet);
> +
> + return dma_cookie_status(dchan, cookie, txstate);
> }
>=20
> /*----------------------------------------------------------------------
> ------*/
> @@ -1013,10 +1078,24 @@ static irqreturn_t fsldma_chan_irq(int irq, void
> *data)
> if (stat)
> chan_err(chan, "irq: unhandled sr 0x%08x\n", stat);
>=20
> + spin_lock(&chan->desc_lock);
> + chan->idle =3D true;
> +
> + /* Move all running descriptors onto the completed list */
> + list_splice_tail_init(&chan->ld_running, &chan->ld_completed);
> +
> + /*
> + * Start any pending transactions automatically
> + *
> + * In the ideal case, we keep the DMA controller busy while we go
> + * ahead and free the descriptors in the tasklet.
> + */
> + fsl_chan_xfer_ld_queue(chan);
> + spin_unlock(&chan->desc_lock);
> +
> /*
> - * Schedule the tasklet to handle all cleanup of the current
> - * transaction. It will start a new transaction if there is
> - * one pending.
> + * Schedule the tasklet to handle all cleanup of the completed
> + * descriptors.
> */
> tasklet_schedule(&chan->tasklet);
> chan_dbg(chan, "irq: Exit\n");
> @@ -1026,53 +1105,9 @@ static irqreturn_t fsldma_chan_irq(int irq, void
> *data)
> static void dma_do_tasklet(unsigned long data)
> {
> struct fsldma_chan *chan =3D (struct fsldma_chan *)data;
> - struct fsl_desc_sw *desc, *_desc;
> - LIST_HEAD(ld_cleanup);
> - unsigned long flags;
>=20
> chan_dbg(chan, "tasklet entry\n");
> -
> - spin_lock_irqsave(&chan->desc_lock, flags);
> -
> - /* update the cookie if we have some descriptors to cleanup */
> - if (!list_empty(&chan->ld_running)) {
> - dma_cookie_t cookie;
> -
> - desc =3D to_fsl_desc(chan->ld_running.prev);
> - cookie =3D desc->async_tx.cookie;
> - dma_cookie_complete(&desc->async_tx);
> -
> - chan_dbg(chan, "completed_cookie=3D%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->ld_running, &ld_cleanup);
> -
> - /* the hardware is now idle and ready for more */
> - chan->idle =3D true;
> -
> - /*
> - * Start any pending transactions automatically
> - *
> - * In the ideal case, we keep the DMA controller busy while we go
> - * ahead and free the descriptors below.
> - */
> - fsl_chan_xfer_ld_queue(chan);
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> -
> - /* Run the callback for each descriptor, in order */
> - list_for_each_entry_safe(desc, _desc, &ld_cleanup, node) {
> -
> - /* Remove from the list of transactions */
> - list_del(&desc->node);
> -
> - /* Run all cleanup for this descriptor */
> - fsldma_cleanup_descriptor(chan, desc);
> - }
> -
> + fsldma_cleanup_descriptors(chan);
> chan_dbg(chan, "tasklet exit\n");
> }
>=20
> @@ -1253,6 +1288,7 @@ static int __devinit fsl_dma_chan_probe(struct
> fsldma_device *fdev,
> spin_lock_init(&chan->desc_lock);
> INIT_LIST_HEAD(&chan->ld_pending);
> INIT_LIST_HEAD(&chan->ld_running);
> + INIT_LIST_HEAD(&chan->ld_completed);
> chan->idle =3D true;
>=20
> chan->common.device =3D &fdev->common;
> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
> index f5c3879..7ede908 100644
> --- a/drivers/dma/fsldma.h
> +++ b/drivers/dma/fsldma.h
> @@ -140,6 +140,7 @@ struct fsldma_chan {
> spinlock_t desc_lock; /* Descriptor operation lock */
> struct list_head ld_pending; /* Link descriptors queue */
> struct list_head ld_running; /* Link descriptors queue */
> + struct list_head ld_completed; /* Link descriptors queue */
> struct dma_chan common; /* DMA common channel */
> struct dma_pool *desc_pool; /* Descriptors pool */
> struct device *dev; /* Channel device */
> --
> 1.7.8.6
>=20
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 6/7] fsl-dma: fix a warning of unitialized cookie
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
` (4 preceding siblings ...)
2012-07-31 23:45 ` [PATCH 5/7] fsl-dma: fix support for async_tx API Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-07-31 23:45 ` [PATCH 7/7] carma: remove unnecessary DMA_INTERRUPT capability Ira W. Snyder
2012-08-01 3:47 ` [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Liu Qiang-B32616
7 siblings, 0 replies; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto; +Cc: Vinod Koul, B32616, Qiang Liu, Dan Williams, linuxppc-dev
From: Qiang Liu <qiang.liu@freescale.com>
Fix a warning of unitialized value when compile with -Wuninitialized.
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Li Yang <leoli@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
Cc: Kim Phillips <kim.phillips@freescale.com>
---
drivers/dma/fsldma.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 380c1b7..8588cf7 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -523,7 +523,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
struct fsldma_chan *chan = to_fsl_chan(tx->chan);
struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
struct fsl_desc_sw *child;
- dma_cookie_t cookie;
+ dma_cookie_t cookie = 0;
spin_lock_irq(&chan->desc_lock);
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 7/7] carma: remove unnecessary DMA_INTERRUPT capability
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
` (5 preceding siblings ...)
2012-07-31 23:45 ` [PATCH 6/7] fsl-dma: fix a warning of unitialized cookie Ira W. Snyder
@ 2012-07-31 23:45 ` Ira W. Snyder
2012-08-01 3:56 ` Liu Qiang-B32616
2012-08-01 3:47 ` [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Liu Qiang-B32616
7 siblings, 1 reply; 11+ messages in thread
From: Ira W. Snyder @ 2012-07-31 23:45 UTC (permalink / raw)
To: linux-crypto; +Cc: B32616, linuxppc-dev, Ira W. Snyder
From: "Ira W. Snyder" <iws@ovro.caltech.edu>
These drivers set the DMA_INTERRUPT capability bit when requesting a DMA
controller channel. This was historical, and is no longer needed.
Recent changes to the drivers/dma/fsldma.c driver have removed support
for this flag. This makes the carma drivers unable to find a DMA channel
with the required capabilities.
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
drivers/misc/carma/carma-fpga-program.c | 1 -
drivers/misc/carma/carma-fpga.c | 3 +--
2 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/carma/carma-fpga-program.c b/drivers/misc/carma/carma-fpga-program.c
index a2d25e4..eaddfe9 100644
--- a/drivers/misc/carma/carma-fpga-program.c
+++ b/drivers/misc/carma/carma-fpga-program.c
@@ -978,7 +978,6 @@ static int fpga_of_probe(struct platform_device *op)
dev_set_drvdata(priv->dev, priv);
dma_cap_zero(mask);
dma_cap_set(DMA_MEMCPY, mask);
- dma_cap_set(DMA_INTERRUPT, mask);
dma_cap_set(DMA_SLAVE, mask);
dma_cap_set(DMA_SG, mask);
diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-fpga.c
index 8c279da..861b298 100644
--- a/drivers/misc/carma/carma-fpga.c
+++ b/drivers/misc/carma/carma-fpga.c
@@ -666,7 +666,7 @@ static int data_submit_dma(struct fpga_device *priv, struct data_buf *buf)
src = SYS_FPGA_BLOCK;
tx = chan->device->device_prep_dma_memcpy(chan, dst, src,
REG_BLOCK_SIZE,
- DMA_PREP_INTERRUPT);
+ 0);
if (!tx) {
dev_err(priv->dev, "unable to prep SYS-FPGA DMA\n");
return -ENOMEM;
@@ -1333,7 +1333,6 @@ static int data_of_probe(struct platform_device *op)
dma_cap_zero(mask);
dma_cap_set(DMA_MEMCPY, mask);
- dma_cap_set(DMA_INTERRUPT, mask);
dma_cap_set(DMA_SLAVE, mask);
dma_cap_set(DMA_SG, mask);
--
1.7.8.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 7/7] carma: remove unnecessary DMA_INTERRUPT capability
2012-07-31 23:45 ` [PATCH 7/7] carma: remove unnecessary DMA_INTERRUPT capability Ira W. Snyder
@ 2012-08-01 3:56 ` Liu Qiang-B32616
0 siblings, 0 replies; 11+ messages in thread
From: Liu Qiang-B32616 @ 2012-08-01 3:56 UTC (permalink / raw)
To: Ira W. Snyder, linux-crypto@vger.kernel.org; +Cc: linuxppc-dev@lists.ozlabs.org
> -----Original Message-----
> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]
> Sent: Wednesday, August 01, 2012 7:46 AM
> To: linux-crypto@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org; Liu Qiang-B32616; Ira W. Snyder
> Subject: [PATCH 7/7] carma: remove unnecessary DMA_INTERRUPT capability
>=20
> From: "Ira W. Snyder" <iws@ovro.caltech.edu>
>=20
> These drivers set the DMA_INTERRUPT capability bit when requesting a DMA
> controller channel. This was historical, and is no longer needed.
>=20
> Recent changes to the drivers/dma/fsldma.c driver have removed support
> for this flag. This makes the carma drivers unable to find a DMA channel
> with the required capabilities.
>=20
> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> ---
> drivers/misc/carma/carma-fpga-program.c | 1 -
> drivers/misc/carma/carma-fpga.c | 3 +--
> 2 files changed, 1 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/misc/carma/carma-fpga-program.c
> b/drivers/misc/carma/carma-fpga-program.c
> index a2d25e4..eaddfe9 100644
> --- a/drivers/misc/carma/carma-fpga-program.c
> +++ b/drivers/misc/carma/carma-fpga-program.c
> @@ -978,7 +978,6 @@ static int fpga_of_probe(struct platform_device *op)
> dev_set_drvdata(priv->dev, priv);
> dma_cap_zero(mask);
> dma_cap_set(DMA_MEMCPY, mask);
> - dma_cap_set(DMA_INTERRUPT, mask);
> dma_cap_set(DMA_SLAVE, mask);
> dma_cap_set(DMA_SG, mask);
>=20
> diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-
> fpga.c index 8c279da..861b298 100644
> --- a/drivers/misc/carma/carma-fpga.c
> +++ b/drivers/misc/carma/carma-fpga.c
> @@ -666,7 +666,7 @@ static int data_submit_dma(struct fpga_device *priv,
> struct data_buf *buf)
> src =3D SYS_FPGA_BLOCK;
> tx =3D chan->device->device_prep_dma_memcpy(chan, dst, src,
> REG_BLOCK_SIZE,
> - DMA_PREP_INTERRUPT);
> + 0);
> if (!tx) {
> dev_err(priv->dev, "unable to prep SYS-FPGA DMA\n");
> return -ENOMEM;
> @@ -1333,7 +1333,6 @@ static int data_of_probe(struct platform_device *op=
)
>=20
> dma_cap_zero(mask);
> dma_cap_set(DMA_MEMCPY, mask);
> - dma_cap_set(DMA_INTERRUPT, mask);
> dma_cap_set(DMA_SLAVE, mask);
> dma_cap_set(DMA_SG, mask);
Hi Ira, attached link is the comments of Dan Williams to the original patch=
of fsl-dma.
http://lkml.indiana.edu/hypermail/linux/kernel/0910.1/03836.html
> --
> 1.7.8.6
>=20
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver
2012-07-31 23:45 [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver Ira W. Snyder
` (6 preceding siblings ...)
2012-07-31 23:45 ` [PATCH 7/7] carma: remove unnecessary DMA_INTERRUPT capability Ira W. Snyder
@ 2012-08-01 3:47 ` Liu Qiang-B32616
7 siblings, 0 replies; 11+ messages in thread
From: Liu Qiang-B32616 @ 2012-08-01 3:47 UTC (permalink / raw)
To: Ira W. Snyder, linux-crypto@vger.kernel.org; +Cc: linuxppc-dev@lists.ozlabs.org
Hi Ira,
My comments inline.
> -----Original Message-----
> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]
> Sent: Wednesday, August 01, 2012 7:46 AM
> To: linux-crypto@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org; Liu Qiang-B32616; Ira W. Snyder
> Subject: [PATCH 0/7] fsl-dma: fixes for Freescale DMA driver
>=20
> From: "Ira W. Snyder" <iws@ovro.caltech.edu>
>=20
> Hello everyone,
>=20
> This is my alternative (simpler) attempt at solving the problems reported
> by Qiang Liu with the async_tx API and MD RAID hardware offload support
> when using the Freescale DMA driver.
>=20
> The bug is caused by this driver freeing descriptors before they have
> been ACKed by software using the async_tx API.
>=20
> I don't like Qiang Liu's code to check where the hardware is in the
> processing of the descriptor chain, and try to free a partial list of
> descriptors. This was a source of bugs in this driver before I fixed them
> several years ago.
It's a bug which you think the whole list is completed when an interrupt is=
raised, there is a potential risk when an interrupt is raised by "Programm=
ed Error". The "ld_running" is a s/w concept, we should not depend on it to=
judge the status of descriptors list.
I know you don't like this process, but it's a safe and common process. You=
can refer to other dma drivers, like ioap-adma, mv-xor and ibm-ppc440x-adm=
a.
Said far point, usb also take this method to judge which descriptor is comp=
leted, I don't know which device can use a s/w list to free all descriptors=
, you can refer to the implement of dl_reverse_done_list().
If you find any problem in my patch, please point out, or you can give a li=
nk about the bug you mentioned many years ago.
Thanks.
>=20
> Instead, the DMA controller raises an interrupt every time it has
> completed a descriptor chain. This means it is ready for new descriptors:
> no need to try and figure out where it is in the middle of a descriptor
> chain.
Attached again,
The interrupt is only report the state of hardware, we cannot assume all de=
scriptors are finished when an interrupt is raised.
>=20
> Qiang Liu: I do not have a hardware setup capable of using MD RAID.
> Please test these patches to see if they fix the bug you reported. You
> may use these patches as-is, or build upon them.
I hope we can discuss it based on my patch. If you think my patch will invo=
lve some issue, please point out. I'm willing to fix it.
Thanks.
>=20
> I have tested this using the drivers/dma/dmatest.c driver, as well as the
> CARMA drivers. There are no regressions that I can find.
>=20
> [ 355.069679] dma0chan3-copy0: terminating after 100000 tests, 0
> failures (status 0) [ 355.192278] dma0chan2-copy0: terminating after
> 100000 tests, 0 failures (status 0)
>=20
> Ira W. Snyder (5):
> fsl-dma: minimize locking overhead
> fsl-dma: add fsl_dma_free_descriptor() to reduce code duplication
> fsl-dma: move functions to avoid forward declarations
> fsl-dma: fix support for async_tx API
> carma: remove unnecessary DMA_INTERRUPT capability
>=20
> Qiang Liu (2):
> fsl-dma: remove attribute DMA_INTERRUPT of dmaengine
> fsl-dma: fix a warning of unitialized cookie
>=20
> drivers/dma/fsldma.c | 318 +++++++++++++++----------
> ------
> drivers/dma/fsldma.h | 1 +
> drivers/misc/carma/carma-fpga-program.c | 1 -
> drivers/misc/carma/carma-fpga.c | 3 +-
> 4 files changed, 159 insertions(+), 164 deletions(-)
>=20
> --
> 1.7.8.6
>=20
^ permalink raw reply [flat|nested] 11+ messages in thread