linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
@ 2020-07-07 16:14 richard.gong
  2020-07-08 17:08 ` Dinh Nguyen
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: richard.gong @ 2020-07-07 16:14 UTC (permalink / raw)
  To: mdf; +Cc: linux-fpga, linux-kernel, dinguyen, richard.gong, richard.gong

From: Richard Gong <richard.gong@intel.com>

When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
reconfiguration process stops and the user can't perform a new FPGA
reconfiguration properly.

Set FPGA complete task to be not interruptible so that the user can
properly perform FPGA reconfiguration after CTRL+C event.

Signed-off-by: Richard Gong <richard.gong@intel.com>
---
 drivers/fpga/stratix10-soc.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 44b7c56..657a70c 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -196,17 +196,13 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
 	if (ret < 0)
 		goto init_done;
 
-	ret = wait_for_completion_interruptible_timeout(
+	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;
 	}
-	if (ret < 0) {
-		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
-		goto init_done;
-	}
 
 	ret = 0;
 	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
@@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 		 */
 		wait_status = 1; /* not timed out */
 		if (!priv->status)
-			wait_status = wait_for_completion_interruptible_timeout(
+			wait_status = wait_for_completion_timeout(
 				&priv->status_return_completion,
 				S10_BUFFER_TIMEOUT);
 
@@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
 			ret = -ETIMEDOUT;
 			break;
 		}
-		if (wait_status < 0) {
-			ret = wait_status;
-			dev_err(dev,
-				"error (%d) waiting for svc layer buffers\n",
-				ret);
-			break;
-		}
 	}
 
 	if (!s10_free_buffers(mgr))
@@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 		if (ret < 0)
 			break;
 
-		ret = wait_for_completion_interruptible_timeout(
+		ret = wait_for_completion_timeout(
 			&priv->status_return_completion, timeout);
 		if (!ret) {
 			dev_err(dev,
@@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
 			ret = -ETIMEDOUT;
 			break;
 		}
-		if (ret < 0) {
-			dev_err(dev,
-				"error (%d) waiting for RECONFIG_COMPLETED\n",
-				ret);
-			break;
-		}
 		/* Not error or timeout, so ret is # of jiffies until timeout */
 		timeout = ret;
 		ret = 0;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-07 16:14 [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible richard.gong
@ 2020-07-08 17:08 ` Dinh Nguyen
  2020-07-08 18:30   ` Richard Gong
  2020-07-09 20:01 ` Tom Rix
  2020-07-20 19:00 ` Richard Gong
  2 siblings, 1 reply; 8+ messages in thread
From: Dinh Nguyen @ 2020-07-08 17:08 UTC (permalink / raw)
  To: richard.gong, mdf; +Cc: linux-fpga, linux-kernel, richard.gong

Hi

On 7/7/20 11:14 AM, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
> 
> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
> reconfiguration process stops and the user can't perform a new FPGA
> reconfiguration properly.
> 
> Set FPGA complete task to be not interruptible so that the user can
> properly perform FPGA reconfiguration after CTRL+C event.
> 
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
>  drivers/fpga/stratix10-soc.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 44b7c56..657a70c 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>  	if (ret < 0)
>  		goto init_done;
>  
> -	ret = wait_for_completion_interruptible_timeout(
> +	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;
>  	}
> -	if (ret < 0) {
> -		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
> -		goto init_done;
> -	}
>  
>  	ret = 0;
>  	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>  		 */
>  		wait_status = 1; /* not timed out */
>  		if (!priv->status)
> -			wait_status = wait_for_completion_interruptible_timeout(
> +			wait_status = wait_for_completion_timeout(
>  				&priv->status_return_completion,
>  				S10_BUFFER_TIMEOUT);
>  
> @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>  			ret = -ETIMEDOUT;
>  			break;
>  		}
> -		if (wait_status < 0) {
> -			ret = wait_status;
> -			dev_err(dev,
> -				"error (%d) waiting for svc layer buffers\n",
> -				ret);
> -			break;
> -		}
>  	}
>  
>  	if (!s10_free_buffers(mgr))
> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>  		if (ret < 0)
>  			break;
>  
> -		ret = wait_for_completion_interruptible_timeout(
> +		ret = wait_for_completion_timeout(
>  			&priv->status_return_completion, timeout);
>  		if (!ret) {
>  			dev_err(dev,
> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>  			ret = -ETIMEDOUT;
>  			break;
>  		}
> -		if (ret < 0) {
> -			dev_err(dev,
> -				"error (%d) waiting for RECONFIG_COMPLETED\n",
> -				ret);
> -			break;
> -		}
>  		/* Not error or timeout, so ret is # of jiffies until timeout */
>  		timeout = ret;
>  		ret = 0;
> 

Do you need the same change in drivers/fpga/socfpga.c? Also, you did not
include Moritz Fisher on this. He's the maintainer.

Dinh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-08 17:08 ` Dinh Nguyen
@ 2020-07-08 18:30   ` Richard Gong
  2020-07-08 18:47     ` Dinh Nguyen
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Gong @ 2020-07-08 18:30 UTC (permalink / raw)
  To: Dinh Nguyen, mdf; +Cc: linux-fpga, linux-kernel, richard.gong

Hi Dinh,


On 7/8/20 12:08 PM, Dinh Nguyen wrote:
> Hi
> 
> On 7/7/20 11:14 AM, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
>> reconfiguration process stops and the user can't perform a new FPGA
>> reconfiguration properly.
>>
>> Set FPGA complete task to be not interruptible so that the user can
>> properly perform FPGA reconfiguration after CTRL+C event.
>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>>   drivers/fpga/stratix10-soc.c | 23 +++--------------------
>>   1 file changed, 3 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>> index 44b7c56..657a70c 100644
>> --- a/drivers/fpga/stratix10-soc.c
>> +++ b/drivers/fpga/stratix10-soc.c
>> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>>   	if (ret < 0)
>>   		goto init_done;
>>   
>> -	ret = wait_for_completion_interruptible_timeout(
>> +	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;
>>   	}
>> -	if (ret < 0) {
>> -		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
>> -		goto init_done;
>> -	}
>>   
>>   	ret = 0;
>>   	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
>> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>>   		 */
>>   		wait_status = 1; /* not timed out */
>>   		if (!priv->status)
>> -			wait_status = wait_for_completion_interruptible_timeout(
>> +			wait_status = wait_for_completion_timeout(
>>   				&priv->status_return_completion,
>>   				S10_BUFFER_TIMEOUT);
>>   
>> @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>>   			ret = -ETIMEDOUT;
>>   			break;
>>   		}
>> -		if (wait_status < 0) {
>> -			ret = wait_status;
>> -			dev_err(dev,
>> -				"error (%d) waiting for svc layer buffers\n",
>> -				ret);
>> -			break;
>> -		}
>>   	}
>>   
>>   	if (!s10_free_buffers(mgr))
>> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>>   		if (ret < 0)
>>   			break;
>>   
>> -		ret = wait_for_completion_interruptible_timeout(
>> +		ret = wait_for_completion_timeout(
>>   			&priv->status_return_completion, timeout);
>>   		if (!ret) {
>>   			dev_err(dev,
>> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>>   			ret = -ETIMEDOUT;
>>   			break;
>>   		}
>> -		if (ret < 0) {
>> -			dev_err(dev,
>> -				"error (%d) waiting for RECONFIG_COMPLETED\n",
>> -				ret);
>> -			break;
>> -
>>   		/* Not error or timeout, so ret is # of jiffies until timeout */
>>   		timeout = ret;
>>   		ret = 0;
>>
> 
> Do you need the same change in drivers/fpga/socfpga.c? 
It is not required.
Also, you did not
> include Moritz Fisher on this. He's the maintainer.
> 
I did include Moritz Fisher <mdf@kernel.org> in the submission, is 
something change recently?


Regards,
Richard
> Dinh
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-08 18:30   ` Richard Gong
@ 2020-07-08 18:47     ` Dinh Nguyen
  2020-07-08 20:08       ` Richard Gong
  0 siblings, 1 reply; 8+ messages in thread
From: Dinh Nguyen @ 2020-07-08 18:47 UTC (permalink / raw)
  To: Richard Gong, mdf; +Cc: linux-fpga, linux-kernel, richard.gong



On 7/8/20 1:30 PM, Richard Gong wrote:
> Hi Dinh,
> 
> 
> On 7/8/20 12:08 PM, Dinh Nguyen wrote:
>> Hi
>>
>> On 7/7/20 11:14 AM, richard.gong@linux.intel.com wrote:
>>> From: Richard Gong <richard.gong@intel.com>
>>>
>>> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
>>> reconfiguration process stops and the user can't perform a new FPGA
>>> reconfiguration properly.
>>>
>>> Set FPGA complete task to be not interruptible so that the user can
>>> properly perform FPGA reconfiguration after CTRL+C event.
>>>
>>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>>> ---
>>>   drivers/fpga/stratix10-soc.c | 23 +++--------------------
>>>   1 file changed, 3 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>>> index 44b7c56..657a70c 100644
>>> --- a/drivers/fpga/stratix10-soc.c
>>> +++ b/drivers/fpga/stratix10-soc.c
>>> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct
>>> fpga_manager *mgr,
>>>       if (ret < 0)
>>>           goto init_done;
>>>   -    ret = wait_for_completion_interruptible_timeout(
>>> +    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;
>>>       }
>>> -    if (ret < 0) {
>>> -        dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
>>> -        goto init_done;
>>> -    }
>>>         ret = 0;
>>>       if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
>>> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager
>>> *mgr, const char *buf,
>>>            */
>>>           wait_status = 1; /* not timed out */
>>>           if (!priv->status)
>>> -            wait_status = wait_for_completion_interruptible_timeout(
>>> +            wait_status = wait_for_completion_timeout(
>>>                   &priv->status_return_completion,
>>>                   S10_BUFFER_TIMEOUT);
>>>   @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager
>>> *mgr, const char *buf,
>>>               ret = -ETIMEDOUT;
>>>               break;
>>>           }
>>> -        if (wait_status < 0) {
>>> -            ret = wait_status;
>>> -            dev_err(dev,
>>> -                "error (%d) waiting for svc layer buffers\n",
>>> -                ret);
>>> -            break;
>>> -        }
>>>       }
>>>         if (!s10_free_buffers(mgr))
>>> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct
>>> fpga_manager *mgr,
>>>           if (ret < 0)
>>>               break;
>>>   -        ret = wait_for_completion_interruptible_timeout(
>>> +        ret = wait_for_completion_timeout(
>>>               &priv->status_return_completion, timeout);
>>>           if (!ret) {
>>>               dev_err(dev,
>>> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct
>>> fpga_manager *mgr,
>>>               ret = -ETIMEDOUT;
>>>               break;
>>>           }
>>> -        if (ret < 0) {
>>> -            dev_err(dev,
>>> -                "error (%d) waiting for RECONFIG_COMPLETED\n",
>>> -                ret);
>>> -            break;
>>> -
>>>           /* Not error or timeout, so ret is # of jiffies until
>>> timeout */
>>>           timeout = ret;
>>>           ret = 0;
>>>
>>
>> Do you need the same change in drivers/fpga/socfpga.c? 
> It is not required.

Why not?

> Also, you did not
>> include Moritz Fisher on this. He's the maintainer.
>>
> I did include Moritz Fisher <mdf@kernel.org> in the submission, is
> something change recently?
> 

My bad, I didn't see his name in the email.

Dinh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-08 18:47     ` Dinh Nguyen
@ 2020-07-08 20:08       ` Richard Gong
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Gong @ 2020-07-08 20:08 UTC (permalink / raw)
  To: Dinh Nguyen, mdf; +Cc: linux-fpga, linux-kernel, richard.gong

Hi Dinh,

On 7/8/20 1:47 PM, Dinh Nguyen wrote:
> 
> 
> On 7/8/20 1:30 PM, Richard Gong wrote:
>> Hi Dinh,
>>
>>
>> On 7/8/20 12:08 PM, Dinh Nguyen wrote:
>>> Hi
>>>
>>> On 7/7/20 11:14 AM, richard.gong@linux.intel.com wrote:
>>>> From: Richard Gong <richard.gong@intel.com>
>>>>
>>>> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
>>>> reconfiguration process stops and the user can't perform a new FPGA
>>>> reconfiguration properly.
>>>>
>>>> Set FPGA complete task to be not interruptible so that the user can
>>>> properly perform FPGA reconfiguration after CTRL+C event.
>>>>
>>>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>>>> ---
>>>>    drivers/fpga/stratix10-soc.c | 23 +++--------------------
>>>>    1 file changed, 3 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>>>> index 44b7c56..657a70c 100644
>>>> --- a/drivers/fpga/stratix10-soc.c
>>>> +++ b/drivers/fpga/stratix10-soc.c
>>>> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct
>>>> fpga_manager *mgr,
>>>>        if (ret < 0)
>>>>            goto init_done;
>>>>    -    ret = wait_for_completion_interruptible_timeout(
>>>> +    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;
>>>>        }
>>>> -    if (ret < 0) {
>>>> -        dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
>>>> -        goto init_done;
>>>> -    }
>>>>          ret = 0;
>>>>        if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
>>>> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager
>>>> *mgr, const char *buf,
>>>>             */
>>>>            wait_status = 1; /* not timed out */
>>>>            if (!priv->status)
>>>> -            wait_status = wait_for_completion_interruptible_timeout(
>>>> +            wait_status = wait_for_completion_timeout(
>>>>                    &priv->status_return_completion,
>>>>                    S10_BUFFER_TIMEOUT);
>>>>    @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager
>>>> *mgr, const char *buf,
>>>>                ret = -ETIMEDOUT;
>>>>                break;
>>>>            }
>>>> -        if (wait_status < 0) {
>>>> -            ret = wait_status;
>>>> -            dev_err(dev,
>>>> -                "error (%d) waiting for svc layer buffers\n",
>>>> -                ret);
>>>> -            break;
>>>> -        }
>>>>        }
>>>>          if (!s10_free_buffers(mgr))
>>>> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct
>>>> fpga_manager *mgr,
>>>>            if (ret < 0)
>>>>                break;
>>>>    -        ret = wait_for_completion_interruptible_timeout(
>>>> +        ret = wait_for_completion_timeout(
>>>>                &priv->status_return_completion, timeout);
>>>>            if (!ret) {
>>>>                dev_err(dev,
>>>> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct
>>>> fpga_manager *mgr,
>>>>                ret = -ETIMEDOUT;
>>>>                break;
>>>>            }
>>>> -        if (ret < 0) {
>>>> -            dev_err(dev,
>>>> -                "error (%d) waiting for RECONFIG_COMPLETED\n",
>>>> -                ret);
>>>> -            break;
>>>> -
>>>>            /* Not error or timeout, so ret is # of jiffies until
>>>> timeout */
>>>>            timeout = ret;
>>>>            ret = 0;
>>>>
>>>
>>> Do you need the same change in drivers/fpga/socfpga.c?
>> It is not required.
> 
> Why not?

The original codes doesn't handle the task completion to be signaled, so 
here it serves the same function as wait_for_completion_timeout call.

Regards,
Richard

> 
>> Also, you did not
>>> include Moritz Fisher on this. He's the maintainer.
>>>
>> I did include Moritz Fisher <mdf@kernel.org> in the submission, is
>> something change recently?
>>
> 
> My bad, I didn't see his name in the email.
> 
> Dinh
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-07 16:14 [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible richard.gong
  2020-07-08 17:08 ` Dinh Nguyen
@ 2020-07-09 20:01 ` Tom Rix
  2020-07-09 21:10   ` Richard Gong
  2020-07-20 19:00 ` Richard Gong
  2 siblings, 1 reply; 8+ messages in thread
From: Tom Rix @ 2020-07-09 20:01 UTC (permalink / raw)
  To: richard.gong, mdf; +Cc: linux-fpga, linux-kernel, dinguyen, richard.gong

Mostly fine, see inline question.

On 7/7/20 9:14 AM, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
>
> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
> reconfiguration process stops and the user can't perform a new FPGA
> reconfiguration properly.
>
> Set FPGA complete task to be not interruptible so that the user can
> properly perform FPGA reconfiguration after CTRL+C event.
>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
>  drivers/fpga/stratix10-soc.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 44b7c56..657a70c 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>  	if (ret < 0)
>  		goto init_done;
>  
> -	ret = wait_for_completion_interruptible_timeout(
> +	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;
>  	}
> -	if (ret < 0) {
> -		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
> -		goto init_done;
> -	}
>  
>  	ret = 0;
>  	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>  		 */
>  		wait_status = 1; /* not timed out */
>  		if (!priv->status)
> -			wait_status = wait_for_completion_interruptible_timeout(
> +			wait_status = wait_for_completion_timeout(
>  				&priv->status_return_completion,
>  				S10_BUFFER_TIMEOUT);
>  
> @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>  			ret = -ETIMEDOUT;
>  			break;
>  		}
> -		if (wait_status < 0) {
> -			ret = wait_status;
> -			dev_err(dev,
> -				"error (%d) waiting for svc layer buffers\n",
> -				ret);
> -			break;
> -		}
>  	}
>  
>  	if (!s10_free_buffers(mgr))
> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,

This part is done in an infinite loop, is the loop still needed ?

Reviewed-by: Tom Rix <trix@redhat.com>

>  		if (ret < 0)
>  			break;
>  
> -		ret = wait_for_completion_interruptible_timeout(
> +		ret = wait_for_completion_timeout(
>  			&priv->status_return_completion, timeout);
>  		if (!ret) {
>  			dev_err(dev,
> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>  			ret = -ETIMEDOUT;
>  			break;
>  		}
> -		if (ret < 0) {
> -			dev_err(dev,
> -				"error (%d) waiting for RECONFIG_COMPLETED\n",
> -				ret);
> -			break;
> -		}
>  		/* Not error or timeout, so ret is # of jiffies until timeout */
>  		timeout = ret;
>  		ret = 0;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-09 20:01 ` Tom Rix
@ 2020-07-09 21:10   ` Richard Gong
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Gong @ 2020-07-09 21:10 UTC (permalink / raw)
  To: Tom Rix, mdf; +Cc: linux-fpga, linux-kernel, dinguyen, richard.gong

Hi Tom,

On 7/9/20 3:01 PM, Tom Rix wrote:
> Mostly fine, see inline question.
> 
> On 7/7/20 9:14 AM, richard.gong@linux.intel.com wrote:
>> From: Richard Gong <richard.gong@intel.com>
>>
>> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
>> reconfiguration process stops and the user can't perform a new FPGA
>> reconfiguration properly.
>>
>> Set FPGA complete task to be not interruptible so that the user can
>> properly perform FPGA reconfiguration after CTRL+C event.
>>
>> Signed-off-by: Richard Gong <richard.gong@intel.com>
>> ---
>>   drivers/fpga/stratix10-soc.c | 23 +++--------------------
>>   1 file changed, 3 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
>> index 44b7c56..657a70c 100644
>> --- a/drivers/fpga/stratix10-soc.c
>> +++ b/drivers/fpga/stratix10-soc.c
>> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>>   	if (ret < 0)
>>   		goto init_done;
>>   
>> -	ret = wait_for_completion_interruptible_timeout(
>> +	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;
>>   	}
>> -	if (ret < 0) {
>> -		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
>> -		goto init_done;
>> -	}
>>   
>>   	ret = 0;
>>   	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
>> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>>   		 */
>>   		wait_status = 1; /* not timed out */
>>   		if (!priv->status)
>> -			wait_status = wait_for_completion_interruptible_timeout(
>> +			wait_status = wait_for_completion_timeout(
>>   				&priv->status_return_completion,
>>   				S10_BUFFER_TIMEOUT);
>>   
>> @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>>   			ret = -ETIMEDOUT;
>>   			break;
>>   		}
>> -		if (wait_status < 0) {
>> -			ret = wait_status;
>> -			dev_err(dev,
>> -				"error (%d) waiting for svc layer buffers\n",
>> -				ret);
>> -			break;
>> -		}
>>   	}
>>   
>>   	if (!s10_free_buffers(mgr))
>> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
> 
> This part is done in an infinite loop, is the loop still needed 
The loop is still need. FPGA manager driver need polling the completed 
status from the lower level firmware. The lower level firmware can 
return FPGA manager driver with busy, completed or error status.
> 
> Reviewed-by: Tom Rix <trix@redhat.com>
Thanks for your review!

Regards,
Richard
> 
>>   		if (ret < 0)
>>   			break;
>>   
>> -		ret = wait_for_completion_interruptible_timeout(
>> +		ret = wait_for_completion_timeout(
>>   			&priv->status_return_completion, timeout);
>>   		if (!ret) {
>>   			dev_err(dev,
>> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>>   			ret = -ETIMEDOUT;
>>   			break;
>>   		}
>> -		if (ret < 0) {
>> -			dev_err(dev,
>> -				"error (%d) waiting for RECONFIG_COMPLETED\n",
>> -				ret);
>> -			break;
>> -		}
>>   		/* Not error or timeout, so ret is # of jiffies until timeout */
>>   		timeout = ret;
>>   		ret = 0;
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible
  2020-07-07 16:14 [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible richard.gong
  2020-07-08 17:08 ` Dinh Nguyen
  2020-07-09 20:01 ` Tom Rix
@ 2020-07-20 19:00 ` Richard Gong
  2 siblings, 0 replies; 8+ messages in thread
From: Richard Gong @ 2020-07-20 19:00 UTC (permalink / raw)
  To: mdf; +Cc: linux-fpga, linux-kernel, dinguyen, richard.gong


Hi Moritz,

Sorry for asking.

When you get chance, can you review this FPGA patch?

Regards,
Richard


On 7/7/20 11:14 AM, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
> 
> When CTRL+C occurs during the process of FPGA reconfiguration, the FPGA
> reconfiguration process stops and the user can't perform a new FPGA
> reconfiguration properly.
> 
> Set FPGA complete task to be not interruptible so that the user can
> properly perform FPGA reconfiguration after CTRL+C event.
> 
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> ---
>   drivers/fpga/stratix10-soc.c | 23 +++--------------------
>   1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
> index 44b7c56..657a70c 100644
> --- a/drivers/fpga/stratix10-soc.c
> +++ b/drivers/fpga/stratix10-soc.c
> @@ -196,17 +196,13 @@ static int s10_ops_write_init(struct fpga_manager *mgr,
>   	if (ret < 0)
>   		goto init_done;
>   
> -	ret = wait_for_completion_interruptible_timeout(
> +	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;
>   	}
> -	if (ret < 0) {
> -		dev_err(dev, "error (%d) waiting for RECONFIG_REQUEST\n", ret);
> -		goto init_done;
> -	}
>   
>   	ret = 0;
>   	if (!test_and_clear_bit(SVC_STATUS_OK, &priv->status)) {
> @@ -318,7 +314,7 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>   		 */
>   		wait_status = 1; /* not timed out */
>   		if (!priv->status)
> -			wait_status = wait_for_completion_interruptible_timeout(
> +			wait_status = wait_for_completion_timeout(
>   				&priv->status_return_completion,
>   				S10_BUFFER_TIMEOUT);
>   
> @@ -340,13 +336,6 @@ static int s10_ops_write(struct fpga_manager *mgr, const char *buf,
>   			ret = -ETIMEDOUT;
>   			break;
>   		}
> -		if (wait_status < 0) {
> -			ret = wait_status;
> -			dev_err(dev,
> -				"error (%d) waiting for svc layer buffers\n",
> -				ret);
> -			break;
> -		}
>   	}
>   
>   	if (!s10_free_buffers(mgr))
> @@ -372,7 +361,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>   		if (ret < 0)
>   			break;
>   
> -		ret = wait_for_completion_interruptible_timeout(
> +		ret = wait_for_completion_timeout(
>   			&priv->status_return_completion, timeout);
>   		if (!ret) {
>   			dev_err(dev,
> @@ -380,12 +369,6 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
>   			ret = -ETIMEDOUT;
>   			break;
>   		}
> -		if (ret < 0) {
> -			dev_err(dev,
> -				"error (%d) waiting for RECONFIG_COMPLETED\n",
> -				ret);
> -			break;
> -		}
>   		/* Not error or timeout, so ret is # of jiffies until timeout */
>   		timeout = ret;
>   		ret = 0;
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-07-20 18:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 16:14 [PATCH] fpga: stratix10-soc: make FPGA task un-interruptible richard.gong
2020-07-08 17:08 ` Dinh Nguyen
2020-07-08 18:30   ` Richard Gong
2020-07-08 18:47     ` Dinh Nguyen
2020-07-08 20:08       ` Richard Gong
2020-07-09 20:01 ` Tom Rix
2020-07-09 21:10   ` Richard Gong
2020-07-20 19:00 ` Richard Gong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).