Re: [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers

2014-03-01 Thread Alexander Popov
Hello Andy.

2014-02-24 17:03 GMT+04:00 Andy Shevchenko :
> On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote:
>> Introduce support for slave s/g transfer preparation and the associated
>> device control callback in the MPC512x DMA controller driver, which adds
>> support for data transfers between memory and peripheral I/O to the
>> previously supported mem-to-mem transfers.
>>
>
> Few comments below.

Thanks for your feedback. I agree with your points and will fix my code.

Best regards,
Alexander
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers

2014-02-24 Thread Andy Shevchenko
On Mon, 2014-02-24 at 15:09 +0400, Alexander Popov wrote:
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.
> 

Few comments below.

> Signed-off-by: Alexander Popov 
> ---
>  drivers/dma/mpc512x_dma.c | 235 
> +-
>  1 file changed, 230 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 2ce248b..8f504cb 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -2,6 +2,7 @@
>   * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
>   * Copyright (C) Semihalf 2009
>   * Copyright (C) Ilya Yanok, Emcraft Systems 2010
> + * Copyright (C) Alexander Popov, Promcontroller 2013
>   *
>   * Written by Piotr Ziecik . Hardware description
>   * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -29,8 +30,17 @@
>   */
>  
>  /*
> - * This is initial version of MPC5121 DMA driver. Only memory to memory
> - * transfers are supported (tested using dmatest module).
> + * MPC512x and MPC8308 DMA driver. It supports
> + * memory to memory data transfers (tested using dmatest module) and
> + * data transfers between memory and peripheral I/O memory
> + * by means of slave s/g with these limitations:
> + *  - chunked transfers (transfers with more than one part) are refused
> + * as long as proper support for scatter/gather is missing;
> + *  - transfers on MPC8308 always start from software as this SoC appears
> + * not to have external request lines for peripheral flow control;
> + *  - minimal memory <-> I/O memory transfer chunk is 4 bytes and 
> consequently
> + * source and destination addresses must be 4-byte aligned
> + * and transfer size must be aligned on (4 * maxburst) boundary;
>   */
>  
>  #include 
> @@ -189,6 +199,7 @@ struct mpc_dma_desc {
>   dma_addr_t  tcd_paddr;
>   int error;
>   struct list_headnode;
> + int will_access_peripheral;
>  };
>  
>  struct mpc_dma_chan {
> @@ -201,6 +212,10 @@ struct mpc_dma_chan {
>   struct mpc_dma_tcd  *tcd;
>   dma_addr_t  tcd_paddr;
>  
> + /* Settings for access to peripheral FIFO */
> + dma_addr_t  per_paddr;  /* FIFO address */
> + u32 tcd_nunits;
> +
>   /* Lock for this structure */
>   spinlock_t  lock;
>  };
> @@ -251,8 +266,23 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>   struct mpc_dma_desc *mdesc;
>   int cid = mchan->chan.chan_id;
>  
> - /* Move all queued descriptors to active list */
> - list_splice_tail_init(&mchan->queued, &mchan->active);
> + while (!list_empty(&mchan->queued)) {
> + mdesc = list_first_entry(&mchan->queued,
> + struct mpc_dma_desc, node);
> + /*
> +  * Grab either several mem-to-mem transfer descriptors
> +  * or one peripheral transfer descriptor,
> +  * don't mix mem-to-mem and peripheral transfer descriptors
> +  * within the same 'active' list.
> +  */
> + if (mdesc->will_access_peripheral) {
> + if (list_empty(&mchan->active))
> + list_move_tail(&mdesc->node, &mchan->active);
> + break;
> + } else {
> + list_move_tail(&mdesc->node, &mchan->active);
> + }
> + }
>  
>   /* Chain descriptors into one transaction */
>   list_for_each_entry(mdesc, &mchan->active, node) {
> @@ -278,7 +308,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>  
>   if (first != prev)
>   mdma->tcd[cid].e_sg = 1;
> - out_8(&mdma->regs->dmassrt, cid);
> +
> + if (mdma->is_mpc8308) {
> + /* MPC8308, no request lines, software initiated start */
> + out_8(&mdma->regs->dmassrt, cid);
> + } else if (first->will_access_peripheral) {
> + /* peripherals involved, start by external request signal */

Probably you have to keep style of all comments in the code. For
example, let's start sentences from a capital letter.

> + out_8(&mdma->regs->dmaserq, cid);
> + } else {
> + /* memory to memory transfer, software initiated start */
> + out_8(&mdma->regs->dmassrt, cid);
> + }
>  }
>  
>  /* Handle interrupt on one half of DMA controller (32 channels) */
> @@ -596,6 +636,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t 
> dst, dma_addr_t src,
>   }
>  
>   mdesc->error = 0;
> + mdesc->will_access_peripheral = 0;
>   tcd = 

[PATCH RFC v8 2/5] dma: mpc512x: add support for peripheral transfers

2014-02-24 Thread Alexander Popov
Introduce support for slave s/g transfer preparation and the associated
device control callback in the MPC512x DMA controller driver, which adds
support for data transfers between memory and peripheral I/O to the
previously supported mem-to-mem transfers.

Signed-off-by: Alexander Popov 
---
 drivers/dma/mpc512x_dma.c | 235 +-
 1 file changed, 230 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2ce248b..8f504cb 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -2,6 +2,7 @@
  * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
  * Copyright (C) Semihalf 2009
  * Copyright (C) Ilya Yanok, Emcraft Systems 2010
+ * Copyright (C) Alexander Popov, Promcontroller 2013
  *
  * Written by Piotr Ziecik . Hardware description
  * (defines, structures and comments) was taken from MPC5121 DMA driver
@@ -29,8 +30,17 @@
  */
 
 /*
- * This is initial version of MPC5121 DMA driver. Only memory to memory
- * transfers are supported (tested using dmatest module).
+ * MPC512x and MPC8308 DMA driver. It supports
+ * memory to memory data transfers (tested using dmatest module) and
+ * data transfers between memory and peripheral I/O memory
+ * by means of slave s/g with these limitations:
+ *  - chunked transfers (transfers with more than one part) are refused
+ * as long as proper support for scatter/gather is missing;
+ *  - transfers on MPC8308 always start from software as this SoC appears
+ * not to have external request lines for peripheral flow control;
+ *  - minimal memory <-> I/O memory transfer chunk is 4 bytes and consequently
+ * source and destination addresses must be 4-byte aligned
+ * and transfer size must be aligned on (4 * maxburst) boundary;
  */
 
 #include 
@@ -189,6 +199,7 @@ struct mpc_dma_desc {
dma_addr_t  tcd_paddr;
int error;
struct list_headnode;
+   int will_access_peripheral;
 };
 
 struct mpc_dma_chan {
@@ -201,6 +212,10 @@ struct mpc_dma_chan {
struct mpc_dma_tcd  *tcd;
dma_addr_t  tcd_paddr;
 
+   /* Settings for access to peripheral FIFO */
+   dma_addr_t  per_paddr;  /* FIFO address */
+   u32 tcd_nunits;
+
/* Lock for this structure */
spinlock_t  lock;
 };
@@ -251,8 +266,23 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
struct mpc_dma_desc *mdesc;
int cid = mchan->chan.chan_id;
 
-   /* Move all queued descriptors to active list */
-   list_splice_tail_init(&mchan->queued, &mchan->active);
+   while (!list_empty(&mchan->queued)) {
+   mdesc = list_first_entry(&mchan->queued,
+   struct mpc_dma_desc, node);
+   /*
+* Grab either several mem-to-mem transfer descriptors
+* or one peripheral transfer descriptor,
+* don't mix mem-to-mem and peripheral transfer descriptors
+* within the same 'active' list.
+*/
+   if (mdesc->will_access_peripheral) {
+   if (list_empty(&mchan->active))
+   list_move_tail(&mdesc->node, &mchan->active);
+   break;
+   } else {
+   list_move_tail(&mdesc->node, &mchan->active);
+   }
+   }
 
/* Chain descriptors into one transaction */
list_for_each_entry(mdesc, &mchan->active, node) {
@@ -278,7 +308,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
 
if (first != prev)
mdma->tcd[cid].e_sg = 1;
-   out_8(&mdma->regs->dmassrt, cid);
+
+   if (mdma->is_mpc8308) {
+   /* MPC8308, no request lines, software initiated start */
+   out_8(&mdma->regs->dmassrt, cid);
+   } else if (first->will_access_peripheral) {
+   /* peripherals involved, start by external request signal */
+   out_8(&mdma->regs->dmaserq, cid);
+   } else {
+   /* memory to memory transfer, software initiated start */
+   out_8(&mdma->regs->dmassrt, cid);
+   }
 }
 
 /* Handle interrupt on one half of DMA controller (32 channels) */
@@ -596,6 +636,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, 
dma_addr_t src,
}
 
mdesc->error = 0;
+   mdesc->will_access_peripheral = 0;
tcd = mdesc->tcd;
 
/* Prepare Transfer Control Descriptor for this transaction */
@@ -643,6 +684,187 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t 
dst, dma_addr_t src,
return &mdesc->desc;
 }
 
+static struct dma_async_tx_descriptor *
+mpc_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
+