[PATCH] dmaengine: at_xdmac: fix slave configuration issue

[Vinod Koul]

On Thu, Apr 16, 2015 at 05:04:00PM +0200, Ludovic Desroches wrote:
> When doing the slave configuration, an error is returned if the maxburst
> value is not supported. The bug comes from the fact that we always check
> the maxburst for both directions but in the case of an unidirectional
> channel, only one is set.
While setting the slave configuration we are not tied to a channel
direction, the direction is passed usin prep_ method. So from that
prespective a channle can be used for tx and rx with same slave config set.

Now if we were invoking at_xdmac_set_slave_config from prep_ calls then it
would have been fine but here we are checking when the slave config is set
so this is not right. You should check maxburst at runtime then...

-- 
~Vinod
> Signed-off-by: Ludovic Desroches <ludovic.desroches at atmel.com>
> Cc: <stable at vger.kernel.org> # 3.19 and later
> ---
>  drivers/dma/at_xdmac.c | 97 +++++++++++++++++++++++++++-----------------------
>  1 file changed, 52 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index d9891d3..30e161b 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -501,54 +501,61 @@ static int at_xdmac_set_slave_config(struct dma_chan *chan,
>  	u8 dwidth;
>  	int csize;
>  
> -	atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG] =
> -		AT91_XDMAC_DT_PERID(atchan->perid)
> -		| AT_XDMAC_CC_DAM_INCREMENTED_AM
> -		| AT_XDMAC_CC_SAM_FIXED_AM
> -		| AT_XDMAC_CC_DIF(atchan->memif)
> -		| AT_XDMAC_CC_SIF(atchan->perif)
> -		| AT_XDMAC_CC_SWREQ_HWR_CONNECTED
> -		| AT_XDMAC_CC_DSYNC_PER2MEM
> -		| AT_XDMAC_CC_MBSIZE_SIXTEEN
> -		| AT_XDMAC_CC_TYPE_PER_TRAN;
> -	csize = at_xdmac_csize(sconfig->src_maxburst);
> -	if (csize < 0) {
> -		dev_err(chan2dev(chan), "invalid src maxburst value\n");
> -		return -EINVAL;
> -	}
> -	atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG] |= AT_XDMAC_CC_CSIZE(csize);
> -	dwidth = ffs(sconfig->src_addr_width) - 1;
> -	atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG] |= AT_XDMAC_CC_DWIDTH(dwidth);
> -
> -
> -	atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG] =
> -		AT91_XDMAC_DT_PERID(atchan->perid)
> -		| AT_XDMAC_CC_DAM_FIXED_AM
> -		| AT_XDMAC_CC_SAM_INCREMENTED_AM
> -		| AT_XDMAC_CC_DIF(atchan->perif)
> -		| AT_XDMAC_CC_SIF(atchan->memif)
> -		| AT_XDMAC_CC_SWREQ_HWR_CONNECTED
> -		| AT_XDMAC_CC_DSYNC_MEM2PER
> -		| AT_XDMAC_CC_MBSIZE_SIXTEEN
> -		| AT_XDMAC_CC_TYPE_PER_TRAN;
> -	csize = at_xdmac_csize(sconfig->dst_maxburst);
> -	if (csize < 0) {
> -		dev_err(chan2dev(chan), "invalid src maxburst value\n");
> -		return -EINVAL;
> +	if (sconfig->src_addr) {
> +		atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG] =
> +			AT91_XDMAC_DT_PERID(atchan->perid)
> +			| AT_XDMAC_CC_DAM_INCREMENTED_AM
> +			| AT_XDMAC_CC_SAM_FIXED_AM
> +			| AT_XDMAC_CC_DIF(atchan->memif)
> +			| AT_XDMAC_CC_SIF(atchan->perif)
> +			| AT_XDMAC_CC_SWREQ_HWR_CONNECTED
> +			| AT_XDMAC_CC_DSYNC_PER2MEM
> +			| AT_XDMAC_CC_MBSIZE_SIXTEEN
> +			| AT_XDMAC_CC_TYPE_PER_TRAN;
> +		csize = at_xdmac_csize(sconfig->src_maxburst);
> +		if (csize < 0) {
> +			dev_err(chan2dev(chan), "invalid src maxburst value\n");
> +			return -EINVAL;
> +		}
> +		atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG] |= AT_XDMAC_CC_CSIZE(csize);
> +		dwidth = ffs(sconfig->src_addr_width) - 1;
> +		atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG] |= AT_XDMAC_CC_DWIDTH(dwidth);
> +		/* Src addr is needed to configure the link list descriptor. */
> +		atchan->per_src_addr = sconfig->src_addr;
> +
> +		dev_dbg(chan2dev(chan),
> +			"%s: cfg[dev2mem]=0x%08x, per_src_addr=0x%08x\n",
> +			__func__, atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG],
> +			atchan->per_src_addr);
>  	}
> -	atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG] |= AT_XDMAC_CC_CSIZE(csize);
> -	dwidth = ffs(sconfig->dst_addr_width) - 1;
> -	atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG] |= AT_XDMAC_CC_DWIDTH(dwidth);
>  
> -	/* Src and dst addr are needed to configure the link list descriptor. */
> -	atchan->per_src_addr = sconfig->src_addr;
> -	atchan->per_dst_addr = sconfig->dst_addr;
> +	if (sconfig->dst_addr) {
> +		atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG] =
> +			AT91_XDMAC_DT_PERID(atchan->perid)
> +			| AT_XDMAC_CC_DAM_FIXED_AM
> +			| AT_XDMAC_CC_SAM_INCREMENTED_AM
> +			| AT_XDMAC_CC_DIF(atchan->perif)
> +			| AT_XDMAC_CC_SIF(atchan->memif)
> +			| AT_XDMAC_CC_SWREQ_HWR_CONNECTED
> +			| AT_XDMAC_CC_DSYNC_MEM2PER
> +			| AT_XDMAC_CC_MBSIZE_SIXTEEN
> +			| AT_XDMAC_CC_TYPE_PER_TRAN;
> +		csize = at_xdmac_csize(sconfig->dst_maxburst);
> +		if (csize < 0) {
> +			dev_err(chan2dev(chan), "invalid src maxburst value\n");
> +			return -EINVAL;
> +		}
> +		atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG] |= AT_XDMAC_CC_CSIZE(csize);
> +		dwidth = ffs(sconfig->dst_addr_width) - 1;
> +		atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG] |= AT_XDMAC_CC_DWIDTH(dwidth);
> +		/* Dst addr is needed to configure the link list descriptor. */
> +		atchan->per_dst_addr = sconfig->dst_addr;
>  
> -	dev_dbg(chan2dev(chan),
> -		"%s: cfg[dev2mem]=0x%08x, cfg[mem2dev]=0x%08x, per_src_addr=0x%08x, per_dst_addr=0x%08x\n",
> -		__func__, atchan->cfg[AT_XDMAC_DEV_TO_MEM_CFG],
> -		atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG],
> -		atchan->per_src_addr, atchan->per_dst_addr);
> +		dev_dbg(chan2dev(chan),
> +			"%s: cfg[mem2dev]=0x%08x, per_dst_addr=0x%08x\n",
> +			__func__, atchan->cfg[AT_XDMAC_MEM_TO_DEV_CFG],
> +			atchan->per_dst_addr);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.2.0
> 

--