Re: [linux-sunxi] [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Priit Laes]

On Mon, 2016-03-07 at 10:59 +0100, Boris Brezillon wrote:
> Some drivers might need to tweak the block size and wait cycles
> values
> to get better performances.
> Create and export the sun4i_dma_set_chan_config() to do that.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/dma/sun4i-dma.c       | 44 ++++++++++++++++++++++++++++++---
> ----------
>  include/linux/dma/sun4i-dma.h | 38
> +++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 13 deletions(-)
>  create mode 100644 include/linux/dma/sun4i-dma.h
> 
> diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
> index 1661d518..e48f537 100644
> --- a/drivers/dma/sun4i-dma.c
> +++ b/drivers/dma/sun4i-dma.c
> @@ -12,6 +12,7 @@
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
>  #include <linux/dmaengine.h>
> +#include <linux/dma/sun4i-dma.h>
>  #include <linux/dmapool.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> @@ -138,6 +139,7 @@ struct sun4i_dma_pchan {
>  struct sun4i_dma_vchan {
>  	struct virt_dma_chan		vc;
>  	struct dma_slave_config		cfg;
> +	struct sun4i_dma_chan_config	scfg;
>  	struct sun4i_dma_pchan		*pchan;
>  	struct sun4i_dma_promise	*processing;
>  	struct sun4i_dma_contract	*contract;
> @@ -779,7 +781,7 @@ sun4i_dma_prep_slave_sg(struct dma_chan *chan,
> struct scatterlist *sgl,
>  	u8 ram_type, io_mode, linear_mode;
>  	struct scatterlist *sg;
>  	dma_addr_t srcaddr, dstaddr;
> -	u32 endpoints, para;
> +	u32 endpoints;
>  	int i;
>  
>  	if (!sgl)
> @@ -825,17 +827,6 @@ sun4i_dma_prep_slave_sg(struct dma_chan *chan,
> struct scatterlist *sgl,
>  			dstaddr = sg_dma_address(sg);
>  		}
>  
> -		/*
> -		 * These are the magic DMA engine timings that keep
> SPI going.
> -		 * I haven't seen any interface on DMAEngine to
> configure
> -		 * timings, and so far they seem to work for
> everything we
> -		 * support, so I've kept them here. I don't know if
> other
> -		 * devices need different timings because, as usual,
> we only
> -		 * have the "para" bitfield meanings, but no comment
> on what
> -		 * the values should be when doing a certain
> operation :|
> -		 */
> -		para = SUN4I_DDMA_MAGIC_SPI_PARAMETERS;
> -
>  		/* And make a suitable promise */
>  		if (vchan->is_dedicated)
>  			promise = generate_ddma_promise(chan,
> srcaddr, dstaddr,
> @@ -850,7 +841,7 @@ sun4i_dma_prep_slave_sg(struct dma_chan *chan,
> struct scatterlist *sgl,
>  			return NULL; /* TODO: should we free
> everything? */
>  
>  		promise->cfg |= endpoints;
> -		promise->para = para;
> +		promise->para = vchan->scfg.para;
>  
>  		/* Then add it to the contract */
>  		list_add_tail(&promise->list, &contract->demands);
> @@ -908,6 +899,21 @@ static int sun4i_dma_config(struct dma_chan
> *chan,
>  	return 0;
>  }
>  
> +int sun4i_dma_set_chan_config(struct dma_chan *dchan,
> +			      const struct sun4i_dma_chan_config
> *cfg)
> +{
> +	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(dchan);
> +
> +	if (!vchan->is_dedicated)
> +		return -ENOTSUPP;
> +
> +	/* TODO: control cfg value */
> +	vchan->scfg = *cfg;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(sun4i_dma_set_chan_config);
> +
>  static struct dma_chan *sun4i_dma_of_xlate(struct of_phandle_args
> *dma_spec,
>  					   struct of_dma *ofdma)
>  {
> @@ -1206,6 +1212,18 @@ static int sun4i_dma_probe(struct
> platform_device *pdev)
>  		spin_lock_init(&vchan->vc.lock);
>  		vchan->vc.desc_free = sun4i_dma_free_contract;
>  		vchan_init(&vchan->vc, &priv->slave);
> +
> +		/*
> +		 * These are the magic DMA engine timings that keep
> SPI going.
> +		 * I haven't seen any interface on DMAEngine to
> configure
> +		 * timings, and so far they seem to work for
> everything we
> +		 * support, so I've kept them here. I don't know if
> other
> +		 * devices need different timings because, as usual,
> we only
> +		 * have the "para" bitfield meanings, but no comment
> on what
> +		 * the values should be when doing a certain
> operation :|
> +		 */
> +		vchan->scfg.para = SUN4I_DDMA_MAGIC_SPI_PARAMETERS;

Does SPI refer the Serial Peripheral Interface?

If yes, then I would point out that current sun4i SPI driver doesn't
actually use DMA [1]

http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/411
722.html


> +
>  	}
>  
>  	ret = clk_prepare_enable(priv->clk);
> diff --git a/include/linux/dma/sun4i-dma.h b/include/linux/dma/sun4i
> -dma.h
> new file mode 100644
> index 0000000..f643539
> --- /dev/null
> +++ b/include/linux/dma/sun4i-dma.h
> @@ -0,0 +1,38 @@
> +/*
> + * Sun4i DMA Engine drivers support header file
> + *
> + * Copyright (C) 2016 Free Electrons. All rights reserved.
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef _SUN4I_DMA_H
> +#define _SUN4I_DMA_H
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> +
> +/* Dedicated DMA parameter register layout */
> +#define SUN4I_DDMA_PARA_DST_DATA_BLK_SIZE(n)	(((n) - 1) <<
> 24)
> +#define SUN4I_DDMA_PARA_DST_WAIT_CYCLES(n)	(((n) - 1) << 16)
> +#define SUN4I_DDMA_PARA_SRC_DATA_BLK_SIZE(n)	(((n) - 1) << 8)
> +#define SUN4I_DDMA_PARA_SRC_WAIT_CYCLES(n)	(((n) - 1) << 0)
> +
> +/**
> + * struct sun4i_dma_chan_config - DMA channel config
> + *
> + * @para: contains information about block size and time before
> checking
> + *	  DRQ line. This is device specific and only applicable to
> dedicated
> + *	  DMA channels
> + */
> +struct sun4i_dma_chan_config {
> +	u32 para;
> +};
> +
> +int sun4i_dma_set_chan_config(struct dma_chan *dchan,
> +			      const struct sun4i_dma_chan_config
> *cfg);
> +
> +#endif /* _SUN4I_DMA_H */
> -- 
> 2.1.4
>