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@xxxxxxxxxx>
> ---
>  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
> 
>