[01/14] drivers/firmware/sdei: Remove sdei_is_err()
diff mbox series

Message ID 20200706054732.99387-2-gshan@redhat.com
State New
Headers show
Series
  • Refactor SDEI client driver
Related show

Commit Message

Gavin Shan July 6, 2020, 5:47 a.m. UTC
sdei_is_err() is only called by sdei_to_linux_errno(). The logic of
checking on the error number is common to them. They can be combined
finely.

This removes sdei_is_err() and its logic is combined to the function
sdei_to_linux_errno(). This shouldn't cause functional changes.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 drivers/firmware/arm_sdei.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

Comments

James Morse July 21, 2020, 8:39 p.m. UTC | #1
Hi Gavin,

On 06/07/2020 06:47, Gavin Shan wrote:
> sdei_is_err() is only called by sdei_to_linux_errno().

... so it is! There were a lot more of these out of tree when it was being used to test
the firmware.


> The logic of
> checking on the error number is common to them. They can be combined
> finely.

> This removes sdei_is_err() and its logic is combined to the function
> sdei_to_linux_errno(). 

> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index e7e36aab2386..415c243a8e65 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -114,26 +114,7 @@ static int sdei_to_linux_errno(unsigned long sdei_err)
>  		return -ENOMEM;
>  	}
>  
> -	/* Not an error value ... */
> -	return sdei_err;
> -}
> -
> -/*
> - * If x0 is any of these values, then the call failed, use sdei_to_linux_errno()
> - * to translate.
> - */
> -static int sdei_is_err(struct arm_smccc_res *res)
> -{
> -	switch (res->a0) {
> -	case SDEI_NOT_SUPPORTED:
> -	case SDEI_INVALID_PARAMETERS:
> -	case SDEI_DENIED:
> -	case SDEI_PENDING:
> -	case SDEI_OUT_OF_RESOURCE:
> -		return true;
> -	}
> -
> -	return false;
> +	return 0;
>  }
>  
>  static int invoke_sdei_fn(unsigned long function_id, unsigned long arg0,
> @@ -147,8 +128,7 @@ static int invoke_sdei_fn(unsigned long function_id, unsigned long arg0,

Please also remove the now-pointless initialiser at the top of the function.


>  	if (sdei_firmware_call) {
>  		sdei_firmware_call(function_id, arg0, arg1, arg2, arg3, arg4,
>  				   &res);
> -		if (sdei_is_err(&res))
> -			err = sdei_to_linux_errno(res.a0);
> +		err = sdei_to_linux_errno(res.a0);>  	} else {
>  		/*
>  		 * !sdei_firmware_call means we failed to probe or called
> 

With that:
Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James
Gavin Shan July 22, 2020, 2:04 a.m. UTC | #2
Hi James,

On 7/22/20 6:39 AM, James Morse wrote:
> On 06/07/2020 06:47, Gavin Shan wrote:
>> sdei_is_err() is only called by sdei_to_linux_errno().
> 
> ... so it is! There were a lot more of these out of tree when it was being used to test
> the firmware.
> 

Ok, thanks for the explanation.

> 
>> The logic of
>> checking on the error number is common to them. They can be combined
>> finely.
> 
>> This removes sdei_is_err() and its logic is combined to the function
>> sdei_to_linux_errno().
> 
>> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
>> index e7e36aab2386..415c243a8e65 100644
>> --- a/drivers/firmware/arm_sdei.c
>> +++ b/drivers/firmware/arm_sdei.c
>> @@ -114,26 +114,7 @@ static int sdei_to_linux_errno(unsigned long sdei_err)
>>   		return -ENOMEM;
>>   	}
>>   
>> -	/* Not an error value ... */
>> -	return sdei_err;
>> -}
>> -
>> -/*
>> - * If x0 is any of these values, then the call failed, use sdei_to_linux_errno()
>> - * to translate.
>> - */
>> -static int sdei_is_err(struct arm_smccc_res *res)
>> -{
>> -	switch (res->a0) {
>> -	case SDEI_NOT_SUPPORTED:
>> -	case SDEI_INVALID_PARAMETERS:
>> -	case SDEI_DENIED:
>> -	case SDEI_PENDING:
>> -	case SDEI_OUT_OF_RESOURCE:
>> -		return true;
>> -	}
>> -
>> -	return false;
>> +	return 0;
>>   }
>>   
>>   static int invoke_sdei_fn(unsigned long function_id, unsigned long arg0,
>> @@ -147,8 +128,7 @@ static int invoke_sdei_fn(unsigned long function_id, unsigned long arg0,
> 
> Please also remove the now-pointless initialiser at the top of the function.
> 

Sure, the assignment of @err to zero will be dropped in v2.

> 
>>   	if (sdei_firmware_call) {
>>   		sdei_firmware_call(function_id, arg0, arg1, arg2, arg3, arg4,
>>   				   &res);
>> -		if (sdei_is_err(&res))
>> -			err = sdei_to_linux_errno(res.a0);
>> +		err = sdei_to_linux_errno(res.a0);>  	} else {
>>   		/*
>>   		 * !sdei_firmware_call means we failed to probe or called
>>
> 
> With that:
> Reviewed-by: James Morse <james.morse@arm.com>
> 

Thanks for your review.

Thanks,
Gavin

Patch
diff mbox series

diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index e7e36aab2386..415c243a8e65 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -114,26 +114,7 @@  static int sdei_to_linux_errno(unsigned long sdei_err)
 		return -ENOMEM;
 	}
 
-	/* Not an error value ... */
-	return sdei_err;
-}
-
-/*
- * If x0 is any of these values, then the call failed, use sdei_to_linux_errno()
- * to translate.
- */
-static int sdei_is_err(struct arm_smccc_res *res)
-{
-	switch (res->a0) {
-	case SDEI_NOT_SUPPORTED:
-	case SDEI_INVALID_PARAMETERS:
-	case SDEI_DENIED:
-	case SDEI_PENDING:
-	case SDEI_OUT_OF_RESOURCE:
-		return true;
-	}
-
-	return false;
+	return 0;
 }
 
 static int invoke_sdei_fn(unsigned long function_id, unsigned long arg0,
@@ -147,8 +128,7 @@  static int invoke_sdei_fn(unsigned long function_id, unsigned long arg0,
 	if (sdei_firmware_call) {
 		sdei_firmware_call(function_id, arg0, arg1, arg2, arg3, arg4,
 				   &res);
-		if (sdei_is_err(&res))
-			err = sdei_to_linux_errno(res.a0);
+		err = sdei_to_linux_errno(res.a0);
 	} else {
 		/*
 		 * !sdei_firmware_call means we failed to probe or called