[PATCH 1/5] dmaengine: at_hdmac: modify way to use interrupts

[PATCH 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Nicolas Ferre]

Now we use Buffer Transfer Completed interrupts. If we
want a chained buffer completed information, we setup the
ATC_IEN bit in CTRLB register in the lli.
This is done by set_desc_eol() function and used by
memcpy/slave_sg functions.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c      |    4 ++--
 drivers/dma/at_hdmac_regs.h |   11 ++++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 3d7d705..13050e6 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -464,7 +464,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
 
 		for (i = 0; i < atdma->dma_common.chancnt; i++) {
 			atchan = &atdma->chan[i];
-			if (pending & (AT_DMA_CBTC(i) | AT_DMA_ERR(i))) {
+			if (pending & (AT_DMA_BTC(i) | AT_DMA_ERR(i))) {
 				if (pending & AT_DMA_ERR(i)) {
 					/* Disable channel on AHB error */
 					dma_writel(atdma, CHDR, atchan->mask);
@@ -549,7 +549,7 @@ atc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 	}
 
 	ctrla =   ATC_DEFAULT_CTRLA;
-	ctrlb =   ATC_DEFAULT_CTRLB
+	ctrlb =   ATC_DEFAULT_CTRLB | ATC_IEN
 		| ATC_SRC_ADDR_MODE_INCR
 		| ATC_DST_ADDR_MODE_INCR
 		| ATC_FC_MEM2MEM;
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index 495457e..8303306 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -309,8 +309,8 @@ static void atc_setup_irq(struct at_dma_chan *atchan, int on)
 	struct at_dma	*atdma = to_at_dma(atchan->chan_common.device);
 	u32		ebci;
 
-	/* enable interrupts on buffer chain completion & error */
-	ebci =    AT_DMA_CBTC(atchan->chan_common.chan_id)
+	/* enable interrupts on buffer transfer completion & error */
+	ebci =    AT_DMA_BTC(atchan->chan_common.chan_id)
 		| AT_DMA_ERR(atchan->chan_common.chan_id);
 	if (on)
 		dma_writel(atdma, EBCIER, ebci);
@@ -347,7 +347,12 @@ static inline int atc_chan_is_enabled(struct at_dma_chan *atchan)
  */
 static void set_desc_eol(struct at_desc *desc)
 {
-	desc->lli.ctrlb |= ATC_SRC_DSCR_DIS | ATC_DST_DSCR_DIS;
+	u32 ctrlb = desc->lli.ctrlb;
+
+	ctrlb &= ~ATC_IEN;
+	ctrlb |= ATC_SRC_DSCR_DIS | ATC_DST_DSCR_DIS;
+
+	desc->lli.ctrlb = ctrlb;
 	desc->lli.dscr = 0;
 }
 
-- 
1.7.3

[PATCH 2/5] dmaengine: at_hdmac: add cyclic DMA operation support

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c      |  188 +++++++++++++++++++++++++++++++++++++++---
 drivers/dma/at_hdmac_regs.h |   14 +++-
 2 files changed, 186 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 13050e6..fe9e1de 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -14,6 +14,8 @@
  * The driver has currently been tested with the Atmel AT91SAM9RL
  * and AT91SAM9G45 series.
  */
+#define DEBUG 12
+#define VERBOSE_DEBUG 12
 
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
@@ -237,16 +239,12 @@ static void atc_dostart(struct at_dma_chan *atchan, struct at_desc *first)
 static void
 atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
 {
-	dma_async_tx_callback		callback;
-	void				*param;
 	struct dma_async_tx_descriptor	*txd = &desc->txd;
 
 	dev_vdbg(chan2dev(&atchan->chan_common),
 		"descriptor %u complete\n", txd->cookie);
 
 	atchan->completed_cookie = txd->cookie;
-	callback = txd->callback;
-	param = txd->callback_param;
 
 	/* move children to free_list */
 	list_splice_init(&desc->tx_list, &atchan->free_list);
@@ -278,12 +276,19 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
 		}
 	}
 
-	/*
-	 * The API requires that no submissions are done from a
-	 * callback, so we don't need to drop the lock here
-	 */
-	if (callback)
-		callback(param);
+	/* for cyclic transfers,
+	 * no need to replay callback function while stopping */
+	if (!test_bit(ATC_IS_CYCLIC, &atchan->status)) {
+		dma_async_tx_callback	callback = txd->callback;
+		void			*param = txd->callback_param;
+
+		/*
+		 * The API requires that no submissions are done from a
+		 * callback, so we don't need to drop the lock here
+		 */
+		if (callback)
+			callback(param);
+	}
 
 	dma_run_dependencies(txd);
 }
@@ -419,6 +424,26 @@ static void atc_handle_error(struct at_dma_chan *atchan)
 	atc_chain_complete(atchan, bad_desc);
 }
 
+/**
+ * atc_handle_cyclic - at the end of a period, run callback function
+ * @atchan: channel used for cyclic operations
+ *
+ * Called with atchan->lock held and bh disabled
+ */
+static void atc_handle_cyclic(struct at_dma_chan *atchan)
+{
+	struct at_desc			*first = atc_first_active(atchan);
+	struct dma_async_tx_descriptor	*txd = &first->txd;
+	dma_async_tx_callback		callback = txd->callback;
+	void				*param = txd->callback_param;
+
+	dev_vdbg(chan2dev(&atchan->chan_common),
+			"new cyclic period llp 0x%08x\n",
+			channel_readl(atchan, DSCR));
+
+	if (callback)
+		callback(param);
+}
 
 /*--  IRQ & Tasklet  ---------------------------------------------------*/
 
@@ -434,8 +459,10 @@ static void atc_tasklet(unsigned long data)
 	}
 
 	spin_lock(&atchan->lock);
-	if (test_and_clear_bit(0, &atchan->error_status))
+	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
 		atc_handle_error(atchan);
+	else if (test_bit(ATC_IS_CYCLIC, &atchan->status))
+		atc_handle_cyclic(atchan);
 	else
 		atc_advance_work(atchan);
 
@@ -469,7 +496,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
 					/* Disable channel on AHB error */
 					dma_writel(atdma, CHDR, atchan->mask);
 					/* Give information to tasklet */
-					set_bit(0, &atchan->error_status);
+					set_bit(ATC_IS_ERROR, &atchan->status);
 				}
 				tasklet_schedule(&atchan->tasklet);
 				ret = IRQ_HANDLED;
@@ -759,6 +786,127 @@ err_desc_get:
 	return NULL;
 }
 
+/**
+ * atc_dma_cyclic_prep - prepare the cyclic DMA transfer
+ * @chan: the DMA channel to prepare
+ * @buf_addr: physical DMA address where the buffer starts
+ * @buf_len: total number of bytes for the entire buffer
+ * @period_len: number of bytes for each period
+ * @direction: transfer direction, to or from device
+ */
+static struct dma_async_tx_descriptor *
+atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
+		size_t period_len, enum dma_data_direction direction)
+{
+	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
+	struct at_dma_slave	*atslave = chan->private;
+	struct at_desc		*first = NULL;
+	struct at_desc		*prev = NULL;
+	unsigned long		was_cyclic;
+	unsigned int		periods = buf_len / period_len;
+	unsigned int		reg_width;
+	u32			ctrla;
+	u32			ctrlb;
+	unsigned int		i;
+
+	dev_vdbg(chan2dev(chan), "prep_dma_cyclic: %s buf at 0x%08x - %d (%d/%d)\n",
+			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
+			buf_addr,
+			periods, buf_len, period_len);
+
+	if (unlikely(!atslave || !buf_len || !period_len)) {
+		dev_dbg(chan2dev(chan), "prep_dma_cyclic: length is zero!\n");
+		return NULL;
+	}
+
+	was_cyclic = test_and_set_bit(ATC_IS_CYCLIC, &atchan->status);
+	if (was_cyclic) {
+		dev_dbg(chan2dev(chan), "prep_dma_cyclic: channel in use!\n");
+		return NULL;
+	}
+
+	reg_width = atslave->reg_width;
+
+	/* Check for too big/unaligned periods and unaligned DMA buffer */
+	if (period_len > (ATC_BTSIZE_MAX << reg_width))
+		goto err_out;
+	if (unlikely(period_len & ((1 << reg_width) - 1)))
+		goto err_out;
+	if (unlikely(buf_addr & ((1 << reg_width) - 1)))
+		goto err_out;
+	if (unlikely(!(direction & (DMA_TO_DEVICE | DMA_FROM_DEVICE))))
+		goto err_out;
+
+	/* prepare common CRTLA/CTRLB values */
+	ctrla =   ATC_DEFAULT_CTRLA | atslave->ctrla
+		| ATC_DST_WIDTH(reg_width)
+		| ATC_SRC_WIDTH(reg_width)
+		| period_len >> reg_width;
+	ctrlb = ATC_DEFAULT_CTRLB;
+
+	/* build cyclic linked list */
+	for (i = 0; i < periods; i++) {
+		struct at_desc	*desc;
+
+		desc = atc_desc_get(atchan);
+		if (!desc)
+			goto err_desc_get;
+
+		switch (direction) {
+		case DMA_TO_DEVICE:
+			desc->lli.saddr = buf_addr + (period_len * i);
+			desc->lli.daddr = atslave->tx_reg;
+			desc->lli.ctrla = ctrla;
+			desc->lli.ctrlb = ctrlb
+					| ATC_DST_ADDR_MODE_FIXED
+					| ATC_SRC_ADDR_MODE_INCR
+					| ATC_FC_MEM2PER;
+			break;
+
+		case DMA_FROM_DEVICE:
+			desc->lli.saddr = atslave->rx_reg;
+			desc->lli.daddr = buf_addr + (period_len * i);
+			desc->lli.ctrla = ctrla;
+			desc->lli.ctrlb = ctrlb
+					| ATC_DST_ADDR_MODE_INCR
+					| ATC_SRC_ADDR_MODE_FIXED
+					| ATC_FC_PER2MEM;
+			break;
+
+		default:
+			return NULL;
+		}
+
+		if (!first) {
+			first = desc;
+		} else {
+			/* inform the HW lli about chaining */
+			prev->lli.dscr = desc->txd.phys;
+			/* insert the link descriptor to the LD ring */
+			list_add_tail(&desc->desc_node,
+					&first->tx_list);
+		}
+		prev = desc;
+	}
+
+	/* lets make a cyclic list */
+	prev->lli.dscr = first->txd.phys;
+
+	/* First descriptor of the chain embedds additional information */
+	first->txd.cookie = -EBUSY;
+	first->len = buf_len;
+
+	return &first->txd;
+
+err_desc_get:
+	dev_err(chan2dev(chan), "not enough descriptors available\n");
+	atc_desc_put(atchan, first);
+err_out:
+	clear_bit(ATC_IS_CYCLIC, &atchan->status);
+	return NULL;
+}
+
+
 static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 		       unsigned long arg)
 {
@@ -793,6 +941,9 @@ static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 	list_for_each_entry_safe(desc, _desc, &list, desc_node)
 		atc_chain_complete(atchan, desc);
 
+	/* if channel dedicated to cyclic operations, free it */
+	clear_bit(ATC_IS_CYCLIC, &atchan->status);
+
 	spin_unlock_bh(&atchan->lock);
 
 	return 0;
@@ -853,6 +1004,10 @@ static void atc_issue_pending(struct dma_chan *chan)
 
 	dev_vdbg(chan2dev(chan), "issue_pending\n");
 
+	/* Not needed for cyclic transfers */
+	if (test_bit(ATC_IS_CYCLIC, &atchan->status))
+		return;
+
 	spin_lock_bh(&atchan->lock);
 	if (!atc_chan_is_enabled(atchan)) {
 		atc_advance_work(atchan);
@@ -1092,10 +1247,15 @@ static int __init at_dma_probe(struct platform_device *pdev)
 	if (dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask))
 		atdma->dma_common.device_prep_dma_memcpy = atc_prep_dma_memcpy;
 
-	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)) {
+	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask))
 		atdma->dma_common.device_prep_slave_sg = atc_prep_slave_sg;
+
+	if (dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
+		atdma->dma_common.device_prep_dma_cyclic = atc_prep_dma_cyclic;
+
+	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ||
+	    dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
 		atdma->dma_common.device_control = atc_control;
-	}
 
 	dma_writel(atdma, EN, AT_DMA_ENABLE);
 
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index 8303306..c79a9e0 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -181,12 +181,22 @@ txd_to_at_desc(struct dma_async_tx_descriptor *txd)
 /*--  Channels  --------------------------------------------------------*/
 
 /**
+ * atc_status - information bits stored in channel status flag
+ *
+ * Manipulated with atomic operations.
+ */
+enum atc_status {
+	ATC_IS_ERROR = 0,
+	ATC_IS_CYCLIC = 24,
+};
+
+/**
  * struct at_dma_chan - internal representation of an Atmel HDMAC channel
  * @chan_common: common dmaengine channel object members
  * @device: parent device
  * @ch_regs: memory mapped register base
  * @mask: channel index in a mask
- * @error_status: transmit error status information from irq handler
+ * @status: transmit status information from irq/prep* functions
  *                to tasklet (use atomic operations)
  * @tasklet: bottom half to finish transaction work
  * @lock: serializes enqueue/dequeue operations to descriptors lists
@@ -201,7 +211,7 @@ struct at_dma_chan {
 	struct at_dma		*device;
 	void __iomem		*ch_regs;
 	u8			mask;
-	unsigned long		error_status;
+	unsigned long		status;
 	struct tasklet_struct	tasklet;
 
 	spinlock_t		lock;
-- 
1.7.3

[PATCH 2/5] dmaengine: at_hdmac: add cyclic DMA operation support

[Koul, Vinod]

On Fri, 2011-04-01 at 19:19 +0200, Nicolas Ferre wrote:
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  drivers/dma/at_hdmac.c      |  188 +++++++++++++++++++++++++++++++++++++++---
>  drivers/dma/at_hdmac_regs.h |   14 +++-
>  2 files changed, 186 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 13050e6..fe9e1de 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -14,6 +14,8 @@
>   * The driver has currently been tested with the Atmel AT91SAM9RL
>   * and AT91SAM9G45 series.
>   */
> +#define DEBUG 12
> +#define VERBOSE_DEBUG 12
Please don't define here and use debug options available for build

>  
> +/**
> + * atc_handle_cyclic - at the end of a period, run callback function
> + * @atchan: channel used for cyclic operations
> + *
> + * Called with atchan->lock held and bh disabled
> + */
> +static void atc_handle_cyclic(struct at_dma_chan *atchan)
> +{
> +	struct at_desc			*first = atc_first_active(atchan);
> +	struct dma_async_tx_descriptor	*txd = &first->txd;
> +	dma_async_tx_callback		callback = txd->callback;
> +	void				*param = txd->callback_param;
> +
> +	dev_vdbg(chan2dev(&atchan->chan_common),
> +			"new cyclic period llp 0x%08x\n",
> +			channel_readl(atchan, DSCR));
> +
> +	if (callback)
> +		callback(param);
> +}
You dont seem to be doing much expect calling callback, so doesn't it
make sense to write so much code for just calling callback? 
>  
>  /*--  IRQ & Tasklet  ---------------------------------------------------*/
>  
> @@ -434,8 +459,10 @@ static void atc_tasklet(unsigned long data)
>  	}
>  
>  	spin_lock(&atchan->lock);
> -	if (test_and_clear_bit(0, &atchan->error_status))
> +	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
>  		atc_handle_error(atchan);
> +	else if (test_bit(ATC_IS_CYCLIC, &atchan->status))
> +		atc_handle_cyclic(atchan);
>  	else
>  		atc_advance_work(atchan);
>  
> @@ -469,7 +496,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
>  					/* Disable channel on AHB error */
>  					dma_writel(atdma, CHDR, atchan->mask);
>  					/* Give information to tasklet */
> -					set_bit(0, &atchan->error_status);
> +					set_bit(ATC_IS_ERROR, &atchan->status);
>  				}
>  				tasklet_schedule(&atchan->tasklet);
>  				ret = IRQ_HANDLED;
> @@ -759,6 +786,127 @@ err_desc_get:
>  	return NULL;
>  }
>  
> +/**
> + * atc_dma_cyclic_prep - prepare the cyclic DMA transfer
> + * @chan: the DMA channel to prepare
> + * @buf_addr: physical DMA address where the buffer starts
> + * @buf_len: total number of bytes for the entire buffer
> + * @period_len: number of bytes for each period
> + * @direction: transfer direction, to or from device
> + */
> +static struct dma_async_tx_descriptor *
> +atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> +		size_t period_len, enum dma_data_direction direction)
> +{
> +	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
> +	struct at_dma_slave	*atslave = chan->private;
> +	struct at_desc		*first = NULL;
> +	struct at_desc		*prev = NULL;
> +	unsigned long		was_cyclic;
> +	unsigned int		periods = buf_len / period_len;
> +	unsigned int		reg_width;
> +	u32			ctrla;
> +	u32			ctrlb;
> +	unsigned int		i;
> +
> +	dev_vdbg(chan2dev(chan), "prep_dma_cyclic: %s buf at 0x%08x - %d (%d/%d)\n",
> +			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
> +			buf_addr,
> +			periods, buf_len, period_len);
> +
> +	if (unlikely(!atslave || !buf_len || !period_len)) {
> +		dev_dbg(chan2dev(chan), "prep_dma_cyclic: length is zero!\n");
> +		return NULL;
> +	}
> +
> +	was_cyclic = test_and_set_bit(ATC_IS_CYCLIC, &atchan->status);
> +	if (was_cyclic) {
> +		dev_dbg(chan2dev(chan), "prep_dma_cyclic: channel in use!\n");
> +		return NULL;
> +	}
> +
> +	reg_width = atslave->reg_width;
> +
> +	/* Check for too big/unaligned periods and unaligned DMA buffer */
> +	if (period_len > (ATC_BTSIZE_MAX << reg_width))
> +		goto err_out;
> +	if (unlikely(period_len & ((1 << reg_width) - 1)))
> +		goto err_out;
> +	if (unlikely(buf_addr & ((1 << reg_width) - 1)))
> +		goto err_out;
> +	if (unlikely(!(direction & (DMA_TO_DEVICE | DMA_FROM_DEVICE))))
> +		goto err_out;
> +
> +	/* prepare common CRTLA/CTRLB values */
> +	ctrla =   ATC_DEFAULT_CTRLA | atslave->ctrla
> +		| ATC_DST_WIDTH(reg_width)
> +		| ATC_SRC_WIDTH(reg_width)
> +		| period_len >> reg_width;
> +	ctrlb = ATC_DEFAULT_CTRLB;
> +
> +	/* build cyclic linked list */
> +	for (i = 0; i < periods; i++) {
would help making code look cleaner if you split this one to seprate
function
> +		struct at_desc	*desc;
> +
> +		desc = atc_desc_get(atchan);
> +		if (!desc)
> +			goto err_desc_get;
> +
> +		switch (direction) {
> +		case DMA_TO_DEVICE:
> +			desc->lli.saddr = buf_addr + (period_len * i);
> +			desc->lli.daddr = atslave->tx_reg;
> +			desc->lli.ctrla = ctrla;
> +			desc->lli.ctrlb = ctrlb
> +					| ATC_DST_ADDR_MODE_FIXED
> +					| ATC_SRC_ADDR_MODE_INCR
> +					| ATC_FC_MEM2PER;
> +			break;
> +
> +		case DMA_FROM_DEVICE:
> +			desc->lli.saddr = atslave->rx_reg;
> +			desc->lli.daddr = buf_addr + (period_len * i);
> +			desc->lli.ctrla = ctrla;
> +			desc->lli.ctrlb = ctrlb
> +					| ATC_DST_ADDR_MODE_INCR
> +					| ATC_SRC_ADDR_MODE_FIXED
> +					| ATC_FC_PER2MEM;
> +			break;
> +
> +		default:
> +			return NULL;
> +		}
> +
> +		if (!first) {
> +			first = desc;
> +		} else {
> +			/* inform the HW lli about chaining */
> +			prev->lli.dscr = desc->txd.phys;
> +			/* insert the link descriptor to the LD ring */
> +			list_add_tail(&desc->desc_node,
> +					&first->tx_list);
> +		}
> +		prev = desc;
> +	}
> +
> +	/* lets make a cyclic list */
> +	prev->lli.dscr = first->txd.phys;
> +
> +	/* First descriptor of the chain embedds additional information */
> +	first->txd.cookie = -EBUSY;
> +	first->len = buf_len;
> +
> +	return &first->txd;
> +
> +err_desc_get:
> +	dev_err(chan2dev(chan), "not enough descriptors available\n");
> +	atc_desc_put(atchan, first);
> +err_out:
> +	clear_bit(ATC_IS_CYCLIC, &atchan->status);
> +	return NULL;
> +}
> +
> +
>  static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  		       unsigned long arg)
>  {
> @@ -793,6 +941,9 @@ static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  	list_for_each_entry_safe(desc, _desc, &list, desc_node)
>  		atc_chain_complete(atchan, desc);
>  
> +	/* if channel dedicated to cyclic operations, free it */
> +	clear_bit(ATC_IS_CYCLIC, &atchan->status);
> +
>  	spin_unlock_bh(&atchan->lock);
>  
>  	return 0;
> @@ -853,6 +1004,10 @@ static void atc_issue_pending(struct dma_chan *chan)
>  
>  	dev_vdbg(chan2dev(chan), "issue_pending\n");
>  
> +	/* Not needed for cyclic transfers */
> +	if (test_bit(ATC_IS_CYCLIC, &atchan->status))
> +		return;
> +
>  	spin_lock_bh(&atchan->lock);
>  	if (!atc_chan_is_enabled(atchan)) {
>  		atc_advance_work(atchan);
> @@ -1092,10 +1247,15 @@ static int __init at_dma_probe(struct platform_device *pdev)
>  	if (dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask))
>  		atdma->dma_common.device_prep_dma_memcpy = atc_prep_dma_memcpy;
>  
> -	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)) {
> +	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask))
>  		atdma->dma_common.device_prep_slave_sg = atc_prep_slave_sg;
> +
> +	if (dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
> +		atdma->dma_common.device_prep_dma_cyclic = atc_prep_dma_cyclic;
> +
> +	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ||
> +	    dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
>  		atdma->dma_common.device_control = atc_control;
> -	}
>  
>  	dma_writel(atdma, EN, AT_DMA_ENABLE);
>  
> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
> index 8303306..c79a9e0 100644
> --- a/drivers/dma/at_hdmac_regs.h
> +++ b/drivers/dma/at_hdmac_regs.h
> @@ -181,12 +181,22 @@ txd_to_at_desc(struct dma_async_tx_descriptor *txd)
>  /*--  Channels  --------------------------------------------------------*/
>  
>  /**
> + * atc_status - information bits stored in channel status flag
> + *
> + * Manipulated with atomic operations.
> + */
> +enum atc_status {
> +	ATC_IS_ERROR = 0,
> +	ATC_IS_CYCLIC = 24,
> +};
> +
> +/**
>   * struct at_dma_chan - internal representation of an Atmel HDMAC channel
>   * @chan_common: common dmaengine channel object members
>   * @device: parent device
>   * @ch_regs: memory mapped register base
>   * @mask: channel index in a mask
> - * @error_status: transmit error status information from irq handler
> + * @status: transmit status information from irq/prep* functions
>   *                to tasklet (use atomic operations)
>   * @tasklet: bottom half to finish transaction work
>   * @lock: serializes enqueue/dequeue operations to descriptors lists
> @@ -201,7 +211,7 @@ struct at_dma_chan {
>  	struct at_dma		*device;
>  	void __iomem		*ch_regs;
>  	u8			mask;
> -	unsigned long		error_status;
> +	unsigned long		status;
>  	struct tasklet_struct	tasklet;
>  
>  	spinlock_t		lock;



-- 
~Vinod

[PATCH 2/5] dmaengine: at_hdmac: add cyclic DMA operation support

[Nicolas Ferre]

Le 06/04/2011 12:08, Koul, Vinod :
> On Fri, 2011-04-01 at 19:19 +0200, Nicolas Ferre wrote:
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> ---
>>  drivers/dma/at_hdmac.c      |  188 +++++++++++++++++++++++++++++++++++++++---
>>  drivers/dma/at_hdmac_regs.h |   14 +++-
>>  2 files changed, 186 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
>> index 13050e6..fe9e1de 100644
>> --- a/drivers/dma/at_hdmac.c
>> +++ b/drivers/dma/at_hdmac.c
>> @@ -14,6 +14,8 @@
>>   * The driver has currently been tested with the Atmel AT91SAM9RL
>>   * and AT91SAM9G45 series.
>>   */
>> +#define DEBUG 12
>> +#define VERBOSE_DEBUG 12
> Please don't define here and use debug options available for build

Yes, sure sorry.

>> +/**
>> + * atc_handle_cyclic - at the end of a period, run callback function
>> + * @atchan: channel used for cyclic operations
>> + *
>> + * Called with atchan->lock held and bh disabled
>> + */
>> +static void atc_handle_cyclic(struct at_dma_chan *atchan)
>> +{
>> +	struct at_desc			*first = atc_first_active(atchan);
>> +	struct dma_async_tx_descriptor	*txd = &first->txd;
>> +	dma_async_tx_callback		callback = txd->callback;
>> +	void				*param = txd->callback_param;
>> +
>> +	dev_vdbg(chan2dev(&atchan->chan_common),
>> +			"new cyclic period llp 0x%08x\n",
>> +			channel_readl(atchan, DSCR));
>> +
>> +	if (callback)
>> +		callback(param);
>> +}
> You dont seem to be doing much expect calling callback, so doesn't it
> make sense to write so much code for just calling callback? 

I do not totally follow you here: you mean that I should reduce the
amount of variables in this function or is it a more global comment
about my way of handling cyclic operations?

>>  
>>  /*--  IRQ & Tasklet  ---------------------------------------------------*/
>>  
>> @@ -434,8 +459,10 @@ static void atc_tasklet(unsigned long data)
>>  	}
>>  
>>  	spin_lock(&atchan->lock);
>> -	if (test_and_clear_bit(0, &atchan->error_status))
>> +	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
>>  		atc_handle_error(atchan);
>> +	else if (test_bit(ATC_IS_CYCLIC, &atchan->status))
>> +		atc_handle_cyclic(atchan);
>>  	else
>>  		atc_advance_work(atchan);
>>  
>> @@ -469,7 +496,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
>>  					/* Disable channel on AHB error */
>>  					dma_writel(atdma, CHDR, atchan->mask);
>>  					/* Give information to tasklet */
>> -					set_bit(0, &atchan->error_status);
>> +					set_bit(ATC_IS_ERROR, &atchan->status);
>>  				}
>>  				tasklet_schedule(&atchan->tasklet);
>>  				ret = IRQ_HANDLED;
>> @@ -759,6 +786,127 @@ err_desc_get:
>>  	return NULL;
>>  }
>>  
>> +/**
>> + * atc_dma_cyclic_prep - prepare the cyclic DMA transfer
>> + * @chan: the DMA channel to prepare
>> + * @buf_addr: physical DMA address where the buffer starts
>> + * @buf_len: total number of bytes for the entire buffer
>> + * @period_len: number of bytes for each period
>> + * @direction: transfer direction, to or from device
>> + */
>> +static struct dma_async_tx_descriptor *
>> +atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
>> +		size_t period_len, enum dma_data_direction direction)
>> +{
>> +	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
>> +	struct at_dma_slave	*atslave = chan->private;
>> +	struct at_desc		*first = NULL;
>> +	struct at_desc		*prev = NULL;
>> +	unsigned long		was_cyclic;
>> +	unsigned int		periods = buf_len / period_len;
>> +	unsigned int		reg_width;
>> +	u32			ctrla;
>> +	u32			ctrlb;
>> +	unsigned int		i;
>> +
>> +	dev_vdbg(chan2dev(chan), "prep_dma_cyclic: %s buf at 0x%08x - %d (%d/%d)\n",
>> +			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
>> +			buf_addr,
>> +			periods, buf_len, period_len);
>> +
>> +	if (unlikely(!atslave || !buf_len || !period_len)) {
>> +		dev_dbg(chan2dev(chan), "prep_dma_cyclic: length is zero!\n");
>> +		return NULL;
>> +	}
>> +
>> +	was_cyclic = test_and_set_bit(ATC_IS_CYCLIC, &atchan->status);
>> +	if (was_cyclic) {
>> +		dev_dbg(chan2dev(chan), "prep_dma_cyclic: channel in use!\n");
>> +		return NULL;
>> +	}
>> +
>> +	reg_width = atslave->reg_width;
>> +
>> +	/* Check for too big/unaligned periods and unaligned DMA buffer */
>> +	if (period_len > (ATC_BTSIZE_MAX << reg_width))
>> +		goto err_out;
>> +	if (unlikely(period_len & ((1 << reg_width) - 1)))
>> +		goto err_out;
>> +	if (unlikely(buf_addr & ((1 << reg_width) - 1)))
>> +		goto err_out;
>> +	if (unlikely(!(direction & (DMA_TO_DEVICE | DMA_FROM_DEVICE))))
>> +		goto err_out;
>> +
>> +	/* prepare common CRTLA/CTRLB values */
>> +	ctrla =   ATC_DEFAULT_CTRLA | atslave->ctrla
>> +		| ATC_DST_WIDTH(reg_width)
>> +		| ATC_SRC_WIDTH(reg_width)
>> +		| period_len >> reg_width;
>> +	ctrlb = ATC_DEFAULT_CTRLB;
>> +
>> +	/* build cyclic linked list */
>> +	for (i = 0; i < periods; i++) {
> would help making code look cleaner if you split this one to seprate
> function

Ok, I will see what I can do... and will post a v2 soon.

>> +		struct at_desc	*desc;
>> +
>> +		desc = atc_desc_get(atchan);
>> +		if (!desc)
>> +			goto err_desc_get;
>> +
>> +		switch (direction) {
>> +		case DMA_TO_DEVICE:
>> +			desc->lli.saddr = buf_addr + (period_len * i);
>> +			desc->lli.daddr = atslave->tx_reg;
>> +			desc->lli.ctrla = ctrla;
>> +			desc->lli.ctrlb = ctrlb
>> +					| ATC_DST_ADDR_MODE_FIXED
>> +					| ATC_SRC_ADDR_MODE_INCR
>> +					| ATC_FC_MEM2PER;
>> +			break;
>> +
>> +		case DMA_FROM_DEVICE:
>> +			desc->lli.saddr = atslave->rx_reg;
>> +			desc->lli.daddr = buf_addr + (period_len * i);
>> +			desc->lli.ctrla = ctrla;
>> +			desc->lli.ctrlb = ctrlb
>> +					| ATC_DST_ADDR_MODE_INCR
>> +					| ATC_SRC_ADDR_MODE_FIXED
>> +					| ATC_FC_PER2MEM;
>> +			break;
>> +
>> +		default:
>> +			return NULL;
>> +		}
>> +
>> +		if (!first) {
>> +			first = desc;
>> +		} else {
>> +			/* inform the HW lli about chaining */
>> +			prev->lli.dscr = desc->txd.phys;
>> +			/* insert the link descriptor to the LD ring */
>> +			list_add_tail(&desc->desc_node,
>> +					&first->tx_list);
>> +		}
>> +		prev = desc;
>> +	}
>> +
>> +	/* lets make a cyclic list */
>> +	prev->lli.dscr = first->txd.phys;
>> +
>> +	/* First descriptor of the chain embedds additional information */
>> +	first->txd.cookie = -EBUSY;
>> +	first->len = buf_len;
>> +
>> +	return &first->txd;
>> +
>> +err_desc_get:
>> +	dev_err(chan2dev(chan), "not enough descriptors available\n");
>> +	atc_desc_put(atchan, first);
>> +err_out:
>> +	clear_bit(ATC_IS_CYCLIC, &atchan->status);
>> +	return NULL;
>> +}
>> +
>> +
>>  static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>>  		       unsigned long arg)
>>  {
>> @@ -793,6 +941,9 @@ static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>>  	list_for_each_entry_safe(desc, _desc, &list, desc_node)
>>  		atc_chain_complete(atchan, desc);
>>  
>> +	/* if channel dedicated to cyclic operations, free it */
>> +	clear_bit(ATC_IS_CYCLIC, &atchan->status);
>> +
>>  	spin_unlock_bh(&atchan->lock);
>>  
>>  	return 0;
>> @@ -853,6 +1004,10 @@ static void atc_issue_pending(struct dma_chan *chan)
>>  
>>  	dev_vdbg(chan2dev(chan), "issue_pending\n");
>>  
>> +	/* Not needed for cyclic transfers */
>> +	if (test_bit(ATC_IS_CYCLIC, &atchan->status))
>> +		return;
>> +
>>  	spin_lock_bh(&atchan->lock);
>>  	if (!atc_chan_is_enabled(atchan)) {
>>  		atc_advance_work(atchan);
>> @@ -1092,10 +1247,15 @@ static int __init at_dma_probe(struct platform_device *pdev)
>>  	if (dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask))
>>  		atdma->dma_common.device_prep_dma_memcpy = atc_prep_dma_memcpy;
>>  
>> -	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)) {
>> +	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask))
>>  		atdma->dma_common.device_prep_slave_sg = atc_prep_slave_sg;
>> +
>> +	if (dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
>> +		atdma->dma_common.device_prep_dma_cyclic = atc_prep_dma_cyclic;
>> +
>> +	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ||
>> +	    dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
>>  		atdma->dma_common.device_control = atc_control;
>> -	}
>>  
>>  	dma_writel(atdma, EN, AT_DMA_ENABLE);
>>  
>> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
>> index 8303306..c79a9e0 100644
>> --- a/drivers/dma/at_hdmac_regs.h
>> +++ b/drivers/dma/at_hdmac_regs.h
>> @@ -181,12 +181,22 @@ txd_to_at_desc(struct dma_async_tx_descriptor *txd)
>>  /*--  Channels  --------------------------------------------------------*/
>>  
>>  /**
>> + * atc_status - information bits stored in channel status flag
>> + *
>> + * Manipulated with atomic operations.
>> + */
>> +enum atc_status {
>> +	ATC_IS_ERROR = 0,
>> +	ATC_IS_CYCLIC = 24,
>> +};
>> +
>> +/**
>>   * struct at_dma_chan - internal representation of an Atmel HDMAC channel
>>   * @chan_common: common dmaengine channel object members
>>   * @device: parent device
>>   * @ch_regs: memory mapped register base
>>   * @mask: channel index in a mask
>> - * @error_status: transmit error status information from irq handler
>> + * @status: transmit status information from irq/prep* functions
>>   *                to tasklet (use atomic operations)
>>   * @tasklet: bottom half to finish transaction work
>>   * @lock: serializes enqueue/dequeue operations to descriptors lists
>> @@ -201,7 +211,7 @@ struct at_dma_chan {
>>  	struct at_dma		*device;
>>  	void __iomem		*ch_regs;
>>  	u8			mask;
>> -	unsigned long		error_status;
>> +	unsigned long		status;
>>  	struct tasklet_struct	tasklet;
>>  
>>  	spinlock_t		lock;


Thanks for your review, best regards,
-- 
Nicolas Ferre

[PATCH 2/5] dmaengine: at_hdmac: add cyclic DMA operation support

[Koul, Vinod]

On Wed, 2011-04-06 at 14:50 +0200, Nicolas Ferre wrote:
> Le 06/04/2011 12:08, Koul, Vinod :
> > On Fri, 2011-04-01 at 19:19 +0200, Nicolas Ferre wrote:
> >> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> >> ---
> >>  drivers/dma/at_hdmac.c      |  188 +++++++++++++++++++++++++++++++++++++++---
> >>  drivers/dma/at_hdmac_regs.h |   14 +++-
> >>  2 files changed, 186 insertions(+), 16 deletions(-)
> >>
> >> +static void atc_handle_cyclic(struct at_dma_chan *atchan)
> >> +{
> >> +	struct at_desc			*first = atc_first_active(atchan);
> >> +	struct dma_async_tx_descriptor	*txd = &first->txd;
> >> +	dma_async_tx_callback		callback = txd->callback;
> >> +	void				*param = txd->callback_param;
> >> +
> >> +	dev_vdbg(chan2dev(&atchan->chan_common),
> >> +			"new cyclic period llp 0x%08x\n",
> >> +			channel_readl(atchan, DSCR));
> >> +
> >> +	if (callback)
> >> +		callback(param);
> >> +}
> > You dont seem to be doing much expect calling callback, so doesn't it
> > make sense to write so much code for just calling callback? 
> 
> I do not totally follow you here: you mean that I should reduce the
> amount of variables in this function or is it a more global comment
> about my way of handling cyclic operations?
Well, in handling of cyclic operation you are only calling the callback
if set. If you plan to add more functionality to this function in future
then okay, otherwise you may reconsider this and avoid the function as
you are not doing much here, if it reduces code size. This is okay this
way also.
I had no comment on variables as I understand first two are unavoidable
> 
> >>  
> >>  /*--  IRQ & Tasklet  ---------------------------------------------------*/
> >>  
> >> @@ -434,8 +459,10 @@ static void atc_tasklet(unsigned long data)
> >>  	}
> >>  
> >>  	spin_lock(&atchan->lock);
> >> -	if (test_and_clear_bit(0, &atchan->error_status))
> >> +	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
> >>  		atc_handle_error(atchan);
> >> +	else if (test_bit(ATC_IS_CYCLIC, &atchan->status))
> >> +		atc_handle_cyclic(atchan);
> >>  	else
> >>  		atc_advance_work(atchan);
> >>  
> >> @@ -469,7 +496,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
> >>  					/* Disable channel on AHB error */
> >>  					dma_writel(atdma, CHDR, atchan->mask);
> >>  					/* Give information to tasklet */
> >> -					set_bit(0, &atchan->error_status);
> >> +					set_bit(ATC_IS_ERROR, &atchan->status);
> >>  				}
> >>  				tasklet_schedule(&atchan->tasklet);
> >>  				ret = IRQ_HANDLED;
> >> @@ -759,6 +786,127 @@ err_desc_get:
> >>  	return NULL;
> >>  }
> >>  
> >> +/**
> >> + * atc_dma_cyclic_prep - prepare the cyclic DMA transfer
> >> + * @chan: the DMA channel to prepare
> >> + * @buf_addr: physical DMA address where the buffer starts
> >> + * @buf_len: total number of bytes for the entire buffer
> >> + * @period_len: number of bytes for each period
> >> + * @direction: transfer direction, to or from device
> >> + */
> >> +static struct dma_async_tx_descriptor *
> >> +atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> >> +		size_t period_len, enum dma_data_direction direction)
> >> +{
> >> +	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
> >> +	struct at_dma_slave	*atslave = chan->private;
> >> +	struct at_desc		*first = NULL;
> >> +	struct at_desc		*prev = NULL;
> >> +	unsigned long		was_cyclic;
> >> +	unsigned int		periods = buf_len / period_len;
> >> +	unsigned int		reg_width;
> >> +	u32			ctrla;
> >> +	u32			ctrlb;
> >> +	unsigned int		i;
> >> +
> >> +	dev_vdbg(chan2dev(chan), "prep_dma_cyclic: %s buf at 0x%08x - %d (%d/%d)\n",
> >> +			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
> >> +			buf_addr,
> >> +			periods, buf_len, period_len);
> >> +
> >> +	if (unlikely(!atslave || !buf_len || !period_len)) {
> >> +		dev_dbg(chan2dev(chan), "prep_dma_cyclic: length is zero!\n");
> >> +		return NULL;
> >> +	}
> >> +
> >> +	was_cyclic = test_and_set_bit(ATC_IS_CYCLIC, &atchan->status);
> >> +	if (was_cyclic) {
> >> +		dev_dbg(chan2dev(chan), "prep_dma_cyclic: channel in use!\n");
> >> +		return NULL;
> >> +	}
> >> +
> >> +	reg_width = atslave->reg_width;
> >> +
> >> +	/* Check for too big/unaligned periods and unaligned DMA buffer */
> >> +	if (period_len > (ATC_BTSIZE_MAX << reg_width))
> >> +		goto err_out;
> >> +	if (unlikely(period_len & ((1 << reg_width) - 1)))
> >> +		goto err_out;
> >> +	if (unlikely(buf_addr & ((1 << reg_width) - 1)))
> >> +		goto err_out;
> >> +	if (unlikely(!(direction & (DMA_TO_DEVICE | DMA_FROM_DEVICE))))
> >> +		goto err_out;
> >> +
> >> +	/* prepare common CRTLA/CTRLB values */
> >> +	ctrla =   ATC_DEFAULT_CTRLA | atslave->ctrla
> >> +		| ATC_DST_WIDTH(reg_width)
> >> +		| ATC_SRC_WIDTH(reg_width)
> >> +		| period_len >> reg_width;
> >> +	ctrlb = ATC_DEFAULT_CTRLB;
> >> +
> >> +	/* build cyclic linked list */
> >> +	for (i = 0; i < periods; i++) {
> > would help making code look cleaner if you split this one to seprate
> > function
> 
> Ok, I will see what I can do... and will post a v2 soon.
> 
> >> +		struct at_desc	*desc;
> >> +
> >> +		desc = atc_desc_get(atchan);
> >> +		if (!desc)
> >> +			goto err_desc_get;
> >> +
> >> +		switch (direction) {
> >> +		case DMA_TO_DEVICE:
> >> +			desc->lli.saddr = buf_addr + (period_len * i);
> >> +			desc->lli.daddr = atslave->tx_reg;
> >> +			desc->lli.ctrla = ctrla;
> >> +			desc->lli.ctrlb = ctrlb
> >> +					| ATC_DST_ADDR_MODE_FIXED
> >> +					| ATC_SRC_ADDR_MODE_INCR
> >> +					| ATC_FC_MEM2PER;
> >> +			break;
> >> +
> >> +		case DMA_FROM_DEVICE:
> >> +			desc->lli.saddr = atslave->rx_reg;
> >> +			desc->lli.daddr = buf_addr + (period_len * i);
> >> +			desc->lli.ctrla = ctrla;
> >> +			desc->lli.ctrlb = ctrlb
> >> +					| ATC_DST_ADDR_MODE_INCR
> >> +					| ATC_SRC_ADDR_MODE_FIXED
> >> +					| ATC_FC_PER2MEM;
> >> +			break;
> >> +
> >> +		default:
> >> +			return NULL;
> >> +		}
> >> +
> >> +		if (!first) {
> >> +			first = desc;
> >> +		} else {
> >> +			/* inform the HW lli about chaining */
> >> +			prev->lli.dscr = desc->txd.phys;
> >> +			/* insert the link descriptor to the LD ring */
> >> +			list_add_tail(&desc->desc_node,
> >> +					&first->tx_list);
> >> +		}
> >> +		prev = desc;
> >> +	}
> >> +
> >> +	/* lets make a cyclic list */
> >> +	prev->lli.dscr = first->txd.phys;
> >> +
> >> +	/* First descriptor of the chain embedds additional information */
> >> +	first->txd.cookie = -EBUSY;
> >> +	first->len = buf_len;
> >> +
> >> +	return &first->txd;
> >> +
> >> +err_desc_get:
> >> +	dev_err(chan2dev(chan), "not enough descriptors available\n");
> >> +	atc_desc_put(atchan, first);
> >> +err_out:
> >> +	clear_bit(ATC_IS_CYCLIC, &atchan->status);
> >> +	return NULL;
> >> +}
> >> +
> >> +
> >>  static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> >>  		       unsigned long arg)
> >>  {
> >> @@ -793,6 +941,9 @@ static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> >>  	list_for_each_entry_safe(desc, _desc, &list, desc_node)
> >>  		atc_chain_complete(atchan, desc);
> >>  
> >> +	/* if channel dedicated to cyclic operations, free it */
> >> +	clear_bit(ATC_IS_CYCLIC, &atchan->status);
> >> +
> >>  	spin_unlock_bh(&atchan->lock);
> >>  
> >>  	return 0;
> >> @@ -853,6 +1004,10 @@ static void atc_issue_pending(struct dma_chan *chan)
> >>  
> >>  	dev_vdbg(chan2dev(chan), "issue_pending\n");
> >>  
> >> +	/* Not needed for cyclic transfers */
> >> +	if (test_bit(ATC_IS_CYCLIC, &atchan->status))
> >> +		return;
> >> +
> >>  	spin_lock_bh(&atchan->lock);
> >>  	if (!atc_chan_is_enabled(atchan)) {
> >>  		atc_advance_work(atchan);
> >> @@ -1092,10 +1247,15 @@ static int __init at_dma_probe(struct platform_device *pdev)
> >>  	if (dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask))
> >>  		atdma->dma_common.device_prep_dma_memcpy = atc_prep_dma_memcpy;
> >>  
> >> -	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)) {
> >> +	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask))
> >>  		atdma->dma_common.device_prep_slave_sg = atc_prep_slave_sg;
> >> +
> >> +	if (dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
> >> +		atdma->dma_common.device_prep_dma_cyclic = atc_prep_dma_cyclic;
> >> +
> >> +	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ||
> >> +	    dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
> >>  		atdma->dma_common.device_control = atc_control;
> >> -	}
> >>  
> >>  	dma_writel(atdma, EN, AT_DMA_ENABLE);
> >>  
> >> diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
> >> index 8303306..c79a9e0 100644
> >> --- a/drivers/dma/at_hdmac_regs.h
> >> +++ b/drivers/dma/at_hdmac_regs.h
> >> @@ -181,12 +181,22 @@ txd_to_at_desc(struct dma_async_tx_descriptor *txd)
> >>  /*--  Channels  --------------------------------------------------------*/
> >>  
> >>  /**
> >> + * atc_status - information bits stored in channel status flag
> >> + *
> >> + * Manipulated with atomic operations.
> >> + */
> >> +enum atc_status {
> >> +	ATC_IS_ERROR = 0,
> >> +	ATC_IS_CYCLIC = 24,
> >> +};
> >> +
> >> +/**
> >>   * struct at_dma_chan - internal representation of an Atmel HDMAC channel
> >>   * @chan_common: common dmaengine channel object members
> >>   * @device: parent device
> >>   * @ch_regs: memory mapped register base
> >>   * @mask: channel index in a mask
> >> - * @error_status: transmit error status information from irq handler
> >> + * @status: transmit status information from irq/prep* functions
> >>   *                to tasklet (use atomic operations)
> >>   * @tasklet: bottom half to finish transaction work
> >>   * @lock: serializes enqueue/dequeue operations to descriptors lists
> >> @@ -201,7 +211,7 @@ struct at_dma_chan {
> >>  	struct at_dma		*device;
> >>  	void __iomem		*ch_regs;
> >>  	u8			mask;
> >> -	unsigned long		error_status;
> >> +	unsigned long		status;
> >>  	struct tasklet_struct	tasklet;
> >>  
> >>  	spinlock_t		lock;
> 
> 
> Thanks for your review, best regards,


-- 
~Vinod

[PATCH 2/5] dmaengine: at_hdmac: add cyclic DMA operation support

[Nicolas Ferre]

Le 06/04/2011 16:02, Koul, Vinod :
> On Wed, 2011-04-06 at 14:50 +0200, Nicolas Ferre wrote:
>> Le 06/04/2011 12:08, Koul, Vinod :
>>> On Fri, 2011-04-01 at 19:19 +0200, Nicolas Ferre wrote:
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> ---
>>>>  drivers/dma/at_hdmac.c      |  188 +++++++++++++++++++++++++++++++++++++++---
>>>>  drivers/dma/at_hdmac_regs.h |   14 +++-
>>>>  2 files changed, 186 insertions(+), 16 deletions(-)
>>>>
>>>> +static void atc_handle_cyclic(struct at_dma_chan *atchan)
>>>> +{
>>>> +	struct at_desc			*first = atc_first_active(atchan);
>>>> +	struct dma_async_tx_descriptor	*txd = &first->txd;
>>>> +	dma_async_tx_callback		callback = txd->callback;
>>>> +	void				*param = txd->callback_param;
>>>> +
>>>> +	dev_vdbg(chan2dev(&atchan->chan_common),
>>>> +			"new cyclic period llp 0x%08x\n",
>>>> +			channel_readl(atchan, DSCR));
>>>> +
>>>> +	if (callback)
>>>> +		callback(param);
>>>> +}
>>> You dont seem to be doing much expect calling callback, so doesn't it
>>> make sense to write so much code for just calling callback? 
>>
>> I do not totally follow you here: you mean that I should reduce the
>> amount of variables in this function or is it a more global comment
>> about my way of handling cyclic operations?
> Well, in handling of cyclic operation you are only calling the callback
> if set. If you plan to add more functionality to this function in future
> then okay, otherwise you may reconsider this and avoid the function as
> you are not doing much here, if it reduces code size. This is okay this
> way also.
> I had no comment on variables as I understand first two are unavoidable

As far as code size and execution path is concerned, this function is
expanded by the compiler as an inline one and I checked that it only
takes a few lines of assembly (without function call) if the debug trace
is not requested (~7 lines into atc_tasklet() with one call to
atc_first_active() which is needed anyway).

I would prefer to keep this simple "pseudo-inline" function just for a
matter of code clarity.

Best regards,
-- 
Nicolas Ferre

[PATCH 3/5] dmaengine: at_hdmac: debug information sg_len for prep_slave_sg

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index fe9e1de..ce1d1a5 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -666,7 +666,8 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	struct scatterlist	*sg;
 	size_t			total_len = 0;
 
-	dev_vdbg(chan2dev(chan), "prep_slave_sg: %s f0x%lx\n",
+	dev_vdbg(chan2dev(chan), "prep_slave_sg (%d): %s f0x%lx\n",
+			sg_len,
 			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
 			flags);
 
-- 
1.7.3

[PATCH 4/5] dmaengine: at_hdmac: remove channel status testing in tasklet

[Nicolas Ferre]

There is no need to test if channel is enabled in tasklet:
- in error path, channel is disabled in interrupt routine
- in normal path, this test is performed in sub functions to report
a misuse of the engine.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index ce1d1a5..6d0fb1f 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -451,13 +451,6 @@ static void atc_tasklet(unsigned long data)
 {
 	struct at_dma_chan *atchan = (struct at_dma_chan *)data;
 
-	/* Channel cannot be enabled here */
-	if (atc_chan_is_enabled(atchan)) {
-		dev_err(chan2dev(&atchan->chan_common),
-			"BUG: channel enabled in tasklet\n");
-		return;
-	}
-
 	spin_lock(&atchan->lock);
 	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
 		atc_handle_error(atchan);
-- 
1.7.3

[PATCH 5/5] dmaengine: at_hdmac: specialize AHB interfaces to optimize transfers

[Nicolas Ferre]

DMA controller has two AHB interfaces on the SOC internal
matrix.
It is more efficient to specialize each interface as the
access to memory can introduce latencies that are not compatible
with peripheral accesses requirements.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c      |   20 ++++++++++----------
 drivers/dma/at_hdmac_regs.h |    2 ++
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 6d0fb1f..634cca7 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -39,8 +39,7 @@
 
 #define	ATC_DEFAULT_CFG		(ATC_FIFOCFG_HALFFIFO)
 #define	ATC_DEFAULT_CTRLA	(0)
-#define	ATC_DEFAULT_CTRLB	(ATC_SIF(0)	\
-				|ATC_DIF(1))
+#define	ATC_DEFAULT_CTRLB	(ATC_SIF(MEM_IF) | ATC_DIF(MEM_IF))
 
 /*
  * Initial number of descriptors to allocate for each channel. This could
@@ -672,14 +671,14 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	reg_width = atslave->reg_width;
 
 	ctrla = ATC_DEFAULT_CTRLA | atslave->ctrla;
-	ctrlb = ATC_DEFAULT_CTRLB | ATC_IEN;
+	ctrlb = ATC_IEN;
 
 	switch (direction) {
 	case DMA_TO_DEVICE:
 		ctrla |=  ATC_DST_WIDTH(reg_width);
 		ctrlb |=  ATC_DST_ADDR_MODE_FIXED
 			| ATC_SRC_ADDR_MODE_INCR
-			| ATC_FC_MEM2PER;
+			| ATC_FC_MEM2PER | ATC_SIF(MEM_IF) | ATC_DIF(PER_IF);
 		reg = atslave->tx_reg;
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct at_desc	*desc;
@@ -720,7 +719,7 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		ctrla |=  ATC_SRC_WIDTH(reg_width);
 		ctrlb |=  ATC_DST_ADDR_MODE_INCR
 			| ATC_SRC_ADDR_MODE_FIXED
-			| ATC_FC_PER2MEM;
+			| ATC_FC_PER2MEM | ATC_SIF(PER_IF) | ATC_DIF(MEM_IF);
 
 		reg = atslave->rx_reg;
 		for_each_sg(sgl, sg, sg_len, i) {
@@ -800,7 +799,7 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 	unsigned int		periods = buf_len / period_len;
 	unsigned int		reg_width;
 	u32			ctrla;
-	u32			ctrlb;
+	u32			ctrlb = 0;
 	unsigned int		i;
 
 	dev_vdbg(chan2dev(chan), "prep_dma_cyclic: %s buf at 0x%08x - %d (%d/%d)\n",
@@ -831,12 +830,11 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 	if (unlikely(!(direction & (DMA_TO_DEVICE | DMA_FROM_DEVICE))))
 		goto err_out;
 
-	/* prepare common CRTLA/CTRLB values */
+	/* prepare common CRTLA value */
 	ctrla =   ATC_DEFAULT_CTRLA | atslave->ctrla
 		| ATC_DST_WIDTH(reg_width)
 		| ATC_SRC_WIDTH(reg_width)
 		| period_len >> reg_width;
-	ctrlb = ATC_DEFAULT_CTRLB;
 
 	/* build cyclic linked list */
 	for (i = 0; i < periods; i++) {
@@ -854,7 +852,8 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 			desc->lli.ctrlb = ctrlb
 					| ATC_DST_ADDR_MODE_FIXED
 					| ATC_SRC_ADDR_MODE_INCR
-					| ATC_FC_MEM2PER;
+					| ATC_FC_MEM2PER
+					| ATC_SIF(MEM_IF) | ATC_DIF(PER_IF);
 			break;
 
 		case DMA_FROM_DEVICE:
@@ -864,7 +863,8 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 			desc->lli.ctrlb = ctrlb
 					| ATC_DST_ADDR_MODE_INCR
 					| ATC_SRC_ADDR_MODE_FIXED
-					| ATC_FC_PER2MEM;
+					| ATC_FC_PER2MEM
+					| ATC_SIF(PER_IF) | ATC_DIF(MEM_IF);
 			break;
 
 		default:
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index c79a9e0..9afcb8d 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -103,6 +103,8 @@
 /* Bitfields in CTRLB */
 #define	ATC_SIF(i)		(0x3 & (i))	/* Src tx done via AHB-Lite Interface i */
 #define	ATC_DIF(i)		((0x3 & (i)) <<  4)	/* Dst tx done via AHB-Lite Interface i */
+#define MEM_IF			0		/* specify AHB interface 0 as memory interface */
+#define PER_IF			1		/* specify AHB interface 1 as peripheral interface */
 #define	ATC_SRC_PIP		(0x1 <<  8)	/* Source Picture-in-Picture enabled */
 #define	ATC_DST_PIP		(0x1 << 12)	/* Destination Picture-in-Picture enabled */
 #define	ATC_SRC_DSCR_DIS	(0x1 << 16)	/* Src Descriptor fetch disable */
-- 
1.7.3

[PATCH v2 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Nicolas Ferre]

Now we use Buffer Transfer Completed interrupts. If we
want a chained buffer completed information, we setup the
ATC_IEN bit in CTRLB register in the lli.
This is done by set_desc_eol() function and used by
memcpy/slave_sg functions.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c      |    4 ++--
 drivers/dma/at_hdmac_regs.h |   11 ++++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 3d7d705..13050e6 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -464,7 +464,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
 
 		for (i = 0; i < atdma->dma_common.chancnt; i++) {
 			atchan = &atdma->chan[i];
-			if (pending & (AT_DMA_CBTC(i) | AT_DMA_ERR(i))) {
+			if (pending & (AT_DMA_BTC(i) | AT_DMA_ERR(i))) {
 				if (pending & AT_DMA_ERR(i)) {
 					/* Disable channel on AHB error */
 					dma_writel(atdma, CHDR, atchan->mask);
@@ -549,7 +549,7 @@ atc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 	}
 
 	ctrla =   ATC_DEFAULT_CTRLA;
-	ctrlb =   ATC_DEFAULT_CTRLB
+	ctrlb =   ATC_DEFAULT_CTRLB | ATC_IEN
 		| ATC_SRC_ADDR_MODE_INCR
 		| ATC_DST_ADDR_MODE_INCR
 		| ATC_FC_MEM2MEM;
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index 495457e..8303306 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -309,8 +309,8 @@ static void atc_setup_irq(struct at_dma_chan *atchan, int on)
 	struct at_dma	*atdma = to_at_dma(atchan->chan_common.device);
 	u32		ebci;
 
-	/* enable interrupts on buffer chain completion & error */
-	ebci =    AT_DMA_CBTC(atchan->chan_common.chan_id)
+	/* enable interrupts on buffer transfer completion & error */
+	ebci =    AT_DMA_BTC(atchan->chan_common.chan_id)
 		| AT_DMA_ERR(atchan->chan_common.chan_id);
 	if (on)
 		dma_writel(atdma, EBCIER, ebci);
@@ -347,7 +347,12 @@ static inline int atc_chan_is_enabled(struct at_dma_chan *atchan)
  */
 static void set_desc_eol(struct at_desc *desc)
 {
-	desc->lli.ctrlb |= ATC_SRC_DSCR_DIS | ATC_DST_DSCR_DIS;
+	u32 ctrlb = desc->lli.ctrlb;
+
+	ctrlb &= ~ATC_IEN;
+	ctrlb |= ATC_SRC_DSCR_DIS | ATC_DST_DSCR_DIS;
+
+	desc->lli.ctrlb = ctrlb;
 	desc->lli.dscr = 0;
 }
 
-- 
1.7.3

[PATCH v2 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Koul, Vinod]

On Fri, 2011-04-22 at 19:41 +0200, Nicolas Ferre wrote:
> Now we use Buffer Transfer Completed interrupts. If we
> want a chained buffer completed information, we setup the
> ATC_IEN bit in CTRLB register in the lli.
> This is done by set_desc_eol() function and used by
> memcpy/slave_sg functions.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  drivers/dma/at_hdmac.c      |    4 ++--
>  drivers/dma/at_hdmac_regs.h |   11 ++++++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
>  
Looks like your forgot to run checkpatch
WARNING: line over 80 characters
#401: FILE: drivers/dma/at_hdmac_regs.h:106:
+#define MEM_IF			0		/* specify AHB interface 0 as memory interface */

WARNING: line over 80 characters
#402: FILE: drivers/dma/at_hdmac_regs.h:107:
+#define PER_IF			1		/* specify AHB interface 1 as peripheral interface
*/

WARNING: line over 80 characters
#689: FILE: drivers/dma/at_hdmac.c:899:
+			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",

total: 0 errors, 3 warnings, 465 lines checked

While I am okay with 3rd one, you can easily reformat code for first two
and avoid these two warnings...

-- 
~Vinod

[PATCH v2 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Koul, Vinod]

On Tue, 2011-04-26 at 09:32 +0530, Koul, Vinod wrote:
> On Fri, 2011-04-22 at 19:41 +0200, Nicolas Ferre wrote:
> > Now we use Buffer Transfer Completed interrupts. If we
> > want a chained buffer completed information, we setup the
> > ATC_IEN bit in CTRLB register in the lli.
> > This is done by set_desc_eol() function and used by
> > memcpy/slave_sg functions.
> > 
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> >  drivers/dma/at_hdmac.c      |    4 ++--
> >  drivers/dma/at_hdmac_regs.h |   11 ++++++++---
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> >  
> Looks like your forgot to run checkpatch
> WARNING: line over 80 characters
> #401: FILE: drivers/dma/at_hdmac_regs.h:106:
> +#define MEM_IF			0		/* specify AHB interface 0 as memory interface */
> 
> WARNING: line over 80 characters
> #402: FILE: drivers/dma/at_hdmac_regs.h:107:
> +#define PER_IF			1		/* specify AHB interface 1 as peripheral interface
> */
> 
> WARNING: line over 80 characters
> #689: FILE: drivers/dma/at_hdmac.c:899:
> +			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
> 
> total: 0 errors, 3 warnings, 465 lines checked
> 
> While I am okay with 3rd one, you can easily reformat code for first two
> and avoid these two warnings...
> 
And your 4th patch fails to apply on my tree, please rebase these to my
tree as well

-- 
~Vinod

[PATCH v2 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:32 Tue 26 Apr     , Koul, Vinod wrote:
> On Fri, 2011-04-22 at 19:41 +0200, Nicolas Ferre wrote:
> > Now we use Buffer Transfer Completed interrupts. If we
> > want a chained buffer completed information, we setup the
> > ATC_IEN bit in CTRLB register in the lli.
> > This is done by set_desc_eol() function and used by
> > memcpy/slave_sg functions.
> > 
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> >  drivers/dma/at_hdmac.c      |    4 ++--
> >  drivers/dma/at_hdmac_regs.h |   11 ++++++++---
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> >  
> Looks like your forgot to run checkpatch
> WARNING: line over 80 characters
> #401: FILE: drivers/dma/at_hdmac_regs.h:106:
> +#define MEM_IF			0		/* specify AHB interface 0 as memory interface */
> 
> WARNING: line over 80 characters
> #402: FILE: drivers/dma/at_hdmac_regs.h:107:
> +#define PER_IF			1		/* specify AHB interface 1 as peripheral interface
> */
> 
> WARNING: line over 80 characters
> #689: FILE: drivers/dma/at_hdmac.c:899:
> +			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
> 
> total: 0 errors, 3 warnings, 465 lines checked
> 
> While I am okay with 3rd one, you can easily reformat code for first two
> and avoid these two warnings...
It's comments it make it more readable

Best Regards,
J.

[PATCH v2 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Koul, Vinod]

On Tue, 2011-04-26 at 10:04 +0530, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> On 09:32 Tue 26 Apr     , Koul, Vinod wrote:
> > On Fri, 2011-04-22 at 19:41 +0200, Nicolas Ferre wrote:
> > > Now we use Buffer Transfer Completed interrupts. If we
> > > want a chained buffer completed information, we setup the
> > > ATC_IEN bit in CTRLB register in the lli.
> > > This is done by set_desc_eol() function and used by
> > > memcpy/slave_sg functions.
> > > 
> > > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > ---
> > >  drivers/dma/at_hdmac.c      |    4 ++--
> > >  drivers/dma/at_hdmac_regs.h |   11 ++++++++---
> > >  2 files changed, 10 insertions(+), 5 deletions(-)
> > > 
> > >  
> > Looks like your forgot to run checkpatch
> > WARNING: line over 80 characters
> > #401: FILE: drivers/dma/at_hdmac_regs.h:106:
> > +#define MEM_IF			0		/* specify AHB interface 0 as memory interface */
> > 
> > WARNING: line over 80 characters
> > #402: FILE: drivers/dma/at_hdmac_regs.h:107:
> > +#define PER_IF			1		/* specify AHB interface 1 as peripheral interface
> > */
> > 
> > WARNING: line over 80 characters
> > #689: FILE: drivers/dma/at_hdmac.c:899:
> > +			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
> > 
> > total: 0 errors, 3 warnings, 465 lines checked
> > 
> > While I am okay with 3rd one, you can easily reformat code for first two
> > and avoid these two warnings...
> It's comments it make it more readable
And that's why it can be easily reformatted without sacrificing code
readability and also avoid the warnings.
These warnings should be removed wherever its possible without
sacrificing code readability, something like:
+/* AHB interface defines */
+#define MEM_IF 0 /* 0 as memory interface */
+#define PER_IF 1 /* 1 as peripheral interface

and while at it, you need to fix these for namespace

-- 
~Vinod

[PATCH v3 4/5] dmaengine: at_hdmac: remove channel status testing in tasklet

[Nicolas Ferre]

There is no need to test if channel is enabled in tasklet:
- in error path, channel is disabled in interrupt routine
- in normal path, this test is performed in sub functions to report
a misuse of the engine.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 8f50a0f..65bd52a 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -472,13 +472,6 @@ static void atc_tasklet(unsigned long data)
 {
 	struct at_dma_chan *atchan = (struct at_dma_chan *)data;
 
-	/* Channel cannot be enabled here */
-	if (atc_chan_is_enabled(atchan)) {
-		dev_err(chan2dev(&atchan->chan_common),
-			"BUG: channel enabled in tasklet\n");
-		return;
-	}
-
 	spin_lock(&atchan->lock);
 	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
 		atc_handle_error(atchan);
-- 
1.7.3

[PATCH v3 1/5] dmaengine: at_hdmac: modify way to use interrupts

[Koul, Vinod]

On Sat, 2011-04-30 at 16:57 +0200, Nicolas Ferre wrote:
> Now we use Buffer Transfer Completed interrupts. If we
> want a chained buffer completed information, we setup the
> ATC_IEN bit in CTRLB register in the lli.
> This is done by set_desc_eol() function and used by
> memcpy/slave_sg functions.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  drivers/dma/at_hdmac.c      |    4 ++--
>  drivers/dma/at_hdmac_regs.h |   11 ++++++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
All, Applied, Thanks

-- 
~Vinod

[PATCH v2 2/5] dmaengine: at_hdmac: add cyclic DMA operation support

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
v2:	- remove debug directives
	- split atc_prep_dma_cyclic() into function to ease code readability.
	- atc_desc_chain() helper function will be use in a folling patch series for
	  other prep_ functions.

 drivers/dma/at_hdmac.c      |  231 ++++++++++++++++++++++++++++++++++++++++---
 drivers/dma/at_hdmac_regs.h |   14 +++-
 2 files changed, 229 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 13050e6..ed9d924 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -165,6 +165,29 @@ static void atc_desc_put(struct at_dma_chan *atchan, struct at_desc *desc)
 }
 
 /**
+ * atc_desc_chain - build chain adding a descripor
+ * @first: address of first descripor of the chain
+ * @prev: address of previous descripor of the chain
+ * @desc: descriptor to queue
+ *
+ * Called from prep_* functions
+ */
+static void atc_desc_chain(struct at_desc **first, struct at_desc **prev,
+			   struct at_desc *desc)
+{
+	if (!(*first)) {
+		*first = desc;
+	} else {
+		/* inform the HW lli about chaining */
+		(*prev)->lli.dscr = desc->txd.phys;
+		/* insert the link descriptor to the LD ring */
+		list_add_tail(&desc->desc_node,
+				&(*first)->tx_list);
+	}
+	*prev = desc;
+}
+
+/**
  * atc_assign_cookie - compute and assign new cookie
  * @atchan: channel we work on
  * @desc: descriptor to asign cookie for
@@ -237,16 +260,12 @@ static void atc_dostart(struct at_dma_chan *atchan, struct at_desc *first)
 static void
 atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
 {
-	dma_async_tx_callback		callback;
-	void				*param;
 	struct dma_async_tx_descriptor	*txd = &desc->txd;
 
 	dev_vdbg(chan2dev(&atchan->chan_common),
 		"descriptor %u complete\n", txd->cookie);
 
 	atchan->completed_cookie = txd->cookie;
-	callback = txd->callback;
-	param = txd->callback_param;
 
 	/* move children to free_list */
 	list_splice_init(&desc->tx_list, &atchan->free_list);
@@ -278,12 +297,19 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
 		}
 	}
 
-	/*
-	 * The API requires that no submissions are done from a
-	 * callback, so we don't need to drop the lock here
-	 */
-	if (callback)
-		callback(param);
+	/* for cyclic transfers,
+	 * no need to replay callback function while stopping */
+	if (!test_bit(ATC_IS_CYCLIC, &atchan->status)) {
+		dma_async_tx_callback	callback = txd->callback;
+		void			*param = txd->callback_param;
+
+		/*
+		 * The API requires that no submissions are done from a
+		 * callback, so we don't need to drop the lock here
+		 */
+		if (callback)
+			callback(param);
+	}
 
 	dma_run_dependencies(txd);
 }
@@ -419,6 +445,26 @@ static void atc_handle_error(struct at_dma_chan *atchan)
 	atc_chain_complete(atchan, bad_desc);
 }
 
+/**
+ * atc_handle_cyclic - at the end of a period, run callback function
+ * @atchan: channel used for cyclic operations
+ *
+ * Called with atchan->lock held and bh disabled
+ */
+static void atc_handle_cyclic(struct at_dma_chan *atchan)
+{
+	struct at_desc			*first = atc_first_active(atchan);
+	struct dma_async_tx_descriptor	*txd = &first->txd;
+	dma_async_tx_callback		callback = txd->callback;
+	void				*param = txd->callback_param;
+
+	dev_vdbg(chan2dev(&atchan->chan_common),
+			"new cyclic period llp 0x%08x\n",
+			channel_readl(atchan, DSCR));
+
+	if (callback)
+		callback(param);
+}
 
 /*--  IRQ & Tasklet  ---------------------------------------------------*/
 
@@ -434,8 +480,10 @@ static void atc_tasklet(unsigned long data)
 	}
 
 	spin_lock(&atchan->lock);
-	if (test_and_clear_bit(0, &atchan->error_status))
+	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
 		atc_handle_error(atchan);
+	else if (test_bit(ATC_IS_CYCLIC, &atchan->status))
+		atc_handle_cyclic(atchan);
 	else
 		atc_advance_work(atchan);
 
@@ -469,7 +517,7 @@ static irqreturn_t at_dma_interrupt(int irq, void *dev_id)
 					/* Disable channel on AHB error */
 					dma_writel(atdma, CHDR, atchan->mask);
 					/* Give information to tasklet */
-					set_bit(0, &atchan->error_status);
+					set_bit(ATC_IS_ERROR, &atchan->status);
 				}
 				tasklet_schedule(&atchan->tasklet);
 				ret = IRQ_HANDLED;
@@ -759,6 +807,148 @@ err_desc_get:
 	return NULL;
 }
 
+/**
+ * atc_dma_cyclic_check_values
+ * Check for too big/unaligned periods and unaligned DMA buffer
+ */
+static int
+atc_dma_cyclic_check_values(unsigned int reg_width, dma_addr_t buf_addr,
+		size_t period_len, enum dma_data_direction direction)
+{
+	if (period_len > (ATC_BTSIZE_MAX << reg_width))
+		goto err_out;
+	if (unlikely(period_len & ((1 << reg_width) - 1)))
+		goto err_out;
+	if (unlikely(buf_addr & ((1 << reg_width) - 1)))
+		goto err_out;
+	if (unlikely(!(direction & (DMA_TO_DEVICE | DMA_FROM_DEVICE))))
+		goto err_out;
+
+	return 0;
+
+err_out:
+	return -EINVAL;
+}
+
+/**
+ * atc_dma_cyclic_fill_desc - Fill one period decriptor
+ */
+static int
+atc_dma_cyclic_fill_desc(struct at_dma_slave *atslave, struct at_desc *desc,
+		unsigned int period_index, dma_addr_t buf_addr,
+		size_t period_len, enum dma_data_direction direction)
+{
+	u32		ctrla;
+	unsigned int	reg_width = atslave->reg_width;
+
+	/* prepare common CRTLA value */
+	ctrla =   ATC_DEFAULT_CTRLA | atslave->ctrla
+		| ATC_DST_WIDTH(reg_width)
+		| ATC_SRC_WIDTH(reg_width)
+		| period_len >> reg_width;
+
+	switch (direction) {
+	case DMA_TO_DEVICE:
+		desc->lli.saddr = buf_addr + (period_len * period_index);
+		desc->lli.daddr = atslave->tx_reg;
+		desc->lli.ctrla = ctrla;
+		desc->lli.ctrlb = ATC_DEFAULT_CTRLB
+				| ATC_DST_ADDR_MODE_FIXED
+				| ATC_SRC_ADDR_MODE_INCR
+				| ATC_FC_MEM2PER;
+		break;
+
+	case DMA_FROM_DEVICE:
+		desc->lli.saddr = atslave->rx_reg;
+		desc->lli.daddr = buf_addr + (period_len * period_index);
+		desc->lli.ctrla = ctrla;
+		desc->lli.ctrlb = ATC_DEFAULT_CTRLB
+				| ATC_DST_ADDR_MODE_INCR
+				| ATC_SRC_ADDR_MODE_FIXED
+				| ATC_FC_PER2MEM;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * atc_prep_dma_cyclic - prepare the cyclic DMA transfer
+ * @chan: the DMA channel to prepare
+ * @buf_addr: physical DMA address where the buffer starts
+ * @buf_len: total number of bytes for the entire buffer
+ * @period_len: number of bytes for each period
+ * @direction: transfer direction, to or from device
+ */
+static struct dma_async_tx_descriptor *
+atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
+		size_t period_len, enum dma_data_direction direction)
+{
+	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
+	struct at_dma_slave	*atslave = chan->private;
+	struct at_desc		*first = NULL;
+	struct at_desc		*prev = NULL;
+	unsigned long		was_cyclic;
+	unsigned int		periods = buf_len / period_len;
+	unsigned int		i;
+
+	dev_vdbg(chan2dev(chan), "prep_dma_cyclic: %s buf at 0x%08x - %d (%d/%d)\n",
+			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
+			buf_addr,
+			periods, buf_len, period_len);
+
+	if (unlikely(!atslave || !buf_len || !period_len)) {
+		dev_dbg(chan2dev(chan), "prep_dma_cyclic: length is zero!\n");
+		return NULL;
+	}
+
+	was_cyclic = test_and_set_bit(ATC_IS_CYCLIC, &atchan->status);
+	if (was_cyclic) {
+		dev_dbg(chan2dev(chan), "prep_dma_cyclic: channel in use!\n");
+		return NULL;
+	}
+
+	/* Check for too big/unaligned periods and unaligned DMA buffer */
+	if (atc_dma_cyclic_check_values(atslave->reg_width, buf_addr,
+					period_len, direction))
+		goto err_out;
+
+	/* build cyclic linked list */
+	for (i = 0; i < periods; i++) {
+		struct at_desc	*desc;
+
+		desc = atc_desc_get(atchan);
+		if (!desc)
+			goto err_desc_get;
+
+		if (atc_dma_cyclic_fill_desc(atslave, desc, i, buf_addr,
+						period_len, direction))
+			goto err_desc_get;
+
+		atc_desc_chain(&first, &prev, desc);
+	}
+
+	/* lets make a cyclic list */
+	prev->lli.dscr = first->txd.phys;
+
+	/* First descriptor of the chain embedds additional information */
+	first->txd.cookie = -EBUSY;
+	first->len = buf_len;
+
+	return &first->txd;
+
+err_desc_get:
+	dev_err(chan2dev(chan), "not enough descriptors available\n");
+	atc_desc_put(atchan, first);
+err_out:
+	clear_bit(ATC_IS_CYCLIC, &atchan->status);
+	return NULL;
+}
+
+
 static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 		       unsigned long arg)
 {
@@ -793,6 +983,9 @@ static int atc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 	list_for_each_entry_safe(desc, _desc, &list, desc_node)
 		atc_chain_complete(atchan, desc);
 
+	/* if channel dedicated to cyclic operations, free it */
+	clear_bit(ATC_IS_CYCLIC, &atchan->status);
+
 	spin_unlock_bh(&atchan->lock);
 
 	return 0;
@@ -853,6 +1046,10 @@ static void atc_issue_pending(struct dma_chan *chan)
 
 	dev_vdbg(chan2dev(chan), "issue_pending\n");
 
+	/* Not needed for cyclic transfers */
+	if (test_bit(ATC_IS_CYCLIC, &atchan->status))
+		return;
+
 	spin_lock_bh(&atchan->lock);
 	if (!atc_chan_is_enabled(atchan)) {
 		atc_advance_work(atchan);
@@ -959,6 +1156,7 @@ static void atc_free_chan_resources(struct dma_chan *chan)
 	}
 	list_splice_init(&atchan->free_list, &list);
 	atchan->descs_allocated = 0;
+	atchan->status = 0;
 
 	dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
 }
@@ -1092,10 +1290,15 @@ static int __init at_dma_probe(struct platform_device *pdev)
 	if (dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask))
 		atdma->dma_common.device_prep_dma_memcpy = atc_prep_dma_memcpy;
 
-	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)) {
+	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask))
 		atdma->dma_common.device_prep_slave_sg = atc_prep_slave_sg;
+
+	if (dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
+		atdma->dma_common.device_prep_dma_cyclic = atc_prep_dma_cyclic;
+
+	if (dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ||
+	    dma_has_cap(DMA_CYCLIC, atdma->dma_common.cap_mask))
 		atdma->dma_common.device_control = atc_control;
-	}
 
 	dma_writel(atdma, EN, AT_DMA_ENABLE);
 
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index 8303306..c79a9e0 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -181,12 +181,22 @@ txd_to_at_desc(struct dma_async_tx_descriptor *txd)
 /*--  Channels  --------------------------------------------------------*/
 
 /**
+ * atc_status - information bits stored in channel status flag
+ *
+ * Manipulated with atomic operations.
+ */
+enum atc_status {
+	ATC_IS_ERROR = 0,
+	ATC_IS_CYCLIC = 24,
+};
+
+/**
  * struct at_dma_chan - internal representation of an Atmel HDMAC channel
  * @chan_common: common dmaengine channel object members
  * @device: parent device
  * @ch_regs: memory mapped register base
  * @mask: channel index in a mask
- * @error_status: transmit error status information from irq handler
+ * @status: transmit status information from irq/prep* functions
  *                to tasklet (use atomic operations)
  * @tasklet: bottom half to finish transaction work
  * @lock: serializes enqueue/dequeue operations to descriptors lists
@@ -201,7 +211,7 @@ struct at_dma_chan {
 	struct at_dma		*device;
 	void __iomem		*ch_regs;
 	u8			mask;
-	unsigned long		error_status;
+	unsigned long		status;
 	struct tasklet_struct	tasklet;
 
 	spinlock_t		lock;
-- 
1.7.3

[PATCH v2 3/5] dmaengine: at_hdmac: debug information sg_len for prep_slave_sg

[Nicolas Ferre]

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index ed9d924..8f50a0f 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -687,7 +687,8 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	struct scatterlist	*sg;
 	size_t			total_len = 0;
 
-	dev_vdbg(chan2dev(chan), "prep_slave_sg: %s f0x%lx\n",
+	dev_vdbg(chan2dev(chan), "prep_slave_sg (%d): %s f0x%lx\n",
+			sg_len,
 			direction == DMA_TO_DEVICE ? "TO DEVICE" : "FROM DEVICE",
 			flags);
 
-- 
1.7.3

[PATCH v2 4/5] dmaengine: at_hdmac: remove channel status testing in tasklet

[Nicolas Ferre]

There is no need to test if channel is enabled in tasklet:
- in error path, channel is disabled in interrupt routine
- in normal path, this test is performed in sub functions to report
a misuse of the engine.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 8f50a0f..65bd52a 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -472,13 +472,6 @@ static void atc_tasklet(unsigned long data)
 {
 	struct at_dma_chan *atchan = (struct at_dma_chan *)data;
 
-	/* Channel cannot be enabled here */
-	if (atc_chan_is_enabled(atchan)) {
-		dev_err(chan2dev(&atchan->chan_common),
-			"BUG: channel enabled in tasklet\n");
-		return;
-	}
-
 	spin_lock(&atchan->lock);
 	if (test_and_clear_bit(ATC_IS_ERROR, &atchan->status))
 		atc_handle_error(atchan);
-- 
1.7.3

[PATCH v2 5/5] dmaengine: at_hdmac: specialize AHB interfaces to optimize transfers

[Nicolas Ferre]

DMA controller has two AHB interfaces on the SOC internal
matrix.
It is more efficient to specialize each interface as the
access to memory can introduce latencies that are not compatible
with peripheral accesses requirements.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/dma/at_hdmac.c      |   21 ++++++++++-----------
 drivers/dma/at_hdmac_regs.h |    2 ++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 65bd52a..73c2e99 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -37,8 +37,7 @@
 
 #define	ATC_DEFAULT_CFG		(ATC_FIFOCFG_HALFFIFO)
 #define	ATC_DEFAULT_CTRLA	(0)
-#define	ATC_DEFAULT_CTRLB	(ATC_SIF(0)	\
-				|ATC_DIF(1))
+#define	ATC_DEFAULT_CTRLB	(ATC_SIF(MEM_IF) | ATC_DIF(MEM_IF))
 
 /*
  * Initial number of descriptors to allocate for each channel. This could
@@ -693,14 +692,14 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	reg_width = atslave->reg_width;
 
 	ctrla = ATC_DEFAULT_CTRLA | atslave->ctrla;
-	ctrlb = ATC_DEFAULT_CTRLB | ATC_IEN;
+	ctrlb = ATC_IEN;
 
 	switch (direction) {
 	case DMA_TO_DEVICE:
 		ctrla |=  ATC_DST_WIDTH(reg_width);
 		ctrlb |=  ATC_DST_ADDR_MODE_FIXED
 			| ATC_SRC_ADDR_MODE_INCR
-			| ATC_FC_MEM2PER;
+			| ATC_FC_MEM2PER | ATC_SIF(MEM_IF) | ATC_DIF(PER_IF);
 		reg = atslave->tx_reg;
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct at_desc	*desc;
@@ -741,7 +740,7 @@ atc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		ctrla |=  ATC_SRC_WIDTH(reg_width);
 		ctrlb |=  ATC_DST_ADDR_MODE_INCR
 			| ATC_SRC_ADDR_MODE_FIXED
-			| ATC_FC_PER2MEM;
+			| ATC_FC_PER2MEM | ATC_SIF(PER_IF) | ATC_DIF(MEM_IF);
 
 		reg = atslave->rx_reg;
 		for_each_sg(sgl, sg, sg_len, i) {
@@ -846,20 +845,20 @@ atc_dma_cyclic_fill_desc(struct at_dma_slave *atslave, struct at_desc *desc,
 		desc->lli.saddr = buf_addr + (period_len * period_index);
 		desc->lli.daddr = atslave->tx_reg;
 		desc->lli.ctrla = ctrla;
-		desc->lli.ctrlb = ATC_DEFAULT_CTRLB
-				| ATC_DST_ADDR_MODE_FIXED
+		desc->lli.ctrlb = ATC_DST_ADDR_MODE_FIXED
 				| ATC_SRC_ADDR_MODE_INCR
-				| ATC_FC_MEM2PER;
+				| ATC_FC_MEM2PER
+				| ATC_SIF(MEM_IF) | ATC_DIF(PER_IF);
 		break;
 
 	case DMA_FROM_DEVICE:
 		desc->lli.saddr = atslave->rx_reg;
 		desc->lli.daddr = buf_addr + (period_len * period_index);
 		desc->lli.ctrla = ctrla;
-		desc->lli.ctrlb = ATC_DEFAULT_CTRLB
-				| ATC_DST_ADDR_MODE_INCR
+		desc->lli.ctrlb = ATC_DST_ADDR_MODE_INCR
 				| ATC_SRC_ADDR_MODE_FIXED
-				| ATC_FC_PER2MEM;
+				| ATC_FC_PER2MEM
+				| ATC_SIF(PER_IF) | ATC_DIF(MEM_IF);
 		break;
 
 	default:
diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h
index c79a9e0..9afcb8d 100644
--- a/drivers/dma/at_hdmac_regs.h
+++ b/drivers/dma/at_hdmac_regs.h
@@ -103,6 +103,8 @@
 /* Bitfields in CTRLB */
 #define	ATC_SIF(i)		(0x3 & (i))	/* Src tx done via AHB-Lite Interface i */
 #define	ATC_DIF(i)		((0x3 & (i)) <<  4)	/* Dst tx done via AHB-Lite Interface i */
+#define MEM_IF			0		/* specify AHB interface 0 as memory interface */
+#define PER_IF			1		/* specify AHB interface 1 as peripheral interface */
 #define	ATC_SRC_PIP		(0x1 <<  8)	/* Source Picture-in-Picture enabled */
 #define	ATC_DST_PIP		(0x1 << 12)	/* Destination Picture-in-Picture enabled */
 #define	ATC_SRC_DSCR_DIS	(0x1 << 16)	/* Src Descriptor fetch disable */
-- 
1.7.3