[PATCH] crypto: ccp - Fix to return the correct return value

[PATCH] crypto: ccp - Fix to return the correct return value

[Tian Tao]

ccp_dev_suspend and ccp_dev_resume return 0 on error, which causes
ret to equal 0 in sp_suspend and sp_resume, making the if condition
impossible to use.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 drivers/crypto/ccp/ccp-dev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
index 0971ee6..6f2af7b 100644
--- a/drivers/crypto/ccp/ccp-dev.c
+++ b/drivers/crypto/ccp/ccp-dev.c
@@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp)
 
 	/* If there's no device there's nothing to do */
 	if (!ccp)
-		return 0;
+		return -ENXIO;
 
 	spin_lock_irqsave(&ccp->cmd_lock, flags);
 
@@ -584,7 +584,7 @@ int ccp_dev_resume(struct sp_device *sp)
 
 	/* If there's no device there's nothing to do */
 	if (!ccp)
-		return 0;
+		return -ENXIO;
 
 	spin_lock_irqsave(&ccp->cmd_lock, flags);
 
-- 
2.7.4

Re: [PATCH] crypto: ccp - Fix to return the correct return value

[Tom Lendacky]

On 4/14/21 4:17 AM, Tian Tao wrote:
> ccp_dev_suspend and ccp_dev_resume return 0 on error, which causes
> ret to equal 0 in sp_suspend and sp_resume, making the if condition
> impossible to use.

Why do you think that is an error and why do you think it should return
-ENXIO? Since ccp_dev_suspend() and ccp_dev_resume() only return 0 it
might be a more appropriate fix to have these be void functions and
eliminate the if condition in sp_suspend() and sp_resume().

Thanks,
Tom

> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> ---
>  drivers/crypto/ccp/ccp-dev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
> index 0971ee6..6f2af7b 100644
> --- a/drivers/crypto/ccp/ccp-dev.c
> +++ b/drivers/crypto/ccp/ccp-dev.c
> @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp)
>  
>  	/* If there's no device there's nothing to do */
>  	if (!ccp)
> -		return 0;
> +		return -ENXIO;
>  
>  	spin_lock_irqsave(&ccp->cmd_lock, flags);
>  
> @@ -584,7 +584,7 @@ int ccp_dev_resume(struct sp_device *sp)
>  
>  	/* If there's no device there's nothing to do */
>  	if (!ccp)
> -		return 0;
> +		return -ENXIO;
>  
>  	spin_lock_irqsave(&ccp->cmd_lock, flags);
>  
> 

Re: [PATCH] crypto: ccp - Fix to return the correct return value

[tiantao (H)]

在 2021/4/15 6:48, Tom Lendacky 写道:
> On 4/14/21 4:17 AM, Tian Tao wrote:
>> ccp_dev_suspend and ccp_dev_resume return 0 on error, which causes
>> ret to equal 0 in sp_suspend and sp_resume, making the if condition
>> impossible to use.
> Why do you think that is an error and why do you think it should return
> -ENXIO? Since ccp_dev_suspend() and ccp_dev_resume() only return 0 it

thank you for helping reivew.

I think that ccp equals null might just be wrong, now after listening to 
your

explanation, my understanding was wrong, I will send a new patch as you 
suggested.

> might be a more appropriate fix to have these be void functions and
> eliminate the if condition in sp_suspend() and sp_resume().
>
> Thanks,
> Tom
>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> ---
>>   drivers/crypto/ccp/ccp-dev.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
>> index 0971ee6..6f2af7b 100644
>> --- a/drivers/crypto/ccp/ccp-dev.c
>> +++ b/drivers/crypto/ccp/ccp-dev.c
>> @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp)
>>   
>>   	/* If there's no device there's nothing to do */
>>   	if (!ccp)
>> -		return 0;
>> +		return -ENXIO;
>>   
>>   	spin_lock_irqsave(&ccp->cmd_lock, flags);
>>   
>> @@ -584,7 +584,7 @@ int ccp_dev_resume(struct sp_device *sp)
>>   
>>   	/* If there's no device there's nothing to do */
>>   	if (!ccp)
>> -		return 0;
>> +		return -ENXIO;
>>   
>>   	spin_lock_irqsave(&ccp->cmd_lock, flags);
>>   
>>
> .
>