Re: [PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[Vinod Koul]

On Mon, 2012-01-02 at 11:08 +0100, javier Martin wrote:
> Hi Vinod,
> thank you for your review.
> 
> On 23 December 2011 18:38, Vinod Koul <vinod.koul@xxxxxxxxx> wrote:
> >> @@ -328,6 +329,37 @@ static struct dma_async_tx_descriptor *imxdma_prep_dma_cyclic(
> >>       return &imxdmac->desc;
> >>  }
> >>
> >> +static struct dma_async_tx_descriptor *imxdma_prep_dma_memcpy(
> >> +     struct dma_chan *chan, dma_addr_t dest,
> >> +     dma_addr_t src, size_t len, unsigned long flags)
> >> +{
> >> +     struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
> >> +     struct imxdma_engine *imxdma = imxdmac->imxdma;
> >> +     int ret;
> >> +
> >> +     dev_dbg(imxdma->dev, "%s channel: %d src=0x%x dst=0x%x len=%d\n",
> >> +                     __func__, imxdmac->channel, src, dest, len);
> >> +
> >> +     if (imxdmac->status == DMA_IN_PROGRESS)
> >> +             return NULL;
> >> +
> >> +     imxdmac->status = DMA_IN_PROGRESS;
> > why? prepare doesn't mean DMA is in progress. You can get multiple
> > descriptors for same channel by calling prepare multiple times.
> > Even one descriptor can be memcpy whereas other is slave...
> > In issue_pending this should be moved to DMA_IN_PROGRESS
> 
> According to the original developer of this file it only supports a
> single descriptor. Please, take a look at the following function:
> 
> static void imxdma_issue_pending(struct dma_chan *chan)
> {
> 	/*
> 	 * Nothing to do. We only have a single descriptor
> 	 */
> }
> 
> 
> Also, the same behavior is present in the other prepare functions:
> "imxdma_prep_slave_sg" and "imxdma_prep_dma_cyclic".
> 
> I think the less painful approach here is merging my patch so that
> multiple descriptors support can be added later.
This behavior has nothing to do with single/multiple descriptors.
Prepare means to prepare the transfer and not to submit. There are other
APIs given to do the job. Please go thru the Documentation/dmaengine.txt
as well.

I know some drivers are broken in this respect, but would be great if
you can fix this behavior here as well.

-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/