Hi,

El 01/02/15 a las 07:03, Priit Laes escibi?:

On Sat, 2015-01-31 at 19:58 -0300, Emilio L?pez wrote:

quoted

This patch adds support for the DMA engine present on Allwinner A10,
A13, A10S and A20 SoCs. This engine has two kinds of channels:
normal and dedicated. The main difference is in the mode of
operation; while a single normal channel may be operating at any
given time, dedicated channels may operate simultaneously provided
there is no overlap of source or destination.

Hardware documentation can be found on A10 User Manual (section 12),
A13 User Manual (section 14) and A20 User Manual (section 1.12)

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>

(...)

quoted

diff --git a/Documentation/devicetree/bindings/dma/sun4i-dma.txt b/Documentation/devicetree/bindings/dma/sun4i-dma.txt
new file mode 100644
index 0000000..f1634a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/sun4i-dma.txt

@@ -0,0 +1,46 @@
+Allwinner A10 DMA Controller

Don't you want to mention A13, A10S and A20 too?

What if a new SoC shows up with this controller? :) I'd rather give a
single name to the IP, like we do with the compatible strings.

(...)

quoted

diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 2022b54..675b98f 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile

@@ -50,3 +50,4 @@ obj-y += xilinx/
 obj-$(CONFIG_INTEL_MIC_X100_DMA) += mic_x100_dma.o
 obj-$(CONFIG_NBPFAXI_DMA) += nbpfaxi.o
 obj-$(CONFIG_DMA_SUN6I) += sun6i-dma.o
+obj-$(CONFIG_SUN4I_DMA) += sun4i-dma.o

diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c new file mode 100644
index 0000000..a025405
--- /dev/null
+++ b/drivers/dma/sun4i-dma.c

@@ -0,0 +1,1264 @@
+/*
+ * Copyright (C) 2014 Emilio L?pez

2014, 2015 ?

I guess so :)

(...)

quoted

+static int convert_burst(u32 maxburst)
+{
+       if (maxburst > 8)
+               return -EINVAL;

Would it make sense to add check for maxburst = 0 too?

I can add one if you feel it's needed.

quoted

+
+       /* 1 -> 0, 4 -> 1, 8 -> 2 */
+       return (maxburst >> 2);
+}
+
+static int convert_buswidth(enum dma_slave_buswidth addr_width)
+{
+       if (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES)
+               return -EINVAL;
+
+       /* 8 (1 byte) -> 0, 16 (2 bytes) -> 1, 32 (4 bytes) -> 2 */
+       return (addr_width >> 1);
+}
+
+static int choose_optimal_buswidth(dma_addr_t addr)
+{
+       /* On 32 bit aligned addresses, we can use a 32 bit bus width */
+       if (addr % 4 == 0)

Not sure, whether it makes sense to optimize or not, but this can be
calculated like this:

(addr & (4 - 1)) == 0

It looks like IS_ALIGNED() in <linux/kernel.h> is what we need here.

quoted

+static struct sun4i_dma_promise *
+generate_ddma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest,
+                                                               size_t len, struct dma_slave_config *sconfig)
+{
+       struct sun4i_dma_promise *promise;
+       int ret;
+
+       promise = kzalloc(sizeof(*promise), GFP_NOWAIT);
+       if (!promise)
+               return NULL;
+
+       promise->src = src;
+       promise->dst = dest;
+       promise->len = len;
+

No timing parameters or this is just a quirk required for SPI?

They're filled together with the endpoints, in case we need different
ones for some other device. There's a big comment block explaining the
situation on top of their assignment.

quoted

       promise->cfg = DDMA_CFG_LOADING |
DDMA_CFG_BYTE_COUNT_MODE_REMAIN;
+
+       /* Source burst */
+       ret = convert_burst(sconfig->src_maxburst);
+       if (IS_ERR_VALUE(ret))
+               goto fail;
+       promise->cfg |= DDMA_CFG_SRC_BURST_LENGTH(ret);
+

Thanks for reviewing this! :)

Emilio