This patchset fixes few issues and extends its support.
Changes in V3:
- lflags is removed from dw_dma_chan and local flag variables are created.
- An extra argument is added to routines calling dwc_descriptor_complete()
directly or indirectly
- spin_lock() in tasklet is also changed to irqsave variants.
Changes in V2:
- lflags added in dw_dma_chan instead of dw_dma
- Patch from Linus Walleij added for pause and resume functionality.
Linus Walleij (1):
dmaengine/dw_dmac: implement pause and resume in dwc_control
Viresh Kumar (6):
dmaengine/dw_dmac: call dwc_descriptor_complete from dwc_control with
lock held
dmaengine/dw_dmac: Replace spin_lock* with irqsave variants
dmaengine/dw_dmac: don't call callback routine in case
dmaengine_terminate_all() is called
dmaengine/dw_dmac: Enable resubmission from callback routine.
dmaengine/dw_dmac: set residue as total len in dwc_tx_status if
status is !DMA_SUCCESS
dmaengine/dw_dmac: Divide one sg to many desc, if sg len is greater
than DWC_MAX_COUNT
drivers/dma/dw_dmac.c | 260 ++++++++++++++++++++++++++++----------------
drivers/dma/dw_dmac_regs.h | 1 +
2 files changed, 165 insertions(+), 96 deletions(-)
--
1.7.3.4
dmaengine routines can be called from interrupt context and with interrupts
disabled. Whereas spin_unlock_bh can't be called from such contexts. So this
patch converts all spin_*_bh routines to irqsave variants.
Also, spin_lock() used in tasklet is converted to irqsave variants, as tasklet
can be interrupted, and dma requests from such interruptions may also call
spin_lock.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 82 +++++++++++++++++++++++++++++-------------------
1 files changed, 49 insertions(+), 33 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index b8985af..b6dfc39 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -93,8 +93,9 @@ static struct dw_desc *dwc_desc_get(struct dw_dma_chan *dwc)
struct dw_desc *desc, *_desc;
struct dw_desc *ret = NULL;
unsigned int i = 0;
+ unsigned long flags;
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
list_for_each_entry_safe(desc, _desc, &dwc->free_list, desc_node) {
if (async_tx_test_ack(&desc->txd)) {
list_del(&desc->desc_node);
@@ -104,7 +105,7 @@ static struct dw_desc *dwc_desc_get(struct dw_dma_chan *dwc)
dev_dbg(chan2dev(&dwc->chan), "desc %p not ACKed\n", desc);
i++;
}
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
dev_vdbg(chan2dev(&dwc->chan), "scanned %u descriptors on freelist\n", i);
@@ -130,12 +131,14 @@ static void dwc_sync_desc_for_cpu(struct dw_dma_chan *dwc, struct dw_desc *desc)
*/
static void dwc_desc_put(struct dw_dma_chan *dwc, struct dw_desc *desc)
{
+ unsigned long flags;
+
if (desc) {
struct dw_desc *child;
dwc_sync_desc_for_cpu(dwc, desc);
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
list_for_each_entry(child, &desc->tx_list, desc_node)
dev_vdbg(chan2dev(&dwc->chan),
"moving child desc %p to freelist\n",
@@ -143,7 +146,7 @@ static void dwc_desc_put(struct dw_dma_chan *dwc, struct dw_desc *desc)
list_splice_init(&desc->tx_list, &dwc->free_list);
dev_vdbg(chan2dev(&dwc->chan), "moving desc %p to freelist\n", desc);
list_add(&desc->desc_node, &dwc->free_list);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
}
}
@@ -407,6 +410,8 @@ EXPORT_SYMBOL(dw_dma_get_dst_addr);
static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
u32 status_block, u32 status_err, u32 status_xfer)
{
+ unsigned long flags;
+
if (status_block & dwc->mask) {
void (*callback)(void *param);
void *callback_param;
@@ -418,9 +423,9 @@ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
callback = dwc->cdesc->period_callback;
callback_param = dwc->cdesc->period_callback_param;
if (callback) {
- spin_unlock(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
callback(callback_param);
- spin_lock(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
}
}
@@ -470,6 +475,7 @@ static void dw_dma_tasklet(unsigned long data)
u32 status_block;
u32 status_xfer;
u32 status_err;
+ unsigned long flags;
int i;
status_block = dma_readl(dw, RAW.BLOCK);
@@ -481,7 +487,7 @@ static void dw_dma_tasklet(unsigned long data)
for (i = 0; i < dw->dma.chancnt; i++) {
dwc = &dw->chan[i];
- spin_lock(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
if (test_bit(DW_DMA_IS_CYCLIC, &dwc->flags))
dwc_handle_cyclic(dw, dwc, status_block, status_err,
status_xfer);
@@ -489,7 +495,7 @@ static void dw_dma_tasklet(unsigned long data)
dwc_handle_error(dw, dwc);
else if ((status_block | status_xfer) & (1 << i))
dwc_scan_descriptors(dw, dwc);
- spin_unlock(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
}
/*
@@ -544,8 +550,9 @@ static dma_cookie_t dwc_tx_submit(struct dma_async_tx_descriptor *tx)
struct dw_desc *desc = txd_to_dw_desc(tx);
struct dw_dma_chan *dwc = to_dw_dma_chan(tx->chan);
dma_cookie_t cookie;
+ unsigned long flags;
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
cookie = dwc_assign_cookie(dwc, desc);
/*
@@ -565,7 +572,7 @@ static dma_cookie_t dwc_tx_submit(struct dma_async_tx_descriptor *tx)
list_add_tail(&desc->desc_node, &dwc->queue);
}
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
return cookie;
}
@@ -804,6 +811,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
struct dw_dma *dw = to_dw_dma(chan->device);
struct dw_desc *desc, *_desc;
+ unsigned long flags;
LIST_HEAD(list);
/* Only supports DMA_TERMINATE_ALL */
@@ -816,7 +824,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
* channel. We still have to poll the channel enable bit due
* to AHB/HSB limitations.
*/
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
channel_clear_bit(dw, CH_EN, dwc->mask);
@@ -831,7 +839,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
list_for_each_entry_safe(desc, _desc, &list, desc_node)
dwc_descriptor_complete(dwc, desc);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
return 0;
}
@@ -845,15 +853,16 @@ dwc_tx_status(struct dma_chan *chan,
dma_cookie_t last_used;
dma_cookie_t last_complete;
int ret;
+ unsigned long flags;
last_complete = dwc->completed;
last_used = chan->cookie;
ret = dma_async_is_complete(cookie, last_complete, last_used);
if (ret != DMA_SUCCESS) {
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
last_complete = dwc->completed;
last_used = chan->cookie;
@@ -869,11 +878,12 @@ dwc_tx_status(struct dma_chan *chan,
static void dwc_issue_pending(struct dma_chan *chan)
{
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
+ unsigned long flags;
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
if (!list_empty(&dwc->queue))
dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
}
static int dwc_alloc_chan_resources(struct dma_chan *chan)
@@ -885,6 +895,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
int i;
u32 cfghi;
u32 cfglo;
+ unsigned long flags;
dev_vdbg(chan2dev(chan), "alloc_chan_resources\n");
@@ -922,16 +933,16 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
* doesn't mean what you think it means), and status writeback.
*/
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
i = dwc->descs_allocated;
while (dwc->descs_allocated < NR_DESCS_PER_CHANNEL) {
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
desc = kzalloc(sizeof(struct dw_desc), GFP_KERNEL);
if (!desc) {
dev_info(chan2dev(chan),
"only allocated %d descriptors\n", i);
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
break;
}
@@ -943,7 +954,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
sizeof(desc->lli), DMA_TO_DEVICE);
dwc_desc_put(dwc, desc);
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
i = ++dwc->descs_allocated;
}
@@ -952,7 +963,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
channel_set_bit(dw, MASK.BLOCK, dwc->mask);
channel_set_bit(dw, MASK.ERROR, dwc->mask);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
dev_dbg(chan2dev(chan),
"alloc_chan_resources allocated %d descriptors\n", i);
@@ -965,6 +976,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
struct dw_dma *dw = to_dw_dma(chan->device);
struct dw_desc *desc, *_desc;
+ unsigned long flags;
LIST_HEAD(list);
dev_dbg(chan2dev(chan), "free_chan_resources (descs allocated=%u)\n",
@@ -975,7 +987,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
BUG_ON(!list_empty(&dwc->queue));
BUG_ON(dma_readl(to_dw_dma(chan->device), CH_EN) & dwc->mask);
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
list_splice_init(&dwc->free_list, &list);
dwc->descs_allocated = 0;
@@ -984,7 +996,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
channel_clear_bit(dw, MASK.BLOCK, dwc->mask);
channel_clear_bit(dw, MASK.ERROR, dwc->mask);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
list_for_each_entry_safe(desc, _desc, &list, desc_node) {
dev_vdbg(chan2dev(chan), " freeing descriptor %p\n", desc);
@@ -1009,13 +1021,14 @@ int dw_dma_cyclic_start(struct dma_chan *chan)
{
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
struct dw_dma *dw = to_dw_dma(dwc->chan.device);
+ unsigned long flags;
if (!test_bit(DW_DMA_IS_CYCLIC, &dwc->flags)) {
dev_err(chan2dev(&dwc->chan), "missing prep for cyclic DMA\n");
return -ENODEV;
}
- spin_lock(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
/* assert channel is idle */
if (dma_readl(dw, CH_EN) & dwc->mask) {
@@ -1028,7 +1041,7 @@ int dw_dma_cyclic_start(struct dma_chan *chan)
channel_readl(dwc, LLP),
channel_readl(dwc, CTL_HI),
channel_readl(dwc, CTL_LO));
- spin_unlock(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
return -EBUSY;
}
@@ -1043,7 +1056,7 @@ int dw_dma_cyclic_start(struct dma_chan *chan)
channel_set_bit(dw, CH_EN, dwc->mask);
- spin_unlock(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
return 0;
}
@@ -1059,14 +1072,15 @@ void dw_dma_cyclic_stop(struct dma_chan *chan)
{
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
struct dw_dma *dw = to_dw_dma(dwc->chan.device);
+ unsigned long flags;
- spin_lock(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
channel_clear_bit(dw, CH_EN, dwc->mask);
while (dma_readl(dw, CH_EN) & dwc->mask)
cpu_relax();
- spin_unlock(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
}
EXPORT_SYMBOL(dw_dma_cyclic_stop);
@@ -1095,17 +1109,18 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
unsigned int reg_width;
unsigned int periods;
unsigned int i;
+ unsigned long flags;
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
if (!list_empty(&dwc->queue) || !list_empty(&dwc->active_list)) {
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
dev_dbg(chan2dev(&dwc->chan),
"queue and/or active list are not empty\n");
return ERR_PTR(-EBUSY);
}
was_cyclic = test_and_set_bit(DW_DMA_IS_CYCLIC, &dwc->flags);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
if (was_cyclic) {
dev_dbg(chan2dev(&dwc->chan),
"channel already prepared for cyclic DMA\n");
@@ -1219,13 +1234,14 @@ void dw_dma_cyclic_free(struct dma_chan *chan)
struct dw_dma *dw = to_dw_dma(dwc->chan.device);
struct dw_cyclic_desc *cdesc = dwc->cdesc;
int i;
+ unsigned long flags;
dev_dbg(chan2dev(&dwc->chan), "cyclic free\n");
if (!cdesc)
return;
- spin_lock_bh(&dwc->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
channel_clear_bit(dw, CH_EN, dwc->mask);
while (dma_readl(dw, CH_EN) & dwc->mask)
@@ -1235,7 +1251,7 @@ void dw_dma_cyclic_free(struct dma_chan *chan)
dma_writel(dw, CLEAR.ERROR, dwc->mask);
dma_writel(dw, CLEAR.XFER, dwc->mask);
- spin_unlock_bh(&dwc->lock);
+ spin_unlock_irqrestore(&dwc->lock, flags);
for (i = 0; i < cdesc->periods; i++)
dwc_desc_put(dwc, cdesc->desc[i]);
--
1.7.3.4
If dmaengine_terminate_all() is called for dma channel, then it doesn't make
much sense to call registered callback routine. While in case of success or
failure it must be called.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 27 ++++++++++++++-------------
1 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index b6dfc39..f590109 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -198,7 +198,8 @@ static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
/*----------------------------------------------------------------------*/
static void
-dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
+dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
+ bool callback_required)
{
dma_async_tx_callback callback;
void *param;
@@ -208,8 +209,10 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
dev_vdbg(chan2dev(&dwc->chan), "descriptor %u complete\n", txd->cookie);
dwc->completed = txd->cookie;
- callback = txd->callback;
- param = txd->callback_param;
+ if (callback_required) {
+ callback = txd->callback;
+ param = txd->callback_param;
+ }
dwc_sync_desc_for_cpu(dwc, desc);
@@ -241,12 +244,10 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
}
}
- /*
- * The API requires that no submissions are done from a
- * callback, so we don't need to drop the lock here
- */
- if (callback)
- callback(param);
+ if (callback_required) {
+ if (callback)
+ callback(param);
+ }
}
static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
@@ -275,7 +276,7 @@ static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
}
list_for_each_entry_safe(desc, _desc, &list, desc_node)
- dwc_descriptor_complete(dwc, desc);
+ dwc_descriptor_complete(dwc, desc, 1);
}
static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
@@ -325,7 +326,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
* No descriptors so far seem to be in progress, i.e.
* this one must be done.
*/
- dwc_descriptor_complete(dwc, desc);
+ dwc_descriptor_complete(dwc, desc, 1);
}
dev_err(chan2dev(&dwc->chan),
@@ -387,7 +388,7 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
dwc_dump_lli(dwc, &child->lli);
/* Pretend the descriptor completed successfully */
- dwc_descriptor_complete(dwc, bad_desc);
+ dwc_descriptor_complete(dwc, bad_desc, 1);
}
/* --------------------- Cyclic DMA API extensions -------------------- */
@@ -837,7 +838,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
/* Flush all pending and queued descriptors */
list_for_each_entry_safe(desc, _desc, &list, desc_node)
- dwc_descriptor_complete(dwc, desc);
+ dwc_descriptor_complete(dwc, desc, 0);
spin_unlock_irqrestore(&dwc->lock, flags);
--
1.7.3.4
Resubmission of new transfer must be allowed from callbacks. For this release
lock before calling callback routine and enable them again.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 37 +++++++++++++++++++++----------------
1 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index f590109..3285ae9 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -199,10 +199,10 @@ static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
static void
dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
- bool callback_required)
+ bool callback_required, unsigned long flags)
{
- dma_async_tx_callback callback;
- void *param;
+ dma_async_tx_callback callback = NULL;
+ void *param = NULL;
struct dma_async_tx_descriptor *txd = &desc->txd;
struct dw_desc *child;
@@ -245,12 +245,15 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
}
if (callback_required) {
+ spin_unlock_irqrestore(&dwc->lock, flags);
if (callback)
callback(param);
+ spin_lock_irqsave(&dwc->lock, flags);
}
}
-static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
+static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc,
+ unsigned long flags)
{
struct dw_desc *desc, *_desc;
LIST_HEAD(list);
@@ -276,10 +279,11 @@ static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
}
list_for_each_entry_safe(desc, _desc, &list, desc_node)
- dwc_descriptor_complete(dwc, desc, 1);
+ dwc_descriptor_complete(dwc, desc, 1, flags);
}
-static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
+static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc,
+ unsigned long flags)
{
dma_addr_t llp;
struct dw_desc *desc, *_desc;
@@ -298,7 +302,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
if (status_xfer & dwc->mask) {
/* Everything we've submitted is done */
dma_writel(dw, CLEAR.XFER, dwc->mask);
- dwc_complete_all(dw, dwc);
+ dwc_complete_all(dw, dwc, flags);
return;
}
@@ -326,7 +330,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
* No descriptors so far seem to be in progress, i.e.
* this one must be done.
*/
- dwc_descriptor_complete(dwc, desc, 1);
+ dwc_descriptor_complete(dwc, desc, 1, flags);
}
dev_err(chan2dev(&dwc->chan),
@@ -351,12 +355,13 @@ static void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_lli *lli)
lli->ctlhi, lli->ctllo);
}
-static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
+static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc,
+ unsigned long flags)
{
struct dw_desc *bad_desc;
struct dw_desc *child;
- dwc_scan_descriptors(dw, dwc);
+ dwc_scan_descriptors(dw, dwc, flags);
/*
* The descriptor currently at the head of the active list is
@@ -388,7 +393,7 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
dwc_dump_lli(dwc, &child->lli);
/* Pretend the descriptor completed successfully */
- dwc_descriptor_complete(dwc, bad_desc, 1);
+ dwc_descriptor_complete(dwc, bad_desc, 1, flags);
}
/* --------------------- Cyclic DMA API extensions -------------------- */
@@ -493,9 +498,9 @@ static void dw_dma_tasklet(unsigned long data)
dwc_handle_cyclic(dw, dwc, status_block, status_err,
status_xfer);
else if (status_err & (1 << i))
- dwc_handle_error(dw, dwc);
+ dwc_handle_error(dw, dwc, flags);
else if ((status_block | status_xfer) & (1 << i))
- dwc_scan_descriptors(dw, dwc);
+ dwc_scan_descriptors(dw, dwc, flags);
spin_unlock_irqrestore(&dwc->lock, flags);
}
@@ -838,7 +843,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
/* Flush all pending and queued descriptors */
list_for_each_entry_safe(desc, _desc, &list, desc_node)
- dwc_descriptor_complete(dwc, desc, 0);
+ dwc_descriptor_complete(dwc, desc, 0, flags);
spin_unlock_irqrestore(&dwc->lock, flags);
@@ -862,7 +867,7 @@ dwc_tx_status(struct dma_chan *chan,
ret = dma_async_is_complete(cookie, last_complete, last_used);
if (ret != DMA_SUCCESS) {
spin_lock_irqsave(&dwc->lock, flags);
- dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
+ dwc_scan_descriptors(to_dw_dma(chan->device), dwc, flags);
spin_unlock_irqrestore(&dwc->lock, flags);
last_complete = dwc->completed;
@@ -883,7 +888,7 @@ static void dwc_issue_pending(struct dma_chan *chan)
spin_lock_irqsave(&dwc->lock, flags);
if (!list_empty(&dwc->queue))
- dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
+ dwc_scan_descriptors(to_dw_dma(chan->device), dwc, flags);
spin_unlock_irqrestore(&dwc->lock, flags);
}
--
1.7.3.4
If len passed in sg for slave_sg transfers is greater than DWC_MAX_COUNT, then
driver programmes controller incorrectly. This patch adds code to handle this
situation by allocation more than one desc for same sg.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 65 +++++++++++++++++++++++++++++++++----------------
1 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 964d2d6..014654d 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -707,9 +707,15 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
reg = dws->tx_reg;
for_each_sg(sgl, sg, sg_len, i) {
struct dw_desc *desc;
- u32 len;
- u32 mem;
+ u32 len, dlen, mem;
+ mem = sg_phys(sg);
+ len = sg_dma_len(sg);
+ mem_width = 2;
+ if (unlikely(mem & 3 || len & 3))
+ mem_width = 0;
+
+slave_sg_todev_fill_desc:
desc = dwc_desc_get(dwc);
if (!desc) {
dev_err(chan2dev(chan),
@@ -717,16 +723,19 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
goto err_desc_get;
}
- mem = sg_phys(sg);
- len = sg_dma_len(sg);
- mem_width = 2;
- if (unlikely(mem & 3 || len & 3))
- mem_width = 0;
-
desc->lli.sar = mem;
desc->lli.dar = reg;
desc->lli.ctllo = ctllo | DWC_CTLL_SRC_WIDTH(mem_width);
- desc->lli.ctlhi = len >> mem_width;
+ if ((len >> mem_width) > DWC_MAX_COUNT) {
+ dlen = DWC_MAX_COUNT << mem_width;
+ mem += dlen;
+ len -= dlen;
+ } else {
+ dlen = len;
+ len = 0;
+ }
+
+ desc->lli.ctlhi = dlen >> mem_width;
if (!first) {
first = desc;
@@ -740,7 +749,10 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
&first->tx_list);
}
prev = desc;
- total_len += len;
+ total_len += dlen;
+
+ if (len)
+ goto slave_sg_todev_fill_desc;
}
break;
case DMA_FROM_DEVICE:
@@ -753,15 +765,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
reg = dws->rx_reg;
for_each_sg(sgl, sg, sg_len, i) {
struct dw_desc *desc;
- u32 len;
- u32 mem;
-
- desc = dwc_desc_get(dwc);
- if (!desc) {
- dev_err(chan2dev(chan),
- "not enough descriptors available\n");
- goto err_desc_get;
- }
+ u32 len, dlen, mem;
mem = sg_phys(sg);
len = sg_dma_len(sg);
@@ -769,10 +773,26 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
if (unlikely(mem & 3 || len & 3))
mem_width = 0;
+slave_sg_fromdev_fill_desc:
+ desc = dwc_desc_get(dwc);
+ if (!desc) {
+ dev_err(chan2dev(chan),
+ "not enough descriptors available\n");
+ goto err_desc_get;
+ }
+
desc->lli.sar = reg;
desc->lli.dar = mem;
desc->lli.ctllo = ctllo | DWC_CTLL_DST_WIDTH(mem_width);
- desc->lli.ctlhi = len >> reg_width;
+ if ((len >> reg_width) > DWC_MAX_COUNT) {
+ dlen = DWC_MAX_COUNT << reg_width;
+ mem += dlen;
+ len -= dlen;
+ } else {
+ dlen = len;
+ len = 0;
+ }
+ desc->lli.ctlhi = dlen >> reg_width;
if (!first) {
first = desc;
@@ -786,7 +806,10 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
&first->tx_list);
}
prev = desc;
- total_len += len;
+ total_len += dlen;
+
+ if (len)
+ goto slave_sg_fromdev_fill_desc;
}
break;
default:
--
1.7.3.4
If transfer status is !=DMA_SUCCESS, return total transfer len as residue,
instead of zero.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 3285ae9..964d2d6 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -876,7 +876,11 @@ dwc_tx_status(struct dma_chan *chan,
ret = dma_async_is_complete(cookie, last_complete, last_used);
}
- dma_set_tx_state(txstate, last_complete, last_used, 0);
+ if (ret != DMA_SUCCESS)
+ dma_set_tx_state(txstate, last_complete, last_used,
+ dwc_first_active(dwc)->len);
+ else
+ dma_set_tx_state(txstate, last_complete, last_used, 0);
return ret;
}
--
1.7.3.4
From: Linus Walleij <[email protected]>
Some peripherals like amba-pl011 needs pause to be implemented in DMA controller
drivers. This also returns correct status from dwc_tx_status() in case chan is
paused.
Signed-off-by: Linus Walleij <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 59 +++++++++++++++++++++++++++++---------------
drivers/dma/dw_dmac_regs.h | 1 +
2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 014654d..ba1a2a4 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -841,34 +841,50 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
struct dw_dma *dw = to_dw_dma(chan->device);
struct dw_desc *desc, *_desc;
unsigned long flags;
+ u32 cfglo;
LIST_HEAD(list);
- /* Only supports DMA_TERMINATE_ALL */
- if (cmd != DMA_TERMINATE_ALL)
- return -ENXIO;
+ if (cmd == DMA_PAUSE) {
+ spin_lock_irqsave(&dwc->lock, flags);
- /*
- * This is only called when something went wrong elsewhere, so
- * we don't really care about the data. Just disable the
- * channel. We still have to poll the channel enable bit due
- * to AHB/HSB limitations.
- */
- spin_lock_irqsave(&dwc->lock, flags);
+ cfglo = channel_readl(dwc, CFG_LO);
+ channel_writel(dwc, CFG_LO, cfglo | DWC_CFGL_CH_SUSP);
+ while (!(channel_readl(dwc, CFG_LO) & DWC_CFGL_FIFO_EMPTY))
+ cpu_relax();
- channel_clear_bit(dw, CH_EN, dwc->mask);
+ dwc->paused = true;
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ } else if (cmd == DMA_RESUME) {
+ if (!dwc->paused)
+ return 0;
- while (dma_readl(dw, CH_EN) & dwc->mask)
- cpu_relax();
+ spin_lock_irqsave(&dwc->lock, flags);
- /* active_list entries will end up before queued entries */
- list_splice_init(&dwc->queue, &list);
- list_splice_init(&dwc->active_list, &list);
+ cfglo = channel_readl(dwc, CFG_LO);
+ channel_writel(dwc, CFG_LO, cfglo & ~DWC_CFGL_CH_SUSP);
+ dwc->paused = false;
- /* Flush all pending and queued descriptors */
- list_for_each_entry_safe(desc, _desc, &list, desc_node)
- dwc_descriptor_complete(dwc, desc, 0, flags);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ } else if (cmd == DMA_TERMINATE_ALL) {
+ spin_lock_irqsave(&dwc->lock, flags);
- spin_unlock_irqrestore(&dwc->lock, flags);
+ channel_clear_bit(dw, CH_EN, dwc->mask);
+ while (dma_readl(dw, CH_EN) & dwc->mask)
+ cpu_relax();
+
+ dwc->paused = false;
+
+ /* active_list entries will end up before queued entries */
+ list_splice_init(&dwc->queue, &list);
+ list_splice_init(&dwc->active_list, &list);
+
+ /* Flush all pending and queued descriptors */
+ list_for_each_entry_safe(desc, _desc, &list, desc_node)
+ dwc_descriptor_complete(dwc, desc, 0, flags);
+
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ } else
+ return -ENXIO;
return 0;
}
@@ -905,6 +921,9 @@ dwc_tx_status(struct dma_chan *chan,
else
dma_set_tx_state(txstate, last_complete, last_used, 0);
+ if (dwc->paused)
+ return DMA_PAUSED;
+
return ret;
}
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index 720f821..c968597 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -138,6 +138,7 @@ struct dw_dma_chan {
void __iomem *ch_regs;
u8 mask;
u8 priority;
+ bool paused;
spinlock_t lock;
--
1.7.3.4
dwc_descriptor_complete must always be called with channel lock held. This patch
moves unlock code, in dwc_control(), untill after dwc_descriptor_complete is
called.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index b15c32c..b8985af 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -827,12 +827,12 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
list_splice_init(&dwc->queue, &list);
list_splice_init(&dwc->active_list, &list);
- spin_unlock_bh(&dwc->lock);
-
/* Flush all pending and queued descriptors */
list_for_each_entry_safe(desc, _desc, &list, desc_node)
dwc_descriptor_complete(dwc, desc);
+ spin_unlock_bh(&dwc->lock);
+
return 0;
}
--
1.7.3.4
On 04/27/2011 03:06 PM, Viresh KUMAR wrote:
> This patchset fixes few issues and extends its support.
>
Sorry for two copies of same pathset :(
--
viresh
On Wed, Apr 27, 2011 at 03:06:44PM +0530, Viresh Kumar wrote:
> @@ -407,6 +410,8 @@ EXPORT_SYMBOL(dw_dma_get_dst_addr);
> static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
> u32 status_block, u32 status_err, u32 status_xfer)
> {
> + unsigned long flags;
> +
> if (status_block & dwc->mask) {
> void (*callback)(void *param);
> void *callback_param;
> @@ -418,9 +423,9 @@ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
> callback = dwc->cdesc->period_callback;
> callback_param = dwc->cdesc->period_callback_param;
> if (callback) {
> - spin_unlock(&dwc->lock);
> + spin_unlock_irqrestore(&dwc->lock, flags);
> callback(callback_param);
> - spin_lock(&dwc->lock);
> + spin_lock_irqsave(&dwc->lock, flags);
I'm really not convinced that this is anywhere near correct. I'm
surprised this doesn't spit out a compiler warning.
spin_unlock_irqrestore() reads the flags argument and puts it into
the PSR. spin_lock_irqsave() reads the PSR, puts it into the flags
argument, sets the interrupt mask bit and writes back to the PSR.
So, if you do:
unsigned long flags;
spin_unlock_irqrestore(&dwc->lock, flags);
...
spin_lock_irqsave(&dwc->lock, flags);
you're going to end up corrupting the PSR.
In any case, releasing a spinlock temporarily within a called function
is _really_ not a nice thing to do. It makes code review rather
difficult as called functions become non-atomic when called within
an atomic region.
On Wed, Apr 27, 2011 at 03:06:45PM +0530, Viresh Kumar wrote:
> If dmaengine_terminate_all() is called for dma channel, then it doesn't make
> much sense to call registered callback routine. While in case of success or
> failure it must be called.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/dma/dw_dmac.c | 27 ++++++++++++++-------------
> 1 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index b6dfc39..f590109 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -198,7 +198,8 @@ static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
> /*----------------------------------------------------------------------*/
>
> static void
> -dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
> +dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
> + bool callback_required)
If you're using 'bool' then using 'true' and 'false' with it rather than
'1' and '0' is a good idea.
On Wed, Apr 27, 2011 at 03:06:46PM +0530, Viresh Kumar wrote:
> static void
> dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
> - bool callback_required)
> + bool callback_required, unsigned long flags)
> {
...
> if (callback_required) {
> + spin_unlock_irqrestore(&dwc->lock, flags);
> if (callback)
> callback(param);
> + spin_lock_irqsave(&dwc->lock, flags);
Again, this isn't really on. It seems to me that this code needs some
serious reworking.
On 04/28/2011 10:41 PM, Russell King - ARM Linux wrote:
>> > static void
>> > -dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
>> > +dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
>> > + bool callback_required)
> If you're using 'bool' then using 'true' and 'false' with it rather than
> '1' and '0' is a good idea.
Yes, will correct this.
--
viresh
On Thu, Apr 28, 2011 at 06:10:20PM +0100, Russell King - ARM Linux wrote:
> On Wed, Apr 27, 2011 at 03:06:44PM +0530, Viresh Kumar wrote:
> > @@ -407,6 +410,8 @@ EXPORT_SYMBOL(dw_dma_get_dst_addr);
> > static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
> > u32 status_block, u32 status_err, u32 status_xfer)
> > {
> > + unsigned long flags;
> > +
> > if (status_block & dwc->mask) {
> > void (*callback)(void *param);
> > void *callback_param;
> > @@ -418,9 +423,9 @@ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
> > callback = dwc->cdesc->period_callback;
> > callback_param = dwc->cdesc->period_callback_param;
> > if (callback) {
> > - spin_unlock(&dwc->lock);
> > + spin_unlock_irqrestore(&dwc->lock, flags);
> > callback(callback_param);
> > - spin_lock(&dwc->lock);
> > + spin_lock_irqsave(&dwc->lock, flags);
>
> I'm really not convinced that this is anywhere near correct. I'm
> surprised this doesn't spit out a compiler warning.
>
> spin_unlock_irqrestore() reads the flags argument and puts it into
> the PSR. spin_lock_irqsave() reads the PSR, puts it into the flags
> argument, sets the interrupt mask bit and writes back to the PSR.
>
> So, if you do:
>
> unsigned long flags;
>
> spin_unlock_irqrestore(&dwc->lock, flags);
> ...
> spin_lock_irqsave(&dwc->lock, flags);
>
> you're going to end up corrupting the PSR.
>
> In any case, releasing a spinlock temporarily within a called function
> is _really_ not a nice thing to do. It makes code review rather
> difficult as called functions become non-atomic when called within
> an atomic region.
BTW, how this gets handled in other drivers is basically as follows in
the tasklet:
tasklet()
{
LIST_HEAD(completed);
spin_lock_irqsave(lock, flags);
for each txd(txd) {
if (completed(txd))
list_move_tail(&txd->node, &completed);
}
try to start new txd();
spin_unlock_irqrestore(lock, flags);
for each list entry safe(txd, &completed) {
void (*callback)(void *) = txd->callback;
void *param = txd->callback_param;
free_txd(txd);
if (callback)
callback(param);
}
}
I'm not sure how easy it is to move dw_dmac to that kind of structure,
but I think this is what is required rather than dropping locks within
functions which they haven't themselves taken.
On 04/29/2011 02:45 PM, Russell King - ARM Linux wrote:
> BTW, how this gets handled in other drivers is basically as follows in
> the tasklet:
>
> tasklet()
> {
> LIST_HEAD(completed);
>
> spin_lock_irqsave(lock, flags);
> for each txd(txd) {
> if (completed(txd))
> list_move_tail(&txd->node, &completed);
> }
> try to start new txd();
> spin_unlock_irqrestore(lock, flags);
>
> for each list entry safe(txd, &completed) {
> void (*callback)(void *) = txd->callback;
> void *param = txd->callback_param;
>
> free_txd(txd);
>
> if (callback)
> callback(param);
> }
> }
>
> I'm not sure how easy it is to move dw_dmac to that kind of structure,
I will check that.
--
viresh
On 04/28/2011 10:40 PM, Russell King - ARM Linux wrote:
> On Wed, Apr 27, 2011 at 03:06:44PM +0530, Viresh Kumar wrote:
>> @@ -407,6 +410,8 @@ EXPORT_SYMBOL(dw_dma_get_dst_addr);
>> static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
>> u32 status_block, u32 status_err, u32 status_xfer)
>> {
>> + unsigned long flags;
>> +
>> if (status_block & dwc->mask) {
>> void (*callback)(void *param);
>> void *callback_param;
>> @@ -418,9 +423,9 @@ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
>> callback = dwc->cdesc->period_callback;
>> callback_param = dwc->cdesc->period_callback_param;
>> if (callback) {
>> - spin_unlock(&dwc->lock);
>> + spin_unlock_irqrestore(&dwc->lock, flags);
>> callback(callback_param);
>> - spin_lock(&dwc->lock);
>> + spin_lock_irqsave(&dwc->lock, flags);
>
> I'm really not convinced that this is anywhere near correct. I'm
> surprised this doesn't spit out a compiler warning.
>
Sorry, this is done by mistake.
--
viresh