[v6,04/15] DMA: PL330: Add DMA_CYCLIC capability
diff mbox

Message ID 1313744097-27289-5-git-send-email-boojin.kim@samsung.com
State New, archived
Headers show

Commit Message

boojin.kim Aug. 19, 2011, 8:54 a.m. UTC
This patch adds DMA_CYCLIC capability that is used for audio driver.
DMA driver activated with it reuses the dma requests that were submitted
through tx_submit().

Signed-off-by: Boojin Kim <boojin.kim@samsung.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/dma/pl330.c |   97 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 95 insertions(+), 2 deletions(-)

Comments

Jassi Brar Aug. 22, 2011, 10:09 a.m. UTC | #1
On Fri, Aug 19, 2011 at 2:24 PM, Boojin Kim <boojin.kim@samsung.com> wrote:

> @@ -324,6 +362,9 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
>        pl330_release_channel(pch->pl330_chid);
>        pch->pl330_chid = NULL;
>
> +       if (pch->cyclic)
> +               list_splice_tail_init(&pch->work_list, &pch->dmac->desc_pool);
'cyclic' member is 'enum cyclic_mode', please observe the rule and compare
it only against the enum values.


> +static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
> +               struct dma_chan *chan, dma_addr_t dma_addr, size_t len,
> +               size_t period_len, enum dma_data_direction direction)
> +{
> +       struct dma_pl330_desc *desc;
> +       struct dma_pl330_chan *pch = to_pchan(chan);
> +       dma_addr_t dst;
> +       dma_addr_t src;
> +
> +       desc = pl330_get_desc(pch);
> +       if (!desc) {
> +               dev_err(pch->dmac->pif.dev, "%s:%d Unable to fetch desc\n",
> +                       __func__, __LINE__);
> +               return NULL;
> +       }
> +
> +       switch (direction) {
> +       case DMA_TO_DEVICE:
> +               desc->rqcfg.src_inc = 1;
> +               desc->rqcfg.dst_inc = 0;
> +               src = dma_addr;
> +               dst = pch->fifo_addr;
> +               break;
> +       case DMA_FROM_DEVICE:
> +               desc->rqcfg.src_inc = 0;
> +               desc->rqcfg.dst_inc = 1;
> +               src = pch->fifo_addr;
> +               dst = dma_addr;
> +               break;
> +       default:
> +               dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma direction\n",
> +               __func__, __LINE__);
> +               return NULL;
> +       }
> +
> +       desc->rqcfg.brst_size = pch->burst_sz;
> +       desc->rqcfg.brst_len = 1;
> +
> +       if (!pch->cyclic)
> +               pch->cyclic = CYCLIC_PREP;
The need for check here seems suspicious.
Is it really needed? If not, please remove it.
boojin.kim Aug. 22, 2011, 12:03 p.m. UTC | #2
Jassi Brar wrote:

>
> > @@ -324,6 +362,9 @@ static void pl330_free_chan_resources(struct
> dma_chan *chan)
> >        pl330_release_channel(pch->pl330_chid);
> >        pch->pl330_chid = NULL;
> >
> > +       if (pch->cyclic)
> > +               list_splice_tail_init(&pch->work_list, &pch->dmac-
> >desc_pool);
> 'cyclic' member is 'enum cyclic_mode', please observe the rule and
> compare
> it only against the enum values.
I will address your comment.

>
>
> > +static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
> > +               struct dma_chan *chan, dma_addr_t dma_addr, size_t len,
> > +               size_t period_len, enum dma_data_direction direction)
> > +{
> > +       struct dma_pl330_desc *desc;
> > +       struct dma_pl330_chan *pch = to_pchan(chan);
> > +       dma_addr_t dst;
> > +       dma_addr_t src;
> > +
> > +       desc = pl330_get_desc(pch);
> > +       if (!desc) {
> > +               dev_err(pch->dmac->pif.dev, "%s:%d Unable to fetch
> desc\n",
> > +                       __func__, __LINE__);
> > +               return NULL;
> > +       }
> > +
> > +       switch (direction) {
> > +       case DMA_TO_DEVICE:
> > +               desc->rqcfg.src_inc = 1;
> > +               desc->rqcfg.dst_inc = 0;
> > +               src = dma_addr;
> > +               dst = pch->fifo_addr;
> > +               break;
> > +       case DMA_FROM_DEVICE:
> > +               desc->rqcfg.src_inc = 0;
> > +               desc->rqcfg.dst_inc = 1;
> > +               src = pch->fifo_addr;
> > +               dst = dma_addr;
> > +               break;
> > +       default:
> > +               dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma
> direction\n",
> > +               __func__, __LINE__);
> > +               return NULL;
> > +       }
> > +
> > +       desc->rqcfg.brst_size = pch->burst_sz;
> > +       desc->rqcfg.brst_len = 1;
> > +
> > +       if (!pch->cyclic)
> > +               pch->cyclic = CYCLIC_PREP;
> The need for check here seems suspicious.
> Is it really needed? If not, please remove it.
It's required because Cyclic operation is passed from CYCLIC_PREP to 
CYCLIC_TRIGGER.

Thanks
Boojin
Jassi Brar Aug. 23, 2011, 5:41 a.m. UTC | #3
On Mon, Aug 22, 2011 at 5:33 PM, Boojin Kim <boojin.kim@samsung.com> wrote:
>>
>> > +static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
>> > +               struct dma_chan *chan, dma_addr_t dma_addr, size_t len,
>> > +               size_t period_len, enum dma_data_direction direction)
>> > +{
>> > +       struct dma_pl330_desc *desc;
>> > +       struct dma_pl330_chan *pch = to_pchan(chan);
>> > +       dma_addr_t dst;
>> > +       dma_addr_t src;
>> > +
>> > +       desc = pl330_get_desc(pch);
>> > +       if (!desc) {
>> > +               dev_err(pch->dmac->pif.dev, "%s:%d Unable to fetch
>> desc\n",
>> > +                       __func__, __LINE__);
>> > +               return NULL;
>> > +       }
>> > +
>> > +       switch (direction) {
>> > +       case DMA_TO_DEVICE:
>> > +               desc->rqcfg.src_inc = 1;
>> > +               desc->rqcfg.dst_inc = 0;
>> > +               src = dma_addr;
>> > +               dst = pch->fifo_addr;
>> > +               break;
>> > +       case DMA_FROM_DEVICE:
>> > +               desc->rqcfg.src_inc = 0;
>> > +               desc->rqcfg.dst_inc = 1;
>> > +               src = pch->fifo_addr;
>> > +               dst = dma_addr;
>> > +               break;
>> > +       default:
>> > +               dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma
>> direction\n",
>> > +               __func__, __LINE__);
>> > +               return NULL;
>> > +       }
>> > +
>> > +       desc->rqcfg.brst_size = pch->burst_sz;
>> > +       desc->rqcfg.brst_len = 1;
>> > +
>> > +       if (!pch->cyclic)
>> > +               pch->cyclic = CYCLIC_PREP;
>> The need for check here seems suspicious.
>> Is it really needed? If not, please remove it.
> It's required because Cyclic operation is passed from CYCLIC_PREP to
> CYCLIC_TRIGGER.
I don't see why you need to differentiate between PREP and TRIGGER in
cyclic mode. I think, you should simply have 'bool cyclic' instead of enum.
boojin.kim Aug. 23, 2011, 7:08 a.m. UTC | #4
Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Tuesday, August 23, 2011 2:42 PM
> To: Boojin Kim
> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; Vinod Koul; Kukjin Kim; Dan Williams; Mark Brown;
> Grant Likely; Russell King
> Subject: Re: [PATCH v6 04/15] DMA: PL330: Add DMA_CYCLIC capability
>
> On Mon, Aug 22, 2011 at 5:33 PM, Boojin Kim <boojin.kim@samsung.com>
> wrote:
> >>
> >> > +static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
> >> > +               struct dma_chan *chan, dma_addr_t dma_addr, size_t
> len,
> >> > +               size_t period_len, enum dma_data_direction
> direction)
> >> > +{
> >> > +       struct dma_pl330_desc *desc;
> >> > +       struct dma_pl330_chan *pch = to_pchan(chan);
> >> > +       dma_addr_t dst;
> >> > +       dma_addr_t src;
> >> > +
> >> > +       desc = pl330_get_desc(pch);
> >> > +       if (!desc) {
> >> > +               dev_err(pch->dmac->pif.dev, "%s:%d Unable to fetch
> >> desc\n",
> >> > +                       __func__, __LINE__);
> >> > +               return NULL;
> >> > +       }
> >> > +
> >> > +       switch (direction) {
> >> > +       case DMA_TO_DEVICE:
> >> > +               desc->rqcfg.src_inc = 1;
> >> > +               desc->rqcfg.dst_inc = 0;
> >> > +               src = dma_addr;
> >> > +               dst = pch->fifo_addr;
> >> > +               break;
> >> > +       case DMA_FROM_DEVICE:
> >> > +               desc->rqcfg.src_inc = 0;
> >> > +               desc->rqcfg.dst_inc = 1;
> >> > +               src = pch->fifo_addr;
> >> > +               dst = dma_addr;
> >> > +               break;
> >> > +       default:
> >> > +               dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma
> >> direction\n",
> >> > +               __func__, __LINE__);
> >> > +               return NULL;
> >> > +       }
> >> > +
> >> > +       desc->rqcfg.brst_size = pch->burst_sz;
> >> > +       desc->rqcfg.brst_len = 1;
> >> > +
> >> > +       if (!pch->cyclic)
> >> > +               pch->cyclic = CYCLIC_PREP;
> >> The need for check here seems suspicious.
> >> Is it really needed? If not, please remove it.
> > It's required because Cyclic operation is passed from CYCLIC_PREP to
> > CYCLIC_TRIGGER.
> I don't see why you need to differentiate between PREP and TRIGGER in
> cyclic mode. I think, you should simply have 'bool cyclic' instead of
> enum.
On cyclic mode, Pl330_tasklet() operation is changed according to the value of 
'cyclic'.
In case of CYCLIC_PREP, Pl330_tasklet()operation is same with normal 
operation.
In case of CYCLIC_TRIGGER, pl330_tasklet()reloads the done data into 
work_list.
So, 'Cyclic' needs two status(CYCLIC_PREP and CYCLIC_TRIGGER) to distinguish 
the operation on pl330_tasklet().
The sequence is following.
device_prep_dma_cyclic() -> set CYCLIC_PREP -> device_issue_pending() -> 
Pl330_tasklet() with CYCLIC_PREP -> set CYCLIC_TRIGGER -> PL330 IRQ -> 
Pl330_tasklet() with CYCLIC_TRIGGER.

Thanks
Boojin
Jassi Brar Aug. 23, 2011, 8:05 a.m. UTC | #5
On Tue, Aug 23, 2011 at 12:38 PM, Boojin Kim <boojin.kim@samsung.com> wrote:
> Jassi Brar [mailto:jassisinghbrar@gmail.com]
>> Sent: Tuesday, August 23, 2011 2:42 PM
>> To: Boojin Kim
>> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
>> soc@vger.kernel.org; Vinod Koul; Kukjin Kim; Dan Williams; Mark Brown;
>> Grant Likely; Russell King
>> Subject: Re: [PATCH v6 04/15] DMA: PL330: Add DMA_CYCLIC capability
>>
>> On Mon, Aug 22, 2011 at 5:33 PM, Boojin Kim <boojin.kim@samsung.com>
>> wrote:
>> >>
>> >> > +static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
>> >> > +               struct dma_chan *chan, dma_addr_t dma_addr, size_t
>> len,
>> >> > +               size_t period_len, enum dma_data_direction
>> direction)
>> >> > +{
>> >> > +       struct dma_pl330_desc *desc;
>> >> > +       struct dma_pl330_chan *pch = to_pchan(chan);
>> >> > +       dma_addr_t dst;
>> >> > +       dma_addr_t src;
>> >> > +
>> >> > +       desc = pl330_get_desc(pch);
>> >> > +       if (!desc) {
>> >> > +               dev_err(pch->dmac->pif.dev, "%s:%d Unable to fetch
>> >> desc\n",
>> >> > +                       __func__, __LINE__);
>> >> > +               return NULL;
>> >> > +       }
>> >> > +
>> >> > +       switch (direction) {
>> >> > +       case DMA_TO_DEVICE:
>> >> > +               desc->rqcfg.src_inc = 1;
>> >> > +               desc->rqcfg.dst_inc = 0;
>> >> > +               src = dma_addr;
>> >> > +               dst = pch->fifo_addr;
>> >> > +               break;
>> >> > +       case DMA_FROM_DEVICE:
>> >> > +               desc->rqcfg.src_inc = 0;
>> >> > +               desc->rqcfg.dst_inc = 1;
>> >> > +               src = pch->fifo_addr;
>> >> > +               dst = dma_addr;
>> >> > +               break;
>> >> > +       default:
>> >> > +               dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma
>> >> direction\n",
>> >> > +               __func__, __LINE__);
>> >> > +               return NULL;
>> >> > +       }
>> >> > +
>> >> > +       desc->rqcfg.brst_size = pch->burst_sz;
>> >> > +       desc->rqcfg.brst_len = 1;
>> >> > +
>> >> > +       if (!pch->cyclic)
>> >> > +               pch->cyclic = CYCLIC_PREP;
>> >> The need for check here seems suspicious.
>> >> Is it really needed? If not, please remove it.
>> > It's required because Cyclic operation is passed from CYCLIC_PREP to
>> > CYCLIC_TRIGGER.
>> I don't see why you need to differentiate between PREP and TRIGGER in
>> cyclic mode. I think, you should simply have 'bool cyclic' instead of
>> enum.
> On cyclic mode, Pl330_tasklet() operation is changed according to the value of
> 'cyclic'.
> In case of CYCLIC_PREP, Pl330_tasklet()operation is same with normal
> operation.
By 'normal' you mean non-cyclic, right?  That doesn't seem correct to do.
Once the desc has been marked cyclic by device_prep_dma_cyclic(), you
shouldn't treat it like a non-cyclic anymore.


> The sequence is following.
> device_prep_dma_cyclic() -> set CYCLIC_PREP -> device_issue_pending() ->
> Pl330_tasklet() with CYCLIC_PREP -> set CYCLIC_TRIGGER -> PL330 IRQ ->
> Pl330_tasklet() with CYCLIC_TRIGGER.
"device_issue_pending() ->Pl330_tasklet() with CYCLIC_PREP" is no different
from "device_issue_pending() ->Pl330_tasklet() with CYCLIC_TRIGGER"
because the list of 'done' xfers would be null yet.
boojin.kim Aug. 23, 2011, 11:28 p.m. UTC | #6
Jassi Brar wrote:
> On Tue, Aug 23, 2011 at 12:38 PM, Boojin Kim <boojin.kim@samsung.com>
> wrote:
> > Jassi Brar [mailto:jassisinghbrar@gmail.com]
> >> Sent: Tuesday, August 23, 2011 2:42 PM
> >> To: Boojin Kim
> >> Cc: linux-arm-kernel@lists.infradead.org; linux-samsung-
> >> soc@vger.kernel.org; Vinod Koul; Kukjin Kim; Dan Williams; Mark
> Brown;
> >> Grant Likely; Russell King
> >> Subject: Re: [PATCH v6 04/15] DMA: PL330: Add DMA_CYCLIC capability
> >>
> >> On Mon, Aug 22, 2011 at 5:33 PM, Boojin Kim
> <boojin.kim@samsung.com>
> >> wrote:
> >> >>
> >> >> > +static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
> >> >> > +               struct dma_chan *chan, dma_addr_t dma_addr,
> size_t
> >> len,
> >> >> > +               size_t period_len, enum dma_data_direction
> >> direction)
> >> >> > +{
> >> >> > +       struct dma_pl330_desc *desc;
> >> >> > +       struct dma_pl330_chan *pch = to_pchan(chan);
> >> >> > +       dma_addr_t dst;
> >> >> > +       dma_addr_t src;
> >> >> > +
> >> >> > +       desc = pl330_get_desc(pch);
> >> >> > +       if (!desc) {
> >> >> > +               dev_err(pch->dmac->pif.dev, "%s:%d Unable to
> fetch
> >> >> desc\n",
> >> >> > +                       __func__, __LINE__);
> >> >> > +               return NULL;
> >> >> > +       }
> >> >> > +
> >> >> > +       switch (direction) {
> >> >> > +       case DMA_TO_DEVICE:
> >> >> > +               desc->rqcfg.src_inc = 1;
> >> >> > +               desc->rqcfg.dst_inc = 0;
> >> >> > +               src = dma_addr;
> >> >> > +               dst = pch->fifo_addr;
> >> >> > +               break;
> >> >> > +       case DMA_FROM_DEVICE:
> >> >> > +               desc->rqcfg.src_inc = 0;
> >> >> > +               desc->rqcfg.dst_inc = 1;
> >> >> > +               src = pch->fifo_addr;
> >> >> > +               dst = dma_addr;
> >> >> > +               break;
> >> >> > +       default:
> >> >> > +               dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma
> >> >> direction\n",
> >> >> > +               __func__, __LINE__);
> >> >> > +               return NULL;
> >> >> > +       }
> >> >> > +
> >> >> > +       desc->rqcfg.brst_size = pch->burst_sz;
> >> >> > +       desc->rqcfg.brst_len = 1;
> >> >> > +
> >> >> > +       if (!pch->cyclic)
> >> >> > +               pch->cyclic = CYCLIC_PREP;
> >> >> The need for check here seems suspicious.
> >> >> Is it really needed? If not, please remove it.
> >> > It's required because Cyclic operation is passed from CYCLIC_PREP
> to
> >> > CYCLIC_TRIGGER.
> >> I don't see why you need to differentiate between PREP and TRIGGER
> in
> >> cyclic mode. I think, you should simply have 'bool cyclic' instead
> of
> >> enum.
> > On cyclic mode, Pl330_tasklet() operation is changed according to
> the value of
> > 'cyclic'.
> > In case of CYCLIC_PREP, Pl330_tasklet()operation is same with normal
> > operation.
> By 'normal' you mean non-cyclic, right?  That doesn't seem correct to
> do.
> Once the desc has been marked cyclic by device_prep_dma_cyclic(), you
> shouldn't treat it like a non-cyclic anymore.
>
>
> > The sequence is following.
> > device_prep_dma_cyclic() -> set CYCLIC_PREP -> device_issue_pending()
> ->
> > Pl330_tasklet() with CYCLIC_PREP -> set CYCLIC_TRIGGER -> PL330 IRQ
> ->
> > Pl330_tasklet() with CYCLIC_TRIGGER.
> "device_issue_pending() ->Pl330_tasklet() with CYCLIC_PREP" is no
> different
> from "device_issue_pending() ->Pl330_tasklet() with CYCLIC_TRIGGER"
> because the list of 'done' xfers would be null yet.
Okay, You're right. CYCLIC_PREP is a redundancy status.
I will address your comment.

Thanks
Boojin Kim

Patch
diff mbox

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 59943ec..3a0baac 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -43,6 +43,12 @@  enum desc_status {
 	DONE,
 };
 
+enum cyclic_mode {
+	NO_CYCLIC,
+	CYCLIC_PREP,
+	CYCLIC_TRIGGER,
+};
+
 struct dma_pl330_chan {
 	/* Schedule desc completion */
 	struct tasklet_struct task;
@@ -75,6 +81,9 @@  struct dma_pl330_chan {
 	int burst_sz; /* the peripheral fifo width */
 	int burst_len; /* the number of burst */
 	dma_addr_t fifo_addr;
+
+	/* for cyclic capability */
+	enum cyclic_mode cyclic;
 };
 
 struct dma_pl330_dmac {
@@ -161,6 +170,31 @@  static inline void free_desc_list(struct list_head *list)
 	spin_unlock_irqrestore(&pdmac->pool_lock, flags);
 }
 
+static inline void handle_cyclic_desc_list(struct list_head *list)
+{
+	struct dma_pl330_desc *desc;
+	struct dma_pl330_chan *pch;
+	unsigned long flags;
+
+	if (list_empty(list))
+		return;
+
+	list_for_each_entry(desc, list, node) {
+		dma_async_tx_callback callback;
+
+		/* Change status to reload it */
+		desc->status = PREP;
+		pch = desc->pchan;
+		callback = desc->txd.callback;
+		if (callback)
+			callback(desc->txd.callback_param);
+	}
+
+	spin_lock_irqsave(&pch->lock, flags);
+	list_splice_tail_init(list, &pch->work_list);
+	spin_unlock_irqrestore(&pch->lock, flags);
+}
+
 static inline void fill_queue(struct dma_pl330_chan *pch)
 {
 	struct dma_pl330_desc *desc;
@@ -214,7 +248,10 @@  static void pl330_tasklet(unsigned long data)
 
 	spin_unlock_irqrestore(&pch->lock, flags);
 
-	free_desc_list(&list);
+	if (pch->cyclic == CYCLIC_TRIGGER)
+		handle_cyclic_desc_list(&list);
+	else
+		free_desc_list(&list);
 }
 
 static void dma_pl330_rqcb(void *token, enum pl330_op_err err)
@@ -245,6 +282,7 @@  static int pl330_alloc_chan_resources(struct dma_chan *chan)
 	spin_lock_irqsave(&pch->lock, flags);
 
 	pch->completed = chan->cookie = 1;
+	pch->cyclic = NO_CYCLIC;
 
 	pch->pl330_chid = pl330_request_channel(&pdmac->pif);
 	if (!pch->pl330_chid) {
@@ -324,6 +362,9 @@  static void pl330_free_chan_resources(struct dma_chan *chan)
 	pl330_release_channel(pch->pl330_chid);
 	pch->pl330_chid = NULL;
 
+	if (pch->cyclic)
+		list_splice_tail_init(&pch->work_list, &pch->dmac->desc_pool);
+
 	spin_unlock_irqrestore(&pch->lock, flags);
 }
 
@@ -347,7 +388,11 @@  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 
 static void pl330_issue_pending(struct dma_chan *chan)
 {
-	pl330_tasklet((unsigned long) to_pchan(chan));
+	struct dma_pl330_chan *pch = to_pchan(chan);
+	pl330_tasklet((unsigned long) pch);
+
+	if (pch->cyclic == CYCLIC_PREP)
+		pch->cyclic = CYCLIC_TRIGGER;
 }
 
 /*
@@ -560,6 +605,52 @@  static inline int get_burst_len(struct dma_pl330_desc *desc, size_t len)
 	return burst_len;
 }
 
+static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
+		struct dma_chan *chan, dma_addr_t dma_addr, size_t len,
+		size_t period_len, enum dma_data_direction direction)
+{
+	struct dma_pl330_desc *desc;
+	struct dma_pl330_chan *pch = to_pchan(chan);
+	dma_addr_t dst;
+	dma_addr_t src;
+
+	desc = pl330_get_desc(pch);
+	if (!desc) {
+		dev_err(pch->dmac->pif.dev, "%s:%d Unable to fetch desc\n",
+			__func__, __LINE__);
+		return NULL;
+	}
+
+	switch (direction) {
+	case DMA_TO_DEVICE:
+		desc->rqcfg.src_inc = 1;
+		desc->rqcfg.dst_inc = 0;
+		src = dma_addr;
+		dst = pch->fifo_addr;
+		break;
+	case DMA_FROM_DEVICE:
+		desc->rqcfg.src_inc = 0;
+		desc->rqcfg.dst_inc = 1;
+		src = pch->fifo_addr;
+		dst = dma_addr;
+		break;
+	default:
+		dev_err(pch->dmac->pif.dev, "%s:%d Invalid dma direction\n",
+		__func__, __LINE__);
+		return NULL;
+	}
+
+	desc->rqcfg.brst_size = pch->burst_sz;
+	desc->rqcfg.brst_len = 1;
+
+	if (!pch->cyclic)
+		pch->cyclic = CYCLIC_PREP;
+
+	fill_px(&desc->px, dst, src, period_len);
+
+	return &desc->txd;
+}
+
 static struct dma_async_tx_descriptor *
 pl330_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dst,
 		dma_addr_t src, size_t len, unsigned long flags)
@@ -791,6 +882,7 @@  pl330_probe(struct amba_device *adev, const struct amba_id *id)
 			case MEMTODEV:
 			case DEVTOMEM:
 				dma_cap_set(DMA_SLAVE, pd->cap_mask);
+				dma_cap_set(DMA_CYCLIC, pd->cap_mask);
 				break;
 			default:
 				dev_err(&adev->dev, "DEVTODEV Not Supported\n");
@@ -819,6 +911,7 @@  pl330_probe(struct amba_device *adev, const struct amba_id *id)
 	pd->device_alloc_chan_resources = pl330_alloc_chan_resources;
 	pd->device_free_chan_resources = pl330_free_chan_resources;
 	pd->device_prep_dma_memcpy = pl330_prep_dma_memcpy;
+	pd->device_prep_dma_cyclic = pl330_prep_dma_cyclic;
 	pd->device_tx_status = pl330_tx_status;
 	pd->device_prep_slave_sg = pl330_prep_slave_sg;
 	pd->device_control = pl330_control;