dmaengine/dw_dmac updates
v3 cover dmaengine/dw_dmac updates
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(-)
Signature · 1 line
1.7.3.4
PATCH v3 1/7 dmaengine/dw_dmac: call dwc_descriptor_complete from dwc_control with lock held
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.
--- drivers/dma/dw_dmac.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/dma/dw_dmac.c +2 -2
@@ -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 ; }
Signature · 1 line
1.7.3.4
PATCH v3 2/7 dmaengine/dw_dmac: Replace spin_lock* with irqsave variants
In reply to Viresh Kumardmaengine 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.
--- drivers/dma/dw_dmac.c | 82 +++++++++++++++++++++++++++++------------------- 1 files changed, 49 insertions(+), 33 deletions(-)
drivers/dma/dw_dmac.c +49 -33
@@ -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]);
Signature · 1 line
1.7.3.4
Re: [PATCH V3 2/7] dmaengine/dw_dmac: Replace spin_lock* with irqsave variants
Quoted 10 lines · Viresh Kumar
drivers/dma/dw_dmac.c +2
-0@@ -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;
drivers/dma/dw_dmac.c static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc, · line 428
Quoted 8 lines
drivers/dma/dw_dmac.c +2
-2@@ -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.
Re: [PATCH V3 2/7] dmaengine/dw_dmac: Replace spin_lock* with irqsave variants
Quoted 35 lines · Russell King - ARM Linux
Quoted 10 lines · Viresh Kumar
drivers/dma/dw_dmac.c +2
-0@@ -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;drivers/dma/dw_dmac.c static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc, · line 428
Quoted 8 lines
drivers/dma/dw_dmac.c +2
-2@@ -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);
Quoted 6 lines
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.
Re: [PATCH V3 2/7] dmaengine/dw_dmac: Replace spin_lock* with irqsave variants
Quoted 26 lines · Russell King - ARM Linux
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); } }
Quoted 1 line
I'm not sure how easy it is to move
dw_dmacto that kind of structure,
I will check that.
Signature · 1 line
viresh
Re: [PATCH V3 2/7] dmaengine/dw_dmac: Replace spin_lock* with irqsave variants
In reply to Russell King - ARM LinuxQuoted 14 lines · Russell King - ARM Linux
Quoted 13 lines · Viresh Kumar
drivers/dma/dw_dmac.c +2
-0@@ -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) {
Quoted 9 lines
drivers/dma/dw_dmac.c static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc, · line 428
Quoted 5 lines
drivers/dma/dw_dmac.c +2
-2-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.
Signature · 1 line
viresh
PATCH v3 3/7 dmaengine/dw_dmac: don't call callback routine in case dmaengine_terminate_all() is called
In reply to Viresh KumarIf 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.
--- drivers/dma/dw_dmac.c | 27 ++++++++++++++------------- 1 files changed, 14 insertions(+), 13 deletions(-)
drivers/dma/dw_dmac.c +14 -13
@@ -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);
Signature · 1 line
1.7.3.4
Re: [PATCH V3 3/7] dmaengine/dw_dmac: don't call callback routine in case dmaengine_terminate_all() is called
Quoted 14 lines · Viresh Kumar
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(-)drivers/dma/dw_dmac.c +0
-0
drivers/dma/dw_dmac.c static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first) · line 202
Quoted 6 lines
drivers/dma/dw_dmac.c +2
-1@@ -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.
Re: [PATCH V3 3/7] dmaengine/dw_dmac: don't call callback routine in case dmaengine_terminate_all() is called
Quoted 6 lines · Russell King - ARM Linux
Quoted 4 lines
drivers/dma/dw_dmac.c static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first) · line 202
Quoted 4 lines
static void
drivers/dma/dw_dmac.c +2
-1-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.
Signature · 1 line
viresh
PATCH v3 4/7 dmaengine/dw_dmac: Enable resubmission from callback routine.
In reply to Viresh KumarResubmission of new transfer must be allowed from callbacks. For this release lock before calling callback routine and enable them again.
--- drivers/dma/dw_dmac.c | 37 +++++++++++++++++++++---------------- 1 files changed, 21 insertions(+), 16 deletions(-)
drivers/dma/dw_dmac.c +21 -16
@@ -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); }
Signature · 1 line
1.7.3.4
Re: [PATCH V3 4/7] dmaengine/dw_dmac: Enable resubmission from callback routine.
Quoted 5 lines · Viresh Kumar
static void
dwc_descriptor_complete(structdw_dma_chan*dwc, structdw_desc*desc,drivers/dma/dw_dmac.c +1
-1- bool callback_required) + bool callback_required, unsigned long flags) {
...
drivers/dma/dw_dmac.c dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc, · line 251
Quoted 5 lines
drivers/dma/dw_dmac.c +2
-0if (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.
PATCH v3 5/7 dmaengine/dw_dmac: set residue as total len in dwc_tx_status if status is !DMA_SUCCESS
In reply to Viresh KumarIf transfer status is !=DMA_SUCCESS, return total transfer len as residue, instead of zero.
--- drivers/dma/dw_dmac.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/dma/dw_dmac.c +5 -1
@@ -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; }
Signature · 1 line
1.7.3.4
PATCH v3 6/7 dmaengine/dw_dmac: Divide one sg to many desc, if sg len is greater than DWC_MAX_COUNT
In reply to Viresh KumarIf 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.
--- drivers/dma/dw_dmac.c | 65 +++++++++++++++++++++++++++++++++---------------- 1 files changed, 44 insertions(+), 21 deletions(-)
drivers/dma/dw_dmac.c +44 -21
@@ -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 :
Signature · 1 line
1.7.3.4
PATCH v3 7/7 dmaengine/dw_dmac: implement pause and resume in dwc_control
In reply to Viresh KumarFrom: 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.
--- drivers/dma/dw_dmac.c | 59 +++++++++++++++++++++++++++++--------------- drivers/dma/dw_dmac_regs.h | 1 + 2 files changed, 40 insertions(+), 20 deletions(-)
drivers/dma/dw_dmac.c +39 -20
@@ -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; }
drivers/dma/dw_dmac_regs.h +1 -0
@@ -138,6 +138,7 @@ struct dw_dma_chan {void __iomem *ch_regs;u8 mask;u8 priority; +bool paused;spinlock_t lock;
Signature · 1 line
1.7.3.4
Quoted 2 lines · Viresh KUMAR
This patchset fixes few issues and extends its support.
Sorry for two copies of same pathset :(
Signature · 1 line
viresh