[PATCH] dmaengine: dw-edma: Fix multiple times setting of the CYCLE_STATE and CYCLE_BIT bits for HDMA.

LUO Haowen posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/dma/dw-edma/dw-hdma-v0-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] dmaengine: dw-edma: Fix multiple times setting of the CYCLE_STATE and CYCLE_BIT bits for HDMA.
Posted by LUO Haowen 1 month, 1 week ago
Others have submitted this issue (https://lore.kernel.org/dmaengine/
20240722030405.3385-1-zhengdongxiong@gxmicro.cn/), 
but it has not been fixed yet. Therefore, more supplementary information 
is provided here.

As mentioned in the "PCS-CCS-CB-TCB" Producer-Consumer Synchronization of
"DesignWare Cores PCI Express Controller Databook, version 6.00a":

1. The Consumer CYCLE_STATE (CCS) bit in the register only needs to be 
initialized once; the value will update automatically to be 
~CYCLE_BIT (CB) in the next chunk.
2. The Consumer CYCLE_BIT bit in the register is loaded from the LL 
element and tested against CCS. When CB = CCS, the data transfer is 
executed. Otherwise not.

The current logic sets customer (HDMA) CS and CB bits to 1 in each chunk
while setting the producer (software) CB of odd chunks to 0 and even 
chunks to 1 in the linked list. This is leading to a mismatch between 
the producer CB and consumer CS bits.

This issue can be reproduced by setting the transmission data size to
exceed one chunk. By the way, in the EDMA using the same "PCS-CCS-CB-TCB"
mechanism, the CS bit is only initialized once and this issue was not 
found. Refer to 
drivers/dma/dw-edma/dw-edma-v0-core.c:dw_edma_v0_core_start.

So fix this issue by initializing the CYCLE_STATE and CYCLE_BIT bits 
only once.

Signed-off-by: LUO Haowen <luo-hw@foxmail.com>
---
 drivers/dma/dw-edma/dw-hdma-v0-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index e3f8db4fe..ce8f7254b 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -252,10 +252,10 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 			  lower_32_bits(chunk->ll_region.paddr));
 		SET_CH_32(dw, chan->dir, chan->id, llp.msb,
 			  upper_32_bits(chunk->ll_region.paddr));
+		/* Set consumer cycle */
+		SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
+			HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
 	}
-	/* Set consumer cycle */
-	SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
-		  HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
 
 	dw_hdma_v0_sync_ll_data(chunk);
 
-- 
2.34.1
Re: [PATCH] dmaengine: dw-edma: Fix multiple times setting of the CYCLE_STATE and CYCLE_BIT bits for HDMA.
Posted by Frank Li 1 month, 1 week ago
On Wed, Mar 04, 2026 at 10:50:49AM +0800, LUO Haowen wrote:
> Others have submitted this issue (https://lore.kernel.org/dmaengine/
> 20240722030405.3385-1-zhengdongxiong@gxmicro.cn/),
> but it has not been fixed yet. Therefore, more supplementary information
> is provided here.
>
> As mentioned in the "PCS-CCS-CB-TCB" Producer-Consumer Synchronization of
> "DesignWare Cores PCI Express Controller Databook, version 6.00a":
>
> 1. The Consumer CYCLE_STATE (CCS) bit in the register only needs to be
> initialized once; the value will update automatically to be
> ~CYCLE_BIT (CB) in the next chunk.
> 2. The Consumer CYCLE_BIT bit in the register is loaded from the LL
> element and tested against CCS. When CB = CCS, the data transfer is
> executed. Otherwise not.
>
> The current logic sets customer (HDMA) CS and CB bits to 1 in each chunk
> while setting the producer (software) CB of odd chunks to 0 and even
> chunks to 1 in the linked list. This is leading to a mismatch between
> the producer CB and consumer CS bits.
>
> This issue can be reproduced by setting the transmission data size to
> exceed one chunk. By the way, in the EDMA using the same "PCS-CCS-CB-TCB"
> mechanism, the CS bit is only initialized once and this issue was not
> found. Refer to
> drivers/dma/dw-edma/dw-edma-v0-core.c:dw_edma_v0_core_start.
>
> So fix this issue by initializing the CYCLE_STATE and CYCLE_BIT bits
> only once.

Need fixes tags here!

Frank
>
> Signed-off-by: LUO Haowen <luo-hw@foxmail.com>
> ---
>  drivers/dma/dw-edma/dw-hdma-v0-core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index e3f8db4fe..ce8f7254b 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -252,10 +252,10 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  			  lower_32_bits(chunk->ll_region.paddr));
>  		SET_CH_32(dw, chan->dir, chan->id, llp.msb,
>  			  upper_32_bits(chunk->ll_region.paddr));
> +		/* Set consumer cycle */
> +		SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
> +			HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
>  	}
> -	/* Set consumer cycle */
> -	SET_CH_32(dw, chan->dir, chan->id, cycle_sync,
> -		  HDMA_V0_CONSUMER_CYCLE_STAT | HDMA_V0_CONSUMER_CYCLE_BIT);
>
>  	dw_hdma_v0_sync_ll_data(chunk);
>
> --
> 2.34.1
>