Re: [PATCH 2/2] mfd: smsc-ece1099: Refactoring for smsc_i2c_probe()

From: Lee Jones
Date: Mon Jan 11 2016 - 03:12:28 EST


Oh, the subject line is also duff. Please describe the changes properly.

"refactoring X", "making changes to Y" are not good subject lines.

On Mon, 11 Jan 2016, Lee Jones wrote:
> On Tue, 29 Dec 2015, SF Markus Elfring wrote:
>
> > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> > Date: Tue, 29 Dec 2015 15:03:31 +0100
> >
> > This issue was detected by using the Coccinelle software.
> >
> > * Let us return directly if a call of the function "devm_regmap_init_i2c"
> > or "regmap_write" failed.
> >
> > * Delete the jump label "err" then.
>
> Why are you bullet pointing here?
>
> Just use proper sentences to tell us what's going on.
>
> > Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/mfd/smsc-ece1099.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
>
> Code looks fine however.
>
> Please re-submit the set with the aforementioned changes along with my:
>
> Acked-by: Lee Jones <lee.jones@xxxxxxxxxx>
>
> > diff --git a/drivers/mfd/smsc-ece1099.c b/drivers/mfd/smsc-ece1099.c
> > index bcac488..951333a 100644
> > --- a/drivers/mfd/smsc-ece1099.c
> > +++ b/drivers/mfd/smsc-ece1099.c
> > @@ -46,10 +46,8 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
> > }
> >
> > smsc->regmap = devm_regmap_init_i2c(i2c, &smsc_regmap_config);
> > - if (IS_ERR(smsc->regmap)) {
> > - ret = PTR_ERR(smsc->regmap);
> > - goto err;
> > - }
> > + if (IS_ERR(smsc->regmap))
> > + return PTR_ERR(smsc->regmap);
> >
> > i2c_set_clientdata(i2c, smsc);
> > smsc->dev = &i2c->dev;
> > @@ -68,7 +66,7 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
> >
> > ret = regmap_write(smsc->regmap, SMSC_CLK_CTRL, smsc->clk);
> > if (ret)
> > - goto err;
> > + return ret;
> >
> > #ifdef CONFIG_OF
> > if (i2c->dev.of_node)
> > @@ -76,7 +74,6 @@ static int smsc_i2c_probe(struct i2c_client *i2c,
> > NULL, NULL, &i2c->dev);
> > #endif
> >
> > -err:
> > return ret;
> > }
> >
>

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog