Messages in this thread | | | Date | Fri, 13 Dec 2013 16:33:39 +0100 | From | Lukasz Czerwinski <> | Subject | Re: [RFC PATCH] dma: pl330: Change pl330_tx_status() |
| |
On 12/09/2013 01:00 PM, Lars-Peter Clausen wrote: > On 12/09/2013 12:19 PM, Lukasz Czerwinski wrote: >> This patch adds possibility to read status of unfinished transfers >> before termination. >> >> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> >> I tested patch with Exynos4 board. This is only proof of concept. >> Any comments are welcome. > > The patch does not look as if it is going to work correctly. And I think > that this can't be implemented correctly with the current structure of the > driver (at least not without jumping through a couple of hoops). I do have a > couple of WIP patches that restructure the driver and then implement residue > reporting on top of it. > >>
Thanks for your comments.
I will be waiting for your patch series then I will implement residue reporting in proper way.
br Lukasz
>> Thanks >> Lukasz >> >> drivers/dma/pl330.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 60 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c >> index 24f8ae3..c88c36e 100644 >> --- a/drivers/dma/pl330.c >> +++ b/drivers/dma/pl330.c >> @@ -566,6 +566,7 @@ struct dma_pl330_chan { >> /* For D-to-M and M-to-D channels */ >> int burst_sz; /* the peripheral fifo width */ >> int burst_len; /* the number of burst */ >> + int transfered; > > This is always set to 0. > >> dma_addr_t fifo_addr; >> >> /* for cyclic capability */ >> @@ -602,6 +603,9 @@ struct dma_pl330_desc { >> >> enum desc_status status; >> >> + int bytes_requested; >> + int direction; >> + >> /* The channel which currently holds this desc */ >> struct dma_pl330_chan *pchan; >> }; >> @@ -2366,6 +2370,29 @@ static int pl330_alloc_chan_resources(struct dma_chan *chan) >> return 1; >> } >> >> +int pl330_get_current_xferred_count(struct dma_pl330_chan *pch, >> + struct dma_pl330_desc *desc) >> +{ >> + u32 val, addr; >> + struct pl330_thread *thrd = pch->pl330_chid; >> + void __iomem *regs = thrd->dmac->pinfo->base; >> + >> + val = addr = 0; >> + switch (desc->direction) { >> + case DMA_MEM_TO_DEV: >> + val = readl(regs + SA(thrd->id)); >> + addr = desc->px.src_addr; >> + break; >> + case DMA_DEV_TO_MEM: >> + val = readl(regs + DA(thrd->id)); >> + addr = desc->px.dst_addr; >> + break; >> + default: >> + break; >> + } >> + return val - addr; >> +} >> + >> static int pl330_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, unsigned long arg) >> { >> struct dma_pl330_chan *pch = to_pchan(chan); >> @@ -2446,7 +2473,33 @@ static enum dma_status >> pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie, >> struct dma_tx_state *txstate) >> { >> - return dma_cookie_status(chan, cookie, txstate); >> + enum dma_status ret; >> + unsigned long flags; >> + struct dma_pl330_desc *desc; >> + struct dma_pl330_chan *pch = to_pchan(chan); >> + unsigned int bytes_transferred; >> + unsigned int residual; >> + >> + /* Check in pending list */ >> + spin_lock_irqsave(&pch->lock, flags); >> + list_for_each_entry(desc, &pch->work_list, node) { >> + if (desc->txd.cookie == cookie) { >> + bytes_transferred = >> + pl330_get_current_xferred_count(pch, desc); > > pl330_get_current_xferred_count() will only return the correct result if it > is called for the currently active transfer. > > And then there is also the issue that the driver creates multiple internal > descriptors for each segment in the transfer. Each of these descriptors gets > different cookie. pl330_tx_submit() returns the cookie to the descriptor to > the first segment in the transfer. This means if you pass that cookie to > pl330_tx_status() you'll only get the residue for the first segment and not > for the whole transfer, as it is supposed to be. > >> + residual = desc->bytes_requested - >> + bytes_transferred % desc->bytes_requested; > > If bytes_transferred % desc->bytes_requested != bytes_transferred there is a > bug somewhere. > >> + dma_set_residue(txstate, residual); >> + ret = desc->status; >> + spin_unlock_irqrestore(&pch->lock, flags); >> + return ret; >> + } >> + } >> + spin_unlock_irqrestore(&pch->lock, flags); >> + >> + ret = dma_cookie_status(chan, cookie, txstate); >> + dma_set_residue(txstate, pch->transfered); >> + >> + return ret; >> } >> > [...] > . >
* English - detected * Polish
* Polish
<javascript:void(0);>
| |