[PATCH net] net: airoha: Add retry mechanism to airoha_qdma_set_trtcm_param()

[PATCH net] net: airoha: Add retry mechanism to airoha_qdma_set_trtcm_param()

[Lorenzo Bianconi]

From: Brown Huang <brown.huang@airoha.com>

CPU accesses QDMA via the bus. When multiple modules are using the bus
simultaneously, CPU access to QDMA may encounter bus timeouts and fails,
resulting in QDMA configuration failures and potentially causing packet
transmission issues. In order to mitigate the issue, introduce a retry
mechanism to airoha_qdma_set_trtcm_param routine in order to ensure the
configuration is correctly applied to the hardware.

Fixes: ef1ca9271313b ("net: airoha: Add sched HTB offload support")
Signed-off-by: Brown Huang <brown.huang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index eab6a98d62b9..aaf7d8c8ccd3 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -2493,14 +2493,23 @@ static int airoha_qdma_set_trtcm_param(struct airoha_qdma *qdma, int channel,
 		     FIELD_PREP(TRTCM_METER_GROUP_MASK, group) |
 		     FIELD_PREP(TRTCM_PARAM_INDEX_MASK, idx) |
 		     FIELD_PREP(TRTCM_PARAM_RATE_TYPE_MASK, mode);
+	int i;
 
-	airoha_qdma_wr(qdma, REG_TRTCM_DATA_LOW(addr), val);
-	airoha_qdma_wr(qdma, REG_TRTCM_CFG_PARAM(addr), config);
+	for (i = 0; i < 100; i++) {
+		u32 data;
 
-	return read_poll_timeout(airoha_qdma_rr, val,
-				 val & TRTCM_PARAM_RW_DONE_MASK,
-				 USEC_PER_MSEC, 10 * USEC_PER_MSEC, true,
-				 qdma, REG_TRTCM_CFG_PARAM(addr));
+		airoha_qdma_wr(qdma, REG_TRTCM_DATA_LOW(addr), val);
+		wmb();
+		airoha_qdma_wr(qdma, REG_TRTCM_CFG_PARAM(addr), config);
+		if (airoha_qdma_get_trtcm_param(qdma, channel, addr, param,
+						mode, &data, NULL))
+			continue;
+
+		if (data == val)
+			break;
+	}
+
+	return i == 100 ? -EBUSY : 0;
 }
 
 static int airoha_qdma_set_trtcm_config(struct airoha_qdma *qdma, int channel,

---
base-commit: 4aacf509e537a711fa71bca9f234e5eb6968850e
change-id: 20260605-airoha_qdma_set_trtcm_param-retry-fix-a9d2956b9b2f

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

Re: [PATCH net] net: airoha: Add retry mechanism to airoha_qdma_set_trtcm_param()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

> From: Brown Huang <brown.huang@airoha.com>
> 
> CPU accesses QDMA via the bus. When multiple modules are using the bus
> simultaneously, CPU access to QDMA may encounter bus timeouts and fails,
> resulting in QDMA configuration failures and potentially causing packet
> transmission issues. In order to mitigate the issue, introduce a retry
> mechanism to airoha_qdma_set_trtcm_param routine in order to ensure the
> configuration is correctly applied to the hardware.
> 
> Fixes: ef1ca9271313b ("net: airoha: Add sched HTB offload support")
> Signed-off-by: Brown Huang <brown.huang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index eab6a98d62b9..aaf7d8c8ccd3 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2493,14 +2493,23 @@ static int airoha_qdma_set_trtcm_param(struct airoha_qdma *qdma, int channel,
>  		     FIELD_PREP(TRTCM_METER_GROUP_MASK, group) |
>  		     FIELD_PREP(TRTCM_PARAM_INDEX_MASK, idx) |
>  		     FIELD_PREP(TRTCM_PARAM_RATE_TYPE_MASK, mode);
> +	int i;
>  
> -	airoha_qdma_wr(qdma, REG_TRTCM_DATA_LOW(addr), val);
> -	airoha_qdma_wr(qdma, REG_TRTCM_CFG_PARAM(addr), config);
> +	for (i = 0; i < 100; i++) {
> +		u32 data;
>  
> -	return read_poll_timeout(airoha_qdma_rr, val,
> -				 val & TRTCM_PARAM_RW_DONE_MASK,
> -				 USEC_PER_MSEC, 10 * USEC_PER_MSEC, true,
> -				 qdma, REG_TRTCM_CFG_PARAM(addr));
> +		airoha_qdma_wr(qdma, REG_TRTCM_DATA_LOW(addr), val);
> +		wmb();
> +		airoha_qdma_wr(qdma, REG_TRTCM_CFG_PARAM(addr), config);
> +		if (airoha_qdma_get_trtcm_param(qdma, channel, addr, param,
> +						mode, &data, NULL))
> +			continue;
> +
> +		if (data == val)
> +			break;
> +	}
> +
> +	return i == 100 ? -EBUSY : 0;
>  }
>  
>  static int airoha_qdma_set_trtcm_config(struct airoha_qdma *qdma, int channel,
> 
> ---
> base-commit: 4aacf509e537a711fa71bca9f234e5eb6968850e
> change-id: 20260605-airoha_qdma_set_trtcm_param-retry-fix-a9d2956b9b2f
> 
> Best regards,
> -- 
> Lorenzo Bianconi <lorenzo@kernel.org>

I think the issue reported by sashiko in [0] is valid and it needs to be
addressed in v2, but since I am not the original author of the patch, I will
let Brown or Leto comment on it.

Regards,
Lorenzo

[0] https://sashiko.dev/#/patchset/20260608-airoha_qdma_set_trtcm_param-retry-fix-v1-1-f07704f0d8c5%40kernel.org

> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net] net: airoha: Add retry mechanism to airoha_qdma_set_trtcm_param()

[Leto Liu (刘涛)]

>I think the issue reported by sashiko in [0] is valid and it needs to be addressed in v2, but since I am not the original author of the patch, I will let Brown or Leto comment on it.
>
>Regards,
>Lorenzo
>
Thanks for the careful review — you are absolutely right.
In the current version, the immediate call to airoha_qdma_get_trtcm_param() can indeed clobber the write command we just issued. That helper programs REG_TRTCM_CFG_PARAM() to start a read transaction, so without first waiting for TRTCM_PARAM_RW_DONE_MASK after the write, we may overwrite the command register before the hardware has latched/finished the write. The added wmb() only enforces the ordering between the DATA and CFG writes, but it does not guarantee the write transaction has completed.
I will address this by restoring the DONE polling after the write, and only then performing the read-back verification (i.e. “write → poll DONE → read-back → compare”, with retries as needed). Also, to avoid clobbering the val argument, the poll will use a separate temporary variable.
I have revised the patch into a v2 version based on this approach, and it is currently under testing and validation. Since this is an intermittent issue, I am running long-duration stability tests. I expect to provide the v2 patch by next Monday.

Best Regards, Leto