[RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()

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(-)

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!