Messages in this thread | | | Date | Mon, 3 Jun 2013 11:22:13 +0800 | Subject | Re: [PATCH] dma: mmp_pdma: support for getting residual bytes | From | Xiang Wang <> |
| |
2013/5/31 Andy Shevchenko <andy.shevchenko@gmail.com>: > On Fri, May 31, 2013 at 11:21 AM, Xiang Wang <wangxfdu@gmail.com> wrote: >> In some of our drivers (e.g. UART) we may stop a running DMA >> before it finishes. So we need to know how many bytes have >> been transferred. > > Couple of comments below. > >> --- a/drivers/dma/mmp_pdma.c >> +++ b/drivers/dma/mmp_pdma.c > >> @@ -589,7 +638,13 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, >> mmp_pdma_free_desc_list(chan, &chan->chain_pending); >> mmp_pdma_free_desc_list(chan, &chan->chain_running); >> spin_unlock_irqrestore(&chan->desc_lock, flags); >> - chan->idle = true; >> + chan->status = DMA_SUCCESS; >> + chan->bytes_residue = 0; >> + break; >> + case DMA_PAUSE: >> + disable_chan(chan->phy); >> + chan->status = DMA_PAUSED; >> + chan->bytes_residue = mmp_pdma_get_bytes_residue(chan); > > Does it mean user has to do DMA_PAUSE first to get more or less > accurate residue? > Logically that sound correct, but in general we may allow user to get > approximate residue value of on going transfer. Hi, Andy Your comment makes sense. But if the user is allowed to query the residue value in real-time, we cannot just return a saved value to him. Why I use a saved value (chan->bytes_residue)? In current mmp pdma driver, a phy channel will be freed after the transmission finishes (chan->phy is set to NULL). So we cannot get the physical channel information after we call DMA_TERMINATE_ALL or DMA finishes itself. That is to say, when the use queries the channel information at these points, the chan->phy is usually NULL. > >> break; >> case DMA_SLAVE_CONFIG: >> if (cfg->direction == DMA_DEV_TO_MEM) { >> @@ -637,7 +692,8 @@ static enum dma_status mmp_pdma_tx_status(struct dma_chan *dchan, >> unsigned long flags; >> >> spin_lock_irqsave(&chan->desc_lock, flags); >> - ret = dma_cookie_status(dchan, cookie, txstate); >> + ret = chan->status; >> + dma_set_residue(txstate, chan->bytes_residue); >> spin_unlock_irqrestore(&chan->desc_lock, flags); > > Besides my patch which removes this spinlock I think the workflow > should be something like > > status = dma_cookie_status() > if status == DMA_SUCCESS or !txstate: > return status > > dma_set_residue() > return status > > Because there is no reason to return residue of successfully finished > transfer. It should be 0. Hi, Andy There is one exception from my point of view. When we are receiving data from peripheral devices, we usually start a DMA transaction with a target length of 4K for example. When a timed-out event occurs in peripheral device, it will notify DMA controller and DMA controller will send out a End of Receive interrupt (Marvell specific?). In such situation, DMA status is also DMA_SUCCESS. But the residual bytes is not 0 and the user must query it. Thanks! > > -- > With Best Regards, > Andy Shevchenko
| |