diff mbox

[15/18] dmaengine/amba-pl08x: Pass flow controller information with slave channel data

Message ID 2f6001455c4e54f56716a245c7e319eb25a9bb0d.1311936524.git.viresh.kumar@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Viresh KUMAR July 29, 2011, 10:49 a.m. UTC
At least, on SPEAr platforms there is one peripheral, JPEG, which can be flow
controller for DMA transfer. Currently DMA controller driver didn't support
peripheral flow controller configurations.

This patch adds fc field in struct pl08x_channel_data, which will be used only
for slave transfers and is not used in case of mem2mem transfers. In case this
is passed as 0, then flow controller is decided based on DMA_DIRECTION.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/dma/amba-pl08x.c   |   46 +++++++++++++++++++++++++++++++++++++++----
 include/linux/amba/pl08x.h |    5 ++++
 2 files changed, 46 insertions(+), 5 deletions(-)

Comments

Russell King - ARM Linux July 29, 2011, 11:10 a.m. UTC | #1
Apart from my initial question in this thread...

On Fri, Jul 29, 2011 at 04:19:25PM +0530, Viresh Kumar wrote:
> +	if (plchan->cd->fc)
> +		tmp = plchan->cd->fc;
> +	else
> +		tmp = (direction == DMA_TO_DEVICE) ? PL080_FLOW_MEM2PER :
> +			PL080_FLOW_PER2MEM;
> +
> +	txd->ccfg |= tmp << PL080_CONFIG_FLOW_CONTROL_SHIFT;
> +

Please, no, don't go back to encoding the flow mask into the platform
data.  Instead use 'fc' as an indication of who should be the flow
controller, and hav ethis select the flow control based on both
'fc' and the direction.
Viresh KUMAR July 29, 2011, 11:31 a.m. UTC | #2
On 07/29/2011 04:40 PM, Russell King - ARM Linux wrote:
> Please, no, don't go back to encoding the flow mask into the platform
> data.  Instead use 'fc' as an indication of who should be the flow
> controller, and hav ethis select the flow control based on both
> 'fc' and the direction.

Ok.
Linus Walleij July 31, 2011, 12:40 a.m. UTC | #3
2011/7/29 Viresh Kumar <viresh.kumar@st.com>:

I have the same comment as Russell basically:

> @@ -65,6 +69,7 @@ struct pl08x_channel_data {
>        int max_signal;
>        u32 muxval;
>        u32 cctl;
> +       u32 fc;

bool device_fc;

or similar. Then encode the necessary bits if the device controls
flow, and fall back to default (DMAC control) otherwise.

>        dma_addr_t addr;
>        bool circular_buffer;
>        bool single;
> --
> 1.7.2.2

Thanks,
Linus Walleij
viresh kumar July 31, 2011, 5:36 a.m. UTC | #4
On 7/30/11, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2011/7/29 Viresh Kumar <viresh.kumar@st.com>:
>
> I have the same comment as Russell basically:
>
>> @@ -65,6 +69,7 @@ struct pl08x_channel_data {
>>        int max_signal;
>>        u32 muxval;
>>        u32 cctl;
>> +       u32 fc;
>
> bool device_fc;
>
> or similar. Then encode the necessary bits if the device controls
> flow, and fall back to default (DMAC control) otherwise.
>

Yes. I thought of exactly the same code.

--
viresh
diff mbox

Patch

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 6b0ca26..9eea0d1 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -632,6 +632,38 @@  static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 
 	if (bd.remainder) {
 		/*
+		 * Zero length is only allowed if all these requirements are
+		 * met:
+		 * - flow controller is peripheral.
+		 * - src.addr is aligned to src.width
+		 * - dst.addr is aligned to dst.width
+		 */
+		if (!bd.remainder) {
+			u32 fc = (txd->cctl & PL080_CONFIG_FLOW_CONTROL_MASK) >>
+				PL080_CONFIG_FLOW_CONTROL_SHIFT;
+			if (!((fc >= PL080_FLOW_SRC2DST_DST) &&
+					(fc <= PL080_FLOW_SRC2DST_SRC))) {
+				dev_err(&pl08x->adev->dev, "%s sg len can't be "
+						"zero", __func__);
+				return 0;
+			}
+
+			if ((bd.srcbus.addr % bd.srcbus.buswidth) ||
+					(bd.srcbus.addr % bd.srcbus.buswidth)) {
+				dev_err(&pl08x->adev->dev, "%s src & dst "
+						"address must be aligned to src"
+						" & dst width if peripheral is "
+						"flow controller", __func__);
+				return 0;
+			}
+
+			cctl = pl08x_cctl_bits(cctl, bd.srcbus.buswidth,
+					bd.dstbus.buswidth, 0);
+			pl08x_fill_lli_for_desc(&bd, num_llis++, 0, cctl);
+			continue;
+		}
+
+		/*
 		 * Master now aligned
 		 * - if slave is not then we must set its width down
 		 */
@@ -1255,7 +1287,7 @@  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;
-	int ret;
+	int ret, tmp;
 
 	/*
 	 * Current implementation ASSUMES only one sg
@@ -1289,14 +1321,10 @@  static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 	txd->len = sgl->length;
 
 	if (direction == DMA_TO_DEVICE) {
-		txd->ccfg |= PL080_FLOW_MEM2PER <<
-			PL080_CONFIG_FLOW_CONTROL_SHIFT;
 		txd->cctl = plchan->dst_cctl;
 		txd->src_addr = sgl->dma_address;
 		txd->dst_addr = plchan->dst_addr;
 	} else if (direction == DMA_FROM_DEVICE) {
-		txd->ccfg |= PL080_FLOW_PER2MEM <<
-			PL080_CONFIG_FLOW_CONTROL_SHIFT;
 		txd->cctl = plchan->src_cctl;
 		txd->src_addr = plchan->src_addr;
 		txd->dst_addr = sgl->dma_address;
@@ -1306,6 +1334,14 @@  static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
 		return NULL;
 	}
 
+	if (plchan->cd->fc)
+		tmp = plchan->cd->fc;
+	else
+		tmp = (direction == DMA_TO_DEVICE) ? PL080_FLOW_MEM2PER :
+			PL080_FLOW_PER2MEM;
+
+	txd->ccfg |= tmp << PL080_CONFIG_FLOW_CONTROL_SHIFT;
+
 	ret = pl08x_prep_channel_resources(plchan, txd);
 	if (ret)
 		return NULL;
diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h
index 98628a8..febb0da 100644
--- a/include/linux/amba/pl08x.h
+++ b/include/linux/amba/pl08x.h
@@ -47,6 +47,10 @@  enum {
  * @muxval: a number usually used to poke into some mux regiser to
  * mux in the signal to this channel
  * @cctl_opt: default options for the channel control register
+ * @fc: Flow Controller Settings for ccfg register. Only valid for slave
+ * channels. Fill with any PL080_FLOW_* macro's present in
+ * <asm/hardware/pl080.h>. If passed as 0, then Flow control will be decided
+ * according to DMA direction.
  * @addr: source/target address in physical memory for this DMA channel,
  * can be the address of a FIFO register for burst requests for example.
  * This can be left undefined if the PrimeCell API is used for configuring
@@ -65,6 +69,7 @@  struct pl08x_channel_data {
 	int max_signal;
 	u32 muxval;
 	u32 cctl;
+	u32 fc;
 	dma_addr_t addr;
 	bool circular_buffer;
 	bool single;