[PATCH 0/3] fpga: stratix10-soc: harden wait, ERROR order, and timeout

[PATCH 0/3] fpga: stratix10-soc: harden wait, ERROR order, and timeout

[Adrian Ng Ho Yin]

Stratix10 FPGA reconfiguration can fail or hang for three related
reasons in the FPGA manager. This series hardens s10_ops_write() and
s10_ops_write_complete() so those paths cannot skip a completion wait,
treat BUFFER_DONE | ERROR as success, or return immediately when the
DT-supplied config_complete_timeout_us is left at 0. No dependency on
the stratix10-svc accumulate changes.

  1) always wait for svc buffer completion

     s10_ops_write() skips wait_for_completion_timeout() when
     priv->status is already set. That skip is unnecessary and unsafe:
     the callback may fire between the CLAIM/SUBMIT send and the wait
     check. Always call the wait; if complete() already ran, the done
     counter is raised and the wait returns immediately.

  2) prefer ERROR over BUFFER_DONE

     BUFFER_DONE is checked before ERROR, so a combined
     BUFFER_DONE | ERROR status is treated as success. Check ERROR
     first so that status aborts the write. Also clear priv->status at
     the start of write_init so stale bits from an aborted transfer
     cannot leak into the next reconfiguration.

  3) floor config_complete_timeout_us

     of-fpga-region leaves config_complete_timeout_us at 0 when the DT
     property is absent. usecs_to_jiffies(0) makes write_complete return
     immediately, while reconfiguration takes ~600ms in practice.
     Enforce S10_RECONFIG_TIMEOUT as a minimum floor while still
     honouring any larger caller-supplied value. Note this prevents a
     DT property from lowering the timeout below that floor.

Adrian Ng Ho Yin (3):
  fpga: stratix10-soc: always wait for svc buffer completion
  fpga: stratix10-soc: prefer ERROR over BUFFER_DONE
  fpga: stratix10-soc: floor config_complete_timeout_us

 drivers/fpga/stratix10-soc.c | 41 ++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 14 deletions(-)

-- 
2.49.GIT

[PATCH 1/3] fpga: stratix10-soc: always wait for svc buffer completion

[Adrian Ng Ho Yin]

Always call wait_for_completion_timeout() in s10_ops_write(). Skipping
the wait when priv->status is already set is unnecessary: if the
callback already ran, complete() raised the done counter and the wait
returns immediately.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 drivers/fpga/stratix10-soc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index b8ec2e6f615f..3ba0ffa1d9e0 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -310,14 +310,14 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 		}
 
 		/*
-		 * If callback hasn't already happened, wait for buffers to be
-		 * returned from service layer
+		 * Always wait for the completion rather than skipping it when
+		 * priv->status is already set. The callback may have fired
+		 * between the CLAIM/SUBMIT send and here; if so, complete()
+		 * has already incremented the done counter and
+		 * wait_for_completion_timeout() returns immediately.
 		 */
-		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,
-- 
2.49.GIT

[PATCH 2/3] fpga: stratix10-soc: prefer ERROR over BUFFER_DONE

[Adrian Ng Ho Yin]

Check SVC_STATUS_ERROR before BUFFER_DONE so a combined
BUFFER_DONE | ERROR callback aborts instead of being treated as success.
Clear priv->status at the start of write_init so stale bits cannot leak
into the next reconfiguration.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 drivers/fpga/stratix10-soc.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 3ba0ffa1d9e0..c16ed571630b 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -183,6 +183,9 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
 	uint i;
 	int ret;
 
+	/* Drop stale status bits left by a previous aborted write. */
+	priv->status = 0;
+
 	ctype.flags = 0;
 	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
 		dev_dbg(dev, "Requesting partial reconfiguration.\n");
@@ -319,6 +322,13 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 		wait_status = wait_for_completion_timeout(
 			&priv->status_return_completion, S10_BUFFER_TIMEOUT);
 
+		/* ERROR must win over BUFFER_DONE when both are set. */
+		if (test_and_clear_bit(SVC_STATUS_ERROR, &priv->status)) {
+			dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
+			ret = -EFAULT;
+			break;
+		}
+
 		if (test_and_clear_bit(SVC_STATUS_BUFFER_DONE, &priv->status) ||
 		    test_and_clear_bit(SVC_STATUS_BUFFER_SUBMITTED,
 				       &priv->status)) {
@@ -326,12 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 			continue;
 		}
 
-		if (test_and_clear_bit(SVC_STATUS_ERROR, &priv->status)) {
-			dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
-			ret = -EFAULT;
-			break;
-		}
-
 		if (!wait_status) {
 			dev_err(dev, "timeout waiting for svc layer buffers\n");
 			ret = -ETIMEDOUT;
-- 
2.49.GIT

Re: [PATCH 2/3] fpga: stratix10-soc: prefer ERROR over BUFFER_DONE

[Xu Yilun]

On Wed, Sep 23, 2026 at 12:02:42AM +0800, Adrian Ng Ho Yin wrote:
> Check SVC_STATUS_ERROR before BUFFER_DONE so a combined
> BUFFER_DONE | ERROR callback aborts instead of being treated as success.
> Clear priv->status at the start of write_init so stale bits cannot leak
> into the next reconfiguration.

There is also a COMPLETED | ERROR combination, does it need the same
handling?

> 
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> ---
>  drivers/fpga/stratix10-soc.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 3ba0ffa1d9e0..c16ed571630b 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -183,6 +183,9 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>  	uint i;
>  	int ret;
>  
> +	/* Drop stale status bits left by a previous aborted write. */
> +	priv->status = 0;
> +
>  	ctype.flags = 0;
>  	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
>  		dev_dbg(dev, "Requesting partial reconfiguration.\n");
> @@ -319,6 +322,13 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>  		wait_status = wait_for_completion_timeout(
>  			&priv->status_return_completion, S10_BUFFER_TIMEOUT);
>  
> +		/* ERROR must win over BUFFER_DONE when both are set. */
> +		if (test_and_clear_bit(SVC_STATUS_ERROR, &priv->status)) {
> +			dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
> +			ret = -EFAULT;
> +			break;
> +		}
> +
>  		if (test_and_clear_bit(SVC_STATUS_BUFFER_DONE, &priv->status) ||
>  		    test_and_clear_bit(SVC_STATUS_BUFFER_SUBMITTED,
>  				       &priv->status)) {
> @@ -326,12 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>  			continue;
>  		}
>  
> -		if (test_and_clear_bit(SVC_STATUS_ERROR, &priv->status)) {
> -			dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
> -			ret = -EFAULT;
> -			break;
> -		}
> -

I think these command handlings in this series are much of the same
style:

 - init a completion
 - send command
 - wait_for_completion_timeout
 - check error status
 - check successful status
 - stratix10_svc_done

Now they are really a mess, hardly reviewable. Is it possible we make a
helper to regulate them?

>  		if (!wait_status) {
>  			dev_err(dev, "timeout waiting for svc layer buffers\n");
>  			ret = -ETIMEDOUT;
> -- 
> 2.49.GIT
> 
> 

[PATCH 3/3] fpga: stratix10-soc: floor config_complete_timeout_us

[Adrian Ng Ho Yin]

Keep info->config_complete_timeout_us configurable. of-fpga-region
leaves it at 0 when the DT property is absent, and usecs_to_jiffies(0)
makes the wait return immediately. Reconfiguration also takes ~600ms in
practice, so enforce S10_RECONFIG_TIMEOUT as a minimum floor while still
honouring any larger caller-supplied value. Note this prevents a DT
property from lowering the timeout below that floor.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 drivers/fpga/stratix10-soc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index c16ed571630b..83590e6b405f 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -7,6 +7,7 @@
 #include <linux/completion.h>
 #include <linux/fpga/fpga-mgr.h>
 #include <linux/firmware/intel/stratix10-svc-client.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -360,7 +361,15 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 	unsigned long timeout;
 	int ret;
 
-	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
+	/*
+	 * Keep config_complete_timeout_us configurable. of-fpga-region leaves
+	 * it at 0 when the DT property is absent, and usecs_to_jiffies(0)
+	 * makes the wait return immediately. Reconfiguration also takes
+	 * ~600ms in practice, so enforce S10_RECONFIG_TIMEOUT as a minimum
+	 * floor while still honouring any larger caller-supplied value.
+	 */
+	timeout = max(usecs_to_jiffies(info->config_complete_timeout_us),
+		      S10_RECONFIG_TIMEOUT);
 
 	do {
 		reinit_completion(&priv->status_return_completion);
-- 
2.49.GIT

Re: [PATCH 3/3] fpga: stratix10-soc: floor config_complete_timeout_us

[Xu Yilun]

On Wed, Sep 23, 2026 at 12:02:43AM +0800, Adrian Ng Ho Yin wrote:
> Keep info->config_complete_timeout_us configurable. of-fpga-region
> leaves it at 0 when the DT property is absent, and usecs_to_jiffies(0)
> makes the wait return immediately. Reconfiguration also takes ~600ms in
> practice, so enforce S10_RECONFIG_TIMEOUT as a minimum floor while still
> honouring any larger caller-supplied value. Note this prevents a DT
> property from lowering the timeout below that floor.
> 
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> ---
>  drivers/fpga/stratix10-soc.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index c16ed571630b..83590e6b405f 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -7,6 +7,7 @@
>  #include <linux/completion.h>
>  #include <linux/fpga/fpga-mgr.h>
>  #include <linux/firmware/intel/stratix10-svc-client.h>
> +#include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> @@ -360,7 +361,15 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>  	unsigned long timeout;
>  	int ret;
>  
> -	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
> +	/*
> +	 * Keep config_complete_timeout_us configurable. of-fpga-region leaves
> +	 * it at 0 when the DT property is absent, and usecs_to_jiffies(0)
> +	 * makes the wait return immediately. Reconfiguration also takes
> +	 * ~600ms in practice, so enforce S10_RECONFIG_TIMEOUT as a minimum
> +	 * floor while still honouring any larger caller-supplied value.
> +	 */
> +	timeout = max(usecs_to_jiffies(info->config_complete_timeout_us),
> +		      S10_RECONFIG_TIMEOUT);

Basically I think config_complete_timeout_us is used in a wrong way. You
can refer to other vendor fpga driver for reference.

>  
>  	do {
>  		reinit_completion(&priv->status_return_completion);
> -- 
> 2.49.GIT
> 
> 

[PATCH v3 0/3] fpga: stratix10-soc: harden wait, ERROR order, and timeout

[Adrian Ng Ho Yin]

Stratix10 FPGA reconfiguration can fail or hang for three related
reasons in the FPGA manager. This series hardens s10_ops_write_init(),
s10_ops_write() and s10_ops_write_complete(). No dependency on the
stratix10-svc accumulate changes.

  1) always wait for svc buffer completion

     s10_ops_write() skips wait_for_completion_timeout() when
     priv->status is already set. Always call the wait; if complete()
     already ran, the done counter is raised and the wait returns
     immediately.

  2) use config_complete_timeout_us as a deadline

     config_complete_timeout_us was passed as the timeout of every wait
     for a service layer reply, so the value 0 that of-fpga-region
     leaves when the DT property is absent fails write_complete at once.
     Wait for each RECONFIG_STATUS reply with a driver timeout that
     covers the service layer's own polling window, and use
     config_complete_timeout_us only as the deadline for re-polling
     after a BUSY reply, as xilinx-core, efinix-spi and
     altera-pr-ip-core do. Because one reply can take up to the
     service layer polling window, the deadline is coarse.

  3) SVC wait helper; prefer ERROR over success bits

     Add s10_svc_wait() / s10_svc_send_and_wait(). write_init becomes
     one s10_svc_send_and_wait() call, write_complete uses it for each
     status request, and write() uses s10_svc_wait() after sending a
     data buffer or claim request. ERROR takes precedence over
     BUFFER_DONE, BUFFER_SUBMITTED, COMPLETED and OK in the accumulated
     status word.

Changes in v3:
  - Patch 2 replaces "floor config_complete_timeout_us": drop the max()
    floor, use config_complete_timeout_us as a re-poll deadline instead
    of the per-reply wait (Xu Yilun)
  - Patch 3 replaces "prefer ERROR over BUFFER_DONE": add the wait
    helpers and convert all three ops to them (Xu Yilun)
  - ERROR also wins over COMPLETED and OK (Xu Yilun)
  - Timeout patch moved before the helper patch so no intermediate
    commit can re-poll without a bound
  - write_complete ignores a stray wake-up from the last claim callback
    instead of counting it against the deadline
  - v2 was posted without a version tag as "[PATCH n/3]"

Adrian Ng Ho Yin (3):
  fpga: stratix10-soc: always wait for svc buffer completion
  fpga: stratix10-soc: use config_complete_timeout_us as a deadline
  fpga: stratix10-soc: add SVC wait helper and prefer ERROR

 drivers/fpga/stratix10-soc.c | 178 +++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 72 deletions(-)

-- 
2.49.GIT

[PATCH v3 1/3] fpga: stratix10-soc: always wait for svc buffer completion

[Adrian Ng Ho Yin]

Always call wait_for_completion_timeout() in s10_ops_write(). Skipping
the wait when priv->status is already set is unnecessary: if the
callback already ran, complete() raised the done counter and the wait
returns immediately.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---

Notes:
    Changes in v3:
    - None.

 drivers/fpga/stratix10-soc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index b8ec2e6f615f..3ba0ffa1d9e0 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -310,14 +310,14 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 		}
 
 		/*
-		 * If callback hasn't already happened, wait for buffers to be
-		 * returned from service layer
+		 * Always wait for the completion rather than skipping it when
+		 * priv->status is already set. The callback may have fired
+		 * between the CLAIM/SUBMIT send and here; if so, complete()
+		 * has already incremented the done counter and
+		 * wait_for_completion_timeout() returns immediately.
 		 */
-		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,
-- 
2.49.GIT

[PATCH v3 2/3] fpga: stratix10-soc: use config_complete_timeout_us as a deadline

[Adrian Ng Ho Yin]

config_complete_timeout_us is the maximum time the FPGA may take to
enter user mode after programming. Other FPGA managers (xilinx-core,
efinix-spi, altera-pr-ip-core) turn it into a deadline for their own
"is configuration done" polling. s10_ops_write_complete() instead
passes it as the timeout of every wait for a service layer reply.

That couples two unrelated timeouts. of-fpga-region leaves the field at
0 when the DT property is absent, so usecs_to_jiffies(0) makes the very
first wait time out before the service layer has answered, and
write_complete fails even though configuration succeeds.

For COMMAND_RECONFIG_STATUS the service layer polls the firmware for up
to 30 seconds before it replies with COMPLETED, ERROR or BUSY. Wait for
that reply with a driver timeout that covers the polling window
(SVC_COMPLETED_TIMEOUT_MS plus the usual request timeout), and use
config_complete_timeout_us only as the deadline for re-sending the
status request after a BUSY reply. A value of 0 therefore results in a
single status request, as in the other drivers.

The deadline is only checked between replies, and one reply can take up
to the service layer polling window, so values shorter than that behave
like a single request and the deadline can be exceeded by up to one
reply wait. The service layer cannot be interrupted while it polls:
stratix10_svc_done() waits for its thread to finish.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---

Notes:
    Changes in v3:
    - Replaces "floor config_complete_timeout_us". Drop the max() floor and
      stop using config_complete_timeout_us as the per-reply wait. Wait for
      each RECONFIG_STATUS reply with a driver timeout that covers the
      service layer polling window, and use config_complete_timeout_us as
      the deadline for re-polling after BUSY, as other FPGA managers do
      (Xu Yilun).
    - Moved before the helper patch so that no intermediate commit can
      re-poll without a bound.

 drivers/fpga/stratix10-soc.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 3ba0ffa1d9e0..a69f63f0236b 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -25,6 +25,13 @@
 #define S10_BUFFER_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_BUFFER_TIMEOUT_MS))
 #define S10_RECONFIG_TIMEOUT (msecs_to_jiffies(SVC_RECONFIG_REQUEST_TIMEOUT_MS))
 
+/*
+ * The service layer polls the configuration status for up to 30 seconds
+ * (SVC_COMPLETED_TIMEOUT_MS) before it replies to COMMAND_RECONFIG_STATUS.
+ */
+#define S10_RECONFIG_STATUS_TIMEOUT \
+	(msecs_to_jiffies(SVC_COMPLETED_TIMEOUT_MS) + S10_RECONFIG_TIMEOUT)
+
 /*
  * struct s10_svc_buf
  * buf:  virtual address of buf provided by service layer
@@ -353,10 +360,10 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 {
 	struct s10_priv *priv = mgr->priv;
 	struct device *dev = priv->client.dev;
-	unsigned long timeout;
+	unsigned long deadline;
 	int ret;
 
-	timeout = usecs_to_jiffies(info->config_complete_timeout_us);
+	deadline = jiffies + usecs_to_jiffies(info->config_complete_timeout_us);
 
 	do {
 		reinit_completion(&priv->status_return_completion);
@@ -365,17 +372,12 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 		if (ret < 0)
 			break;
 
-		ret = wait_for_completion_timeout(
-			&priv->status_return_completion, timeout);
-		if (!ret) {
-			dev_err(dev,
-				"timeout waiting for RECONFIG_COMPLETED\n");
+		if (!wait_for_completion_timeout(&priv->status_return_completion,
+						 S10_RECONFIG_STATUS_TIMEOUT)) {
+			dev_err(dev, "timeout waiting for RECONFIG_STATUS\n");
 			ret = -ETIMEDOUT;
 			break;
 		}
-		/* Not error or timeout, so ret is # of jiffies until timeout */
-		timeout = ret;
-		ret = 0;
 
 		if (test_and_clear_bit(SVC_STATUS_COMPLETED, &priv->status))
 			break;
@@ -385,6 +387,13 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 			ret = -EFAULT;
 			break;
 		}
+
+		if (time_after_eq(jiffies, deadline)) {
+			dev_err(dev,
+				"timeout waiting for RECONFIG_COMPLETED\n");
+			ret = -ETIMEDOUT;
+			break;
+		}
 	} while (1);
 
 	stratix10_svc_done(priv->chan);
-- 
2.49.GIT

[PATCH v3 3/3] fpga: stratix10-soc: add SVC wait helper and prefer ERROR

[Adrian Ng Ho Yin]

write_init, write and write_complete each open-code the same sequence:
reinit the completion, send a service layer command, wait for the reply,
then test status bits. The order of the status tests differs between the
call sites.

The receive callback ORs each reply's status into priv->status, so bits
from more than one reply can be pending when a caller tests them (for
example several BUFFER_DONE callbacks for one RECONFIG_DATA_CLAIM, or a
reply that arrived after an earlier wait gave up). write() and
write_complete() test a success bit before SVC_STATUS_ERROR, so a status
word holding BUFFER_DONE | ERROR or COMPLETED | ERROR is treated as
success. write_init() ignores ERROR when OK is also set.

Add s10_svc_wait(), which waits for a reply and decodes priv->status with
SVC_STATUS_ERROR taking precedence over the caller's success bits, and
s10_svc_send_and_wait(), which also reinitialises the completion and
sends the command. write_init and write_complete use
s10_svc_send_and_wait(); write() sends either a data buffer or a claim
request and uses s10_svc_wait(). The helpers return 0, -EFAULT, -EBUSY
(SVC_STATUS_BUSY), -ETIMEDOUT, or -EAGAIN for a wake-up that carried
none of those bits. The status word is read and cleared in one step with
xchg().

The callback unlocks the returned buffers before it calls complete(), so
write() can finish while the completion of its last claim callback is
still pending, and that completion can then wake write_complete() before
the RECONFIG_STATUS reply. write_complete() therefore keeps waiting on
-EAGAIN and only re-sends RECONFIG_STATUS, bounded by the deadline, on
an actual BUSY reply.

As in the existing write() loop, status bits are honoured even when the
wait times out. Visible changes: write_init() now returns -EFAULT on
SVC_STATUS_ERROR and -EAGAIN for a reply without OK, where it returned
-ETIMEDOUT for both, and write_init() and write_complete() accept a
success bit that is set when their wait times out.

Also clear priv->status at the start of write_init so that bits set by a
late reply to an aborted previous write do not affect the new request.

stratix10_svc_done() is left at the call sites since it ends the whole
reconfiguration sequence rather than a single command.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---

Notes:
    Changes in v3:
    - Replaces "prefer ERROR over BUFFER_DONE". Add s10_svc_wait() and
      s10_svc_send_and_wait(); write_init and write_complete use
      s10_svc_send_and_wait(), write() uses s10_svc_wait() after its
      data/claim send (Xu Yilun).
    - ERROR now also wins over COMPLETED and over OK (Xu Yilun).
    - write_complete ignores a stray wake-up from the last claim callback
      instead of counting it against the deadline.

 drivers/fpga/stratix10-soc.c | 163 ++++++++++++++++++++---------------
 1 file changed, 94 insertions(+), 69 deletions(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index a69f63f0236b..074a99fc3a4b 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -72,6 +72,69 @@ static int s10_svc_send_msg(struct s10_priv *priv,
 	return ret;
 }
 
+/*
+ * s10_svc_wait - wait for a service layer reply and decode its status
+ * priv: private data
+ * done_bits: mask of SVC_STATUS_* bits that indicate success
+ * timeout: maximum time to wait for the reply, in jiffies
+ *
+ * Status bits accumulate in priv->status until they are consumed here, and
+ * SVC_STATUS_ERROR takes precedence over any success bit set alongside it.
+ * Returns 0 if a bit in done_bits was set, -EFAULT on SVC_STATUS_ERROR,
+ * -EBUSY on SVC_STATUS_BUSY, -ETIMEDOUT if no reply arrived, or -EAGAIN if
+ * the wake-up carried none of these bits (e.g. a late completion whose
+ * status bits were already consumed).
+ */
+static int s10_svc_wait(struct s10_priv *priv, unsigned long done_bits,
+			unsigned long timeout)
+{
+	struct device *dev = priv->client.dev;
+	unsigned long time_left, status;
+
+	time_left = wait_for_completion_timeout(&priv->status_return_completion,
+						timeout);
+	status = xchg(&priv->status, 0);
+
+	if (status & BIT(SVC_STATUS_ERROR)) {
+		dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
+		return -EFAULT;
+	}
+
+	if (status & done_bits)
+		return 0;
+
+	if (status & BIT(SVC_STATUS_BUSY))
+		return -EBUSY;
+
+	if (!time_left) {
+		dev_err(dev, "timeout waiting for service layer reply\n");
+		return -ETIMEDOUT;
+	}
+
+	return -EAGAIN;
+}
+
+/*
+ * s10_svc_send_and_wait - send a command and wait for its reply
+ * Returns a negative errno from s10_svc_send_msg(), otherwise the result of
+ * s10_svc_wait().
+ */
+static int s10_svc_send_and_wait(struct s10_priv *priv,
+				 enum stratix10_svc_command_code command,
+				 void *payload, u32 payload_length,
+				 unsigned long done_bits, unsigned long timeout)
+{
+	int ret;
+
+	reinit_completion(&priv->status_return_completion);
+
+	ret = s10_svc_send_msg(priv, command, payload, payload_length);
+	if (ret < 0)
+		return ret;
+
+	return s10_svc_wait(priv, done_bits, timeout);
+}
+
 /*
  * Free buffers allocated from the service layer's pool that are not in use.
  * Return true when all buffers are freed.
@@ -159,8 +222,8 @@ static void s10_receive_callback(struct stratix10_svc_client *client,
 	status = data->status;
 
 	/*
-	 * Here we set status bits as we receive them.  Elsewhere, we always use
-	 * test_and_clear_bit() to check status in priv->status
+	 * Here we set status bits as we receive them.  s10_svc_wait() consumes
+	 * them from priv->status.
 	 */
 	for (i = 0; i <= SVC_STATUS_ERROR; i++)
 		if (status & (1 << i))
@@ -190,6 +253,9 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
 	uint i;
 	int ret;
 
+	/* A reply to an aborted previous write may have left status bits. */
+	priv->status = 0;
+
 	ctype.flags = 0;
 	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
 		dev_dbg(dev, "Requesting partial reconfiguration.\n");
@@ -198,23 +264,11 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
 		dev_dbg(dev, "Requesting full reconfiguration.\n");
 	}
 
-	reinit_completion(&priv->status_return_completion);
-	ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
-			       &ctype, sizeof(ctype));
-	if (ret < 0)
-		goto init_error;
-
-	if (!wait_for_completion_timeout(&priv->status_return_completion,
-					 S10_RECONFIG_TIMEOUT)) {
-		dev_err(dev, "timeout waiting for RECONFIG_REQUEST\n");
-		ret = -ETIMEDOUT;
-		goto init_error;
-	}
-
-	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
-		ret = -ETIMEDOUT;
+	ret = s10_svc_send_and_wait(priv, COMMAND_RECONFIG,
+				    &ctype, sizeof(ctype),
+				    BIT(SVC_STATUS_OK), S10_RECONFIG_TIMEOUT);
+	if (ret)
 		goto init_error;
-	}
 
 	/* Allocate buffers from the service layer's pool. */
 	for (i = 0; i < NUM_SVC_BUFS; i++) {
@@ -287,7 +341,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 {
 	struct s10_priv *priv = mgr->priv;
 	struct device *dev = priv->client.dev;
-	long wait_status;
 	int sent = 0;
 	int ret = 0;
 
@@ -320,30 +373,15 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 		 * Always wait for the completion rather than skipping it when
 		 * priv->status is already set. The callback may have fired
 		 * between the CLAIM/SUBMIT send and here; if so, complete()
-		 * has already incremented the done counter and
-		 * wait_for_completion_timeout() returns immediately.
+		 * has already incremented the done counter and s10_svc_wait()
+		 * returns immediately.
 		 */
-		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,
-				       &priv->status)) {
-			ret = 0;
-			continue;
-		}
-
-		if (test_and_clear_bit(SVC_STATUS_ERROR, &priv->status)) {
-			dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
-			ret = -EFAULT;
-			break;
-		}
-
-		if (!wait_status) {
-			dev_err(dev, "timeout waiting for svc layer buffers\n");
-			ret = -ETIMEDOUT;
+		ret = s10_svc_wait(priv, BIT(SVC_STATUS_BUFFER_SUBMITTED) |
+					 BIT(SVC_STATUS_BUFFER_DONE),
+				   S10_BUFFER_TIMEOUT);
+		if (ret == -EFAULT || ret == -ETIMEDOUT)
 			break;
-		}
+		ret = 0;
 	}
 
 	if (!s10_free_buffers(mgr))
@@ -366,35 +404,22 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 	deadline = jiffies + usecs_to_jiffies(info->config_complete_timeout_us);
 
 	do {
-		reinit_completion(&priv->status_return_completion);
-
-		ret = s10_svc_send_msg(priv, COMMAND_RECONFIG_STATUS, NULL, 0);
-		if (ret < 0)
-			break;
-
-		if (!wait_for_completion_timeout(&priv->status_return_completion,
-						 S10_RECONFIG_STATUS_TIMEOUT)) {
-			dev_err(dev, "timeout waiting for RECONFIG_STATUS\n");
-			ret = -ETIMEDOUT;
-			break;
-		}
-
-		if (test_and_clear_bit(SVC_STATUS_COMPLETED, &priv->status))
-			break;
-
-		if (test_and_clear_bit(SVC_STATUS_ERROR, &priv->status)) {
-			dev_err(dev, "ERROR - giving up - SVC_STATUS_ERROR\n");
-			ret = -EFAULT;
-			break;
-		}
+		ret = s10_svc_send_and_wait(priv, COMMAND_RECONFIG_STATUS,
+					    NULL, 0, BIT(SVC_STATUS_COMPLETED),
+					    S10_RECONFIG_STATUS_TIMEOUT);
+		/*
+		 * A late completion from the last RECONFIG_DATA_CLAIM callback
+		 * can wake us before the reply to this request arrives.
+		 */
+		while (ret == -EAGAIN)
+			ret = s10_svc_wait(priv, BIT(SVC_STATUS_COMPLETED),
+					   S10_RECONFIG_STATUS_TIMEOUT);
+	} while (ret == -EBUSY && time_before(jiffies, deadline));
 
-		if (time_after_eq(jiffies, deadline)) {
-			dev_err(dev,
-				"timeout waiting for RECONFIG_COMPLETED\n");
-			ret = -ETIMEDOUT;
-			break;
-		}
-	} while (1);
+	if (ret == -EBUSY) {
+		dev_err(dev, "timeout waiting for RECONFIG_COMPLETED\n");
+		ret = -ETIMEDOUT;
+	}
 
 	stratix10_svc_done(priv->chan);
 
-- 
2.49.GIT