| Message ID | 20200706054732.99387-2-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor SDEI client driver | expand |
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
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
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
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(-)