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(-)
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
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.
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
© 2016 - 2026 Red Hat, Inc.