[PATCH 0/9] PL08x further cleanups

[PATCH 0/9] PL08x further cleanups

[Russell King - ARM Linux]

This patch series is the remainder of the PL08x work I did earlier
this year while trying to get the ARM platforms working with DMA.

Unfortunately, due to the state of the hardware, I was not able to
properly test these changes.  However, they do compile, and I think
if others are going to be looking at the PL08x DMA engine driver,
they're worth having.

Linus, can you check these out on your hardware please?

[PATCH 1/9] DMA: PL08x: remove unused constants

[Russell King - ARM Linux]

PL08X_WQ_PERIODMIN and PL08X_MAX_ALLOCS are not used, remove them.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index e6d7228..90db51f 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -156,14 +156,10 @@ struct pl08x_driver_data {
 #define PL08X_BOUNDARY_SHIFT		(10)	/* 1KB 0x400 */
 #define PL08X_BOUNDARY_SIZE		(1 << PL08X_BOUNDARY_SHIFT)
 
-/* Minimum period between work queue runs */
-#define PL08X_WQ_PERIODMIN	20
-
 /* Size (bytes) of each LLI buffer allocated for one transfer */
 # define PL08X_LLI_TSFR_SIZE	0x2000
 
 /* Maximum times we call dma_pool_alloc on this pool without freeing */
-#define PL08X_MAX_ALLOCS	0x40
 #define MAX_NUM_TSFR_LLIS	(PL08X_LLI_TSFR_SIZE/sizeof(struct pl08x_lli))
 #define PL08X_ALIGN		8
 
-- 
1.7.4.4

[PATCH 2/9] DMA: PL08x: select LLI bus only once per LLI setup

[Russell King - ARM Linux]

Avoid re-selecting the LLI bus each time we create an LLI.  Move it out
of the LLI setup loops.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 90db51f..c28eebf 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -491,10 +491,10 @@ static inline u32 pl08x_cctl_bits(u32 cctl, u8 srcwidth, u8 dstwidth,
 
 struct pl08x_lli_build_data {
 	struct pl08x_txd *txd;
-	struct pl08x_driver_data *pl08x;
 	struct pl08x_bus_data srcbus;
 	struct pl08x_bus_data dstbus;
 	size_t remainder;
+	u32 lli_bus;
 };
 
 /*
@@ -547,8 +547,7 @@ static void pl08x_fill_lli_for_desc(struct pl08x_lli_build_data *bd,
 	llis_va[num_llis].src = bd->srcbus.addr;
 	llis_va[num_llis].dst = bd->dstbus.addr;
 	llis_va[num_llis].lli = llis_bus + (num_llis + 1) * sizeof(struct pl08x_lli);
-	if (bd->pl08x->lli_buses & PL08X_AHB2)
-		llis_va[num_llis].lli |= PL080_LLI_LM_AHB2;
+	llis_va[num_llis].lli |= bd.lli_bus;
 
 	if (cctl & PL080_CONTROL_SRC_INCR)
 		bd->srcbus.addr += len;
@@ -601,9 +600,9 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	cctl = txd->cctl;
 
 	bd.txd = txd;
-	bd.pl08x = pl08x;
 	bd.srcbus.addr = txd->src_addr;
 	bd.dstbus.addr = txd->dst_addr;
+	bd.lli_bus = (pl08x->lli_buses & PL08X_AHB2) ? PL080_LLI_LM_AHB2 : 0;
 
 	/* Find maximum width of the source bus */
 	bd.srcbus.maxwidth =
-- 
1.7.4.4

[PATCH 3/9] DMA: PL08x: clean up LLI debugging

[Russell King - ARM Linux]

Clean up debugging when setting up the LLI list.  This reduces the
amount of output while preserving the information, and makes it easier
to read.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index c28eebf..f043284 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -617,25 +617,15 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	/* Set up the bus widths to the maximum */
 	bd.srcbus.buswidth = bd.srcbus.maxwidth;
 	bd.dstbus.buswidth = bd.dstbus.maxwidth;
-	dev_vdbg(&pl08x->adev->dev,
-		 "%s source bus is %d bytes wide, dest bus is %d bytes wide\n",
-		 __func__, bd.srcbus.buswidth, bd.dstbus.buswidth);
-
 
 	/*
 	 * Bytes transferred == tsize * MIN(buswidths), not max(buswidths)
 	 */
 	max_bytes_per_lli = min(bd.srcbus.buswidth, bd.dstbus.buswidth) *
 		PL080_CONTROL_TRANSFER_SIZE_MASK;
-	dev_vdbg(&pl08x->adev->dev,
-		 "%s max bytes per lli = %zu\n",
-		 __func__, max_bytes_per_lli);
 
 	/* We need to count this down to zero */
 	bd.remainder = txd->len;
-	dev_vdbg(&pl08x->adev->dev,
-		 "%s remainder = %zu\n",
-		 __func__, bd.remainder);
 
 	/*
 	 * Choose bus to align to
@@ -644,6 +634,16 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	 */
 	pl08x_choose_master_bus(&bd, &mbus, &sbus, cctl);
 
+	dev_vdbg(&pl08x->adev->dev, "src=0x%08x%s/%u dst=0x%08x%s/%u len=%zu llimax=%zu\n",
+		 bd.srcbus.addr, cctl & PL080_CONTROL_SRC_INCR ? "+" : "",
+		 bd.srcbus.buswidth,
+		 bd.dstbus.addr, cctl & PL080_CONTROL_DST_INCR ? "+" : "",
+		 bd.dstbus.buswidth,
+		 bd.remainder, max_bytes_per_lli);
+	dev_vdbg(&pl08x->adev->dev, "mbus=%s sbus=%s\n",
+		 mbus == &bd.srcbus ? "src" : "dst",
+		 sbus == &bd.srcbus ? "src" : "dst");
+
 	if (txd->len < mbus->buswidth) {
 		/* Less than a bus width available - send as single bytes */
 		while (bd.remainder) {
@@ -835,15 +835,14 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	{
 		int i;
 
+		dev_vdbg(&pl08x->adev->dev,
+			 "%-3s %-9s  %-10s %-10s %-10s %s\n",
+			 "lli", "", "csrc", "cdst", "clli", "cctl");
 		for (i = 0; i < num_llis; i++) {
 			dev_vdbg(&pl08x->adev->dev,
-				 "lli %d @%p: csrc=0x%08x, cdst=0x%08x, cctl=0x%08x, clli=0x%08x\n",
-				 i,
-				 &llis_va[i],
-				 llis_va[i].src,
-				 llis_va[i].dst,
-				 llis_va[i].cctl,
-				 llis_va[i].lli
+				 "%3d @%p: 0x%08x 0x%08x 0x%08x 0x%08x\n",
+				 i, &llis_va[i], llis_va[i].src,
+				 llis_va[i].dst, llis_va[i].lli, llis_va[i].cctl
 				);
 		}
 	}
-- 
1.7.4.4

[PATCH 4/9] DMA: PL08x: separately store source/destination slave address

[Russell King - ARM Linux]

Store the source/destination slave address separately into the channel
structure.  This moves us towards being able to avoid a configuration
call each time we use the channel.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c   |   21 +++++++++------------
 include/linux/amba/pl08x.h |    3 ++-
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index f043284..d587971 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1102,7 +1102,6 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 	struct pl08x_driver_data *pl08x = plchan->host;
 	struct pl08x_channel_data *cd = plchan->cd;
 	enum dma_slave_buswidth addr_width;
-	dma_addr_t addr;
 	u32 maxburst;
 	u32 cctl = 0;
 	int i;
@@ -1113,11 +1112,9 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 	/* Transfer direction */
 	plchan->runtime_direction = config->direction;
 	if (config->direction == DMA_TO_DEVICE) {
-		addr = config->dst_addr;
 		addr_width = config->dst_addr_width;
 		maxburst = config->dst_maxburst;
 	} else if (config->direction == DMA_FROM_DEVICE) {
-		addr = config->src_addr;
 		addr_width = config->src_addr_width;
 		maxburst = config->src_maxburst;
 	} else {
@@ -1161,7 +1158,11 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 		cctl |= burst_sizes[i].reg;
 	}
 
-	plchan->runtime_addr = addr;
+	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
+		plchan->src_addr = config->src_addr;
+	} else {
+		plchan->dst_addr = config->dst_addr;
+	}
 
 	/* Modify the default channel data to fit PrimeCell request */
 	cd->cctl = cctl;
@@ -1396,19 +1397,13 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 		txd->ccfg |= PL080_FLOW_MEM2PER << PL080_CONFIG_FLOW_CONTROL_SHIFT;
 		txd->cctl |= PL080_CONTROL_SRC_INCR;
 		txd->src_addr = sgl->dma_address;
-		if (plchan->runtime_addr)
-			txd->dst_addr = plchan->runtime_addr;
-		else
-			txd->dst_addr = plchan->cd->addr;
+		txd->dst_addr = plchan->dst_addr;
 		src_buses = pl08x->mem_buses;
 		dst_buses = plchan->cd->periph_buses;
 	} else if (direction == DMA_FROM_DEVICE) {
 		txd->ccfg |= PL080_FLOW_PER2MEM << PL080_CONFIG_FLOW_CONTROL_SHIFT;
 		txd->cctl |= PL080_CONTROL_DST_INCR;
-		if (plchan->runtime_addr)
-			txd->src_addr = plchan->runtime_addr;
-		else
-			txd->src_addr = plchan->cd->addr;
+		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
 		src_buses = plchan->cd->periph_buses;
 		dst_buses = pl08x->mem_buses;
@@ -1704,6 +1699,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 			chan->slave = true;
 			chan->name = pl08x->pd->slave_channels[i].bus_id;
 			chan->cd = &pl08x->pd->slave_channels[i];
+			chan->src_addr = chan->cd->addr;
+			chan->dst_addr = chan->cd->addr;
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
 			chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 3111385..072ab28 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -173,7 +173,8 @@ struct pl08x_dma_chan {
 	struct tasklet_struct tasklet;
 	char *name;
 	struct pl08x_channel_data *cd;
-	dma_addr_t runtime_addr;
+	dma_addr_t src_addr;
+	dma_addr_t dst_addr;
 	enum dma_data_direction	runtime_direction;
 	dma_cookie_t lc;
 	struct list_head pend_list;
-- 
1.7.4.4

[PATCH 5/9] DMA: PL08x: separately store source/destination cctl

[Russell King - ARM Linux]

Store the source/destination cctl values into the channel structure.
This moves us towards being able to avoid a configuration call each
time we use the channel.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c   |   30 ++++++++++++++++--------------
 include/linux/amba/pl08x.h |    2 ++
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index d587971..cb17d92 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1095,12 +1095,21 @@ static const struct burst_table burst_sizes[] = {
 	},
 };
 
+static u32 pl08x_cctl(u32 cctl)
+{
+	cctl &= ~(PL080_CONTROL_SRC_AHB2 | PL080_CONTROL_DST_AHB2 |
+		  PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR |
+		  PL080_CONTROL_PROT_MASK);
+
+	/* Access the cell in privileged mode, non-bufferable, non-cacheable */
+	return cctl | PL080_CONTROL_PROT_SYS;
+}
+
 static int dma_set_runtime_config(struct dma_chan *chan,
 				  struct dma_slave_config *config)
 {
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
-	struct pl08x_channel_data *cd = plchan->cd;
 	enum dma_slave_buswidth addr_width;
 	u32 maxburst;
 	u32 cctl = 0;
@@ -1160,13 +1169,12 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 
 	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
 		plchan->src_addr = config->src_addr;
+		plchan->src_cctl = pl08x_cctl(cctl);
 	} else {
 		plchan->dst_addr = config->dst_addr;
+		plchan->dst_cctl = pl08x_cctl(cctl);
 	}
 
-	/* Modify the default channel data to fit PrimeCell request */
-	cd->cctl = cctl;
-
 	dev_dbg(&pl08x->adev->dev,
 		"configured channel %s (%s) for %s, data width %d, "
 		"maxburst %d words, LE, CCTL=0x%08x\n",
@@ -1385,24 +1393,16 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 	txd->direction = direction;
 	txd->len = sgl->length;
 
-	txd->cctl = plchan->cd->cctl &
-			~(PL080_CONTROL_SRC_AHB2 | PL080_CONTROL_DST_AHB2 |
-			  PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR |
-			  PL080_CONTROL_PROT_MASK);
-
-	/* Access the cell in privileged mode, non-bufferable, non-cacheable */
-	txd->cctl |= PL080_CONTROL_PROT_SYS;
-
 	if (direction == DMA_TO_DEVICE) {
 		txd->ccfg |= PL080_FLOW_MEM2PER << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl |= PL080_CONTROL_SRC_INCR;
+		txd->cctl = plchan->dst_cctl | PL080_CONTROL_SRC_INCR;
 		txd->src_addr = sgl->dma_address;
 		txd->dst_addr = plchan->dst_addr;
 		src_buses = pl08x->mem_buses;
 		dst_buses = plchan->cd->periph_buses;
 	} else if (direction == DMA_FROM_DEVICE) {
 		txd->ccfg |= PL080_FLOW_PER2MEM << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl |= PL080_CONTROL_DST_INCR;
+		txd->cctl = plchan->src_cctl | PL080_CONTROL_DST_INCR;
 		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
 		src_buses = plchan->cd->periph_buses;
@@ -1701,6 +1701,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 			chan->cd = &pl08x->pd->slave_channels[i];
 			chan->src_addr = chan->cd->addr;
 			chan->dst_addr = chan->cd->addr;
+			chan->src_cctl = pl08x_cctl(chan->cd->cctl);
+			chan->dst_cctl = pl08x_cctl(chan->cd->cctl);
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
 			chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 072ab28..47cfe31 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -175,6 +175,8 @@ struct pl08x_dma_chan {
 	struct pl08x_channel_data *cd;
 	dma_addr_t src_addr;
 	dma_addr_t dst_addr;
+	u32 src_cctl;
+	u32 dst_cctl;
 	enum dma_data_direction	runtime_direction;
 	dma_cookie_t lc;
 	struct list_head pend_list;
-- 
1.7.4.4

[PATCH 6/9] DMA: PL08x: constify plchan->cd and plat->slave_channels

[Russell King - ARM Linux]

We no longer write to the channel data structure, so we can make it
const throughout the driver.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/linux/amba/pl08x.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 47cfe31..e6e28f3 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -172,7 +172,7 @@ struct pl08x_dma_chan {
 	int phychan_hold;
 	struct tasklet_struct tasklet;
 	char *name;
-	struct pl08x_channel_data *cd;
+	const struct pl08x_channel_data *cd;
 	dma_addr_t src_addr;
 	dma_addr_t dst_addr;
 	u32 src_cctl;
@@ -205,7 +205,7 @@ struct pl08x_dma_chan {
  * @mem_buses: buses which memory can be accessed from: PL08X_AHB1 | PL08X_AHB2
  */
 struct pl08x_platform_data {
-	struct pl08x_channel_data *slave_channels;
+	const struct pl08x_channel_data *slave_channels;
 	unsigned int num_slave_channels;
 	struct pl08x_channel_data memcpy_channel;
 	int (*get_signal)(struct pl08x_dma_chan *);
-- 
1.7.4.4

[PATCH 7/9] DMA: PL08x: cleanup selection of buswidth

[Russell King - ARM Linux]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index cb17d92..985bc29 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1105,13 +1105,26 @@ static u32 pl08x_cctl(u32 cctl)
 	return cctl | PL080_CONTROL_PROT_SYS;
 }
 
+static u32 pl08x_width(enum dma_slave_buswidth width)
+{
+	switch (width) {
+	case DMA_SLAVE_BUSWIDTH_1_BYTE:
+		return PL08X_WIDTH_8BIT;
+	case DMA_SLAVE_BUSWIDTH_2_BYTES:
+		return PL08X_WIDTH_16BIT;
+	case DMA_SLAVE_BUSWIDTH_4_BYTES:
+		return PL08X_WIDTH_32BIT;
+	}
+	return ~0;
+}
+
 static int dma_set_runtime_config(struct dma_chan *chan,
 				  struct dma_slave_config *config)
 {
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
 	enum dma_slave_buswidth addr_width;
-	u32 maxburst;
+	u32 width, maxburst;
 	u32 cctl = 0;
 	int i;
 
@@ -1132,25 +1145,16 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 		return -EINVAL;
 	}
 
-	switch (addr_width) {
-	case DMA_SLAVE_BUSWIDTH_1_BYTE:
-		cctl |= (PL080_WIDTH_8BIT << PL080_CONTROL_SWIDTH_SHIFT) |
-			(PL080_WIDTH_8BIT << PL080_CONTROL_DWIDTH_SHIFT);
-		break;
-	case DMA_SLAVE_BUSWIDTH_2_BYTES:
-		cctl |= (PL080_WIDTH_16BIT << PL080_CONTROL_SWIDTH_SHIFT) |
-			(PL080_WIDTH_16BIT << PL080_CONTROL_DWIDTH_SHIFT);
-		break;
-	case DMA_SLAVE_BUSWIDTH_4_BYTES:
-		cctl |= (PL080_WIDTH_32BIT << PL080_CONTROL_SWIDTH_SHIFT) |
-			(PL080_WIDTH_32BIT << PL080_CONTROL_DWIDTH_SHIFT);
-		break;
-	default:
+	width = pl08x_width(addr_width);
+	if (width == ~0) {
 		dev_err(&pl08x->adev->dev,
 			"bad runtime_config: alien address width\n");
 		return -EINVAL;
 	}
 
+	cctl |= width << PL080_CONTROL_SWIDTH_SHIFT;
+	cctl |= width << PL080_CONTROL_DWIDTH_SHIFT;
+
 	/*
 	 * Now decide on a maxburst:
 	 * If this channel will only request single transfers, set this
-- 
1.7.4.4

[PATCH 8/9] DMA: PL08x: avoid recalculating cctl at each prepare

[Russell King - ARM Linux]

Now that we have separate cctl values for M>P and P>M transfers, we can
avoid calculating the cctl value each time we prepare a transaction.
Move the bus selection and increment setting to the slave configuration
and initialization functions.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   78 ++++++++++++++++++++++++---------------------
 1 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 985bc29..4e139d7 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1095,6 +1095,23 @@ static const struct burst_table burst_sizes[] = {
 	},
 };
 
+/*
+ * Given the source and destination available bus masks, select which
+ * will be routed to each port.  We try to have source and destination
+ * on separate ports, but always respect the allowable settings.
+ */
+static u32 pl08x_select_bus(u8 src, u8 dst)
+{
+	u32 cctl = 0;
+
+	if (!(dst & PL08X_AHB1) || ((dst & PL08X_AHB2) && (src & PL08X_AHB1)))
+		cctl |= PL080_CONTROL_DST_AHB2;
+	if (!(src & PL08X_AHB1) || ((src & PL08X_AHB2) && !(dst & PL08X_AHB2)))
+		cctl |= PL080_CONTROL_SRC_AHB2;
+
+	return cctl;
+}
+
 static u32 pl08x_cctl(u32 cctl)
 {
 	cctl &= ~(PL080_CONTROL_SRC_AHB2 | PL080_CONTROL_DST_AHB2 |
@@ -1173,10 +1190,14 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 
 	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
 		plchan->src_addr = config->src_addr;
-		plchan->src_cctl = pl08x_cctl(cctl);
+		plchan->src_cctl = pl08x_cctl(cctl) | PL080_CONTROL_DST_INCR |
+			pl08x_select_bus(plchan->cd->periph_buses,
+					 pl08x->mem_buses);
 	} else {
 		plchan->dst_addr = config->dst_addr;
-		plchan->dst_cctl = pl08x_cctl(cctl);
+		plchan->dst_cctl = pl08x_cctl(cctl) | PL080_CONTROL_SRC_INCR |
+			pl08x_select_bus(pl08x->mem_buses,
+					 plchan->cd->periph_buses);
 	}
 
 	dev_dbg(&pl08x->adev->dev,
@@ -1277,23 +1298,6 @@ static int pl08x_prep_channel_resources(struct pl08x_dma_chan *plchan,
 	return 0;
 }
 
-/*
- * Given the source and destination available bus masks, select which
- * will be routed to each port.  We try to have source and destination
- * on separate ports, but always respect the allowable settings.
- */
-static u32 pl08x_select_bus(struct pl08x_driver_data *pl08x, u8 src, u8 dst)
-{
-	u32 cctl = 0;
-
-	if (!(dst & PL08X_AHB1) || ((dst & PL08X_AHB2) && (src & PL08X_AHB1)))
-		cctl |= PL080_CONTROL_DST_AHB2;
-	if (!(src & PL08X_AHB1) || ((src & PL08X_AHB2) && !(dst & PL08X_AHB2)))
-		cctl |= PL080_CONTROL_SRC_AHB2;
-
-	return cctl;
-}
-
 static struct pl08x_txd *pl08x_get_txd(struct pl08x_dma_chan *plchan,
 	unsigned long flags)
 {
@@ -1345,8 +1349,8 @@ static struct dma_async_tx_descriptor *pl08x_prep_dma_memcpy(
 	txd->cctl |= PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR;
 
 	if (pl08x->vd->dualmaster)
-		txd->cctl |= pl08x_select_bus(pl08x,
-					pl08x->mem_buses, pl08x->mem_buses);
+		txd->cctl |= pl08x_select_bus(pl08x->mem_buses,
+					      pl08x->mem_buses);
 
 	ret = pl08x_prep_channel_resources(plchan, txd);
 	if (ret)
@@ -1363,7 +1367,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
 	struct pl08x_txd *txd;
-	u8 src_buses, dst_buses;
 	int ret;
 
 	/*
@@ -1399,26 +1402,20 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 
 	if (direction == DMA_TO_DEVICE) {
 		txd->ccfg |= PL080_FLOW_MEM2PER << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl = plchan->dst_cctl | PL080_CONTROL_SRC_INCR;
+		txd->cctl = plchan->dst_cctl;
 		txd->src_addr = sgl->dma_address;
 		txd->dst_addr = plchan->dst_addr;
-		src_buses = pl08x->mem_buses;
-		dst_buses = plchan->cd->periph_buses;
 	} else if (direction == DMA_FROM_DEVICE) {
 		txd->ccfg |= PL080_FLOW_PER2MEM << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl = plchan->src_cctl | PL080_CONTROL_DST_INCR;
+		txd->cctl = plchan->src_cctl;
 		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
-		src_buses = plchan->cd->periph_buses;
-		dst_buses = pl08x->mem_buses;
 	} else {
 		dev_err(&pl08x->adev->dev,
 			"%s direction unsupported\n", __func__);
 		return NULL;
 	}
 
-	txd->cctl |= pl08x_select_bus(pl08x, src_buses, dst_buses);
-
 	ret = pl08x_prep_channel_resources(plchan, txd);
 	if (ret)
 		return NULL;
@@ -1669,6 +1666,20 @@ static irqreturn_t pl08x_irq(int irq, void *dev)
 	return mask ? IRQ_HANDLED : IRQ_NONE;
 }
 
+static void pl08x_dma_slave_init(struct pl08x_dma_chan *chan)
+{
+	u32 cctl = pl08x_cctl(chan->cd->cctl);
+
+	chan->slave = true;
+	chan->name = chan->cd->bus_id;
+	chan->src_addr = chan->cd->addr;
+	chan->dst_addr = chan->cd->addr;
+	chan->src_cctl = cctl | PL080_CONTROL_DST_INCR |
+		pl08x_select_bus(chan->cd->periph_buses, chan->host->mem_buses);
+	chan->dst_cctl = cctl | PL080_CONTROL_SRC_INCR |
+		pl08x_select_bus(chan->host->mem_buses, chan->cd->periph_buses);
+}
+
 /*
  * Initialise the DMAC memcpy/slave channels.
  * Make a local wrapper to hold required data
@@ -1700,13 +1711,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 		chan->state = PL08X_CHAN_IDLE;
 
 		if (slave) {
-			chan->slave = true;
-			chan->name = pl08x->pd->slave_channels[i].bus_id;
 			chan->cd = &pl08x->pd->slave_channels[i];
-			chan->src_addr = chan->cd->addr;
-			chan->dst_addr = chan->cd->addr;
-			chan->src_cctl = pl08x_cctl(chan->cd->cctl);
-			chan->dst_cctl = pl08x_cctl(chan->cd->cctl);
+			pl08x_dma_slave_init(chan);
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
 			chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
-- 
1.7.4.4

[PATCH 9/9] DMA: PL08x: cleanup selection of burst size

[Russell King - ARM Linux]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   58 ++++++++++++++++++++++-----------------------
 1 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 4e139d7..e734fe6 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1048,50 +1048,42 @@ pl08x_dma_tx_status(struct dma_chan *chan,
 
 /* PrimeCell DMA extension */
 struct burst_table {
-	int burstwords;
+	u32 burstwords;
 	u32 reg;
 };
 
 static const struct burst_table burst_sizes[] = {
 	{
 		.burstwords = 256,
-		.reg = (PL080_BSIZE_256 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_256 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_256,
 	},
 	{
 		.burstwords = 128,
-		.reg = (PL080_BSIZE_128 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_128 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_128,
 	},
 	{
 		.burstwords = 64,
-		.reg = (PL080_BSIZE_64 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_64 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_64,
 	},
 	{
 		.burstwords = 32,
-		.reg = (PL080_BSIZE_32 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_32 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_32,
 	},
 	{
 		.burstwords = 16,
-		.reg = (PL080_BSIZE_16 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_16 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_16,
 	},
 	{
 		.burstwords = 8,
-		.reg = (PL080_BSIZE_8 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_8 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_8,
 	},
 	{
 		.burstwords = 4,
-		.reg = (PL080_BSIZE_4 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_4 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_4,
 	},
 	{
-		.burstwords = 1,
-		.reg = (PL080_BSIZE_1 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_1 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.burstwords = 0,
+		.reg = PL080_BSIZE_1,
 	},
 };
 
@@ -1135,15 +1127,25 @@ static u32 pl08x_width(enum dma_slave_buswidth width)
 	return ~0;
 }
 
+static u32 pl08x_burst(u32 maxburst)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(burst_sizes); i++)
+		if (burst_sizes[i].burstwords <= maxburst)
+			break;
+
+	return burst_sizes[i].reg;
+}
+
 static int dma_set_runtime_config(struct dma_chan *chan,
 				  struct dma_slave_config *config)
 {
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
 	enum dma_slave_buswidth addr_width;
-	u32 width, maxburst;
+	u32 width, burst, maxburst;
 	u32 cctl = 0;
-	int i;
 
 	if (!plchan->slave)
 		return -EINVAL;
@@ -1173,20 +1175,16 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 	cctl |= width << PL080_CONTROL_DWIDTH_SHIFT;
 
 	/*
-	 * Now decide on a maxburst:
 	 * If this channel will only request single transfers, set this
 	 * down to ONE element.  Also select one element if no maxburst
 	 * is specified.
 	 */
-	if (plchan->cd->single || maxburst == 0) {
-		cctl |= (PL080_BSIZE_1 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_1 << PL080_CONTROL_DB_SIZE_SHIFT);
-	} else {
-		for (i = 0; i < ARRAY_SIZE(burst_sizes); i++)
-			if (burst_sizes[i].burstwords <= maxburst)
-				break;
-		cctl |= burst_sizes[i].reg;
-	}
+	if (plchan->cd->single)
+		maxburst = 1;
+
+	burst = pl08x_burst(maxburst);
+	cctl |= burst << PL080_CONTROL_SB_SIZE_SHIFT;
+	cctl |= burst << PL080_CONTROL_DB_SIZE_SHIFT;
 
 	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
 		plchan->src_addr = config->src_addr;
-- 
1.7.4.4

[PATCH 0/9] PL08x further cleanups

[Linus Walleij]

2011/7/5 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> Linus, can you check these out on your hardware please?

Sadly I'm on vacation in the Wilderness and that PB11MPCore
sits miles away. I will not be able to test the patches until
august.

However, I think we have no boards configuring in the platform
data for these RealViews/Versatiles yet, I just have some
platform data patches boiling that I was working on the other
week for testing this stuff, so I think the patches should just
be applied, if I find regressions I can surely fix them easily.

So for all of these:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Linus Walleij

[PATCH 0/9] PL08x further cleanups

[Koul, Vinod]

On Tue, 2011-07-05 at 14:10 +0100, Russell King - ARM Linux wrote:
> This patch series is the remainder of the PL08x work I did earlier
> this year while trying to get the ARM platforms working with DMA.
> 
> Unfortunately, due to the state of the hardware, I was not able to
> properly test these changes.  However, they do compile, and I think
> if others are going to be looking at the PL08x DMA engine driver,
> they're worth having.
> 
> Linus, can you check these out on your hardware please?

Russell,

The fail to build for me, on line 550 it refers to bd.llil_bus whereas
the bd is a pointer. Further the WIDTH macros seem to be missing.

While you are fixing this, you may want to fix the minor checkpatch
warning as well.

drivers/dma/amba-pl08x.c: In function 'pl08x_fill_lli_for_desc':
drivers/dma/amba-pl08x.c:550: error: request for member 'lli_bus' in
something not a structure or union
drivers/dma/amba-pl08x.c: In function 'pl08x_width':
drivers/dma/amba-pl08x.c:1121: error: 'PL08X_WIDTH_8BIT' undeclared
(first use in this function)
drivers/dma/amba-pl08x.c:1121: error: (Each undeclared identifier is
reported only once
drivers/dma/amba-pl08x.c:1121: error: for each function it appears in.)
drivers/dma/amba-pl08x.c:1123: error: 'PL08X_WIDTH_16BIT' undeclared
(first use in this function)
drivers/dma/amba-pl08x.c:1125: error: 'PL08X_WIDTH_32BIT' undeclared
(first use in this function)
drivers/dma/amba-pl08x.c:1119: warning: enumeration value
'DMA_SLAVE_BUSWIDTH_UNDEFINED' not handled in switch
drivers/dma/amba-pl08x.c:1119: warning: enumeration value
'DMA_SLAVE_BUSWIDTH_8_BYTES' not handled in switch
make[1]: *** [drivers/dma/amba-pl08x.o] Error 1


-- 
~Vinod

[PATCH 0/9] PL08x further cleanups

[Russell King - ARM Linux]

On Thu, Jul 14, 2011 at 04:35:28AM +0530, Koul, Vinod wrote:
> On Tue, 2011-07-05 at 14:10 +0100, Russell King - ARM Linux wrote:
> > This patch series is the remainder of the PL08x work I did earlier
> > this year while trying to get the ARM platforms working with DMA.
> > 
> > Unfortunately, due to the state of the hardware, I was not able to
> > properly test these changes.  However, they do compile, and I think
> > if others are going to be looking at the PL08x DMA engine driver,
> > they're worth having.
> > 
> > Linus, can you check these out on your hardware please?
> 
> Russell,
> 
> The fail to build for me, on line 550 it refers to bd.llil_bus whereas
> the bd is a pointer. Further the WIDTH macros seem to be missing.

Grr, I've updated the patches and will post the two updated ones.  I
have them in my git tree - would you like them via git instead, or
would you prefer to provide acks and let me push them upstream?

Are there any conflicting changes in amba-pl08x?

> While you are fixing this, you may want to fix the minor checkpatch
> warning as well.

No, the checkpatch warning is inappropriate - and yet again this
illustrates why checkpatch should be a _guide_ and not mandatory.
Checkpatch is warning about:

+       if (plchan->runtime_direction == DMA_FROM_DEVICE) {
+               plchan->src_addr = config->src_addr;
+       } else {
+               plchan->dst_addr = config->dst_addr;
+       }

but in the very next patch this changes to:

        if (plchan->runtime_direction == DMA_FROM_DEVICE) {
                plchan->src_addr = config->src_addr;
+               plchan->src_cctl = pl08x_cctl(cctl);
        } else {
                plchan->dst_addr = config->dst_addr;
+               plchan->dst_cctl = pl08x_cctl(cctl);
        }

Changing this so that the first patch doesn't use the braces, and add
them in the second patch makes the second patch needlessly more noisy
for no benefit.  So no, I'm not fixing that because it makes things
unnecessarily more complex.

[PATCH 1/9] DMA: PL08x: remove unused constants

[Russell King - ARM Linux]

PL08X_WQ_PERIODMIN and PL08X_MAX_ALLOCS are not used, remove them.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index e6d7228..90db51f 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -156,14 +156,10 @@ struct pl08x_driver_data {
 #define PL08X_BOUNDARY_SHIFT		(10)	/* 1KB 0x400 */
 #define PL08X_BOUNDARY_SIZE		(1 << PL08X_BOUNDARY_SHIFT)
 
-/* Minimum period between work queue runs */
-#define PL08X_WQ_PERIODMIN	20
-
 /* Size (bytes) of each LLI buffer allocated for one transfer */
 # define PL08X_LLI_TSFR_SIZE	0x2000
 
 /* Maximum times we call dma_pool_alloc on this pool without freeing */
-#define PL08X_MAX_ALLOCS	0x40
 #define MAX_NUM_TSFR_LLIS	(PL08X_LLI_TSFR_SIZE/sizeof(struct pl08x_lli))
 #define PL08X_ALIGN		8
 
-- 
1.7.4.4

[PATCH 2/9] DMA: PL08x: select LLI bus only once per LLI setup

[Russell King - ARM Linux]

Avoid re-selecting the LLI bus each time we create an LLI.  Move it out
of the LLI setup loops.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 90db51f..6808f7d 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -491,10 +491,10 @@ static inline u32 pl08x_cctl_bits(u32 cctl, u8 srcwidth, u8 dstwidth,
 
 struct pl08x_lli_build_data {
 	struct pl08x_txd *txd;
-	struct pl08x_driver_data *pl08x;
 	struct pl08x_bus_data srcbus;
 	struct pl08x_bus_data dstbus;
 	size_t remainder;
+	u32 lli_bus;
 };
 
 /*
@@ -547,8 +547,7 @@ static void pl08x_fill_lli_for_desc(struct pl08x_lli_build_data *bd,
 	llis_va[num_llis].src = bd->srcbus.addr;
 	llis_va[num_llis].dst = bd->dstbus.addr;
 	llis_va[num_llis].lli = llis_bus + (num_llis + 1) * sizeof(struct pl08x_lli);
-	if (bd->pl08x->lli_buses & PL08X_AHB2)
-		llis_va[num_llis].lli |= PL080_LLI_LM_AHB2;
+	llis_va[num_llis].lli |= bd->lli_bus;
 
 	if (cctl & PL080_CONTROL_SRC_INCR)
 		bd->srcbus.addr += len;
@@ -601,9 +600,9 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	cctl = txd->cctl;
 
 	bd.txd = txd;
-	bd.pl08x = pl08x;
 	bd.srcbus.addr = txd->src_addr;
 	bd.dstbus.addr = txd->dst_addr;
+	bd.lli_bus = (pl08x->lli_buses & PL08X_AHB2) ? PL080_LLI_LM_AHB2 : 0;
 
 	/* Find maximum width of the source bus */
 	bd.srcbus.maxwidth =
-- 
1.7.4.4

[PATCH 3/9] DMA: PL08x: clean up LLI debugging

[Russell King - ARM Linux]

Clean up debugging when setting up the LLI list.  This reduces the
amount of output while preserving the information, and makes it easier
to read.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 6808f7d..1c641bf 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -617,25 +617,15 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	/* Set up the bus widths to the maximum */
 	bd.srcbus.buswidth = bd.srcbus.maxwidth;
 	bd.dstbus.buswidth = bd.dstbus.maxwidth;
-	dev_vdbg(&pl08x->adev->dev,
-		 "%s source bus is %d bytes wide, dest bus is %d bytes wide\n",
-		 __func__, bd.srcbus.buswidth, bd.dstbus.buswidth);
-
 
 	/*
 	 * Bytes transferred == tsize * MIN(buswidths), not max(buswidths)
 	 */
 	max_bytes_per_lli = min(bd.srcbus.buswidth, bd.dstbus.buswidth) *
 		PL080_CONTROL_TRANSFER_SIZE_MASK;
-	dev_vdbg(&pl08x->adev->dev,
-		 "%s max bytes per lli = %zu\n",
-		 __func__, max_bytes_per_lli);
 
 	/* We need to count this down to zero */
 	bd.remainder = txd->len;
-	dev_vdbg(&pl08x->adev->dev,
-		 "%s remainder = %zu\n",
-		 __func__, bd.remainder);
 
 	/*
 	 * Choose bus to align to
@@ -644,6 +634,16 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	 */
 	pl08x_choose_master_bus(&bd, &mbus, &sbus, cctl);
 
+	dev_vdbg(&pl08x->adev->dev, "src=0x%08x%s/%u dst=0x%08x%s/%u len=%zu llimax=%zu\n",
+		 bd.srcbus.addr, cctl & PL080_CONTROL_SRC_INCR ? "+" : "",
+		 bd.srcbus.buswidth,
+		 bd.dstbus.addr, cctl & PL080_CONTROL_DST_INCR ? "+" : "",
+		 bd.dstbus.buswidth,
+		 bd.remainder, max_bytes_per_lli);
+	dev_vdbg(&pl08x->adev->dev, "mbus=%s sbus=%s\n",
+		 mbus == &bd.srcbus ? "src" : "dst",
+		 sbus == &bd.srcbus ? "src" : "dst");
+
 	if (txd->len < mbus->buswidth) {
 		/* Less than a bus width available - send as single bytes */
 		while (bd.remainder) {
@@ -835,15 +835,14 @@ static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	{
 		int i;
 
+		dev_vdbg(&pl08x->adev->dev,
+			 "%-3s %-9s  %-10s %-10s %-10s %s\n",
+			 "lli", "", "csrc", "cdst", "clli", "cctl");
 		for (i = 0; i < num_llis; i++) {
 			dev_vdbg(&pl08x->adev->dev,
-				 "lli %d @%p: csrc=0x%08x, cdst=0x%08x, cctl=0x%08x, clli=0x%08x\n",
-				 i,
-				 &llis_va[i],
-				 llis_va[i].src,
-				 llis_va[i].dst,
-				 llis_va[i].cctl,
-				 llis_va[i].lli
+				 "%3d @%p: 0x%08x 0x%08x 0x%08x 0x%08x\n",
+				 i, &llis_va[i], llis_va[i].src,
+				 llis_va[i].dst, llis_va[i].lli, llis_va[i].cctl
 				);
 		}
 	}
-- 
1.7.4.4

[PATCH 4/9] DMA: PL08x: separately store source/destination slave address

[Russell King - ARM Linux]

Store the source/destination slave address separately into the channel
structure.  This moves us towards being able to avoid a configuration
call each time we use the channel.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c   |   21 +++++++++------------
 include/linux/amba/pl08x.h |    3 ++-
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 1c641bf..077ddee 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1102,7 +1102,6 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 	struct pl08x_driver_data *pl08x = plchan->host;
 	struct pl08x_channel_data *cd = plchan->cd;
 	enum dma_slave_buswidth addr_width;
-	dma_addr_t addr;
 	u32 maxburst;
 	u32 cctl = 0;
 	int i;
@@ -1113,11 +1112,9 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 	/* Transfer direction */
 	plchan->runtime_direction = config->direction;
 	if (config->direction == DMA_TO_DEVICE) {
-		addr = config->dst_addr;
 		addr_width = config->dst_addr_width;
 		maxburst = config->dst_maxburst;
 	} else if (config->direction == DMA_FROM_DEVICE) {
-		addr = config->src_addr;
 		addr_width = config->src_addr_width;
 		maxburst = config->src_maxburst;
 	} else {
@@ -1161,7 +1158,11 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 		cctl |= burst_sizes[i].reg;
 	}
 
-	plchan->runtime_addr = addr;
+	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
+		plchan->src_addr = config->src_addr;
+	} else {
+		plchan->dst_addr = config->dst_addr;
+	}
 
 	/* Modify the default channel data to fit PrimeCell request */
 	cd->cctl = cctl;
@@ -1396,19 +1397,13 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 		txd->ccfg |= PL080_FLOW_MEM2PER << PL080_CONFIG_FLOW_CONTROL_SHIFT;
 		txd->cctl |= PL080_CONTROL_SRC_INCR;
 		txd->src_addr = sgl->dma_address;
-		if (plchan->runtime_addr)
-			txd->dst_addr = plchan->runtime_addr;
-		else
-			txd->dst_addr = plchan->cd->addr;
+		txd->dst_addr = plchan->dst_addr;
 		src_buses = pl08x->mem_buses;
 		dst_buses = plchan->cd->periph_buses;
 	} else if (direction == DMA_FROM_DEVICE) {
 		txd->ccfg |= PL080_FLOW_PER2MEM << PL080_CONFIG_FLOW_CONTROL_SHIFT;
 		txd->cctl |= PL080_CONTROL_DST_INCR;
-		if (plchan->runtime_addr)
-			txd->src_addr = plchan->runtime_addr;
-		else
-			txd->src_addr = plchan->cd->addr;
+		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
 		src_buses = plchan->cd->periph_buses;
 		dst_buses = pl08x->mem_buses;
@@ -1704,6 +1699,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 			chan->slave = true;
 			chan->name = pl08x->pd->slave_channels[i].bus_id;
 			chan->cd = &pl08x->pd->slave_channels[i];
+			chan->src_addr = chan->cd->addr;
+			chan->dst_addr = chan->cd->addr;
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
 			chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 3111385..072ab28 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -173,7 +173,8 @@ struct pl08x_dma_chan {
 	struct tasklet_struct tasklet;
 	char *name;
 	struct pl08x_channel_data *cd;
-	dma_addr_t runtime_addr;
+	dma_addr_t src_addr;
+	dma_addr_t dst_addr;
 	enum dma_data_direction	runtime_direction;
 	dma_cookie_t lc;
 	struct list_head pend_list;
-- 
1.7.4.4

[PATCH 5/9] DMA: PL08x: separately store source/destination cctl

[Russell King - ARM Linux]

Store the source/destination cctl values into the channel structure.
This moves us towards being able to avoid a configuration call each
time we use the channel.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c   |   30 ++++++++++++++++--------------
 include/linux/amba/pl08x.h |    2 ++
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 077ddee..ba617e3f 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1095,12 +1095,21 @@ static const struct burst_table burst_sizes[] = {
 	},
 };
 
+static u32 pl08x_cctl(u32 cctl)
+{
+	cctl &= ~(PL080_CONTROL_SRC_AHB2 | PL080_CONTROL_DST_AHB2 |
+		  PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR |
+		  PL080_CONTROL_PROT_MASK);
+
+	/* Access the cell in privileged mode, non-bufferable, non-cacheable */
+	return cctl | PL080_CONTROL_PROT_SYS;
+}
+
 static int dma_set_runtime_config(struct dma_chan *chan,
 				  struct dma_slave_config *config)
 {
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
-	struct pl08x_channel_data *cd = plchan->cd;
 	enum dma_slave_buswidth addr_width;
 	u32 maxburst;
 	u32 cctl = 0;
@@ -1160,13 +1169,12 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 
 	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
 		plchan->src_addr = config->src_addr;
+		plchan->src_cctl = pl08x_cctl(cctl);
 	} else {
 		plchan->dst_addr = config->dst_addr;
+		plchan->dst_cctl = pl08x_cctl(cctl);
 	}
 
-	/* Modify the default channel data to fit PrimeCell request */
-	cd->cctl = cctl;
-
 	dev_dbg(&pl08x->adev->dev,
 		"configured channel %s (%s) for %s, data width %d, "
 		"maxburst %d words, LE, CCTL=0x%08x\n",
@@ -1385,24 +1393,16 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 	txd->direction = direction;
 	txd->len = sgl->length;
 
-	txd->cctl = plchan->cd->cctl &
-			~(PL080_CONTROL_SRC_AHB2 | PL080_CONTROL_DST_AHB2 |
-			  PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR |
-			  PL080_CONTROL_PROT_MASK);
-
-	/* Access the cell in privileged mode, non-bufferable, non-cacheable */
-	txd->cctl |= PL080_CONTROL_PROT_SYS;
-
 	if (direction == DMA_TO_DEVICE) {
 		txd->ccfg |= PL080_FLOW_MEM2PER << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl |= PL080_CONTROL_SRC_INCR;
+		txd->cctl = plchan->dst_cctl | PL080_CONTROL_SRC_INCR;
 		txd->src_addr = sgl->dma_address;
 		txd->dst_addr = plchan->dst_addr;
 		src_buses = pl08x->mem_buses;
 		dst_buses = plchan->cd->periph_buses;
 	} else if (direction == DMA_FROM_DEVICE) {
 		txd->ccfg |= PL080_FLOW_PER2MEM << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl |= PL080_CONTROL_DST_INCR;
+		txd->cctl = plchan->src_cctl | PL080_CONTROL_DST_INCR;
 		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
 		src_buses = plchan->cd->periph_buses;
@@ -1701,6 +1701,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 			chan->cd = &pl08x->pd->slave_channels[i];
 			chan->src_addr = chan->cd->addr;
 			chan->dst_addr = chan->cd->addr;
+			chan->src_cctl = pl08x_cctl(chan->cd->cctl);
+			chan->dst_cctl = pl08x_cctl(chan->cd->cctl);
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
 			chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 072ab28..47cfe31 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -175,6 +175,8 @@ struct pl08x_dma_chan {
 	struct pl08x_channel_data *cd;
 	dma_addr_t src_addr;
 	dma_addr_t dst_addr;
+	u32 src_cctl;
+	u32 dst_cctl;
 	enum dma_data_direction	runtime_direction;
 	dma_cookie_t lc;
 	struct list_head pend_list;
-- 
1.7.4.4

[PATCH 6/9] DMA: PL08x: constify plchan->cd and plat->slave_channels

[Russell King - ARM Linux]

We no longer write to the channel data structure, so we can make it
const throughout the driver.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/linux/amba/pl08x.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 47cfe31..e6e28f3 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -172,7 +172,7 @@ struct pl08x_dma_chan {
 	int phychan_hold;
 	struct tasklet_struct tasklet;
 	char *name;
-	struct pl08x_channel_data *cd;
+	const struct pl08x_channel_data *cd;
 	dma_addr_t src_addr;
 	dma_addr_t dst_addr;
 	u32 src_cctl;
@@ -205,7 +205,7 @@ struct pl08x_dma_chan {
  * @mem_buses: buses which memory can be accessed from: PL08X_AHB1 | PL08X_AHB2
  */
 struct pl08x_platform_data {
-	struct pl08x_channel_data *slave_channels;
+	const struct pl08x_channel_data *slave_channels;
 	unsigned int num_slave_channels;
 	struct pl08x_channel_data memcpy_channel;
 	int (*get_signal)(struct pl08x_dma_chan *);
-- 
1.7.4.4

[PATCH 7/9] DMA: PL08x: cleanup selection of buswidth

[Russell King - ARM Linux]

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index ba617e3f..2dd37ff 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1105,13 +1105,26 @@ static u32 pl08x_cctl(u32 cctl)
 	return cctl | PL080_CONTROL_PROT_SYS;
 }
 
+static u32 pl08x_width(enum dma_slave_buswidth width)
+{
+	switch (width) {
+	case DMA_SLAVE_BUSWIDTH_1_BYTE:
+		return PL080_WIDTH_8BIT;
+	case DMA_SLAVE_BUSWIDTH_2_BYTES:
+		return PL080_WIDTH_16BIT;
+	case DMA_SLAVE_BUSWIDTH_4_BYTES:
+		return PL080_WIDTH_32BIT;
+	}
+	return ~0;
+}
+
 static int dma_set_runtime_config(struct dma_chan *chan,
 				  struct dma_slave_config *config)
 {
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
 	enum dma_slave_buswidth addr_width;
-	u32 maxburst;
+	u32 width, maxburst;
 	u32 cctl = 0;
 	int i;
 
@@ -1132,25 +1145,16 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 		return -EINVAL;
 	}
 
-	switch (addr_width) {
-	case DMA_SLAVE_BUSWIDTH_1_BYTE:
-		cctl |= (PL080_WIDTH_8BIT << PL080_CONTROL_SWIDTH_SHIFT) |
-			(PL080_WIDTH_8BIT << PL080_CONTROL_DWIDTH_SHIFT);
-		break;
-	case DMA_SLAVE_BUSWIDTH_2_BYTES:
-		cctl |= (PL080_WIDTH_16BIT << PL080_CONTROL_SWIDTH_SHIFT) |
-			(PL080_WIDTH_16BIT << PL080_CONTROL_DWIDTH_SHIFT);
-		break;
-	case DMA_SLAVE_BUSWIDTH_4_BYTES:
-		cctl |= (PL080_WIDTH_32BIT << PL080_CONTROL_SWIDTH_SHIFT) |
-			(PL080_WIDTH_32BIT << PL080_CONTROL_DWIDTH_SHIFT);
-		break;
-	default:
+	width = pl08x_width(addr_width);
+	if (width == ~0) {
 		dev_err(&pl08x->adev->dev,
 			"bad runtime_config: alien address width\n");
 		return -EINVAL;
 	}
 
+	cctl |= width << PL080_CONTROL_SWIDTH_SHIFT;
+	cctl |= width << PL080_CONTROL_DWIDTH_SHIFT;
+
 	/*
 	 * Now decide on a maxburst:
 	 * If this channel will only request single transfers, set this
-- 
1.7.4.4

[PATCH 8/9] DMA: PL08x: avoid recalculating cctl at each prepare

[Russell King - ARM Linux]

Now that we have separate cctl values for M>P and P>M transfers, we can
avoid calculating the cctl value each time we prepare a transaction.
Move the bus selection and increment setting to the slave configuration
and initialization functions.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   78 ++++++++++++++++++++++++---------------------
 1 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 2dd37ff..a84db8b 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1095,6 +1095,23 @@ static const struct burst_table burst_sizes[] = {
 	},
 };
 
+/*
+ * Given the source and destination available bus masks, select which
+ * will be routed to each port.  We try to have source and destination
+ * on separate ports, but always respect the allowable settings.
+ */
+static u32 pl08x_select_bus(u8 src, u8 dst)
+{
+	u32 cctl = 0;
+
+	if (!(dst & PL08X_AHB1) || ((dst & PL08X_AHB2) && (src & PL08X_AHB1)))
+		cctl |= PL080_CONTROL_DST_AHB2;
+	if (!(src & PL08X_AHB1) || ((src & PL08X_AHB2) && !(dst & PL08X_AHB2)))
+		cctl |= PL080_CONTROL_SRC_AHB2;
+
+	return cctl;
+}
+
 static u32 pl08x_cctl(u32 cctl)
 {
 	cctl &= ~(PL080_CONTROL_SRC_AHB2 | PL080_CONTROL_DST_AHB2 |
@@ -1173,10 +1190,14 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 
 	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
 		plchan->src_addr = config->src_addr;
-		plchan->src_cctl = pl08x_cctl(cctl);
+		plchan->src_cctl = pl08x_cctl(cctl) | PL080_CONTROL_DST_INCR |
+			pl08x_select_bus(plchan->cd->periph_buses,
+					 pl08x->mem_buses);
 	} else {
 		plchan->dst_addr = config->dst_addr;
-		plchan->dst_cctl = pl08x_cctl(cctl);
+		plchan->dst_cctl = pl08x_cctl(cctl) | PL080_CONTROL_SRC_INCR |
+			pl08x_select_bus(pl08x->mem_buses,
+					 plchan->cd->periph_buses);
 	}
 
 	dev_dbg(&pl08x->adev->dev,
@@ -1277,23 +1298,6 @@ static int pl08x_prep_channel_resources(struct pl08x_dma_chan *plchan,
 	return 0;
 }
 
-/*
- * Given the source and destination available bus masks, select which
- * will be routed to each port.  We try to have source and destination
- * on separate ports, but always respect the allowable settings.
- */
-static u32 pl08x_select_bus(struct pl08x_driver_data *pl08x, u8 src, u8 dst)
-{
-	u32 cctl = 0;
-
-	if (!(dst & PL08X_AHB1) || ((dst & PL08X_AHB2) && (src & PL08X_AHB1)))
-		cctl |= PL080_CONTROL_DST_AHB2;
-	if (!(src & PL08X_AHB1) || ((src & PL08X_AHB2) && !(dst & PL08X_AHB2)))
-		cctl |= PL080_CONTROL_SRC_AHB2;
-
-	return cctl;
-}
-
 static struct pl08x_txd *pl08x_get_txd(struct pl08x_dma_chan *plchan,
 	unsigned long flags)
 {
@@ -1345,8 +1349,8 @@ static struct dma_async_tx_descriptor *pl08x_prep_dma_memcpy(
 	txd->cctl |= PL080_CONTROL_SRC_INCR | PL080_CONTROL_DST_INCR;
 
 	if (pl08x->vd->dualmaster)
-		txd->cctl |= pl08x_select_bus(pl08x,
-					pl08x->mem_buses, pl08x->mem_buses);
+		txd->cctl |= pl08x_select_bus(pl08x->mem_buses,
+					      pl08x->mem_buses);
 
 	ret = pl08x_prep_channel_resources(plchan, txd);
 	if (ret)
@@ -1363,7 +1367,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
 	struct pl08x_txd *txd;
-	u8 src_buses, dst_buses;
 	int ret;
 
 	/*
@@ -1399,26 +1402,20 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 
 	if (direction == DMA_TO_DEVICE) {
 		txd->ccfg |= PL080_FLOW_MEM2PER << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl = plchan->dst_cctl | PL080_CONTROL_SRC_INCR;
+		txd->cctl = plchan->dst_cctl;
 		txd->src_addr = sgl->dma_address;
 		txd->dst_addr = plchan->dst_addr;
-		src_buses = pl08x->mem_buses;
-		dst_buses = plchan->cd->periph_buses;
 	} else if (direction == DMA_FROM_DEVICE) {
 		txd->ccfg |= PL080_FLOW_PER2MEM << PL080_CONFIG_FLOW_CONTROL_SHIFT;
-		txd->cctl = plchan->src_cctl | PL080_CONTROL_DST_INCR;
+		txd->cctl = plchan->src_cctl;
 		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
-		src_buses = plchan->cd->periph_buses;
-		dst_buses = pl08x->mem_buses;
 	} else {
 		dev_err(&pl08x->adev->dev,
 			"%s direction unsupported\n", __func__);
 		return NULL;
 	}
 
-	txd->cctl |= pl08x_select_bus(pl08x, src_buses, dst_buses);
-
 	ret = pl08x_prep_channel_resources(plchan, txd);
 	if (ret)
 		return NULL;
@@ -1669,6 +1666,20 @@ static irqreturn_t pl08x_irq(int irq, void *dev)
 	return mask ? IRQ_HANDLED : IRQ_NONE;
 }
 
+static void pl08x_dma_slave_init(struct pl08x_dma_chan *chan)
+{
+	u32 cctl = pl08x_cctl(chan->cd->cctl);
+
+	chan->slave = true;
+	chan->name = chan->cd->bus_id;
+	chan->src_addr = chan->cd->addr;
+	chan->dst_addr = chan->cd->addr;
+	chan->src_cctl = cctl | PL080_CONTROL_DST_INCR |
+		pl08x_select_bus(chan->cd->periph_buses, chan->host->mem_buses);
+	chan->dst_cctl = cctl | PL080_CONTROL_SRC_INCR |
+		pl08x_select_bus(chan->host->mem_buses, chan->cd->periph_buses);
+}
+
 /*
  * Initialise the DMAC memcpy/slave channels.
  * Make a local wrapper to hold required data
@@ -1700,13 +1711,8 @@ static int pl08x_dma_init_virtual_channels(struct pl08x_driver_data *pl08x,
 		chan->state = PL08X_CHAN_IDLE;
 
 		if (slave) {
-			chan->slave = true;
-			chan->name = pl08x->pd->slave_channels[i].bus_id;
 			chan->cd = &pl08x->pd->slave_channels[i];
-			chan->src_addr = chan->cd->addr;
-			chan->dst_addr = chan->cd->addr;
-			chan->src_cctl = pl08x_cctl(chan->cd->cctl);
-			chan->dst_cctl = pl08x_cctl(chan->cd->cctl);
+			pl08x_dma_slave_init(chan);
 		} else {
 			chan->cd = &pl08x->pd->memcpy_channel;
 			chan->name = kasprintf(GFP_KERNEL, "memcpy%d", i);
-- 
1.7.4.4

[PATCH 9/9] DMA: PL08x: cleanup selection of burst size

[Russell King - ARM Linux]

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/dma/amba-pl08x.c |   58 ++++++++++++++++++++++-----------------------
 1 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index a84db8b..9aa2bd4 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1048,50 +1048,42 @@ pl08x_dma_tx_status(struct dma_chan *chan,
 
 /* PrimeCell DMA extension */
 struct burst_table {
-	int burstwords;
+	u32 burstwords;
 	u32 reg;
 };
 
 static const struct burst_table burst_sizes[] = {
 	{
 		.burstwords = 256,
-		.reg = (PL080_BSIZE_256 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_256 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_256,
 	},
 	{
 		.burstwords = 128,
-		.reg = (PL080_BSIZE_128 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_128 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_128,
 	},
 	{
 		.burstwords = 64,
-		.reg = (PL080_BSIZE_64 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_64 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_64,
 	},
 	{
 		.burstwords = 32,
-		.reg = (PL080_BSIZE_32 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_32 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_32,
 	},
 	{
 		.burstwords = 16,
-		.reg = (PL080_BSIZE_16 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_16 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_16,
 	},
 	{
 		.burstwords = 8,
-		.reg = (PL080_BSIZE_8 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_8 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_8,
 	},
 	{
 		.burstwords = 4,
-		.reg = (PL080_BSIZE_4 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_4 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.reg = PL080_BSIZE_4,
 	},
 	{
-		.burstwords = 1,
-		.reg = (PL080_BSIZE_1 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_1 << PL080_CONTROL_DB_SIZE_SHIFT),
+		.burstwords = 0,
+		.reg = PL080_BSIZE_1,
 	},
 };
 
@@ -1135,15 +1127,25 @@ static u32 pl08x_width(enum dma_slave_buswidth width)
 	return ~0;
 }
 
+static u32 pl08x_burst(u32 maxburst)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(burst_sizes); i++)
+		if (burst_sizes[i].burstwords <= maxburst)
+			break;
+
+	return burst_sizes[i].reg;
+}
+
 static int dma_set_runtime_config(struct dma_chan *chan,
 				  struct dma_slave_config *config)
 {
 	struct pl08x_dma_chan *plchan = to_pl08x_chan(chan);
 	struct pl08x_driver_data *pl08x = plchan->host;
 	enum dma_slave_buswidth addr_width;
-	u32 width, maxburst;
+	u32 width, burst, maxburst;
 	u32 cctl = 0;
-	int i;
 
 	if (!plchan->slave)
 		return -EINVAL;
@@ -1173,20 +1175,16 @@ static int dma_set_runtime_config(struct dma_chan *chan,
 	cctl |= width << PL080_CONTROL_DWIDTH_SHIFT;
 
 	/*
-	 * Now decide on a maxburst:
 	 * If this channel will only request single transfers, set this
 	 * down to ONE element.  Also select one element if no maxburst
 	 * is specified.
 	 */
-	if (plchan->cd->single || maxburst == 0) {
-		cctl |= (PL080_BSIZE_1 << PL080_CONTROL_SB_SIZE_SHIFT) |
-			(PL080_BSIZE_1 << PL080_CONTROL_DB_SIZE_SHIFT);
-	} else {
-		for (i = 0; i < ARRAY_SIZE(burst_sizes); i++)
-			if (burst_sizes[i].burstwords <= maxburst)
-				break;
-		cctl |= burst_sizes[i].reg;
-	}
+	if (plchan->cd->single)
+		maxburst = 1;
+
+	burst = pl08x_burst(maxburst);
+	cctl |= burst << PL080_CONTROL_SB_SIZE_SHIFT;
+	cctl |= burst << PL080_CONTROL_DB_SIZE_SHIFT;
 
 	if (plchan->runtime_direction == DMA_FROM_DEVICE) {
 		plchan->src_addr = config->src_addr;
-- 
1.7.4.4

[PATCH 0/9] PL08x further cleanups

[Vinod Koul]

On Thu, 2011-07-21 at 17:08 +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 14, 2011 at 04:35:28AM +0530, Koul, Vinod wrote:
> > On Tue, 2011-07-05 at 14:10 +0100, Russell King - ARM Linux wrote:
> > > This patch series is the remainder of the PL08x work I did earlier
> > > this year while trying to get the ARM platforms working with DMA.
> > > 
> > > Unfortunately, due to the state of the hardware, I was not able to
> > > properly test these changes.  However, they do compile, and I think
> > > if others are going to be looking at the PL08x DMA engine driver,
> > > they're worth having.
> > > 
> > > Linus, can you check these out on your hardware please?
> > 
> > Russell,
> > 
> > The fail to build for me, on line 550 it refers to bd.llil_bus whereas
> > the bd is a pointer. Further the WIDTH macros seem to be missing.
> 
> Grr, I've updated the patches and will post the two updated ones.  I
> have them in my git tree - would you like them via git instead, or
> would you prefer to provide acks and let me push them upstream?
> 
Russell,
Thanks I have applied this

Although gcc didnt like not handling other enums so warned:

drivers/dma/amba-pl08x.c: In function 'pl08x_width':
drivers/dma/amba-pl08x.c:1119: warning: enumeration value
'DMA_SLAVE_BUSWIDTH_UNDEFINED' not handled in switch
drivers/dma/amba-pl08x.c:1119: warning: enumeration value
'DMA_SLAVE_BUSWIDTH_8_BYTES' not handled in switch

which can be fixed as:

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 9aa2bd4..4925e0d 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1123,6 +1123,8 @@ static u32 pl08x_width(enum dma_slave_buswidth
width)
 		return PL080_WIDTH_16BIT;
 	case DMA_SLAVE_BUSWIDTH_4_BYTES:
 		return PL080_WIDTH_32BIT;
+	default:
+		return -EINVAL;
 	}
 	return ~0;
 }

If you are okay, pls ack it

-- 
~Vinod Koul
Intel Corp.

[PATCH 0/9] PL08x further cleanups

[Russell King - ARM Linux]

On Mon, Jul 25, 2011 at 07:08:31PM +0530, Vinod Koul wrote:
> Although gcc didnt like not handling other enums so warned:
> 
> drivers/dma/amba-pl08x.c: In function 'pl08x_width':
> drivers/dma/amba-pl08x.c:1119: warning: enumeration value
> 'DMA_SLAVE_BUSWIDTH_UNDEFINED' not handled in switch
> drivers/dma/amba-pl08x.c:1119: warning: enumeration value
> 'DMA_SLAVE_BUSWIDTH_8_BYTES' not handled in switch

Those must be new since I wrote the patch.

> which can be fixed as:
> 
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index 9aa2bd4..4925e0d 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -1123,6 +1123,8 @@ static u32 pl08x_width(enum dma_slave_buswidth
> width)
>  		return PL080_WIDTH_16BIT;
>  	case DMA_SLAVE_BUSWIDTH_4_BYTES:
>  		return PL080_WIDTH_32BIT;
> +	default:
> +		return -EINVAL;
>  	}
>  	return ~0;
>  }
> 
> If you are okay, pls ack it

No.  This function is used as:

+       width = pl08x_width(addr_width);
+       if (width == ~0) {
                dev_err(&pl08x->adev->dev,
                        "bad runtime_config: alien address width\n");
                return -EINVAL;
        }

Notice that it returns a u32 so negative errnos don't make sense.  It
returns ~0 to indicate error.

The code is actually correct as it stands, it's just gcc deciding to
emit a warning for an unhandled enum value which isn't really unhandled.
Just move the 'return ~0;' at the end of the function inside the switch
as a default case to shut it up.

[PATCH 0/9] PL08x further cleanups

[Vinod Koul]

On Mon, 2011-07-25 at 14:43 +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 25, 2011 at 07:08:31PM +0530, Vinod Koul wrote:
> > Although gcc didnt like not handling other enums so warned:
> > 
> > drivers/dma/amba-pl08x.c: In function 'pl08x_width':
> > drivers/dma/amba-pl08x.c:1119: warning: enumeration value
> > 'DMA_SLAVE_BUSWIDTH_UNDEFINED' not handled in switch
> > drivers/dma/amba-pl08x.c:1119: warning: enumeration value
> > 'DMA_SLAVE_BUSWIDTH_8_BYTES' not handled in switch
> 
> Those must be new since I wrote the patch.
> 
> > which can be fixed as:
> > 
> > diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> > index 9aa2bd4..4925e0d 100644
> > --- a/drivers/dma/amba-pl08x.c
> > +++ b/drivers/dma/amba-pl08x.c
> > @@ -1123,6 +1123,8 @@ static u32 pl08x_width(enum dma_slave_buswidth
> > width)
> >  		return PL080_WIDTH_16BIT;
> >  	case DMA_SLAVE_BUSWIDTH_4_BYTES:
> >  		return PL080_WIDTH_32BIT;
> > +	default:
> > +		return -EINVAL;
> >  	}
> >  	return ~0;
> >  }
> > 
> > If you are okay, pls ack it
> 
> No.  This function is used as:
> 
> +       width = pl08x_width(addr_width);
> +       if (width == ~0) {
>                 dev_err(&pl08x->adev->dev,
>                         "bad runtime_config: alien address width\n");
>                 return -EINVAL;
>         }
> 
> Notice that it returns a u32 so negative errnos don't make sense.  It
> returns ~0 to indicate error.
> 
> The code is actually correct as it stands, it's just gcc deciding to
> emit a warning for an unhandled enum value which isn't really unhandled.
> Just move the 'return ~0;' at the end of the function inside the switch
> as a default case to shut it up.
Okay but shouldn't this ideally check for width < 0, that way we can
return proper errors?

If yo

-- 
~Vinod Koul
Intel Corp.

[PATCH 0/9] PL08x further cleanups

[Russell King - ARM Linux]

On Mon, Jul 25, 2011 at 07:21:12PM +0530, Vinod Koul wrote:
> On Mon, 2011-07-25 at 14:43 +0100, Russell King - ARM Linux wrote:
> > No.  This function is used as:
> > 
> > +       width = pl08x_width(addr_width);
> > +       if (width == ~0) {
> >                 dev_err(&pl08x->adev->dev,
> >                         "bad runtime_config: alien address width\n");
> >                 return -EINVAL;
> >         }
> > 
> > Notice that it returns a u32 so negative errnos don't make sense.  It
> > returns ~0 to indicate error.
> > 
> > The code is actually correct as it stands, it's just gcc deciding to
> > emit a warning for an unhandled enum value which isn't really unhandled.
> > Just move the 'return ~0;' at the end of the function inside the switch
> > as a default case to shut it up.
> Okay but shouldn't this ideally check for width < 0, that way we can
> return proper errors?

No.  Two reasons:

1. u32 < 0 does not exist.

2. It's returning a sub bitmask value for the register, so signed numbers
   conceptually don't make sense.

[PATCH 0/9] PL08x further cleanups

[Vinod Koul]

On Mon, 2011-07-25 at 15:22 +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 25, 2011 at 07:21:12PM +0530, Vinod Koul wrote:
> > On Mon, 2011-07-25 at 14:43 +0100, Russell King - ARM Linux wrote:
> > > No.  This function is used as:
> > > 
> > > +       width = pl08x_width(addr_width);
> > > +       if (width == ~0) {
> > >                 dev_err(&pl08x->adev->dev,
> > >                         "bad runtime_config: alien address width\n");
> > >                 return -EINVAL;
> > >         }
> > > 
> > > Notice that it returns a u32 so negative errnos don't make sense.  It
> > > returns ~0 to indicate error.
> > > 
> > > The code is actually correct as it stands, it's just gcc deciding to
> > > emit a warning for an unhandled enum value which isn't really unhandled.
> > > Just move the 'return ~0;' at the end of the function inside the switch
> > > as a default case to shut it up.
> > Okay but shouldn't this ideally check for width < 0, that way we can
> > return proper errors?
> 
> No.  Two reasons:
> 
> 1. u32 < 0 does not exist.
> 
> 2. It's returning a sub bitmask value for the register, so signed numbers
>    conceptually don't make sense.
Agreed, Pushed this with return ~0;
You should be able to see your patchset along with this change in my
tree now

-- 
~Vinod Koul
Intel Corp.

[PATCH 0/9] PL08x further cleanups

[Vinod Koul]

On Mon, 2011-07-25 at 14:43 +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 25, 2011 at 07:08:31PM +0530, Vinod Koul wrote:
> > Although gcc didnt like not handling other enums so warned:
> > 
> > drivers/dma/amba-pl08x.c: In function 'pl08x_width':
> > drivers/dma/amba-pl08x.c:1119: warning: enumeration value
> > 'DMA_SLAVE_BUSWIDTH_UNDEFINED' not handled in switch
> > drivers/dma/amba-pl08x.c:1119: warning: enumeration value
> > 'DMA_SLAVE_BUSWIDTH_8_BYTES' not handled in switch
> 
> Those must be new since I wrote the patch.
> 
> > which can be fixed as:
> > 
> > diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> > index 9aa2bd4..4925e0d 100644
> > --- a/drivers/dma/amba-pl08x.c
> > +++ b/drivers/dma/amba-pl08x.c
> > @@ -1123,6 +1123,8 @@ static u32 pl08x_width(enum dma_slave_buswidth
> > width)
> >  		return PL080_WIDTH_16BIT;
> >  	case DMA_SLAVE_BUSWIDTH_4_BYTES:
> >  		return PL080_WIDTH_32BIT;
> > +	default:
> > +		return -EINVAL;
> >  	}
> >  	return ~0;
> >  }
> > 
> > If you are okay, pls ack it
> 
> No.  This function is used as:
> 
> +       width = pl08x_width(addr_width);
> +       if (width == ~0) {
>                 dev_err(&pl08x->adev->dev,
>                         "bad runtime_config: alien address width\n");
>                 return -EINVAL;
>         }
> 
> Notice that it returns a u32 so negative errnos don't make sense.  It
> returns ~0 to indicate error.
> 
> The code is actually correct as it stands, it's just gcc deciding to
> emit a warning for an unhandled enum value which isn't really unhandled.
> Just move the 'return ~0;' at the end of the function inside the switch
> as a default case to shut it up.
Okay but shouldn't this ideally check for width < 0, that way we can
return proper errors?

-- 
~Vinod Koul
Intel Corp.