Re: [PATCH 2/3] dma: edma: Add support for Cyclic DMA

[Vinod Koul <vinod.koul@intel.com>]

On Thu, Oct 24, 2013 at 12:57:02PM -0500, Joel Fernandes wrote:
> Rebased on slave-dma/next branch and reapplied:
Looks like your MUA caused lines to get wrapped and patch is corrupt, can you
pls resend again using git-send email. I tried even the patch from
patchworks but that too failed!
> 
> ----8<---
> From: Joel Fernandes <joelf@ti.com>
> Subject: [PATCH v2] dma: edma: Add support for Cyclic DMA
> 
> Using the PaRAM configuration function that we split for reuse by the
> different DMA types, we implement Cyclic DMA support.
> For the cyclic case, we pass different configuration parameters to this
> function, and handle all the Cyclic-specific functionality separately.
> 
> Callbacks to the DMA users are handled using vchan_cyclic_callback in
> the virt-dma layer. Linking is handled the same way as the slave SG case
> except for the last slot where we link it back to the first one in a
> cyclic fashion.
> 
> For continuity, we check for cases where no.of periods is great than the
> MAX number of slots the driver can allocate for a particular descriptor
> and error out on such cases.
> 
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
>  drivers/dma/edma.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 151 insertions(+), 8 deletions(-)
> 

>  	struct edma_chan *echan = data;
> @@ -464,24 +602,28 @@ static void edma_callback(unsigned ch_num, u16 ch_status,
> void *data)
This seems bad
>  	unsigned long flags;
>  	struct edmacc_param p;
> 
> -	/* Pause the channel */
> -	edma_pause(echan->ch_num);
> +	edesc = echan->edesc;
> +
> +	/* Pause the channel for non-cyclic */
> +	if (!edesc || (edesc && !edesc->cyclic))
> +		edma_pause(echan->ch_num);
> 
>  	switch (ch_status) {
>  	case DMA_COMPLETE:
>  		spin_lock_irqsave(&echan->vchan.lock, flags);
> 
> -		edesc = echan->edesc;
>  		if (edesc) {
> -			if (edesc->processed == edesc->pset_nr) {
> +			if (edesc->cyclic) {
> +				vchan_cyclic_callback(&edesc->vdesc);
> +			} else if (edesc->processed == edesc->pset_nr) {
>  				dev_dbg(dev, "Transfer complete, stopping channel %d\n", ch_num);
>  				edma_stop(echan->ch_num);
>  				vchan_cookie_complete(&edesc->vdesc);
> +				edma_execute(echan);
>  			} else {
>  				dev_dbg(dev, "Intermediate transfer complete on channel %d\n", ch_num);
> +				edma_execute(echan);
>  			}
> -
> -			edma_execute(echan);
>  		}
> 
>  		spin_unlock_irqrestore(&echan->vchan.lock, flags);
> @@ -677,6 +819,7 @@ static void edma_dma_init(struct edma_cc *ecc, struct
> dma_device *dma,
ditto and few other which checkpatch was not happy about!
>  			  struct device *dev)
>  {
>  	dma->device_prep_slave_sg = edma_prep_slave_sg;
> +	dma->device_prep_dma_cyclic = edma_prep_dma_cyclic;
>  	dma->device_alloc_chan_resources = edma_alloc_chan_resources;
>  	dma->device_free_chan_resources = edma_free_chan_resources;
>  	dma->device_issue_pending = edma_issue_pending;
> -- 
> 1.8.1.2
> 

--
~Vinod