DMA Engine development
 help / color / mirror / Atom feed
* [PATCH 1/2] dmaengine: axi-dmac: Discover length alignment requirement
From: Alexandru Ardelean @ 2019-05-21 11:23 UTC (permalink / raw)
  To: dmaengine; +Cc: Lars-Peter Clausen

From: Lars-Peter Clausen <lars@metafoo.de>

Starting with version 4.1.a the AXI-DMAC is capable of reporting the
required length alignment.

The LSBs that are required to be set for alignment will always read back as
set from the transfer length register. It is not possible to clear them by
writing a 0. This means the driver can discover the length alignment
requirement by writing 0 to that register and reading back the value.

Since the DMA will support length alignment requirements that are different
from the address alignment requirement track both of them independently.

For older versions of the peripheral assume that the length alignment
requirement is equal to the address alignment requirement.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/dma/dma-axi-dmac.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index 0984ae6eb155..edd81ceeeb33 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -44,6 +44,8 @@
  * there is no address than can or needs to be configured for the device side.
  */
 
+#define AXI_DMAC_REG_VERSION		0x00
+
 #define AXI_DMAC_REG_IRQ_MASK		0x80
 #define AXI_DMAC_REG_IRQ_PENDING	0x84
 #define AXI_DMAC_REG_IRQ_SOURCE		0x88
@@ -110,7 +112,8 @@ struct axi_dmac_chan {
 	unsigned int dest_type;
 
 	unsigned int max_length;
-	unsigned int align_mask;
+	unsigned int address_align_mask;
+	unsigned int length_align_mask;
 
 	bool hw_cyclic;
 	bool hw_2d;
@@ -169,14 +172,14 @@ static bool axi_dmac_check_len(struct axi_dmac_chan *chan, unsigned int len)
 {
 	if (len == 0)
 		return false;
-	if ((len & chan->align_mask) != 0) /* Not aligned */
+	if ((len & chan->length_align_mask) != 0) /* Not aligned */
 		return false;
 	return true;
 }
 
 static bool axi_dmac_check_addr(struct axi_dmac_chan *chan, dma_addr_t addr)
 {
-	if ((addr & chan->align_mask) != 0) /* Not aligned */
+	if ((addr & chan->address_align_mask) != 0) /* Not aligned */
 		return false;
 	return true;
 }
@@ -394,7 +397,7 @@ static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan,
 	num_segments = DIV_ROUND_UP(period_len, chan->max_length);
 	segment_size = DIV_ROUND_UP(period_len, num_segments);
 	/* Take care of alignment */
-	segment_size = ((segment_size - 1) | chan->align_mask) + 1;
+	segment_size = ((segment_size - 1) | chan->length_align_mask) + 1;
 
 	for (i = 0; i < num_periods; i++) {
 		len = period_len;
@@ -623,7 +626,7 @@ static int axi_dmac_parse_chan_dt(struct device_node *of_chan,
 		return ret;
 	chan->dest_width = val / 8;
 
-	chan->align_mask = max(chan->dest_width, chan->src_width) - 1;
+	chan->address_align_mask = max(chan->dest_width, chan->src_width) - 1;
 
 	if (axi_dmac_dest_is_mem(chan) && axi_dmac_src_is_mem(chan))
 		chan->direction = DMA_MEM_TO_MEM;
@@ -640,6 +643,9 @@ static int axi_dmac_parse_chan_dt(struct device_node *of_chan,
 static int axi_dmac_detect_caps(struct axi_dmac *dmac)
 {
 	struct axi_dmac_chan *chan = &dmac->chan;
+	unsigned int version;
+
+	version = axi_dmac_read(dmac, AXI_DMAC_REG_VERSION);
 
 	axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, AXI_DMAC_FLAG_CYCLIC);
 	if (axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS) == AXI_DMAC_FLAG_CYCLIC)
@@ -670,6 +676,13 @@ static int axi_dmac_detect_caps(struct axi_dmac *dmac)
 		return -ENODEV;
 	}
 
+	if ((version & 0xff00) >= 0x0100) {
+		axi_dmac_write(dmac, AXI_DMAC_REG_X_LENGTH, 0x00);
+		chan->length_align_mask = axi_dmac_read(dmac, AXI_DMAC_REG_X_LENGTH);
+	} else {
+		chan->length_align_mask = chan->address_align_mask;
+	}
+
 	return 0;
 }
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/2] dmaengine: axi-dmac: assign `copy_align` property
From: Alexandru Ardelean @ 2019-05-21 11:23 UTC (permalink / raw)
  To: dmaengine; +Cc: Alexandru Ardelean
In-Reply-To: <20190521112331.32424-1-alexandru.ardelean@analog.com>

The `copy_align` property is a generic property that describes alignment
for DMA memcpy & sg ops.
It serves mostly an informational purpose, and can be used in DMA tests, to
pass the info to know what alignment to expect.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/dma/dma-axi-dmac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
index edd81ceeeb33..eea994d827ba 100644
--- a/drivers/dma/dma-axi-dmac.c
+++ b/drivers/dma/dma-axi-dmac.c
@@ -763,6 +763,8 @@ static int axi_dmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_clk_disable;
 
+	dma_dev->copy_align = (dmac->chan.address_align_mask + 1);
+
 	axi_dmac_write(dmac, AXI_DMAC_REG_IRQ_MASK, 0x00);
 
 	ret = dma_async_device_register(dma_dev);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 1/2] dmaengine: ti: edma: Clean up the 2x32bit array register accesses
From: Peter Ujfalusi @ 2019-05-21  8:01 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap
In-Reply-To: <20190521075945.14085-2-peter.ujfalusi@ti.com>



On 21/05/2019 10.59, Peter Ujfalusi wrote:
> Introduce defines for getting the array index and the bit number within the
> 64bit array register pairs.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/ti/edma.c | 106 ++++++++++++++++++++++++------------------
>  1 file changed, 61 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
> index ceabdea40ae0..a5822925a327 100644
> --- a/drivers/dma/ti/edma.c
> +++ b/drivers/dma/ti/edma.c
> @@ -133,6 +133,17 @@
>  #define EDMA_CONT_PARAMS_FIXED_EXACT	 1002
>  #define EDMA_CONT_PARAMS_FIXED_NOT_EXACT 1003
>  
> +/*
> + * 64bit array registers are plit into two 32bit registers:

typo s/plit/split

> + * reg0: channel/event 0-31
> + * reg1: channel/event 32-63
> + *
> + * bit 5 in the channel number tells the array index (0/1)
> + * bit 0-4 (0x1f) is the bit offset within the register
> + */
> +#define EDMA_REG_ARRAY_INDEX(channel)	((channel) >> 5)
> +#define EDMA_CHANNEL_BIT(channel)	(BIT((channel) & 0x1f))
> +
>  /* PaRAM slots are laid out like this */
>  struct edmacc_param {
>  	u32 opt;
> @@ -441,15 +452,14 @@ static void edma_setup_interrupt(struct edma_chan *echan, bool enable)
>  {
>  	struct edma_cc *ecc = echan->ecc;
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> +	int idx = EDMA_REG_ARRAY_INDEX(channel);
> +	int ch_bit = EDMA_CHANNEL_BIT(channel);
>  
>  	if (enable) {
> -		edma_shadow0_write_array(ecc, SH_ICR, channel >> 5,
> -					 BIT(channel & 0x1f));
> -		edma_shadow0_write_array(ecc, SH_IESR, channel >> 5,
> -					 BIT(channel & 0x1f));
> +		edma_shadow0_write_array(ecc, SH_ICR, idx, ch_bit);
> +		edma_shadow0_write_array(ecc, SH_IESR, idx, ch_bit);
>  	} else {
> -		edma_shadow0_write_array(ecc, SH_IECR, channel >> 5,
> -					 BIT(channel & 0x1f));
> +		edma_shadow0_write_array(ecc, SH_IECR, idx, ch_bit);
>  	}
>  }
>  
> @@ -587,26 +597,26 @@ static void edma_start(struct edma_chan *echan)
>  {
>  	struct edma_cc *ecc = echan->ecc;
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> -	int j = (channel >> 5);
> -	unsigned int mask = BIT(channel & 0x1f);
> +	int idx = EDMA_REG_ARRAY_INDEX(channel);
> +	int ch_bit = EDMA_CHANNEL_BIT(channel);
>  
>  	if (!echan->hw_triggered) {
>  		/* EDMA channels without event association */
> -		dev_dbg(ecc->dev, "ESR%d %08x\n", j,
> -			edma_shadow0_read_array(ecc, SH_ESR, j));
> -		edma_shadow0_write_array(ecc, SH_ESR, j, mask);
> +		dev_dbg(ecc->dev, "ESR%d %08x\n", idx,
> +			edma_shadow0_read_array(ecc, SH_ESR, idx));
> +		edma_shadow0_write_array(ecc, SH_ESR, idx, ch_bit);
>  	} else {
>  		/* EDMA channel with event association */
> -		dev_dbg(ecc->dev, "ER%d %08x\n", j,
> -			edma_shadow0_read_array(ecc, SH_ER, j));
> +		dev_dbg(ecc->dev, "ER%d %08x\n", idx,
> +			edma_shadow0_read_array(ecc, SH_ER, idx));
>  		/* Clear any pending event or error */
> -		edma_write_array(ecc, EDMA_ECR, j, mask);
> -		edma_write_array(ecc, EDMA_EMCR, j, mask);
> +		edma_write_array(ecc, EDMA_ECR, idx, ch_bit);
> +		edma_write_array(ecc, EDMA_EMCR, idx, ch_bit);
>  		/* Clear any SER */
> -		edma_shadow0_write_array(ecc, SH_SECR, j, mask);
> -		edma_shadow0_write_array(ecc, SH_EESR, j, mask);
> -		dev_dbg(ecc->dev, "EER%d %08x\n", j,
> -			edma_shadow0_read_array(ecc, SH_EER, j));
> +		edma_shadow0_write_array(ecc, SH_SECR, idx, ch_bit);
> +		edma_shadow0_write_array(ecc, SH_EESR, idx, ch_bit);
> +		dev_dbg(ecc->dev, "EER%d %08x\n", idx,
> +			edma_shadow0_read_array(ecc, SH_EER, idx));
>  	}
>  }
>  
> @@ -614,19 +624,19 @@ static void edma_stop(struct edma_chan *echan)
>  {
>  	struct edma_cc *ecc = echan->ecc;
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> -	int j = (channel >> 5);
> -	unsigned int mask = BIT(channel & 0x1f);
> +	int idx = EDMA_REG_ARRAY_INDEX(channel);
> +	int ch_bit = EDMA_CHANNEL_BIT(channel);
>  
> -	edma_shadow0_write_array(ecc, SH_EECR, j, mask);
> -	edma_shadow0_write_array(ecc, SH_ECR, j, mask);
> -	edma_shadow0_write_array(ecc, SH_SECR, j, mask);
> -	edma_write_array(ecc, EDMA_EMCR, j, mask);
> +	edma_shadow0_write_array(ecc, SH_EECR, idx, ch_bit);
> +	edma_shadow0_write_array(ecc, SH_ECR, idx, ch_bit);
> +	edma_shadow0_write_array(ecc, SH_SECR, idx, ch_bit);
> +	edma_write_array(ecc, EDMA_EMCR, idx, ch_bit);
>  
>  	/* clear possibly pending completion interrupt */
> -	edma_shadow0_write_array(ecc, SH_ICR, j, mask);
> +	edma_shadow0_write_array(ecc, SH_ICR, idx, ch_bit);
>  
> -	dev_dbg(ecc->dev, "EER%d %08x\n", j,
> -		edma_shadow0_read_array(ecc, SH_EER, j));
> +	dev_dbg(ecc->dev, "EER%d %08x\n", idx,
> +		edma_shadow0_read_array(ecc, SH_EER, idx));
>  
>  	/* REVISIT:  consider guarding against inappropriate event
>  	 * chaining by overwriting with dummy_paramset.
> @@ -640,45 +650,49 @@ static void edma_stop(struct edma_chan *echan)
>  static void edma_pause(struct edma_chan *echan)
>  {
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> -	unsigned int mask = BIT(channel & 0x1f);
>  
> -	edma_shadow0_write_array(echan->ecc, SH_EECR, channel >> 5, mask);
> +	edma_shadow0_write_array(echan->ecc, SH_EECR,
> +				 EDMA_REG_ARRAY_INDEX(channel),
> +				 EDMA_CHANNEL_BIT(channel));
>  }
>  
>  /* Re-enable EDMA hardware events on the specified channel.  */
>  static void edma_resume(struct edma_chan *echan)
>  {
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> -	unsigned int mask = BIT(channel & 0x1f);
>  
> -	edma_shadow0_write_array(echan->ecc, SH_EESR, channel >> 5, mask);
> +	edma_shadow0_write_array(echan->ecc, SH_EESR,
> +				 EDMA_REG_ARRAY_INDEX(channel),
> +				 EDMA_CHANNEL_BIT(channel));
>  }
>  
>  static void edma_trigger_channel(struct edma_chan *echan)
>  {
>  	struct edma_cc *ecc = echan->ecc;
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> -	unsigned int mask = BIT(channel & 0x1f);
> +	int idx = EDMA_REG_ARRAY_INDEX(channel);
> +	int ch_bit = EDMA_CHANNEL_BIT(channel);
>  
> -	edma_shadow0_write_array(ecc, SH_ESR, (channel >> 5), mask);
> +	edma_shadow0_write_array(ecc, SH_ESR, idx, ch_bit);
>  
> -	dev_dbg(ecc->dev, "ESR%d %08x\n", (channel >> 5),
> -		edma_shadow0_read_array(ecc, SH_ESR, (channel >> 5)));
> +	dev_dbg(ecc->dev, "ESR%d %08x\n", idx,
> +		edma_shadow0_read_array(ecc, SH_ESR, idx));
>  }
>  
>  static void edma_clean_channel(struct edma_chan *echan)
>  {
>  	struct edma_cc *ecc = echan->ecc;
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
> -	int j = (channel >> 5);
> -	unsigned int mask = BIT(channel & 0x1f);
> +	int idx = EDMA_REG_ARRAY_INDEX(channel);
> +	int ch_bit = EDMA_CHANNEL_BIT(channel);
>  
> -	dev_dbg(ecc->dev, "EMR%d %08x\n", j, edma_read_array(ecc, EDMA_EMR, j));
> -	edma_shadow0_write_array(ecc, SH_ECR, j, mask);
> +	dev_dbg(ecc->dev, "EMR%d %08x\n", idx,
> +		edma_read_array(ecc, EDMA_EMR, idx));
> +	edma_shadow0_write_array(ecc, SH_ECR, idx, ch_bit);
>  	/* Clear the corresponding EMR bits */
> -	edma_write_array(ecc, EDMA_EMCR, j, mask);
> +	edma_write_array(ecc, EDMA_EMCR, idx, ch_bit);
>  	/* Clear any SER */
> -	edma_shadow0_write_array(ecc, SH_SECR, j, mask);
> +	edma_shadow0_write_array(ecc, SH_SECR, idx, ch_bit);
>  	edma_write(ecc, EDMA_CCERRCLR, BIT(16) | BIT(1) | BIT(0));
>  }
>  
> @@ -708,7 +722,8 @@ static int edma_alloc_channel(struct edma_chan *echan,
>  	int channel = EDMA_CHAN_SLOT(echan->ch_num);
>  
>  	/* ensure access through shadow region 0 */
> -	edma_or_array2(ecc, EDMA_DRAE, 0, channel >> 5, BIT(channel & 0x1f));
> +	edma_or_array2(ecc, EDMA_DRAE, 0, EDMA_REG_ARRAY_INDEX(channel),
> +		       EDMA_CHANNEL_BIT(channel));
>  
>  	/* ensure no events are pending */
>  	edma_stop(echan);
> @@ -2482,8 +2497,9 @@ static int edma_pm_resume(struct device *dev)
>  	for (i = 0; i < ecc->num_channels; i++) {
>  		if (echan[i].alloced) {
>  			/* ensure access through shadow region 0 */
> -			edma_or_array2(ecc, EDMA_DRAE, 0, i >> 5,
> -				       BIT(i & 0x1f));
> +			edma_or_array2(ecc, EDMA_DRAE, 0,
> +				       EDMA_REG_ARRAY_INDEX(i),
> +				       EDMA_CHANNEL_BIT(i));
>  
>  			edma_setup_interrupt(&echan[i], true);
>  
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [PATCH v2 1/2] dmaengine: ti: edma: Clean up the 2x32bit array register accesses
From: Peter Ujfalusi @ 2019-05-21  7:59 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap
In-Reply-To: <20190521075945.14085-1-peter.ujfalusi@ti.com>

Introduce defines for getting the array index and the bit number within the
64bit array register pairs.

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

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index ceabdea40ae0..a5822925a327 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -133,6 +133,17 @@
 #define EDMA_CONT_PARAMS_FIXED_EXACT	 1002
 #define EDMA_CONT_PARAMS_FIXED_NOT_EXACT 1003
 
+/*
+ * 64bit array registers are plit into two 32bit registers:
+ * reg0: channel/event 0-31
+ * reg1: channel/event 32-63
+ *
+ * bit 5 in the channel number tells the array index (0/1)
+ * bit 0-4 (0x1f) is the bit offset within the register
+ */
+#define EDMA_REG_ARRAY_INDEX(channel)	((channel) >> 5)
+#define EDMA_CHANNEL_BIT(channel)	(BIT((channel) & 0x1f))
+
 /* PaRAM slots are laid out like this */
 struct edmacc_param {
 	u32 opt;
@@ -441,15 +452,14 @@ static void edma_setup_interrupt(struct edma_chan *echan, bool enable)
 {
 	struct edma_cc *ecc = echan->ecc;
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
+	int idx = EDMA_REG_ARRAY_INDEX(channel);
+	int ch_bit = EDMA_CHANNEL_BIT(channel);
 
 	if (enable) {
-		edma_shadow0_write_array(ecc, SH_ICR, channel >> 5,
-					 BIT(channel & 0x1f));
-		edma_shadow0_write_array(ecc, SH_IESR, channel >> 5,
-					 BIT(channel & 0x1f));
+		edma_shadow0_write_array(ecc, SH_ICR, idx, ch_bit);
+		edma_shadow0_write_array(ecc, SH_IESR, idx, ch_bit);
 	} else {
-		edma_shadow0_write_array(ecc, SH_IECR, channel >> 5,
-					 BIT(channel & 0x1f));
+		edma_shadow0_write_array(ecc, SH_IECR, idx, ch_bit);
 	}
 }
 
@@ -587,26 +597,26 @@ static void edma_start(struct edma_chan *echan)
 {
 	struct edma_cc *ecc = echan->ecc;
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
-	int j = (channel >> 5);
-	unsigned int mask = BIT(channel & 0x1f);
+	int idx = EDMA_REG_ARRAY_INDEX(channel);
+	int ch_bit = EDMA_CHANNEL_BIT(channel);
 
 	if (!echan->hw_triggered) {
 		/* EDMA channels without event association */
-		dev_dbg(ecc->dev, "ESR%d %08x\n", j,
-			edma_shadow0_read_array(ecc, SH_ESR, j));
-		edma_shadow0_write_array(ecc, SH_ESR, j, mask);
+		dev_dbg(ecc->dev, "ESR%d %08x\n", idx,
+			edma_shadow0_read_array(ecc, SH_ESR, idx));
+		edma_shadow0_write_array(ecc, SH_ESR, idx, ch_bit);
 	} else {
 		/* EDMA channel with event association */
-		dev_dbg(ecc->dev, "ER%d %08x\n", j,
-			edma_shadow0_read_array(ecc, SH_ER, j));
+		dev_dbg(ecc->dev, "ER%d %08x\n", idx,
+			edma_shadow0_read_array(ecc, SH_ER, idx));
 		/* Clear any pending event or error */
-		edma_write_array(ecc, EDMA_ECR, j, mask);
-		edma_write_array(ecc, EDMA_EMCR, j, mask);
+		edma_write_array(ecc, EDMA_ECR, idx, ch_bit);
+		edma_write_array(ecc, EDMA_EMCR, idx, ch_bit);
 		/* Clear any SER */
-		edma_shadow0_write_array(ecc, SH_SECR, j, mask);
-		edma_shadow0_write_array(ecc, SH_EESR, j, mask);
-		dev_dbg(ecc->dev, "EER%d %08x\n", j,
-			edma_shadow0_read_array(ecc, SH_EER, j));
+		edma_shadow0_write_array(ecc, SH_SECR, idx, ch_bit);
+		edma_shadow0_write_array(ecc, SH_EESR, idx, ch_bit);
+		dev_dbg(ecc->dev, "EER%d %08x\n", idx,
+			edma_shadow0_read_array(ecc, SH_EER, idx));
 	}
 }
 
@@ -614,19 +624,19 @@ static void edma_stop(struct edma_chan *echan)
 {
 	struct edma_cc *ecc = echan->ecc;
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
-	int j = (channel >> 5);
-	unsigned int mask = BIT(channel & 0x1f);
+	int idx = EDMA_REG_ARRAY_INDEX(channel);
+	int ch_bit = EDMA_CHANNEL_BIT(channel);
 
-	edma_shadow0_write_array(ecc, SH_EECR, j, mask);
-	edma_shadow0_write_array(ecc, SH_ECR, j, mask);
-	edma_shadow0_write_array(ecc, SH_SECR, j, mask);
-	edma_write_array(ecc, EDMA_EMCR, j, mask);
+	edma_shadow0_write_array(ecc, SH_EECR, idx, ch_bit);
+	edma_shadow0_write_array(ecc, SH_ECR, idx, ch_bit);
+	edma_shadow0_write_array(ecc, SH_SECR, idx, ch_bit);
+	edma_write_array(ecc, EDMA_EMCR, idx, ch_bit);
 
 	/* clear possibly pending completion interrupt */
-	edma_shadow0_write_array(ecc, SH_ICR, j, mask);
+	edma_shadow0_write_array(ecc, SH_ICR, idx, ch_bit);
 
-	dev_dbg(ecc->dev, "EER%d %08x\n", j,
-		edma_shadow0_read_array(ecc, SH_EER, j));
+	dev_dbg(ecc->dev, "EER%d %08x\n", idx,
+		edma_shadow0_read_array(ecc, SH_EER, idx));
 
 	/* REVISIT:  consider guarding against inappropriate event
 	 * chaining by overwriting with dummy_paramset.
@@ -640,45 +650,49 @@ static void edma_stop(struct edma_chan *echan)
 static void edma_pause(struct edma_chan *echan)
 {
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
-	unsigned int mask = BIT(channel & 0x1f);
 
-	edma_shadow0_write_array(echan->ecc, SH_EECR, channel >> 5, mask);
+	edma_shadow0_write_array(echan->ecc, SH_EECR,
+				 EDMA_REG_ARRAY_INDEX(channel),
+				 EDMA_CHANNEL_BIT(channel));
 }
 
 /* Re-enable EDMA hardware events on the specified channel.  */
 static void edma_resume(struct edma_chan *echan)
 {
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
-	unsigned int mask = BIT(channel & 0x1f);
 
-	edma_shadow0_write_array(echan->ecc, SH_EESR, channel >> 5, mask);
+	edma_shadow0_write_array(echan->ecc, SH_EESR,
+				 EDMA_REG_ARRAY_INDEX(channel),
+				 EDMA_CHANNEL_BIT(channel));
 }
 
 static void edma_trigger_channel(struct edma_chan *echan)
 {
 	struct edma_cc *ecc = echan->ecc;
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
-	unsigned int mask = BIT(channel & 0x1f);
+	int idx = EDMA_REG_ARRAY_INDEX(channel);
+	int ch_bit = EDMA_CHANNEL_BIT(channel);
 
-	edma_shadow0_write_array(ecc, SH_ESR, (channel >> 5), mask);
+	edma_shadow0_write_array(ecc, SH_ESR, idx, ch_bit);
 
-	dev_dbg(ecc->dev, "ESR%d %08x\n", (channel >> 5),
-		edma_shadow0_read_array(ecc, SH_ESR, (channel >> 5)));
+	dev_dbg(ecc->dev, "ESR%d %08x\n", idx,
+		edma_shadow0_read_array(ecc, SH_ESR, idx));
 }
 
 static void edma_clean_channel(struct edma_chan *echan)
 {
 	struct edma_cc *ecc = echan->ecc;
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
-	int j = (channel >> 5);
-	unsigned int mask = BIT(channel & 0x1f);
+	int idx = EDMA_REG_ARRAY_INDEX(channel);
+	int ch_bit = EDMA_CHANNEL_BIT(channel);
 
-	dev_dbg(ecc->dev, "EMR%d %08x\n", j, edma_read_array(ecc, EDMA_EMR, j));
-	edma_shadow0_write_array(ecc, SH_ECR, j, mask);
+	dev_dbg(ecc->dev, "EMR%d %08x\n", idx,
+		edma_read_array(ecc, EDMA_EMR, idx));
+	edma_shadow0_write_array(ecc, SH_ECR, idx, ch_bit);
 	/* Clear the corresponding EMR bits */
-	edma_write_array(ecc, EDMA_EMCR, j, mask);
+	edma_write_array(ecc, EDMA_EMCR, idx, ch_bit);
 	/* Clear any SER */
-	edma_shadow0_write_array(ecc, SH_SECR, j, mask);
+	edma_shadow0_write_array(ecc, SH_SECR, idx, ch_bit);
 	edma_write(ecc, EDMA_CCERRCLR, BIT(16) | BIT(1) | BIT(0));
 }
 
@@ -708,7 +722,8 @@ static int edma_alloc_channel(struct edma_chan *echan,
 	int channel = EDMA_CHAN_SLOT(echan->ch_num);
 
 	/* ensure access through shadow region 0 */
-	edma_or_array2(ecc, EDMA_DRAE, 0, channel >> 5, BIT(channel & 0x1f));
+	edma_or_array2(ecc, EDMA_DRAE, 0, EDMA_REG_ARRAY_INDEX(channel),
+		       EDMA_CHANNEL_BIT(channel));
 
 	/* ensure no events are pending */
 	edma_stop(echan);
@@ -2482,8 +2497,9 @@ static int edma_pm_resume(struct device *dev)
 	for (i = 0; i < ecc->num_channels; i++) {
 		if (echan[i].alloced) {
 			/* ensure access through shadow region 0 */
-			edma_or_array2(ecc, EDMA_DRAE, 0, i >> 5,
-				       BIT(i & 0x1f));
+			edma_or_array2(ecc, EDMA_DRAE, 0,
+				       EDMA_REG_ARRAY_INDEX(i),
+				       EDMA_CHANNEL_BIT(i));
 
 			edma_setup_interrupt(&echan[i], true);
 
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply related

* [PATCH v2 2/2] dmaengine: ti: edma: Enable support for polled (memcpy) completion
From: Peter Ujfalusi @ 2019-05-21  7:59 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap
In-Reply-To: <20190521075945.14085-1-peter.ujfalusi@ti.com>

When a DMA client driver decides that it is not providing callback for
completion of a transfer (and/or does not set the DMA_PREP_INTERRUPT) but
it will poll the status of the transfer (in case of short memcpy for
example) we will not get interrupt for the completion of the transfer and
will not mark the transaction as done.

Check the event registers (ER and EER) and if the channel is inactive then
return with DMA_COMPLETE to let the client know that the transfer is
completed.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/ti/edma.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index a5822925a327..0f4873c2aa12 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -1226,8 +1226,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy(
 
 	edesc->pset[0].param.opt |= ITCCHEN;
 	if (nslots == 1) {
-		/* Enable transfer complete interrupt */
-		edesc->pset[0].param.opt |= TCINTEN;
+		/* Enable transfer complete interrupt if requested */
+		if (tx_flags & DMA_PREP_INTERRUPT)
+			edesc->pset[0].param.opt |= TCINTEN;
 	} else {
 		/* Enable transfer complete chaining for the first slot */
 		edesc->pset[0].param.opt |= TCCHEN;
@@ -1254,7 +1255,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy(
 		}
 
 		edesc->pset[1].param.opt |= ITCCHEN;
-		edesc->pset[1].param.opt |= TCINTEN;
+		/* Enable transfer complete interrupt if requested */
+		if (tx_flags & DMA_PREP_INTERRUPT)
+			edesc->pset[1].param.opt |= TCINTEN;
 	}
 
 	return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
@@ -1816,6 +1819,20 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
 	unsigned long flags;
 
 	ret = dma_cookie_status(chan, cookie, txstate);
+
+	if (ret != DMA_COMPLETE && echan->edesc && !echan->edesc->cyclic) {
+		struct edma_cc *ecc = echan->ecc;
+		int channel = EDMA_CHAN_SLOT(echan->ch_num);
+		int idx = EDMA_REG_ARRAY_INDEX(channel);
+		int ch_bit = EDMA_CHANNEL_BIT(channel);
+		unsigned int sh_er = edma_shadow0_read_array(ecc, SH_ER, idx);
+		unsigned int sh_eer = edma_shadow0_read_array(ecc, SH_EER, idx);
+
+		/* The channel is no longer active */
+		if (!(sh_er & ch_bit) && !(sh_eer & ch_bit))
+			ret = DMA_COMPLETE;
+	}
+
 	if (ret == DMA_COMPLETE || !txstate)
 		return ret;
 
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply related

* [PATCH v2 0/2] dmaengine: ti: edma: Polled completion support
From: Peter Ujfalusi @ 2019-05-21  7:59 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap

Hi,

Changes since v1:
- Cleanup patch for the array register handling
- typo fixed in patch2 commit message

The code around the array register access was pretty confusing for the first
look, so clean them up first then use the cleaner way in the polled handling.

When a DMA client driver decides that it is not providing callback for
completion of a transfer (and/or does not set the DMA_PREP_INTERRUPT) but
it will poll the status of the transfer (in case of short memcpy for
example) we will not get interrupt for the completion of the transfer and
will not mark the transaction as done.

Check the event registers (ER and EER) and if the channel is inactive then
return wioth DMA_COMPLETE to let the client know that the transfer is
completed.

Regards,
Peter
---
Peter Ujfalusi (2):
  dmaengine: ti: edma: Clean up the 2x32bit array register accesses
  dmaengine: ti: edma: Enable support for polled (memcpy) completion

 drivers/dma/ti/edma.c | 129 ++++++++++++++++++++++++++----------------
 1 file changed, 81 insertions(+), 48 deletions(-)

-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply

* Re: [PATCH] dmaengine: ti: edma: Enable support for polled (memcpy) completion
From: Vinod Koul @ 2019-05-21  7:21 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap
In-Reply-To: <ce1a2e96-bc4b-3998-0c36-362867907177@ti.com>

On 21-05-19, 09:16, Peter Ujfalusi wrote:
> On 21/05/2019 8.04, Vinod Koul wrote:
> > On 14-05-19, 11:09, Peter Ujfalusi wrote:

> >>  	return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
> >> @@ -1801,6 +1804,20 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
> >>  	unsigned long flags;
> >>  
> >>  	ret = dma_cookie_status(chan, cookie, txstate);
> >> +
> >> +	if (ret != DMA_COMPLETE && echan->edesc && !echan->edesc->cyclic) {
> >> +		struct edma_cc *ecc = echan->ecc;
> >> +		int channel = EDMA_CHAN_SLOT(echan->ch_num);
> >> +		int j = (channel >> 5);
> >> +		unsigned int mask = BIT(channel & 0x1f);
> > 
> > GENMASK or define a macro instead of a magic number?
> 
> So it is better to change the other places first from where I have just
> copied this.

That would be nice as well :)

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: ti: edma: Enable support for polled (memcpy) completion
From: Peter Ujfalusi @ 2019-05-21  6:16 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap
In-Reply-To: <20190521050430.GS15118@vkoul-mobl>



On 21/05/2019 8.04, Vinod Koul wrote:
> On 14-05-19, 11:09, Peter Ujfalusi wrote:
>> When a DMA client driver decides that it is not providing callback for
>> completion of a transfer (and/or does not set the DMA_PREP_INTERRUPT) but
>> it will poll the status of the transfer (in case of short memcpy for
>> example) we will not get interrupt for the completion of the transfer and
>> will not mark the transaction as done.
>>
>> Check the event registers (ER and EER) and if the channel is inactive then
>> return wioth DMA_COMPLETE to let the client know that the transfer is
>         ^^^^^
> Typo

Ok

> 
>> completed.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/dma/ti/edma.c | 23 ++++++++++++++++++++---
>>  1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
>> index ceabdea40ae0..7501445af069 100644
>> --- a/drivers/dma/ti/edma.c
>> +++ b/drivers/dma/ti/edma.c
>> @@ -1211,8 +1211,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy(
>>  
>>  	edesc->pset[0].param.opt |= ITCCHEN;
>>  	if (nslots == 1) {
>> -		/* Enable transfer complete interrupt */
>> -		edesc->pset[0].param.opt |= TCINTEN;
>> +		/* Enable transfer complete interrupt if requested */
>> +		if (tx_flags & DMA_PREP_INTERRUPT)
>> +			edesc->pset[0].param.opt |= TCINTEN;
>>  	} else {
>>  		/* Enable transfer complete chaining for the first slot */
>>  		edesc->pset[0].param.opt |= TCCHEN;
>> @@ -1239,7 +1240,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy(
>>  		}
>>  
>>  		edesc->pset[1].param.opt |= ITCCHEN;
>> -		edesc->pset[1].param.opt |= TCINTEN;
>> +		/* Enable transfer complete interrupt if requested */
>> +		if (tx_flags & DMA_PREP_INTERRUPT)
>> +			edesc->pset[1].param.opt |= TCINTEN;
>>  	}
>>  
>>  	return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
>> @@ -1801,6 +1804,20 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
>>  	unsigned long flags;
>>  
>>  	ret = dma_cookie_status(chan, cookie, txstate);
>> +
>> +	if (ret != DMA_COMPLETE && echan->edesc && !echan->edesc->cyclic) {
>> +		struct edma_cc *ecc = echan->ecc;
>> +		int channel = EDMA_CHAN_SLOT(echan->ch_num);
>> +		int j = (channel >> 5);
>> +		unsigned int mask = BIT(channel & 0x1f);
> 
> GENMASK or define a macro instead of a magic number?

So it is better to change the other places first from where I have just
copied this.

> 
>> +		unsigned int sh_er = edma_shadow0_read_array(ecc, SH_ER, j);
>> +		unsigned int sh_eer = edma_shadow0_read_array(ecc, SH_EER, j);
>> +
>> +		/* The channel is no longer active */
>> +		if (!(sh_er & mask) && !(sh_eer & mask))
>> +			ret = DMA_COMPLETE;
>> +	}
>> +
>>  	if (ret == DMA_COMPLETE || !txstate)
>>  		return ret;
>>  
>> -- 
>> Peter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* Re: Re: Re: [PATCH v3 11/14] dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
From: Vinod Koul @ 2019-05-21  6:12 UTC (permalink / raw)
  To: Robin Gong
  Cc: robh@kernel.org, broonie@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com, mark.rutland@arm.com,
	u.kleine-koenig@pengutronix.de, plyatov@gmail.com,
	dan.j.williams@intel.com, catalin.marinas@arm.com,
	will.deacon@arm.com, l.stach@pengutronix.de, dl-linux-imx,
	linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, kernel@pengutronix.de
In-Reply-To: <VI1PR04MB4543DEEC702531ED69616B8C89070@VI1PR04MB4543.eurprd04.prod.outlook.com>

On 21-05-19, 05:41, Robin Gong wrote:
> > -----Original Message-----
> > From: Vinod Koul <vkoul@kernel.org>
> > Sent: 2019年5月21日 13:13
> > 
> > On 21-05-19, 04:58, Robin Gong wrote:
> > > > -----Original Message-----
> > > > From: Vinod Koul <vkoul@kernel.org>
> > > > Sent: 2019年5月21日 12:18
> > > >
> > > > On 07-05-19, 09:16, Robin Gong wrote:
> > > > > Because the number of ecspi1 rx event on i.mx8mm is 0, the
> > > > > condition check ignore such special case without dma channel
> > > > > enabled, which caused
> > > > > ecspi1 rx works failed. Actually, no need to check event_id0,
> > > > > checking
> > > > > event_id1 is enough for DEV_2_DEV case because it's so lucky that
> > > > > event_id1 never be 0.
> > > >
> > > > Well is that by chance or design that event_id1 will be never 0?
> > > >
> > > That's by chance. DEV_2_DEV is just for Audio case and non-zero for
> > event_id1 on current i.MX family.
> > 
> > Then it wont be fgood to rely on chance :)
> Yes, I knew that. May I create another independent patch for event_id1 since that's potential issue is not related with this ecspi patch set?

Sure a patch should change one thing but I think it should come before
this one. The log for this should be fixed up as well

-- 
~Vinod

^ permalink raw reply

* RE: Re: Re: [PATCH v3 11/14] dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
From: Robin Gong @ 2019-05-21  5:41 UTC (permalink / raw)
  To: Vinod Koul
  Cc: robh@kernel.org, broonie@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com, mark.rutland@arm.com,
	u.kleine-koenig@pengutronix.de, plyatov@gmail.com,
	dan.j.williams@intel.com, catalin.marinas@arm.com,
	will.deacon@arm.com, l.stach@pengutronix.de, dl-linux-imx,
	linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, kernel@pengutronix.de

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: 2019年5月21日 13:13
> 
> On 21-05-19, 04:58, Robin Gong wrote:
> > > -----Original Message-----
> > > From: Vinod Koul <vkoul@kernel.org>
> > > Sent: 2019年5月21日 12:18
> > >
> > > On 07-05-19, 09:16, Robin Gong wrote:
> > > > Because the number of ecspi1 rx event on i.mx8mm is 0, the
> > > > condition check ignore such special case without dma channel
> > > > enabled, which caused
> > > > ecspi1 rx works failed. Actually, no need to check event_id0,
> > > > checking
> > > > event_id1 is enough for DEV_2_DEV case because it's so lucky that
> > > > event_id1 never be 0.
> > >
> > > Well is that by chance or design that event_id1 will be never 0?
> > >
> > That's by chance. DEV_2_DEV is just for Audio case and non-zero for
> event_id1 on current i.MX family.
> 
> Then it wont be fgood to rely on chance :)
Yes, I knew that. May I create another independent patch for event_id1 since that's potential issue is not related with this ecspi patch set?
> 
> --
> ~Vinod

^ permalink raw reply

* Re: Re: [PATCH v3 11/14] dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
From: Vinod Koul @ 2019-05-21  5:12 UTC (permalink / raw)
  To: Robin Gong
  Cc: robh@kernel.org, broonie@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com, mark.rutland@arm.com,
	u.kleine-koenig@pengutronix.de, plyatov@gmail.com,
	dan.j.williams@intel.com, catalin.marinas@arm.com,
	will.deacon@arm.com, l.stach@pengutronix.de, dl-linux-imx,
	linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, kernel@pengutronix.de
In-Reply-To: <VI1PR04MB45436C98D70C16635CF3CFDE89070@VI1PR04MB4543.eurprd04.prod.outlook.com>

On 21-05-19, 04:58, Robin Gong wrote:
> > -----Original Message-----
> > From: Vinod Koul <vkoul@kernel.org>
> > Sent: 2019年5月21日 12:18
> > 
> > On 07-05-19, 09:16, Robin Gong wrote:
> > > Because the number of ecspi1 rx event on i.mx8mm is 0, the condition
> > > check ignore such special case without dma channel enabled, which
> > > caused
> > > ecspi1 rx works failed. Actually, no need to check event_id0, checking
> > > event_id1 is enough for DEV_2_DEV case because it's so lucky that
> > > event_id1 never be 0.
> > 
> > Well is that by chance or design that event_id1 will be never 0?
> > 
> That's by chance. DEV_2_DEV is just for Audio case and non-zero for event_id1 on current i.MX family.

Then it wont be fgood to rely on chance :)

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH 0/3] dmaengine: tegra210-adma: Fixes for v5.2
From: Vinod Koul @ 2019-05-21  5:11 UTC (permalink / raw)
  To: Jon Hunter; +Cc: Laxman Dewangan, dmaengine, linux-tegra, Sameer Pujar
In-Reply-To: <1558022034-19911-1-git-send-email-jonathanh@nvidia.com>

On 16-05-19, 16:53, Jon Hunter wrote:
> Here are 3 fixes for the Tegra210 ADMA driver for v5.2.
> 

Applied all, thanks
-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: ioatdma: fix unprotected timer deletion
From: Vinod Koul @ 2019-05-21  5:09 UTC (permalink / raw)
  To: Dave Jiang; +Cc: dmaengine, dan.j.williams, fan.du
In-Reply-To: <f80ae09d-82f3-e395-f797-afd79381ce36@intel.com>

On 16-05-19, 09:11, Dave Jiang wrote:
> 
> 
> On 5/9/19 4:37 PM, Dave Jiang wrote:
> > When ioat_free_chan_resources() gets called, ioat_stop() is called without
> > chan->cleanup_lock. ioat_stop modifies IOAT_RUN bit.  It needs to be
> > protected by cleanup_lock. Also, in the __cleanup() path, if IOAT_RUN is
> > cleared, we should not touch the timer again. We observed that the timer
> > routine was run after timer was deleted.
> > 
> > Fixes: 3372de5813e ("dmaengine: ioatdma: removal of dma_v3.c and relevant ioat3
> > references")
> > 
> > Reported-by: Fan Du <fan.du@intel.com>
> > Tested-by: Fan Du <fan.du@intel.com>
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> Vinod, can you hold off on this please? There may be more changes. Thanks.

Okay dropped

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: axi-dmac: Enable TLAST handling
From: Vinod Koul @ 2019-05-21  5:08 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: dmaengine, Michael Hennerich
In-Reply-To: <20190516094430.16121-1-alexandru.ardelean@analog.com>

On 16-05-19, 12:44, Alexandru Ardelean wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> The TLAST flag is used by the DMAC HDL controller to signal to the
> controller that the following segment (to be submitted) is the last one (in
> a series of segments).
> 
> A receiver DMA (typically another DMAC) can read this parameter (from the
> transfer), and terminate the transfer earlier. A typical use-case for this,
> is when the receiver expects a certain amount of segments, but for some
> reason (e.g. an ADC capture which can have an unknown number of digital
> samples) the number of actual segments is smaller. The receiver would read
> this flag, and then the DMAC would finish.

Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: axi-dmac: Sanity check memory mapped interface support
From: Vinod Koul @ 2019-05-21  5:08 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: dmaengine, Lars-Peter Clausen
In-Reply-To: <20190516083134.29460-1-alexandru.ardelean@analog.com>

On 16-05-19, 11:31, Alexandru Ardelean wrote:
> From: Lars-Peter Clausen <lars@metafoo.de>
> 
> The AXI-DMAC supports different types of interface for the data source and
> destination ports. Typically one of those ports is a memory-mapped
> interface while the other is some kind of streaming interface.
> 
> The information about which kind of interface is used for each port is
> encoded in the devicetree.
> 
> It is also possible in the driver to detect whether a port supports
> memory-mapped transfers or not. For streaming interfaces the address
> register is read-only and will always return 0. So in order to check if a
> port supports memory-mapped transfers write a non-zero value to the
> corresponding address register and check that the value read-back is still
> non zero.
> 
> This allows to detect mismatches between the devicetree description and the
> actual hardware configuration.
> 
> Unfortunately it is not possible to autodetect the interface types since
> there is no method to distinguish between the different streaming ports. So
> the best thing that can be done is to error out when a memory mapped port
> is described in the devicetree but none is detected in the hardware.

Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: axi-dmac: Add support for interleaved cyclic transfers
From: Vinod Koul @ 2019-05-21  5:06 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: dmaengine, Dragos Bogdan
In-Reply-To: <20190516070443.16219-1-alexandru.ardelean@analog.com>

On 16-05-19, 10:04, Alexandru Ardelean wrote:
> From: Dragos Bogdan <dragos.bogdan@analog.com>
> 
> The DMAC HDL core supports interleaved & cyclic transfers.
> An example use-case for this mode is when the controller is used as a
> video DMA.
> 
> This change sets the `cyclic` field to true, so that when the IRQ comes and
> the `axi_dmac_transfer_done()` callback is called (from the interrupt
> handler) the proper `vchan_cyclic_callback()` is called. This way the
> DMAEngine framework will process data correctly for interleaved + cyclic
> transfers.
> 
> This doesn't fix anything. It's an enhancement to the driver.

Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: ti: edma: Enable support for polled (memcpy) completion
From: Vinod Koul @ 2019-05-21  5:04 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, linux-arm-kernel, linux-omap
In-Reply-To: <20190514080909.10306-1-peter.ujfalusi@ti.com>

On 14-05-19, 11:09, Peter Ujfalusi wrote:
> When a DMA client driver decides that it is not providing callback for
> completion of a transfer (and/or does not set the DMA_PREP_INTERRUPT) but
> it will poll the status of the transfer (in case of short memcpy for
> example) we will not get interrupt for the completion of the transfer and
> will not mark the transaction as done.
> 
> Check the event registers (ER and EER) and if the channel is inactive then
> return wioth DMA_COMPLETE to let the client know that the transfer is
        ^^^^^
Typo

> completed.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/ti/edma.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
> index ceabdea40ae0..7501445af069 100644
> --- a/drivers/dma/ti/edma.c
> +++ b/drivers/dma/ti/edma.c
> @@ -1211,8 +1211,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy(
>  
>  	edesc->pset[0].param.opt |= ITCCHEN;
>  	if (nslots == 1) {
> -		/* Enable transfer complete interrupt */
> -		edesc->pset[0].param.opt |= TCINTEN;
> +		/* Enable transfer complete interrupt if requested */
> +		if (tx_flags & DMA_PREP_INTERRUPT)
> +			edesc->pset[0].param.opt |= TCINTEN;
>  	} else {
>  		/* Enable transfer complete chaining for the first slot */
>  		edesc->pset[0].param.opt |= TCCHEN;
> @@ -1239,7 +1240,9 @@ static struct dma_async_tx_descriptor *edma_prep_dma_memcpy(
>  		}
>  
>  		edesc->pset[1].param.opt |= ITCCHEN;
> -		edesc->pset[1].param.opt |= TCINTEN;
> +		/* Enable transfer complete interrupt if requested */
> +		if (tx_flags & DMA_PREP_INTERRUPT)
> +			edesc->pset[1].param.opt |= TCINTEN;
>  	}
>  
>  	return vchan_tx_prep(&echan->vchan, &edesc->vdesc, tx_flags);
> @@ -1801,6 +1804,20 @@ static enum dma_status edma_tx_status(struct dma_chan *chan,
>  	unsigned long flags;
>  
>  	ret = dma_cookie_status(chan, cookie, txstate);
> +
> +	if (ret != DMA_COMPLETE && echan->edesc && !echan->edesc->cyclic) {
> +		struct edma_cc *ecc = echan->ecc;
> +		int channel = EDMA_CHAN_SLOT(echan->ch_num);
> +		int j = (channel >> 5);
> +		unsigned int mask = BIT(channel & 0x1f);

GENMASK or define a macro instead of a magic number?

> +		unsigned int sh_er = edma_shadow0_read_array(ecc, SH_ER, j);
> +		unsigned int sh_eer = edma_shadow0_read_array(ecc, SH_EER, j);
> +
> +		/* The channel is no longer active */
> +		if (!(sh_er & mask) && !(sh_eer & mask))
> +			ret = DMA_COMPLETE;
> +	}
> +
>  	if (ret == DMA_COMPLETE || !txstate)
>  		return ret;
>  
> -- 
> Peter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: mediatek-cqdma: sleeping in atomic context
From: Vinod Koul @ 2019-05-21  4:58 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Shun-Chih Yu, Sean Wang, Dan Williams, Matthias Brugger,
	dmaengine, linux-mediatek, kernel-janitors
In-Reply-To: <20190509100923.GA7024@mwanda>

On 09-05-19, 13:09, Dan Carpenter wrote:
> The mtk_cqdma_poll_engine_done() function takes a true/false parameter
> where true means it's called from atomic context.  There are a couple
> places where it was set to false but it's actually in atomic context
> so it should be true.
> 
> All the callers for mtk_cqdma_hard_reset() are holding a spin_lock and
> in mtk_cqdma_free_chan_resources() we take a spin_lock before calling
> the mtk_cqdma_poll_engine_done() function.

Applied, thanks

> 
> Fixes: b1f01e48df5a ("dmaengine: mediatek: Add MediaTek Command-Queue DMA controller for MT6765 SoC")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> The "atomic" parameter is always true so the temptation was to just
> remove it entirely.

a patch is welcome :)

-- 
~Vinod

^ permalink raw reply

* RE:  Re: [PATCH v3 11/14] dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
From: Robin Gong @ 2019-05-21  4:58 UTC (permalink / raw)
  To: Vinod Koul
  Cc: robh@kernel.org, broonie@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com, mark.rutland@arm.com,
	u.kleine-koenig@pengutronix.de, plyatov@gmail.com,
	dan.j.williams@intel.com, catalin.marinas@arm.com,
	will.deacon@arm.com, l.stach@pengutronix.de, dl-linux-imx,
	linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org, kernel@pengutronix.de

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: 2019年5月21日 12:18
> 
> On 07-05-19, 09:16, Robin Gong wrote:
> > Because the number of ecspi1 rx event on i.mx8mm is 0, the condition
> > check ignore such special case without dma channel enabled, which
> > caused
> > ecspi1 rx works failed. Actually, no need to check event_id0, checking
> > event_id1 is enough for DEV_2_DEV case because it's so lucky that
> > event_id1 never be 0.
> 
> Well is that by chance or design that event_id1 will be never 0?
> 
That's by chance. DEV_2_DEV is just for Audio case and non-zero for event_id1 on current i.MX family.

^ permalink raw reply

* Re: [PATCH] dma: dw-axi-dmac: fix null dereference when pointer first is null
From: Vinod Koul @ 2019-05-21  4:57 UTC (permalink / raw)
  To: Colin King
  Cc: Dan Williams, Huang Shijie, dmaengine, kernel-janitors,
	linux-kernel
In-Reply-To: <20190508223329.26796-1-colin.king@canonical.com>

On 08-05-19, 23:33, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> In the unlikely event that axi_desc_get returns a null desc in the
> very first iteration of the while-loop the error exit path ends
> up calling axi_desc_put on a null pointer 'first' and this causes
> a null pointer dereference.  Fix this by adding a null check on
> pointer 'first' before calling axi_desc_put.

Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v1] dmaengine: tegra-apb: Handle DMA_PREP_INTERRUPT flag properly
From: Vinod Koul @ 2019-05-21  4:55 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Dmitry Osipenko, Laxman Dewangan, Thierry Reding, Ben Dooks,
	dmaengine, linux-tegra, linux-kernel
In-Reply-To: <287d7e67-1572-b4f2-d4bb-b1f02f534d47@nvidia.com>

On 08-05-19, 10:24, Jon Hunter wrote:
> 
> On 05/05/2019 19:12, Dmitry Osipenko wrote:
> > The DMA_PREP_INTERRUPT flag means that descriptor's callback should be
> > invoked upon transfer completion and that's it. For some reason driver
> > completely disables the hardware interrupt handling, leaving channel in
> > unusable state if transfer is issued with the flag being unset. Note
> > that there are no occurrences in the relevant drivers that do not set
> > the flag, hence this patch doesn't fix any actual bug and merely fixes
> > potential problem.
> > 
> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> 
> >From having a look at this, I am guessing that we have never really
> tested the case where DMA_PREP_INTERRUPT flag is not set because as you
> mentioned it does not look like this will work at all!

That is a fair argument
> 
> Is there are use-case you are looking at where you don't set the
> DMA_PREP_INTERRUPT flag?
> 
> If not I am wondering if we should even bother supporting this and warn
> if it is not set. AFAICT it does not appear to be mandatory, but maybe
> Vinod can comment more on this.

This is supposed to be used in the cases where you submit a bunch of
descriptors and selectively dont want an interrupt in few cases...

Is this such a case?

Thanks
~Vinod

^ permalink raw reply

* Re: [PATCH] dmaengine: stm32-dma: Fix redundant call to platform_get_irq
From: Vinod Koul @ 2019-05-21  4:39 UTC (permalink / raw)
  To: Amelie Delaunay
  Cc: Dan Williams, Maxime Coquelin, Alexandre Torgue, dmaengine,
	linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <1557215681-18541-1-git-send-email-amelie.delaunay@st.com>

On 07-05-19, 09:54, Amelie Delaunay wrote:
> Commit c6504be53972 ("dmaengine: stm32-dma: Fix unsigned variable compared
> with zero") duplicated the call to platform_get_irq.
> So remove the first call to platform_get_irq.

Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH 3/4] dmaengine: fsl-edma: support little endian for edma driver
From: Vinod Koul @ 2019-05-21  4:38 UTC (permalink / raw)
  To: Peng Ma
  Cc: robh+dt, shawnguo, mark.rutland, leoyang.li, dan.j.williams,
	dmaengine, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20190506090344.37784-3-peng.ma@nxp.com>

On 06-05-19, 09:03, Peng Ma wrote:
> improve edma driver to support little endian.

Can you explain a bit more how adding the below lines adds little endian
support...

> 
> Signed-off-by: Peng Ma <peng.ma@nxp.com>
> ---
>  drivers/dma/fsl-edma-common.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index 680b2a0..6bf238e 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -83,9 +83,14 @@ void fsl_edma_chan_mux(struct fsl_edma_chan *fsl_chan,
>  	u32 ch = fsl_chan->vchan.chan.chan_id;
>  	void __iomem *muxaddr;
>  	unsigned int chans_per_mux, ch_off;
> +	int endian_diff[4] = {3, 1, -1, -3};
>  
>  	chans_per_mux = fsl_chan->edma->n_chans / DMAMUX_NR;
>  	ch_off = fsl_chan->vchan.chan.chan_id % chans_per_mux;
> +
> +	if (!fsl_chan->edma->big_endian)
> +		ch_off += endian_diff[ch_off % 4];
> +
>  	muxaddr = fsl_chan->edma->muxbase[ch / chans_per_mux];
>  	slot = EDMAMUX_CHCFG_SOURCE(slot);
>  
> -- 
> 1.7.1

-- 
~Vinod

^ permalink raw reply

* Re: [V2 2/2] dmaengine: fsl-qdma: Add improvement
From: Vinod Koul @ 2019-05-21  4:35 UTC (permalink / raw)
  To: Peng Ma; +Cc: dan.j.williams, leoyang.li, dmaengine, linux-kernel
In-Reply-To: <20190506022111.31751-2-peng.ma@nxp.com>

On 06-05-19, 10:21, Peng Ma wrote:
> When an error occurs we should clean the error register then to return

Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [V2 1/2] dmaengine: fsl-qdma: fixed the source/destination descriptor format
From: Vinod Koul @ 2019-05-21  4:34 UTC (permalink / raw)
  To: Peng Ma; +Cc: dan.j.williams, leoyang.li, dmaengine, linux-kernel
In-Reply-To: <20190506022111.31751-1-peng.ma@nxp.com>

On 06-05-19, 10:21, Peng Ma wrote:
> CMD of Source/Destination descriptor format should be lower of
> struct fsl_qdma_engine number data address.
> 
> Signed-off-by: Peng Ma <peng.ma@nxp.com>
> ---
> changed for V2:
> 	- Fix descriptor spelling
> 
>  drivers/dma/fsl-qdma.c |   25 +++++++++++++++++--------
>  1 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
> index aa1d0ae..2e8b46b 100644
> --- a/drivers/dma/fsl-qdma.c
> +++ b/drivers/dma/fsl-qdma.c
> @@ -113,6 +113,7 @@
>  /* Field definition for Descriptor offset */
>  #define QDMA_CCDF_STATUS		20
>  #define QDMA_CCDF_OFFSET		20
> +#define QDMA_SDDF_CMD(x)		(((u64)(x)) << 32)
>  
>  /* Field definition for safe loop count*/
>  #define FSL_QDMA_HALT_COUNT		1500
> @@ -214,6 +215,12 @@ struct fsl_qdma_engine {
>  
>  };
>  
> +static inline void
> +qdma_sddf_set_cmd(struct fsl_qdma_format *sddf, u32 val)
> +{
> +	sddf->data = QDMA_SDDF_CMD(val);
> +}

Do you really need this helper which calls another macro!

> +
>  static inline u64
>  qdma_ccdf_addr_get64(const struct fsl_qdma_format *ccdf)
>  {
> @@ -341,6 +348,7 @@ static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
>  static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>  				      dma_addr_t dst, dma_addr_t src, u32 len)
>  {
> +	u32 cmd;
>  	struct fsl_qdma_format *sdf, *ddf;
>  	struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
>  
> @@ -353,6 +361,7 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>  
>  	memset(fsl_comp->virt_addr, 0, FSL_QDMA_COMMAND_BUFFER_SIZE);
>  	memset(fsl_comp->desc_virt_addr, 0, FSL_QDMA_DESCRIPTOR_BUFFER_SIZE);
> +

why did you add a blank line in this 'fix', it does not belong here!

>  	/* Head Command Descriptor(Frame Descriptor) */
>  	qdma_desc_addr_set64(ccdf, fsl_comp->bus_addr + 16);
>  	qdma_ccdf_set_format(ccdf, qdma_ccdf_get_offset(ccdf));
> @@ -369,14 +378,14 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
>  	/* This entry is the last entry. */
>  	qdma_csgf_set_f(csgf_dest, len);
>  	/* Descriptor Buffer */
> -	sdf->data =
> -		cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
> -			    FSL_QDMA_CMD_RWTTYPE_OFFSET);
> -	ddf->data =
> -		cpu_to_le64(FSL_QDMA_CMD_RWTTYPE <<
> -			    FSL_QDMA_CMD_RWTTYPE_OFFSET);
> -	ddf->data |=
> -		cpu_to_le64(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
> +	cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> +			  FSL_QDMA_CMD_RWTTYPE_OFFSET);
> +	qdma_sddf_set_cmd(sdf, cmd);

why not do sddf->data = QDMA_SDDF_CMD(cmd);

> +
> +	cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> +			  FSL_QDMA_CMD_RWTTYPE_OFFSET);
> +	cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
> +	qdma_sddf_set_cmd(ddf, cmd);
>  }
>  
>  /*
> -- 
> 1.7.1

-- 
~Vinod

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox