[PATCH 0/2] dmaengine: core/omap-dma: Support for port window

[PATCH 0/2] dmaengine: core/omap-dma: Support for port window

[Peter Ujfalusi]

Hi,

as I'm trying to convert the remaining OMAP driver to use DMAengine instead of
the legacy omap-dma API I have encountered with the
drivers/usb/musb/tusb6010_omap.c driver.

The TUSB6010 is connected via NOR FLASH interface and it's register space is
mapped in the GPMC memory area. In OMAP SoCs we have support for external DMA
request lines and the TUSB6010 is using those as well.

With asynchronous access the DMA needs to read/write within the FIFO 'window' in
incremental address mode to read/write data.
The constant addressing only works in synchronous mode.

Since the DMA is driven by external DMA requests, the asynchronous mode is also
slave DMA operation, but currently the port window can not be 'swiped' as the
DMAengine only supports single register/address on the slave side.

This series will add support in dma_slave_config to specify the port side window
size and the second patch implements the setup needs in omap-dma driver for such
a transfer.

Regards,
Peter
---
Peter Ujfalusi (2):
  dmaengine: dma_slave_config: add support for slave port window
  dmaengine: omap-dma: Support for slave devices with data port window

 drivers/dma/omap-dma.c    | 45 +++++++++++++++++++++++++++++++++++++++++++--
 include/linux/dmaengine.h |  8 ++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

--
2.10.1

[PATCH 0/2] dmaengine: core/omap-dma: Support for port window

[Peter Ujfalusi]

Hi,

as I'm trying to convert the remaining OMAP driver to use DMAengine instead of
the legacy omap-dma API I have encountered with the
drivers/usb/musb/tusb6010_omap.c driver.

The TUSB6010 is connected via NOR FLASH interface and it's register space is
mapped in the GPMC memory area. In OMAP SoCs we have support for external DMA
request lines and the TUSB6010 is using those as well.

With asynchronous access the DMA needs to read/write within the FIFO 'window' in
incremental address mode to read/write data.
The constant addressing only works in synchronous mode.

Since the DMA is driven by external DMA requests, the asynchronous mode is also
slave DMA operation, but currently the port window can not be 'swiped' as the
DMAengine only supports single register/address on the slave side.

This series will add support in dma_slave_config to specify the port side window
size and the second patch implements the setup needs in omap-dma driver for such
a transfer.

Regards,
Peter
---
Peter Ujfalusi (2):
  dmaengine: dma_slave_config: add support for slave port window
  dmaengine: omap-dma: Support for slave devices with data port window

 drivers/dma/omap-dma.c    | 45 +++++++++++++++++++++++++++++++++++++++++++--
 include/linux/dmaengine.h |  8 ++++++++
 2 files changed, 51 insertions(+), 2 deletions(-)

--
2.10.1

[PATCH 1/2] dmaengine: dma_slave_config: add support for slave port window

[Peter Ujfalusi]

Some slave devices uses address window instead of single register for read
and/or write of data. With the src/dst_port_window_size the address window
can be specified and the DMAengine driver should use this information to
correctly set up the transfer to loop within the provided window.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/linux/dmaengine.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cc535a478bae..689d44327ef3 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -336,6 +336,12 @@ enum dma_slave_buswidth {
  * may or may not be applicable on memory sources.
  * @dst_maxburst: same as src_maxburst but for destination target
  * mutatis mutandis.
+ * @src_port_window_size: The length of the register area the data need to be
+ * written on the device side. It is only used for devices which is using an
+ * area instead of a single register to receive the data. Typically the DMA
+ * loops in this area in order to transfer the data.
+ * @dst_port_window_size: same as src_port_window_size but for the destination
+ * port.
  * @device_fc: Flow Controller Settings. Only valid for slave channels. Fill
  * with 'true' if peripheral should be flow controller. Direction will be
  * selected at Runtime.
@@ -363,6 +369,8 @@ struct dma_slave_config {
 	enum dma_slave_buswidth dst_addr_width;
 	u32 src_maxburst;
 	u32 dst_maxburst;
+	u32 src_port_window_size;
+	u32 dst_port_window_size;
 	bool device_fc;
 	unsigned int slave_id;
 };
-- 
2.10.1

[PATCH 1/2] dmaengine: dma_slave_config: add support for slave port window

[Peter Ujfalusi]

Some slave devices uses address window instead of single register for read
and/or write of data. With the src/dst_port_window_size the address window
can be specified and the DMAengine driver should use this information to
correctly set up the transfer to loop within the provided window.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/linux/dmaengine.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cc535a478bae..689d44327ef3 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -336,6 +336,12 @@ enum dma_slave_buswidth {
  * may or may not be applicable on memory sources.
  * @dst_maxburst: same as src_maxburst but for destination target
  * mutatis mutandis.
+ * @src_port_window_size: The length of the register area the data need to be
+ * written on the device side. It is only used for devices which is using an
+ * area instead of a single register to receive the data. Typically the DMA
+ * loops in this area in order to transfer the data.
+ * @dst_port_window_size: same as src_port_window_size but for the destination
+ * port.
  * @device_fc: Flow Controller Settings. Only valid for slave channels. Fill
  * with 'true' if peripheral should be flow controller. Direction will be
  * selected at Runtime.
@@ -363,6 +369,8 @@ struct dma_slave_config {
 	enum dma_slave_buswidth dst_addr_width;
 	u32 src_maxburst;
 	u32 dst_maxburst;
+	u32 src_port_window_size;
+	u32 dst_port_window_size;
 	bool device_fc;
 	unsigned int slave_id;
 };
-- 
2.10.1

[PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

Based on the src/dst_port_window_size - if it is set - configure the DMA
channel to use double indexing in order to be able to loop within the
address window.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 025f499cb20d..29350f936154 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -166,6 +166,9 @@ enum {
 	CSDP_DST_BURST_16	= 1 << 14,
 	CSDP_DST_BURST_32	= 2 << 14,
 	CSDP_DST_BURST_64	= 3 << 14,
+	CSDP_WRITE_NON_POSTED	= (0 << 16),
+	CSDP_WRITE_POSTED	= (1 << 16),
+	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
 
 	CICR_TOUT_IE		= BIT(0),	/* OMAP1 only */
 	CICR_DROP_IE		= BIT(1),
@@ -883,15 +886,18 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 	unsigned i, es, en, frame_bytes;
 	bool ll_failed = false;
 	u32 burst;
+	int32_t port_window;
 
 	if (dir == DMA_DEV_TO_MEM) {
 		dev_addr = c->cfg.src_addr;
 		dev_width = c->cfg.src_addr_width;
 		burst = c->cfg.src_maxburst;
+		port_window = c->cfg.src_port_window_size;
 	} else if (dir == DMA_MEM_TO_DEV) {
 		dev_addr = c->cfg.dst_addr;
 		dev_width = c->cfg.dst_addr_width;
 		burst = c->cfg.dst_maxburst;
+		port_window = c->cfg.dst_port_window_size;
 	} else {
 		dev_err(chan->device->dev, "%s: bad direction?\n", __func__);
 		return NULL;
@@ -923,11 +929,39 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 
 	d->ccr = c->ccr | CCR_SYNC_FRAME;
 	if (dir == DMA_DEV_TO_MEM) {
-		d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_CONSTANT;
 		d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
+
+		d->ccr |= CCR_DST_AMODE_POSTINC;
+		if (port_window) {
+			d->ccr |= CCR_SRC_AMODE_DBLIDX;
+			d->ei = 1;
+			d->fi = (-1) * (port_window - 1);
+
+			if (port_window / 64)
+				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
+			else if (port_window / 32)
+				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
+			else if (port_window / 16)
+				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
+		} else {
+			d->ccr |= CCR_SRC_AMODE_CONSTANT;
+		}
 	} else {
-		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
 		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
+
+		d->ccr |= CCR_SRC_AMODE_POSTINC;
+		if (port_window) {
+			d->ccr |= CCR_DST_AMODE_DBLIDX;
+
+			if (port_window / 64)
+				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
+			else if (port_window / 32)
+				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
+			else if (port_window / 16)
+				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
+		} else {
+			d->ccr |= CCR_DST_AMODE_CONSTANT;
+		}
 	}
 
 	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
@@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 			d->ccr |= CCR_TRIGGER_SRC;
 
 		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
+
+		if (port_window)
+			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
 	}
 	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
 		d->clnk_ctrl = c->dma_ch;
@@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 		osg->addr = sg_dma_address(sgent);
 		osg->en = en;
 		osg->fn = sg_dma_len(sgent) / frame_bytes;
+		if (port_window && dir == DMA_MEM_TO_DEV) {
+			osg->ei = 1;
+			osg->fi = (-1) * (port_window - 1);
+		}
 
 		if (d->using_ll) {
 			osg->t2_desc = dma_pool_alloc(od->desc_pool, GFP_ATOMIC,
-- 
2.10.1

[PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

Based on the src/dst_port_window_size - if it is set - configure the DMA
channel to use double indexing in order to be able to loop within the
address window.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 025f499cb20d..29350f936154 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -166,6 +166,9 @@ enum {
 	CSDP_DST_BURST_16	= 1 << 14,
 	CSDP_DST_BURST_32	= 2 << 14,
 	CSDP_DST_BURST_64	= 3 << 14,
+	CSDP_WRITE_NON_POSTED	= (0 << 16),
+	CSDP_WRITE_POSTED	= (1 << 16),
+	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
 
 	CICR_TOUT_IE		= BIT(0),	/* OMAP1 only */
 	CICR_DROP_IE		= BIT(1),
@@ -883,15 +886,18 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 	unsigned i, es, en, frame_bytes;
 	bool ll_failed = false;
 	u32 burst;
+	int32_t port_window;
 
 	if (dir == DMA_DEV_TO_MEM) {
 		dev_addr = c->cfg.src_addr;
 		dev_width = c->cfg.src_addr_width;
 		burst = c->cfg.src_maxburst;
+		port_window = c->cfg.src_port_window_size;
 	} else if (dir == DMA_MEM_TO_DEV) {
 		dev_addr = c->cfg.dst_addr;
 		dev_width = c->cfg.dst_addr_width;
 		burst = c->cfg.dst_maxburst;
+		port_window = c->cfg.dst_port_window_size;
 	} else {
 		dev_err(chan->device->dev, "%s: bad direction?\n", __func__);
 		return NULL;
@@ -923,11 +929,39 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 
 	d->ccr = c->ccr | CCR_SYNC_FRAME;
 	if (dir == DMA_DEV_TO_MEM) {
-		d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_CONSTANT;
 		d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
+
+		d->ccr |= CCR_DST_AMODE_POSTINC;
+		if (port_window) {
+			d->ccr |= CCR_SRC_AMODE_DBLIDX;
+			d->ei = 1;
+			d->fi = (-1) * (port_window - 1);
+
+			if (port_window / 64)
+				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
+			else if (port_window / 32)
+				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
+			else if (port_window / 16)
+				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
+		} else {
+			d->ccr |= CCR_SRC_AMODE_CONSTANT;
+		}
 	} else {
-		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
 		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
+
+		d->ccr |= CCR_SRC_AMODE_POSTINC;
+		if (port_window) {
+			d->ccr |= CCR_DST_AMODE_DBLIDX;
+
+			if (port_window / 64)
+				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
+			else if (port_window / 32)
+				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
+			else if (port_window / 16)
+				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
+		} else {
+			d->ccr |= CCR_DST_AMODE_CONSTANT;
+		}
 	}
 
 	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
@@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 			d->ccr |= CCR_TRIGGER_SRC;
 
 		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
+
+		if (port_window)
+			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
 	}
 	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
 		d->clnk_ctrl = c->dma_ch;
@@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
 		osg->addr = sg_dma_address(sgent);
 		osg->en = en;
 		osg->fn = sg_dma_len(sgent) / frame_bytes;
+		if (port_window && dir == DMA_MEM_TO_DEV) {
+			osg->ei = 1;
+			osg->fi = (-1) * (port_window - 1);
+		}
 
 		if (d->using_ll) {
 			osg->t2_desc = dma_pool_alloc(od->desc_pool, GFP_ATOMIC,
-- 
2.10.1

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Vinod Koul]

On Tue, Oct 25, 2016 at 01:50:19PM +0300, Peter Ujfalusi wrote:
> Based on the src/dst_port_window_size - if it is set - configure the DMA
> channel to use double indexing in order to be able to loop within the
> address window.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index 025f499cb20d..29350f936154 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -166,6 +166,9 @@ enum {
>  	CSDP_DST_BURST_16	= 1 << 14,
>  	CSDP_DST_BURST_32	= 2 << 14,
>  	CSDP_DST_BURST_64	= 3 << 14,
> +	CSDP_WRITE_NON_POSTED	= (0 << 16),

and that would be..?

> +	CSDP_WRITE_POSTED	= (1 << 16),
> +	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
>  
>  	CICR_TOUT_IE		= BIT(0),	/* OMAP1 only */
>  	CICR_DROP_IE		= BIT(1),
> @@ -883,15 +886,18 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>  	unsigned i, es, en, frame_bytes;
>  	bool ll_failed = false;
>  	u32 burst;
> +	int32_t port_window;

not u32?

>  
>  	if (dir == DMA_DEV_TO_MEM) {
>  		dev_addr = c->cfg.src_addr;
>  		dev_width = c->cfg.src_addr_width;
>  		burst = c->cfg.src_maxburst;
> +		port_window = c->cfg.src_port_window_size;
>  	} else if (dir == DMA_MEM_TO_DEV) {
>  		dev_addr = c->cfg.dst_addr;
>  		dev_width = c->cfg.dst_addr_width;
>  		burst = c->cfg.dst_maxburst;
> +		port_window = c->cfg.dst_port_window_size;
>  	} else {
>  		dev_err(chan->device->dev, "%s: bad direction?\n", __func__);
>  		return NULL;
> @@ -923,11 +929,39 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>  
>  	d->ccr = c->ccr | CCR_SYNC_FRAME;
>  	if (dir == DMA_DEV_TO_MEM) {
> -		d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_CONSTANT;
>  		d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
> +
> +		d->ccr |= CCR_DST_AMODE_POSTINC;
> +		if (port_window) {
> +			d->ccr |= CCR_SRC_AMODE_DBLIDX;
> +			d->ei = 1;
> +			d->fi = (-1) * (port_window - 1);
> +
> +			if (port_window / 64)
> +				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
> +			else if (port_window / 32)
> +				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
> +			else if (port_window / 16)
> +				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
> +		} else {
> +			d->ccr |= CCR_SRC_AMODE_CONSTANT;

okay does all your hw support this mode, should this not be checked to be
supported, perhpas a DT property which describes thsi capablity and then you
use it, if supported

> +		}
>  	} else {
> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
> +
> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
> +		if (port_window) {
> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
> +
> +			if (port_window / 64)
> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
> +			else if (port_window / 32)
> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
> +			else if (port_window / 16)
> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;

what does these mean?

> +		} else {
> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
> +		}
>  	}
>  
>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>  			d->ccr |= CCR_TRIGGER_SRC;
>  
>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
> +
> +		if (port_window)
> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>  	}
>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>  		d->clnk_ctrl = c->dma_ch;
> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>  		osg->addr = sg_dma_address(sgent);
>  		osg->en = en;
>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
> +		if (port_window && dir == DMA_MEM_TO_DEV) {
> +			osg->ei = 1;
> +			osg->fi = (-1) * (port_window - 1);
> +		}

can you describe what you are trying here..

-- 
~Vinod

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

On 11/14/2016 06:35 AM, Vinod Koul wrote:
> On Tue, Oct 25, 2016 at 01:50:19PM +0300, Peter Ujfalusi wrote:
>> Based on the src/dst_port_window_size - if it is set - configure the DMA
>> channel to use double indexing in order to be able to loop within the
>> address window.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>> index 025f499cb20d..29350f936154 100644
>> --- a/drivers/dma/omap-dma.c
>> +++ b/drivers/dma/omap-dma.c
>> @@ -166,6 +166,9 @@ enum {
>>  	CSDP_DST_BURST_16	= 1 << 14,
>>  	CSDP_DST_BURST_32	= 2 << 14,
>>  	CSDP_DST_BURST_64	= 3 << 14,
>> +	CSDP_WRITE_NON_POSTED	= (0 << 16),
> 
> and that would be..?

0, but I like to have a define for this as we might need to use this mode in
the future and the bit field is defined like this.

> 
>> +	CSDP_WRITE_POSTED	= (1 << 16),
>> +	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
>>  
>>  	CICR_TOUT_IE		= BIT(0),	/* OMAP1 only */
>>  	CICR_DROP_IE		= BIT(1),
>> @@ -883,15 +886,18 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  	unsigned i, es, en, frame_bytes;
>>  	bool ll_failed = false;
>>  	u32 burst;
>> +	int32_t port_window;
> 
> not u32?

Yeah, it can be u32.

> 
>>  
>>  	if (dir == DMA_DEV_TO_MEM) {
>>  		dev_addr = c->cfg.src_addr;
>>  		dev_width = c->cfg.src_addr_width;
>>  		burst = c->cfg.src_maxburst;
>> +		port_window = c->cfg.src_port_window_size;
>>  	} else if (dir == DMA_MEM_TO_DEV) {
>>  		dev_addr = c->cfg.dst_addr;
>>  		dev_width = c->cfg.dst_addr_width;
>>  		burst = c->cfg.dst_maxburst;
>> +		port_window = c->cfg.dst_port_window_size;
>>  	} else {
>>  		dev_err(chan->device->dev, "%s: bad direction?\n", __func__);
>>  		return NULL;
>> @@ -923,11 +929,39 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  
>>  	d->ccr = c->ccr | CCR_SYNC_FRAME;
>>  	if (dir == DMA_DEV_TO_MEM) {
>> -		d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_CONSTANT;
>>  		d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>> +
>> +		d->ccr |= CCR_DST_AMODE_POSTINC;
>> +		if (port_window) {
>> +			d->ccr |= CCR_SRC_AMODE_DBLIDX;
>> +			d->ei = 1;
>> +			d->fi = (-1) * (port_window - 1);
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
>> +		} else {
>> +			d->ccr |= CCR_SRC_AMODE_CONSTANT;
> 
> okay does all your hw support this mode, should this not be checked to be
> supported, perhpas a DT property which describes thsi capablity and then you
> use it, if supported

yes, burst and packed mode is supported by all sDMA versions.

> 
>> +		}
>>  	} else {
>> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +
>> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
>> +		if (port_window) {
>> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
> 
> what does these mean?

To optimize the speed on the write side. First check if the window size is
multiple of 64 bytes, we enable the 64byte burst and packed transfer, if not
try the 32bytes, then 16bytes.
Same for the opposite direction previously.

> 
>> +		} else {
>> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
>> +		}
>>  	}
>>  
>>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
>> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  			d->ccr |= CCR_TRIGGER_SRC;
>>  
>>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
>> +
>> +		if (port_window)
>> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>>  	}
>>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>>  		d->clnk_ctrl = c->dma_ch;
>> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  		osg->addr = sg_dma_address(sgent);
>>  		osg->en = en;
>>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
>> +		if (port_window && dir == DMA_MEM_TO_DEV) {
>> +			osg->ei = 1;
>> +			osg->fi = (-1) * (port_window - 1);
>> +		}
> 
> can you describe what you are trying here..

The DMA is set up so one frame covers the port window. When the frame is
finished we need to start reading the next frame from the start of the window
again. The FI as (-1) * (port_window - 1) will take us to the start of the
window. When the frame is finished the DMA is pointing to the last byte of the
window.

> 


-- 
Péter

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

On 11/14/2016 06:35 AM, Vinod Koul wrote:
> On Tue, Oct 25, 2016 at 01:50:19PM +0300, Peter Ujfalusi wrote:
>> Based on the src/dst_port_window_size - if it is set - configure the DMA
>> channel to use double indexing in order to be able to loop within the
>> address window.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>> index 025f499cb20d..29350f936154 100644
>> --- a/drivers/dma/omap-dma.c
>> +++ b/drivers/dma/omap-dma.c
>> @@ -166,6 +166,9 @@ enum {
>>  	CSDP_DST_BURST_16	= 1 << 14,
>>  	CSDP_DST_BURST_32	= 2 << 14,
>>  	CSDP_DST_BURST_64	= 3 << 14,
>> +	CSDP_WRITE_NON_POSTED	= (0 << 16),
> 
> and that would be..?

0, but I like to have a define for this as we might need to use this mode in
the future and the bit field is defined like this.

> 
>> +	CSDP_WRITE_POSTED	= (1 << 16),
>> +	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
>>  
>>  	CICR_TOUT_IE		= BIT(0),	/* OMAP1 only */
>>  	CICR_DROP_IE		= BIT(1),
>> @@ -883,15 +886,18 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  	unsigned i, es, en, frame_bytes;
>>  	bool ll_failed = false;
>>  	u32 burst;
>> +	int32_t port_window;
> 
> not u32?

Yeah, it can be u32.

> 
>>  
>>  	if (dir == DMA_DEV_TO_MEM) {
>>  		dev_addr = c->cfg.src_addr;
>>  		dev_width = c->cfg.src_addr_width;
>>  		burst = c->cfg.src_maxburst;
>> +		port_window = c->cfg.src_port_window_size;
>>  	} else if (dir == DMA_MEM_TO_DEV) {
>>  		dev_addr = c->cfg.dst_addr;
>>  		dev_width = c->cfg.dst_addr_width;
>>  		burst = c->cfg.dst_maxburst;
>> +		port_window = c->cfg.dst_port_window_size;
>>  	} else {
>>  		dev_err(chan->device->dev, "%s: bad direction?\n", __func__);
>>  		return NULL;
>> @@ -923,11 +929,39 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  
>>  	d->ccr = c->ccr | CCR_SYNC_FRAME;
>>  	if (dir == DMA_DEV_TO_MEM) {
>> -		d->ccr |= CCR_DST_AMODE_POSTINC | CCR_SRC_AMODE_CONSTANT;
>>  		d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>> +
>> +		d->ccr |= CCR_DST_AMODE_POSTINC;
>> +		if (port_window) {
>> +			d->ccr |= CCR_SRC_AMODE_DBLIDX;
>> +			d->ei = 1;
>> +			d->fi = (-1) * (port_window - 1);
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
>> +		} else {
>> +			d->ccr |= CCR_SRC_AMODE_CONSTANT;
> 
> okay does all your hw support this mode, should this not be checked to be
> supported, perhpas a DT property which describes thsi capablity and then you
> use it, if supported

yes, burst and packed mode is supported by all sDMA versions.

> 
>> +		}
>>  	} else {
>> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +
>> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
>> +		if (port_window) {
>> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
> 
> what does these mean?

To optimize the speed on the write side. First check if the window size is
multiple of 64 bytes, we enable the 64byte burst and packed transfer, if not
try the 32bytes, then 16bytes.
Same for the opposite direction previously.

> 
>> +		} else {
>> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
>> +		}
>>  	}
>>  
>>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
>> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  			d->ccr |= CCR_TRIGGER_SRC;
>>  
>>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
>> +
>> +		if (port_window)
>> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>>  	}
>>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>>  		d->clnk_ctrl = c->dma_ch;
>> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>  		osg->addr = sg_dma_address(sgent);
>>  		osg->en = en;
>>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
>> +		if (port_window && dir == DMA_MEM_TO_DEV) {
>> +			osg->ei = 1;
>> +			osg->fi = (-1) * (port_window - 1);
>> +		}
> 
> can you describe what you are trying here..

The DMA is set up so one frame covers the port window. When the frame is
finished we need to start reading the next frame from the start of the window
again. The FI as (-1) * (port_window - 1) will take us to the start of the
window. When the frame is finished the DMA is pointing to the last byte of the
window.

> 


-- 
Péter

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Russell King - ARM Linux]

On Tue, Oct 25, 2016 at 01:50:19PM +0300, Peter Ujfalusi wrote:
> Based on the src/dst_port_window_size - if it is set - configure the DMA
> channel to use double indexing in order to be able to loop within the
> address window.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index 025f499cb20d..29350f936154 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -166,6 +166,9 @@ enum {
>  	CSDP_DST_BURST_16	= 1 << 14,
>  	CSDP_DST_BURST_32	= 2 << 14,
>  	CSDP_DST_BURST_64	= 3 << 14,
> +	CSDP_WRITE_NON_POSTED	= (0 << 16),
> +	CSDP_WRITE_POSTED	= (1 << 16),
> +	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),

Why the useless parens?

> +		if (port_window) {
> +			d->ccr |= CCR_SRC_AMODE_DBLIDX;
> +			d->ei = 1;
> +			d->fi = (-1) * (port_window - 1);

You know that's 1 - port_window, or -(port_window - 1).

> +
> +			if (port_window / 64)
> +				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
> +			else if (port_window / 32)
> +				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
> +			else if (port_window / 16)
> +				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;

Why these divisions?  Wouldn't >= be more suitable here?  Do they even
make sense here?

> +		} else {
> +			d->ccr |= CCR_SRC_AMODE_CONSTANT;
> +		}
>  	} else {
> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
> +
> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
> +		if (port_window) {
> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
> +
> +			if (port_window / 64)
> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
> +			else if (port_window / 32)
> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
> +			else if (port_window / 16)
> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;

Ditto.

> +		} else {
> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
> +		}
>  	}
>  
>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>  			d->ccr |= CCR_TRIGGER_SRC;
>  
>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
> +
> +		if (port_window)
> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>  	}
>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>  		d->clnk_ctrl = c->dma_ch;
> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>  		osg->addr = sg_dma_address(sgent);
>  		osg->en = en;
>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
> +		if (port_window && dir == DMA_MEM_TO_DEV) {
> +			osg->ei = 1;
> +			osg->fi = (-1) * (port_window - 1);

Same as above.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Vinod Koul]

On Mon, Nov 14, 2016 at 11:44:33AM +0200, Peter Ujfalusi wrote:
> On 11/14/2016 06:35 AM, Vinod Koul wrote:

> >>  	} else {
> >> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
> >>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
> >> +
> >> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
> >> +		if (port_window) {
> >> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
> >> +
> >> +			if (port_window / 64)
> >> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
> >> +			else if (port_window / 32)
> >> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
> >> +			else if (port_window / 16)
> >> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
> > 
> > what does these mean?
> 
> To optimize the speed on the write side. First check if the window size is
> multiple of 64 bytes, we enable the 64byte burst and packed transfer, if not
> try the 32bytes, then 16bytes.
> Same for the opposite direction previously.

Ah and how does client know the size of window..?

> 
> > 
> >> +		} else {
> >> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
> >> +		}
> >>  	}
> >>  
> >>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
> >> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
> >>  			d->ccr |= CCR_TRIGGER_SRC;
> >>  
> >>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
> >> +
> >> +		if (port_window)
> >> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
> >>  	}
> >>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
> >>  		d->clnk_ctrl = c->dma_ch;
> >> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
> >>  		osg->addr = sg_dma_address(sgent);
> >>  		osg->en = en;
> >>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
> >> +		if (port_window && dir == DMA_MEM_TO_DEV) {
> >> +			osg->ei = 1;
> >> +			osg->fi = (-1) * (port_window - 1);
> >> +		}
> > 
> > can you describe what you are trying here..
> 
> The DMA is set up so one frame covers the port window. When the frame is
> finished we need to start reading the next frame from the start of the window
> again. The FI as (-1) * (port_window - 1) will take us to the start of the
> window. When the frame is finished the DMA is pointing to the last byte of the
> window.

Sound right to me, would help to add this as a comment..

-- 
~Vinod

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

On 11/14/2016 12:55 PM, Vinod Koul wrote:
> On Mon, Nov 14, 2016 at 11:44:33AM +0200, Peter Ujfalusi wrote:
>> On 11/14/2016 06:35 AM, Vinod Koul wrote:
> 
>>>>  	} else {
>>>> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>>>>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>>>> +
>>>> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
>>>> +		if (port_window) {
>>>> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
>>>> +
>>>> +			if (port_window / 64)
>>>> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>>>> +			else if (port_window / 32)
>>>> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
>>>> +			else if (port_window / 16)
>>>> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
>>>
>>> what does these mean?
>>
>> To optimize the speed on the write side. First check if the window size is
>> multiple of 64 bytes, we enable the 64byte burst and packed transfer, if not
>> try the 32bytes, then 16bytes.
>> Same for the opposite direction previously.
> 
> Ah and how does client know the size of window..?

Clients configuring the DMA should know what they are configuring for. If the
window size can be different they might have DT property if they need.

> 
>>
>>>
>>>> +		} else {
>>>> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
>>>> +		}
>>>>  	}
>>>>  
>>>>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
>>>> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>>>  			d->ccr |= CCR_TRIGGER_SRC;
>>>>  
>>>>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
>>>> +
>>>> +		if (port_window)
>>>> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>>>>  	}
>>>>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>>>>  		d->clnk_ctrl = c->dma_ch;
>>>> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>>>  		osg->addr = sg_dma_address(sgent);
>>>>  		osg->en = en;
>>>>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
>>>> +		if (port_window && dir == DMA_MEM_TO_DEV) {
>>>> +			osg->ei = 1;
>>>> +			osg->fi = (-1) * (port_window - 1);
>>>> +		}
>>>
>>> can you describe what you are trying here..
>>
>> The DMA is set up so one frame covers the port window. When the frame is
>> finished we need to start reading the next frame from the start of the window
>> again. The FI as (-1) * (port_window - 1) will take us to the start of the
>> window. When the frame is finished the DMA is pointing to the last byte of the
>> window.
> 
> Sound right to me, would help to add this as a comment..

OK, I will add a comment describing it.

> 


-- 
Péter

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

On 11/14/2016 12:55 PM, Vinod Koul wrote:
> On Mon, Nov 14, 2016 at 11:44:33AM +0200, Peter Ujfalusi wrote:
>> On 11/14/2016 06:35 AM, Vinod Koul wrote:
> 
>>>>  	} else {
>>>> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>>>>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>>>> +
>>>> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
>>>> +		if (port_window) {
>>>> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
>>>> +
>>>> +			if (port_window / 64)
>>>> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>>>> +			else if (port_window / 32)
>>>> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
>>>> +			else if (port_window / 16)
>>>> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
>>>
>>> what does these mean?
>>
>> To optimize the speed on the write side. First check if the window size is
>> multiple of 64 bytes, we enable the 64byte burst and packed transfer, if not
>> try the 32bytes, then 16bytes.
>> Same for the opposite direction previously.
> 
> Ah and how does client know the size of window..?

Clients configuring the DMA should know what they are configuring for. If the
window size can be different they might have DT property if they need.

> 
>>
>>>
>>>> +		} else {
>>>> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
>>>> +		}
>>>>  	}
>>>>  
>>>>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
>>>> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>>>  			d->ccr |= CCR_TRIGGER_SRC;
>>>>  
>>>>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
>>>> +
>>>> +		if (port_window)
>>>> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>>>>  	}
>>>>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>>>>  		d->clnk_ctrl = c->dma_ch;
>>>> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg(
>>>>  		osg->addr = sg_dma_address(sgent);
>>>>  		osg->en = en;
>>>>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
>>>> +		if (port_window && dir == DMA_MEM_TO_DEV) {
>>>> +			osg->ei = 1;
>>>> +			osg->fi = (-1) * (port_window - 1);
>>>> +		}
>>>
>>> can you describe what you are trying here..
>>
>> The DMA is set up so one frame covers the port window. When the frame is
>> finished we need to start reading the next frame from the start of the window
>> again. The FI as (-1) * (port_window - 1) will take us to the start of the
>> window. When the frame is finished the DMA is pointing to the last byte of the
>> window.
> 
> Sound right to me, would help to add this as a comment..

OK, I will add a comment describing it.

> 


-- 
Péter

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

On 11/14/2016 12:53 PM, Russell King - ARM Linux wrote:
> On Tue, Oct 25, 2016 at 01:50:19PM +0300, Peter Ujfalusi wrote:
>> Based on the src/dst_port_window_size - if it is set - configure the DMA
>> channel to use double indexing in order to be able to loop within the
>> address window.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>> index 025f499cb20d..29350f936154 100644
>> --- a/drivers/dma/omap-dma.c
>> +++ b/drivers/dma/omap-dma.c
>> @@ -166,6 +166,9 @@ enum {
>>  	CSDP_DST_BURST_16	= 1 << 14,
>>  	CSDP_DST_BURST_32	= 2 << 14,
>>  	CSDP_DST_BURST_64	= 3 << 14,
>> +	CSDP_WRITE_NON_POSTED	= (0 << 16),
>> +	CSDP_WRITE_POSTED	= (1 << 16),
>> +	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
>
> Why the useless parens?

I will get rid of them.

>
>> +		if (port_window) {
>> +			d->ccr |= CCR_SRC_AMODE_DBLIDX;
>> +			d->ei = 1;
>> +			d->fi = (-1) * (port_window - 1);
>
> You know that's 1 - port_window, or -(port_window - 1).

Yes, I know. "-(port_window - 1)" might be a bit better.

>
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
>
> Why these divisions?  Wouldn't >= be more suitable here?  Do they even
> make sense here?

I'll revisit these.

>
>> +		} else {
>> +			d->ccr |= CCR_SRC_AMODE_CONSTANT;
>> +		}
>>  	} else {
>> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +
>> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
>> +		if (port_window) {
>> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
>
> Ditto.
>
>> +		} else {
>> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
>> +		}
>>  	}
>>
>>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
>> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor
*omap_dma_prep_slave_sg(
>>  			d->ccr |= CCR_TRIGGER_SRC;
>>
>>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
>> +
>> +		if (port_window)
>> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>>  	}
>>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>>  		d->clnk_ctrl = c->dma_ch;
>> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor
*omap_dma_prep_slave_sg(
>>  		osg->addr = sg_dma_address(sgent);
>>  		osg->en = en;
>>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
>> +		if (port_window && dir == DMA_MEM_TO_DEV) {
>> +			osg->ei = 1;
>> +			osg->fi = (-1) * (port_window - 1);
>
> Same as above.
>


-- 
Péter

Re: [PATCH 2/2] dmaengine: omap-dma: Support for slave devices with data port window

[Peter Ujfalusi]

On 11/14/2016 12:53 PM, Russell King - ARM Linux wrote:
> On Tue, Oct 25, 2016 at 01:50:19PM +0300, Peter Ujfalusi wrote:
>> Based on the src/dst_port_window_size - if it is set - configure the DMA
>> channel to use double indexing in order to be able to loop within the
>> address window.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/dma/omap-dma.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
>> index 025f499cb20d..29350f936154 100644
>> --- a/drivers/dma/omap-dma.c
>> +++ b/drivers/dma/omap-dma.c
>> @@ -166,6 +166,9 @@ enum {
>>  	CSDP_DST_BURST_16	= 1 << 14,
>>  	CSDP_DST_BURST_32	= 2 << 14,
>>  	CSDP_DST_BURST_64	= 3 << 14,
>> +	CSDP_WRITE_NON_POSTED	= (0 << 16),
>> +	CSDP_WRITE_POSTED	= (1 << 16),
>> +	CSDP_WRITE_LAST_NON_POSTED = (2 << 16),
>
> Why the useless parens?

I will get rid of them.

>
>> +		if (port_window) {
>> +			d->ccr |= CCR_SRC_AMODE_DBLIDX;
>> +			d->ei = 1;
>> +			d->fi = (-1) * (port_window - 1);
>
> You know that's 1 - port_window, or -(port_window - 1).

Yes, I know. "-(port_window - 1)" might be a bit better.

>
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_SRC_BURST_32 | CSDP_SRC_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_SRC_BURST_16 | CSDP_SRC_PACKED;
>
> Why these divisions?  Wouldn't >= be more suitable here?  Do they even
> make sense here?

I'll revisit these.

>
>> +		} else {
>> +			d->ccr |= CCR_SRC_AMODE_CONSTANT;
>> +		}
>>  	} else {
>> -		d->ccr |= CCR_DST_AMODE_CONSTANT | CCR_SRC_AMODE_POSTINC;
>>  		d->csdp = CSDP_SRC_BURST_64 | CSDP_SRC_PACKED;
>> +
>> +		d->ccr |= CCR_SRC_AMODE_POSTINC;
>> +		if (port_window) {
>> +			d->ccr |= CCR_DST_AMODE_DBLIDX;
>> +
>> +			if (port_window / 64)
>> +				d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED;
>> +			else if (port_window / 32)
>> +				d->csdp = CSDP_DST_BURST_32 | CSDP_DST_PACKED;
>> +			else if (port_window / 16)
>> +				d->csdp = CSDP_DST_BURST_16 | CSDP_DST_PACKED;
>
> Ditto.
>
>> +		} else {
>> +			d->ccr |= CCR_DST_AMODE_CONSTANT;
>> +		}
>>  	}
>>
>>  	d->cicr = CICR_DROP_IE | CICR_BLOCK_IE;
>> @@ -945,6 +979,9 @@ static struct dma_async_tx_descriptor
*omap_dma_prep_slave_sg(
>>  			d->ccr |= CCR_TRIGGER_SRC;
>>
>>  		d->cicr |= CICR_MISALIGNED_ERR_IE | CICR_TRANS_ERR_IE;
>> +
>> +		if (port_window)
>> +			d->csdp |= CSDP_WRITE_LAST_NON_POSTED;
>>  	}
>>  	if (od->plat->errata & DMA_ERRATA_PARALLEL_CHANNELS)
>>  		d->clnk_ctrl = c->dma_ch;
>> @@ -970,6 +1007,10 @@ static struct dma_async_tx_descriptor
*omap_dma_prep_slave_sg(
>>  		osg->addr = sg_dma_address(sgent);
>>  		osg->en = en;
>>  		osg->fn = sg_dma_len(sgent) / frame_bytes;
>> +		if (port_window && dir == DMA_MEM_TO_DEV) {
>> +			osg->ei = 1;
>> +			osg->fi = (-1) * (port_window - 1);
>
> Same as above.
>


-- 
Péter