Re: [PATCH] dmaengine: dw: add Intel Broxton LPSS Integrated DMA support

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

On Tue, Apr 21, 2015 at 05:41:50AM +0000, Zha, Qipeng wrote:
> + dma maillist
Pls CC maintainer, or your patch will be missed!!

> 
> 
> 
> 
> Best wishes
> Qipeng
> 
> -----Original Message-----
> From: Zha, Qipeng 
> Sent: Tuesday, April 21, 2015 7:34 AM
> To: linux-kernel@vger.kernel.org
> Cc: viresh.linux@gmail.com; andriy.shevchenko@linux.intel.com; Westerberg, Mika; Chen, Jason CJ; Zheng, Qi; Zha, Qipeng; Zhong, Huiquan
> Subject: [PATCH] dmaengine: dw: add Intel Broxton LPSS Integrated DMA support
> 
> From: Huiquan Zhong <huiquan.zhong@intel.com>
> 
> Add Broxton Lower Power Sub System Integrated DMA support, Since the DMA register space is very similar to DesignWare DMA register space.
> 
> Add DW_DMAC_TYPE_IDMA type to distinguish LPSS iDMA register.
> 
> Broxton LPSS iDMA's maximum block size is 0x1ffff(128KB -1).
Looking at the patch aprt from DW_DMAC_TYPE_IDMA changes, rest are not
documented why, perhpas you cna split to multiple changes explainig what and
why. Then this is supporting BXT but I dont see any update to device table?

> 
> Signed-off-by: Huiquan Zhong <huiquan.zhong@intel.com>
> Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
> ---
>  drivers/dma/dw/core.c                | 64 +++++++++++++++++++++++++++++-------
>  drivers/dma/dw/internal.h            |  3 --
>  drivers/dma/dw/regs.h                | 14 ++++++++
>  include/linux/dma/dw.h               |  8 +++++
>  include/linux/platform_data/dma-dw.h |  2 +-
>  5 files changed, 75 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c index a8ad052..1d198c9 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -144,8 +144,19 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
>  		 */
>  		BUG_ON(!dws->dma_dev || dws->dma_dev != dw->dma.dev);
>  
> -		cfghi |= DWC_CFGH_DST_PER(dws->dst_id);
> -		cfghi |= DWC_CFGH_SRC_PER(dws->src_id);
> +		if (dw->type == DW_DMAC_TYPE_IDMA) {
> +			/* Forces channel FIFO to drain while in suspension */
> +			cfglo = IDMA_CFGL_CH_DRAIN;
> +			/* Burst length aligned */
> +			cfglo |= IDMA_CFGL_SRC_BURST_ALIGN
> +				| IDMA_CFGL_DST_BURST_ALIGN;
> +
> +			cfghi |= IDMA_CFGH_DST_PER(dws->dst_id);
> +			cfghi |= IDMA_CFGH_SRC_PER(dws->src_id);
> +		} else {
> +			cfghi |= DWC_CFGH_DST_PER(dws->dst_id);
> +			cfghi |= DWC_CFGH_SRC_PER(dws->src_id);
> +		}
>  	} else {
>  		cfghi |= DWC_CFGH_DST_PER(dwc->dst_id);
>  		cfghi |= DWC_CFGH_SRC_PER(dwc->src_id); @@ -346,9 +357,14 @@ static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  /* Returns how many bytes were already received from source */  static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)  {
> +	struct dw_dma *dw = to_dw_dma(dwc->chan.device);
> +
>  	u32 ctlhi = channel_readl(dwc, CTL_HI);
>  	u32 ctllo = channel_readl(dwc, CTL_LO);
>  
> +	if (dw->type == DW_DMAC_TYPE_IDMA)
> +		return ctlhi & IDMA_CTLH_BLOCK_TS_MASK;
> +
>  	return (ctlhi & DWC_CTLH_BLOCK_TS_MASK) * (1 << (ctllo >> 4 & 7));  }
>  
> @@ -775,6 +791,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	unsigned int		reg_width;
>  	unsigned int		mem_width;
>  	unsigned int		data_width;
> +	unsigned int            width_trf;
>  	unsigned int		i;
>  	struct scatterlist	*sg;
>  	size_t			total_len = 0;
> @@ -823,8 +840,14 @@ slave_sg_todev_fill_desc:
>  			desc->lli.sar = mem;
>  			desc->lli.dar = reg;
>  			desc->lli.ctllo = ctllo | DWC_CTLL_SRC_WIDTH(mem_width);
> -			if ((len >> mem_width) > dwc->block_size) {
> -				dlen = dwc->block_size << mem_width;
> +
> +			if (dw->type == DW_DMAC_TYPE_IDMA)
> +				width_trf = 0;
why should this be zero?

> +			else
> +				width_trf = mem_width;
> +
> +			if ((len >> width_trf) > dwc->block_size) {
> +				dlen = dwc->block_size << width_trf;
>  				mem += dlen;
>  				len -= dlen;
>  			} else {
> @@ -832,7 +855,7 @@ slave_sg_todev_fill_desc:
>  				len = 0;
>  			}
>  
> -			desc->lli.ctlhi = dlen >> mem_width;
> +			desc->lli.ctlhi = dlen >> width_trf;
>  			desc->len = dlen;
>  
>  			if (!first) {
> @@ -883,15 +906,20 @@ slave_sg_fromdev_fill_desc:
>  			desc->lli.sar = reg;
>  			desc->lli.dar = mem;
>  			desc->lli.ctllo = ctllo | DWC_CTLL_DST_WIDTH(mem_width);
> -			if ((len >> reg_width) > dwc->block_size) {
> -				dlen = dwc->block_size << reg_width;
> +			if (dw->type == DW_DMAC_TYPE_IDMA)
> +				width_trf = 0;
> +			else
> +				width_trf = reg_width;
> +
> +			if ((len >> width_trf) > dwc->block_size) {
> +				dlen = dwc->block_size << width_trf;
>  				mem += dlen;
>  				len -= dlen;
>  			} else {
>  				dlen = len;
>  				len = 0;
>  			}
> -			desc->lli.ctlhi = dlen >> reg_width;
> +			desc->lli.ctlhi = dlen >> width_trf;
>  			desc->len = dlen;
>  
>  			if (!first) {
> @@ -954,10 +982,18 @@ EXPORT_SYMBOL_GPL(dw_dma_filter);
>   *
>   * This can be done by finding least significant bit set: n & (n - 1)
>   */
> -static inline void convert_burst(u32 *maxburst)
> +static inline void convert_burst(struct dw_dma_chan *dwc, u32 
> +*maxburst)
you need dma type so why expose teh channel struct here?

>  {
> +	struct dw_dma *dw = to_dw_dma(dwc->chan.device);
> +	int cvt_base;
> +
> +	if (dw->type == DW_DMAC_TYPE_IDMA)
> +		cvt_base = 1;
> +	else
> +		cvt_base = 2;
> +
>  	if (*maxburst > 1)
> -		*maxburst = fls(*maxburst) - 2;
> +		*maxburst = fls(*maxburst) - cvt_base;
>  	else
>  		*maxburst = 0;
>  }
> @@ -973,8 +1009,8 @@ static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
>  	memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig));
>  	dwc->direction = sconfig->direction;
>  
> -	convert_burst(&dwc->dma_sconfig.src_maxburst);
> -	convert_burst(&dwc->dma_sconfig.dst_maxburst);
> +	convert_burst(dwc, &dwc->dma_sconfig.src_maxburst);
> +	convert_burst(dwc, &dwc->dma_sconfig.dst_maxburst);
>  
>  	return 0;
>  }
> @@ -1513,6 +1549,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
>  		return -ENOMEM;
>  
>  	dw->regs = chip->regs;
> +	dw->type = chip->type;
>  	chip->dw = dw;
>  
>  	pm_runtime_get_sync(chip->dev);
> @@ -1649,6 +1686,9 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
>  				(channel_readl(dwc, LLP) & 0xfffffffc) == 0;
>  			channel_writel(dwc, LLP, 0);
>  		}
> +
> +		if (dw->type == DW_DMAC_TYPE_IDMA)
> +			dwc->nollp = 1;
>  	}
>  
>  	/* Clear all interrupts on all channels. */ diff --git a/drivers/dma/dw/internal.h b/drivers/dma/dw/internal.h index 4143973..ad50981 100644
> --- a/drivers/dma/dw/internal.h
> +++ b/drivers/dma/dw/internal.h
> @@ -15,9 +15,6 @@
>  
>  #include "regs.h"
>  
> -int dw_dma_disable(struct dw_dma_chip *chip); -int dw_dma_enable(struct dw_dma_chip *chip);
> -
can you explain why?

-- 
~Vinod

>  extern bool dw_dma_filter(struct dma_chan *chan, void *param);
>  
>  #endif /* _DMA_DW_INTERNAL_H */
> diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h index 241ff2b..7138f33 100644
> --- a/drivers/dma/dw/regs.h
> +++ b/drivers/dma/dw/regs.h
> @@ -213,6 +213,19 @@ enum dw_dma_msize {
>  /* Bitfields in CFG */
>  #define DW_CFG_DMA_EN		(1 << 0)
>  
> +/* Bitfields in LPSS IDMA CFG_LO */
> +#define IDMA_CFGL_CH_DRAIN     (1 << 10)
> +#define IDMA_CFGL_SRC_BURST_ALIGN      (1 << 1)
> +#define IDMA_CFGL_DST_BURST_ALIGN      (1 << 0)
> +
> +/* Bitfields in LPSS IDMA CFG_HI */
> +#define IDMA_CFGH_SRC_PER(x)   (x)
> +#define IDMA_CFGH_DST_PER(x)   ((x) << 4)
> +
> +/* Bitfields in LPSS IDMA CFG_HI */
> +#define IDMA_CTLH_DONE         0x00020000
> +#define IDMA_CTLH_BLOCK_TS_MASK        0x0001ffff
> +
>  enum dw_dmac_flags {
>  	DW_DMA_IS_CYCLIC = 0,
>  	DW_DMA_IS_SOFT_LLP = 1,
> @@ -282,6 +295,7 @@ struct dw_dma {
>  	struct dw_dma_chan	*chan;
>  	u8			all_chan_mask;
>  	u8			in_use;
> +	unsigned int            type;
>  
>  	/* hardware configuration */
>  	unsigned char		nr_masters;
> diff --git a/include/linux/dma/dw.h b/include/linux/dma/dw.h index 7145644..ccf9c67 100644
> --- a/include/linux/dma/dw.h
> +++ b/include/linux/dma/dw.h
> @@ -20,6 +20,11 @@
>  
>  struct dw_dma;
>  
> +enum dw_dmac_type {
> +	DW_DMAC_TYPE_DW = 0,
> +	DW_DMAC_TYPE_IDMA,
> +};
> +
>  /**
>   * struct dw_dma_chip - representation of DesignWare DMA controller hardware
>   * @dev:		struct device of the DMA controller
> @@ -31,6 +36,7 @@ struct dw_dma;
>  struct dw_dma_chip {
>  	struct device	*dev;
>  	int		irq;
> +	unsigned int    type;
>  	void __iomem	*regs;
>  	struct clk	*clk;
>  	struct dw_dma	*dw;
> @@ -39,6 +45,8 @@ struct dw_dma_chip {
>  /* Export to the platform drivers */
>  int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata);  int dw_dma_remove(struct dw_dma_chip *chip);
> +int dw_dma_disable(struct dw_dma_chip *chip); int dw_dma_enable(struct 
> +dw_dma_chip *chip);
>  
>  /* DMA API extensions */
>  struct dw_desc;
> diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
> index 87ac14c..79b4c43 100644
> --- a/include/linux/platform_data/dma-dw.h
> +++ b/include/linux/platform_data/dma-dw.h
> @@ -53,7 +53,7 @@ struct dw_dma_platform_data {
>  #define CHAN_PRIORITY_ASCENDING		0	/* chan0 highest */
>  #define CHAN_PRIORITY_DESCENDING	1	/* chan7 highest */
>  	unsigned char	chan_priority;
> -	unsigned short	block_size;
> +	unsigned int	block_size;
>  	unsigned char	nr_masters;
>  	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
>  };
> --
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--