| Message ID | 20230825143234.38336-1-yann@sionneau.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] i2c: at91: Use dev_err_probe() instead of dev_err() | expand |
On 25/08/2023 at 16:32, Yann Sionneau wrote: > Change > if (IS_ERR(x)) { dev_err(...); return PTR_ERR(x); } > into > return dev_err_probe() > > Also, return the correct error instead of hardcoding -ENODEV > This change has also the advantage of handling the -EPROBE_DEFER situation. Is it found using a tool like Coccinelle or you just ran into it and figured out it could be good to change? Regards, Nicolas > Signed-off-by: Yann Sionneau <yann@sionneau.net> > --- > drivers/i2c/busses/i2c-at91-core.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c > index 05ad3bc3578a..b7bc17b0e5f0 100644 > --- a/drivers/i2c/busses/i2c-at91-core.c > +++ b/drivers/i2c/busses/i2c-at91-core.c > @@ -227,10 +227,9 @@ static int at91_twi_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, dev); > > dev->clk = devm_clk_get(dev->dev, NULL); > - if (IS_ERR(dev->clk)) { > - dev_err(dev->dev, "no clock defined\n"); > - return -ENODEV; > - } > + if (IS_ERR(dev->clk)) > + return dev_err_probe(dev->dev, PTR_ERR(dev->clk), "no clock defined\n"); > + > clk_prepare_enable(dev->clk); > > snprintf(dev->adapter.name, sizeof(dev->adapter.name), "AT91"); > -- > 2.34.1 >
Hi, Le 28/08/2023 à 10:03, Nicolas Ferre a écrit : > On 25/08/2023 at 16:32, Yann Sionneau wrote: >> Change >> if (IS_ERR(x)) { dev_err(...); return PTR_ERR(x); } >> into >> return dev_err_probe() >> >> Also, return the correct error instead of hardcoding -ENODEV >> This change has also the advantage of handling the -EPROBE_DEFER >> situation. > > Is it found using a tool like Coccinelle or you just ran into it and > figured out it could be good to change? I found it by reading the code, I took the time to read the probe functions of a few i2c controller driver to try to find improvements. But I guess one could also find this sort of changes using tools. Regards, Yann > > Regards, > Nicolas > >> Signed-off-by: Yann Sionneau <yann@sionneau.net> >> --- >> drivers/i2c/busses/i2c-at91-core.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-at91-core.c >> b/drivers/i2c/busses/i2c-at91-core.c >> index 05ad3bc3578a..b7bc17b0e5f0 100644 >> --- a/drivers/i2c/busses/i2c-at91-core.c >> +++ b/drivers/i2c/busses/i2c-at91-core.c >> @@ -227,10 +227,9 @@ static int at91_twi_probe(struct platform_device >> *pdev) >> platform_set_drvdata(pdev, dev); >> >> dev->clk = devm_clk_get(dev->dev, NULL); >> - if (IS_ERR(dev->clk)) { >> - dev_err(dev->dev, "no clock defined\n"); >> - return -ENODEV; >> - } >> + if (IS_ERR(dev->clk)) >> + return dev_err_probe(dev->dev, PTR_ERR(dev->clk), "no >> clock defined\n"); >> + >> clk_prepare_enable(dev->clk); >> >> snprintf(dev->adapter.name, sizeof(dev->adapter.name), "AT91"); >> -- >> 2.34.1 >> >
Hi Yann, On Fri, Aug 25, 2023 at 04:32:34PM +0200, Yann Sionneau wrote: > Change > if (IS_ERR(x)) { dev_err(...); return PTR_ERR(x); } > into > return dev_err_probe() > > Also, return the correct error instead of hardcoding -ENODEV > This change has also the advantage of handling the -EPROBE_DEFER situation. > > Signed-off-by: Yann Sionneau <yann@sionneau.net> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
BTW... > > Change > > if (IS_ERR(x)) { dev_err(...); return PTR_ERR(x); } > > into > > return dev_err_probe() > > > > Also, return the correct error instead of hardcoding -ENODEV > > This change has also the advantage of handling the -EPROBE_DEFER situation. > > > > Signed-off-by: Yann Sionneau <yann@sionneau.net> > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> for the next time, please, if you make changes to the patch, starting from the commit title, commit message, patch body, you need to increment the versioning and note the change in the changelog. Use resend only if the patch is taken and sent as it is without any change, anywhere. This should have been [PATCH v2]. Andi
On Fri, Aug 25, 2023 at 04:32:34PM +0200, Yann Sionneau wrote: > Change > if (IS_ERR(x)) { dev_err(...); return PTR_ERR(x); } > into > return dev_err_probe() > > Also, return the correct error instead of hardcoding -ENODEV > This change has also the advantage of handling the -EPROBE_DEFER situation. > > Signed-off-by: Yann Sionneau <yann@sionneau.net> Applied to for-next, thanks!
diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c index 05ad3bc3578a..b7bc17b0e5f0 100644 --- a/drivers/i2c/busses/i2c-at91-core.c +++ b/drivers/i2c/busses/i2c-at91-core.c @@ -227,10 +227,9 @@ static int at91_twi_probe(struct platform_device *pdev) platform_set_drvdata(pdev, dev); dev->clk = devm_clk_get(dev->dev, NULL); - if (IS_ERR(dev->clk)) { - dev_err(dev->dev, "no clock defined\n"); - return -ENODEV; - } + if (IS_ERR(dev->clk)) + return dev_err_probe(dev->dev, PTR_ERR(dev->clk), "no clock defined\n"); + clk_prepare_enable(dev->clk); snprintf(dev->adapter.name, sizeof(dev->adapter.name), "AT91");
Change if (IS_ERR(x)) { dev_err(...); return PTR_ERR(x); } into return dev_err_probe() Also, return the correct error instead of hardcoding -ENODEV This change has also the advantage of handling the -EPROBE_DEFER situation. Signed-off-by: Yann Sionneau <yann@sionneau.net> --- drivers/i2c/busses/i2c-at91-core.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)