[PATCH] intel_soc_pmic_mrfld: simplify the return expression of intel_scu_ipc_dev_iowrite8()

[PATCH] intel_soc_pmic_mrfld: simplify the return expression of intel_scu_ipc_dev_iowrite8()

[Xu Wang]

Simplify the return expression.

Signed-off-by: Xu Wang <vulab@iscas.ac.cn>
---
 drivers/mfd/intel_soc_pmic_mrfld.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/mfd/intel_soc_pmic_mrfld.c b/drivers/mfd/intel_soc_pmic_mrfld.c
index bd94c989d232..71da861e8c27 100644
--- a/drivers/mfd/intel_soc_pmic_mrfld.c
+++ b/drivers/mfd/intel_soc_pmic_mrfld.c
@@ -91,13 +91,8 @@ static int bcove_ipc_byte_reg_write(void *context, unsigned int reg,
 {
 	struct intel_soc_pmic *pmic = context;
 	u8 ipc_in = val;
-	int ret;
 
-	ret = intel_scu_ipc_dev_iowrite8(pmic->scu, reg, ipc_in);
-	if (ret)
-		return ret;
-
-	return 0;
+	return intel_scu_ipc_dev_iowrite8(pmic->scu, reg, ipc_in);
 }
 
 static const struct regmap_config bcove_regmap_config = {
-- 
2.17.1

Re: [PATCH] intel_soc_pmic_mrfld: simplify the return expression of intel_scu_ipc_dev_iowrite8()

[Andy Shevchenko]

On Mon, Jul 27, 2020 at 03:04:07AM +0000, Xu Wang wrote:
> Simplify the return expression.

I understand your intention, but I would rather leave as is.
It's a bit more helpful in the original form in case to add some debugging.
It also keep it symmetrical with read() counterpart.

That said, I'm not objecting the change if Lee finds it good enough.

> Signed-off-by: Xu Wang <vulab@iscas.ac.cn>
> ---
>  drivers/mfd/intel_soc_pmic_mrfld.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/intel_soc_pmic_mrfld.c b/drivers/mfd/intel_soc_pmic_mrfld.c
> index bd94c989d232..71da861e8c27 100644
> --- a/drivers/mfd/intel_soc_pmic_mrfld.c
> +++ b/drivers/mfd/intel_soc_pmic_mrfld.c
> @@ -91,13 +91,8 @@ static int bcove_ipc_byte_reg_write(void *context, unsigned int reg,
>  {
>  	struct intel_soc_pmic *pmic = context;
>  	u8 ipc_in = val;
> -	int ret;
>  
> -	ret = intel_scu_ipc_dev_iowrite8(pmic->scu, reg, ipc_in);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> +	return intel_scu_ipc_dev_iowrite8(pmic->scu, reg, ipc_in);
>  }
>  
>  static const struct regmap_config bcove_regmap_config = {
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] intel_soc_pmic_mrfld: simplify the return expression of intel_scu_ipc_dev_iowrite8()

[Lee Jones]

On Mon, 27 Jul 2020, Andy Shevchenko wrote:

> On Mon, Jul 27, 2020 at 03:04:07AM +0000, Xu Wang wrote:
> > Simplify the return expression.
> 
> I understand your intention, but I would rather leave as is.
> It's a bit more helpful in the original form in case to add some debugging.

If you wish to add debugging, you'll have to add the extra line of
code yourself. :)

> It also keep it symmetrical with read() counterpart.

Can this be fixed-up too?

> That said, I'm not objecting the change if Lee finds it good enough.
> 
> > Signed-off-by: Xu Wang <vulab@iscas.ac.cn>
> > ---
> >  drivers/mfd/intel_soc_pmic_mrfld.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] intel_soc_pmic_mrfld: simplify the return expression of intel_scu_ipc_dev_iowrite8()

[Andy Shevchenko]

On Mon, Jul 27, 2020 at 3:57 PM Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 27 Jul 2020, Andy Shevchenko wrote:
> > On Mon, Jul 27, 2020 at 03:04:07AM +0000, Xu Wang wrote:

...

> > It's a bit more helpful in the original form in case to add some debugging.
>
> If you wish to add debugging, you'll have to add the extra line of
> code yourself. :)

True, that's why I'm not objecting the change.

> > It also keep it symmetrical with read() counterpart.
>
> Can this be fixed-up too?

How? Can you elaborate?

> > That said, I'm not objecting the change if Lee finds it good enough.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] intel_soc_pmic_mrfld: simplify the return expression of intel_scu_ipc_dev_iowrite8()

[Lee Jones]

On Mon, 27 Jul 2020, Andy Shevchenko wrote:

> On Mon, Jul 27, 2020 at 3:57 PM Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 27 Jul 2020, Andy Shevchenko wrote:
> > > On Mon, Jul 27, 2020 at 03:04:07AM +0000, Xu Wang wrote:
> 
> ...
> 
> > > It's a bit more helpful in the original form in case to add some debugging.
> >
> > If you wish to add debugging, you'll have to add the extra line of
> > code yourself. :)
> 
> True, that's why I'm not objecting the change.
> 
> > > It also keep it symmetrical with read() counterpart.
> >
> > Can this be fixed-up too?
> 
> How? Can you elaborate?

I hadn't even looked at the code before my reply.  I just assumed it
was in a similar situation.

Now I have looked at it, could the pointer to the return value be
cast?

If not, just leave it.  Symmetry isn't a good measure of good, clean
code.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog