Re: [PATCH v3] fpga: stratix10-soc: Fix SVC mailbox handling during reconfiguration

["NG, TZE YEE" <tze.yee.ng@altera.com>]

On 27/6/2026 1:11 am, Xu Yilun wrote:
> [You don't often get email from yilun.xu@linux.intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
>> @@ -195,20 +195,20 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>>        ret = s10_svc_send_msg(priv, COMMAND_RECONFIG,
>>                               &ctype, sizeof(ctype));
>>        if (ret < 0)
>> -             goto init_done;
>> +             goto init_error;
>>
>>        ret = wait_for_completion_timeout(
>>                &priv->status_return_completion, S10_RECONFIG_TIMEOUT);
>>        if (!ret) {
>>                dev_err(dev, "timeout waiting for RECONFIG_REQUEST\n");
>>                ret = -ETIMEDOUT;
>> -             goto init_done;
>> +             goto init_error;
>>        }
>>
>>        ret = 0;
> 
> Why re-initialize ret here?
> 

That was clearing the remaining jiffies left in ret by 
wait_for_completion_timeout(). I'll drop it by not assigning the wait 
result to ret.

>>        if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
>>                ret = -ETIMEDOUT;
>> -             goto init_done;
>> +             goto init_error;
>>        }
>>
>>        /* Allocate buffers from the service layer's pool. */
>> @@ -216,16 +216,19 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>>                kbuf = stratix10_svc_allocate_memory(priv->chan, SVC_BUF_SIZE);
>>                if (IS_ERR(kbuf)) {
>>                        s10_free_buffers(mgr);
>> -                     ret = PTR_ERR(kbuf);
>> -                     goto init_done;
>> +                     ret = -ENOMEM;
> 
> Why change the ret?
> 

That was unrelated churn.  I'll revert to PTR_ERR(kbuf).

>> +                     goto init_error;
>>                }
>>
>>                priv->svc_bufs[i].buf = kbuf;
>>                priv->svc_bufs[i].lock = 0;
>>        }
>>
>> -init_done:
>> +     goto init_done;
> 
> Happy path? why not just return 0; And you don't need init_done at all.
> 

Agreed. I'll use return 0 on success and keep only init_error for 
stratix10_svc_done() + return ret.

>> +
>> +init_error:
>>        stratix10_svc_done(priv->chan);
>> +init_done:
>>        return ret;
>>   }