[PATCH] firmware: stratix10-svc: support up to 4 buffer claims and extend timeout

[PATCH] firmware: stratix10-svc: support up to 4 buffer claims and extend timeout

[adrianhoyin.ng]

From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

The service layer previously only returned up to three buffer addresses
per transaction. Extend the logic in svc_thread_cmd_data_claim() to
collect up to four buffer claims. A new field `kaddr4` is added to
struct stratix10_svc_cb_data, and the FPGA manager callback unlocks this
fourth buffer accordingly.

Timeout values for reconfiguration and buffer transactions are also
increased (from ~300–720ms to 5000ms), since real-world processing takes
significantly longer (~600ms or more). The reconfiguration complete
timeout is also replaced with S10_RECONFIG_TIMEOUT (>1s) to avoid
premature aborts.

Additional changes:
- Callbacks are updated to pass all claimed buffers to clients.
- Debug logging is added to trace received status values.
- Simplified buffer wait logic in s10_ops_write() by always using
  wait_for_completion_timeout().

These changes improve robustness of FPGA configuration on SoCFPGA
platforms by handling larger transactions and accommodating realistic
timing.

Signed-off-by: Ang Tien Sung <tiensung.ang@altera.com>
Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
---
 drivers/firmware/stratix10-svc.c              | 36 +++++++++++++------
 drivers/fpga/stratix10-soc.c                  | 18 +++++-----
 .../firmware/intel/stratix10-svc-client.h     |  6 ++--
 3 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index dbed404a71fc..58cf68d9a384 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -342,6 +342,8 @@ static void svc_thread_cmd_data_claim(struct stratix10_svc_controller *ctrl,
 {
 	struct arm_smccc_res res;
 	unsigned long timeout;
+	void *buf_claim_addr[4] = {NULL};
+	int buf_claim_count = 0;
 
 	reinit_completion(&ctrl->complete_status);
 	timeout = msecs_to_jiffies(FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS);
@@ -353,20 +355,32 @@ static void svc_thread_cmd_data_claim(struct stratix10_svc_controller *ctrl,
 
 		if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
 			if (!res.a1) {
+				/* Transaction of 4 blocks are now done */
 				complete(&ctrl->complete_status);
+				cb_data->status = BIT(SVC_STATUS_BUFFER_DONE);
+				cb_data->kaddr1 = buf_claim_addr[0];
+				cb_data->kaddr2 = buf_claim_addr[1];
+				cb_data->kaddr3 = buf_claim_addr[2];
+				cb_data->kaddr4 = buf_claim_addr[3];
+				p_data->chan->scl->receive_cb(p_data->chan->scl,
+				cb_data);
 				break;
 			}
-			cb_data->status = BIT(SVC_STATUS_BUFFER_DONE);
-			cb_data->kaddr1 = svc_pa_to_va(res.a1);
-			cb_data->kaddr2 = (res.a2) ?
-					  svc_pa_to_va(res.a2) : NULL;
-			cb_data->kaddr3 = (res.a3) ?
-					  svc_pa_to_va(res.a3) : NULL;
-			p_data->chan->scl->receive_cb(p_data->chan->scl,
-						      cb_data);
-		} else {
-			pr_debug("%s: secure world busy, polling again\n",
-				 __func__);
+
+			if (buf_claim_count < 4) {
+				buf_claim_addr[buf_claim_count] = svc_pa_to_va(res.a1);
+				buf_claim_count++;
+			}
+
+			if (res.a2 && buf_claim_count < 4) {
+				buf_claim_addr[buf_claim_count] = svc_pa_to_va(res.a2);
+				buf_claim_count++;
+			}
+			if (res.a3 && buf_claim_count < 4) {
+				buf_claim_addr[buf_claim_count] = svc_pa_to_va(res.a3);
+				buf_claim_count++;
+			}
+
 		}
 	} while (res.a0 == INTEL_SIP_SMC_STATUS_OK ||
 		 res.a0 == INTEL_SIP_SMC_STATUS_BUSY ||
diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 0a295ccf1644..4fea9458f92b 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -147,6 +147,7 @@ static void s10_receive_callback(struct stratix10_svc_client *client,
 	u32 status;
 	int i;
 
+	pr_debug("%s data %x\n", __func__, data->status);
 	WARN_ONCE(!data, "%s: stratix10_svc_rc_data = NULL", __func__);
 
 	status = data->status;
@@ -163,6 +164,7 @@ static void s10_receive_callback(struct stratix10_svc_client *client,
 		s10_unlock_bufs(priv, data->kaddr1);
 		s10_unlock_bufs(priv, data->kaddr2);
 		s10_unlock_bufs(priv, data->kaddr3);
+		s10_unlock_bufs(priv, data->kaddr4);
 	}
 
 	complete(&priv->status_return_completion);
@@ -309,15 +311,8 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 				break;
 		}
 
-		/*
-		 * If callback hasn't already happened, wait for buffers to be
-		 * returned from service layer
-		 */
-		wait_status = 1; /* not timed out */
-		if (!priv->status)
-			wait_status = wait_for_completion_timeout(
-				&priv->status_return_completion,
-				S10_BUFFER_TIMEOUT);
+		wait_status = wait_for_completion_timeout(&priv->status_return_completion,
+							  S10_BUFFER_TIMEOUT);
 
 		if (test_and_clear_bit(SVC_STATUS_BUFFER_DONE, &priv->status) ||
 		    test_and_clear_bit(SVC_STATUS_BUFFER_SUBMITTED,
@@ -353,7 +348,10 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 	unsigned long timeout;
 	int ret;
 
-	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
+	/* The time taken to process this is close to 600ms
+	 * This MUST be increased over 1 second
+	 */
+	timeout = S10_RECONFIG_TIMEOUT;
 
 	do {
 		reinit_completion(&priv->status_return_completion);
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
index d290060f4c73..abe04f4fad1d 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -68,8 +68,8 @@
  * timeout value used in Stratix10 FPGA manager driver.
  * timeout value used in RSU driver
  */
-#define SVC_RECONFIG_REQUEST_TIMEOUT_MS         300
-#define SVC_RECONFIG_BUFFER_TIMEOUT_MS          720
+#define SVC_RECONFIG_REQUEST_TIMEOUT_MS         5000
+#define SVC_RECONFIG_BUFFER_TIMEOUT_MS          5000
 #define SVC_RSU_REQUEST_TIMEOUT_MS              300
 #define SVC_FCS_REQUEST_TIMEOUT_MS		2000
 #define SVC_COMPLETED_TIMEOUT_MS		30000
@@ -222,12 +222,14 @@ struct stratix10_svc_command_config_type {
  * @kaddr1: address of 1st completed data block
  * @kaddr2: address of 2nd completed data block
  * @kaddr3: address of 3rd completed data block
+ * @kaddr4: address of 4th completed data block
  */
 struct stratix10_svc_cb_data {
 	u32 status;
 	void *kaddr1;
 	void *kaddr2;
 	void *kaddr3;
+	void *kaddr4;
 };
 
 /**
-- 
2.49.GIT

Re: [PATCH] firmware: stratix10-svc: support up to 4 buffer claims and extend timeout

[Dinh Nguyen]

On 2/10/26 01:16, adrianhoyin.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> 
> The service layer previously only returned up to three buffer addresses
> per transaction. Extend the logic in svc_thread_cmd_data_claim() to
> collect up to four buffer claims. A new field `kaddr4` is added to
> struct stratix10_svc_cb_data, and the FPGA manager callback unlocks this
> fourth buffer accordingly.
> 
> Timeout values for reconfiguration and buffer transactions are also
> increased (from ~300–720ms to 5000ms), since real-world processing takes
> significantly longer (~600ms or more). The reconfiguration complete
> timeout is also replaced with S10_RECONFIG_TIMEOUT (>1s) to avoid
> premature aborts.
> 
> Additional changes:
> - Callbacks are updated to pass all claimed buffers to clients.
> - Debug logging is added to trace received status values.
> - Simplified buffer wait logic in s10_ops_write() by always using
>    wait_for_completion_timeout().
> 
> These changes improve robustness of FPGA configuration on SoCFPGA
> platforms by handling larger transactions and accommodating realistic
> timing.
> 
> Signed-off-by: Ang Tien Sung <tiensung.ang@altera.com>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> ---
>   drivers/firmware/stratix10-svc.c              | 36 +++++++++++++------
>   drivers/fpga/stratix10-soc.c                  | 18 +++++-----
>   .../firmware/intel/stratix10-svc-client.h     |  6 ++--
>   3 files changed, 37 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index dbed404a71fc..58cf68d9a384 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -342,6 +342,8 @@ static void svc_thread_cmd_data_claim(struct stratix10_svc_controller *ctrl,
>   {
>   	struct arm_smccc_res res;
>   	unsigned long timeout;
> +	void *buf_claim_addr[4] = {NULL};
> +	int buf_claim_count = 0;
>   
>   	reinit_completion(&ctrl->complete_status);
>   	timeout = msecs_to_jiffies(FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS);
> @@ -353,20 +355,32 @@ static void svc_thread_cmd_data_claim(struct stratix10_svc_controller *ctrl,
>   
>   		if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
>   			if (!res.a1) {
> +				/* Transaction of 4 blocks are now done */

This comment is a bit misleading. How do you know you finish exactly 4 
blocks?

>   				complete(&ctrl->complete_status);
> +				cb_data->status = BIT(SVC_STATUS_BUFFER_DONE);
> +				cb_data->kaddr1 = buf_claim_addr[0];
> +				cb_data->kaddr2 = buf_claim_addr[1];
> +				cb_data->kaddr3 = buf_claim_addr[2];
> +				cb_data->kaddr4 = buf_claim_addr[3];
> +				p_data->chan->scl->receive_cb(p_data->chan->scl,
> +				cb_data);
>   				break;
>   			}
> -			cb_data->status = BIT(SVC_STATUS_BUFFER_DONE);
> -			cb_data->kaddr1 = svc_pa_to_va(res.a1);
> -			cb_data->kaddr2 = (res.a2) ?
> -					  svc_pa_to_va(res.a2) : NULL;
> -			cb_data->kaddr3 = (res.a3) ?
> -					  svc_pa_to_va(res.a3) : NULL;
> -			p_data->chan->scl->receive_cb(p_data->chan->scl,
> -						      cb_data);

The callback used to be only dependent on INTEL_SIP_SMC_STATUS_OK, but 
this change put the dependency on res.a1 == 0 as well. Please add 
comment or add to the commit message the reason for this change.

> -		} else {
> -			pr_debug("%s: secure world busy, polling again\n",
> -				 __func__);
> +
> +			if (buf_claim_count < 4) {
> +				buf_claim_addr[buf_claim_count] = svc_pa_to_va(res.a1);
> +				buf_claim_count++;
> +			}
> +
> +			if (res.a2 && buf_claim_count < 4) {
> +				buf_claim_addr[buf_claim_count] = svc_pa_to_va(res.a2);
> +				buf_claim_count++;
> +			}
> +			if (res.a3 && buf_claim_count < 4) {
> +				buf_claim_addr[buf_claim_count] = svc_pa_to_va(res.a3);
> +				buf_claim_count++;
> +			}

Could there be a case where buf_claim_count >= 4, if so you should add a 
WARN_ON.

> +
>   		}
>   	} while (res.a0 == INTEL_SIP_SMC_STATUS_OK ||
>   		 res.a0 == INTEL_SIP_SMC_STATUS_BUSY ||
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 0a295ccf1644..4fea9458f92b 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -147,6 +147,7 @@ static void s10_receive_callback(struct stratix10_svc_client *client,
>   	u32 status;
>   	int i;
>   
> +	pr_debug("%s data %x\n", __func__, data->status);

Use %#x would be more informative for a status bitmask.

>   	WARN_ONCE(!data, "%s: stratix10_svc_rc_data = NULL", __func__);
>   
>   	status = data->status;
> @@ -163,6 +164,7 @@ static void s10_receive_callback(struct stratix10_svc_client *client,
>   		s10_unlock_bufs(priv, data->kaddr1);
>   		s10_unlock_bufs(priv, data->kaddr2);
>   		s10_unlock_bufs(priv, data->kaddr3);
> +		s10_unlock_bufs(priv, data->kaddr4);
>   	}
>   
>   	complete(&priv->status_return_completion);
> @@ -309,15 +311,8 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>   				break;
>   		}
>   
> -		/*
> -		 * If callback hasn't already happened, wait for buffers to be
> -		 * returned from service layer
> -		 */
> -		wait_status = 1; /* not timed out */
> -		if (!priv->status)
> -			wait_status = wait_for_completion_timeout(
> -				&priv->status_return_completion,
> -				S10_BUFFER_TIMEOUT);
> +		wait_status = wait_for_completion_timeout(&priv->status_return_completion,
> +							  S10_BUFFER_TIMEOUT);

Can you add a comment why you remove the check for !priv->status to call 
wait_for_completion_timeout() ?

>   
>   		if (test_and_clear_bit(SVC_STATUS_BUFFER_DONE, &priv->status) ||
>   		    test_and_clear_bit(SVC_STATUS_BUFFER_SUBMITTED,
> @@ -353,7 +348,10 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>   	unsigned long timeout;
>   	int ret;
>   
> -	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
> +	/* The time taken to process this is close to 600ms

nit: comment style is wrong

> +	 * This MUST be increased over 1 second
> +	 */
> +	timeout = S10_RECONFIG_TIMEOUT;
>   
>   	do {
>   		reinit_completion(&priv->status_return_completion);
> diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
> index d290060f4c73..abe04f4fad1d 100644
> --- a/include/linux/firmware/intel/stratix10-svc-client.h
> +++ b/include/linux/firmware/intel/stratix10-svc-client.h
> @@ -68,8 +68,8 @@
>    * timeout value used in Stratix10 FPGA manager driver.
>    * timeout value used in RSU driver
>    */
> -#define SVC_RECONFIG_REQUEST_TIMEOUT_MS         300
> -#define SVC_RECONFIG_BUFFER_TIMEOUT_MS          720
> +#define SVC_RECONFIG_REQUEST_TIMEOUT_MS         5000
> +#define SVC_RECONFIG_BUFFER_TIMEOUT_MS          5000

These are huge jumps in timeout values. You may be masking real firmware 
hangs. Please document why such a large change.

Dinh

Re: [PATCH] firmware: stratix10-svc: support up to 4 buffer claims and extend timeout

[Xu Yilun]

On Tue, Feb 10, 2026 at 03:16:42PM +0800, adrianhoyin.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> 
> The service layer previously only returned up to three buffer addresses
> per transaction. Extend the logic in svc_thread_cmd_data_claim() to
> collect up to four buffer claims. A new field `kaddr4` is added to

Why previously use 3 buffers per transaction, and why now use 4, not
5, 6...  I.e., I saw some association from struct
arm_smccc_res::a1/a2/a3 so we used 3 buffers previouly (correct me if
wrong). But the 4th buffer seems pure SVC decision so please elaborate.

> struct stratix10_svc_cb_data, and the FPGA manager callback unlocks this
> fourth buffer accordingly.
> 
> Timeout values for reconfiguration and buffer transactions are also
> increased (from ~300–720ms to 5000ms), since real-world processing takes
> significantly longer (~600ms or more). The reconfiguration complete
> timeout is also replaced with S10_RECONFIG_TIMEOUT (>1s) to avoid
> premature aborts.
> 
> Additional changes:
> - Callbacks are updated to pass all claimed buffers to clients.
> - Debug logging is added to trace received status values.
> - Simplified buffer wait logic in s10_ops_write() by always using
>   wait_for_completion_timeout().

The patch includes too much independent changes which makes reviewers
hard to distinguish which line is for which purpose, please split your
patch into a series. This also gives chances for you to explain why each
change is necessary or nice to have.

> 
> These changes improve robustness of FPGA configuration on SoCFPGA
> platforms by handling larger transactions and accommodating realistic
> timing.
> 
...

> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 0a295ccf1644..4fea9458f92b 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -147,6 +147,7 @@ static void s10_receive_callback(struct stratix10_svc_client *client,
>  	u32 status;
>  	int i;
>  
> +	pr_debug("%s data %x\n", __func__, data->status);

I'd prefer you delete it when you think the patches are ready to get merged.

...

> @@ -353,7 +348,10 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>  	unsigned long timeout;
>  	int ret;
>  
> -	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
> +	/* The time taken to process this is close to 600ms
> +	 * This MUST be increased over 1 second
> +	 */
> +	timeout = S10_RECONFIG_TIMEOUT;

This was a configurable option and you now hard code it, I don't see the
rationale.