On Thu, Jul 11, 2013 at 10:54:29AM +0800, Xiang Wang wrote:
> >> +static u32 mmp_pdma_get_bytes_residue(struct mmp_pdma_chan *chan)
> >> +{
> >> + u32 reg, orig_pos, cur_pos, ddadr, residue = 0;
> >> + bool running_desc_found = false;
> >> + struct mmp_pdma_desc_sw *desc_sw;
> >> +
> >> + /*
> >> + * When a phy channel is unavailable, maybe it has been freed, return
> >> + * last stored value for safe.
> >> + */
> >> + if (!chan->phy)
> >> + return chan->bytes_residue;
> >> +
> >> + reg = (chan->phy->idx << 4) + DDADR;
> >> + ddadr = readl_relaxed(chan->phy->base + reg);
> >> +
> >> + /* iterate over all descriptors to sum up the number of pending
> >> bytes */
> > and why?
> >
> > Residue does not mean sum of all pending bytes across all descriptors
> > submitted
> > You need to find the residue of current descriptor only and return
> >
> Here are my thoughts:
> We tell dma engine to transmit x bytes for us and it creates n
> descriptors to transmit which is transparent to us.
> So when we want to find out how many bytes are left, dma engine should
> sum up all pending bytes across all descriptors.
> What's your opinion?
but you may have multiple transactions submmited. Iterating over _all_
descriptors doesnt make sense then. You need to iterate over descriptors for
current transaction only.
> >> + list_for_each_entry(desc_sw, &chan->chain_running, node) {
> >> + /* for the case of a running descriptor */
> >> + if (desc_sw->desc.ddadr == ddadr && !running_desc_found) {
> >> + switch (chan->dir) {
> >> + case DMA_DEV_TO_MEM:
> >> + case DMA_MEM_TO_MEM:
> >> + reg = (chan->phy->idx << 4) + DTADR;
> >> + cur_pos = readl_relaxed(chan->phy->base +
> >> reg);
> >> + orig_pos = desc_sw->desc.dtadr;
> >> + break;
> >> +
> >> + case DMA_MEM_TO_DEV:
> >> + reg = (chan->phy->idx << 4) + DSADR;
> >> + cur_pos = readl_relaxed(chan->phy->base +
> >> reg);
> >> + orig_pos = desc_sw->desc.dsadr;
> >> + break;
> >> +
> >> + default:
> >> + cur_pos = 0;
> >> + orig_pos = 0;
> > This makes no sense...
> What do you recommend for the default case? Just return 0?
why should direction be anything other than these? This is error!
~Vinod
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
