LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()
@ 2023-08-25 14:32 Yann Sionneau
  2023-08-28  8:03 ` Nicolas Ferre
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Yann Sionneau @ 2023-08-25 14:32 UTC (permalink / raw
  To: Codrin Ciubotariu, Andi Shyti, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, Yann Sionneau

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

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()
  2023-08-25 14:32 [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err() Yann Sionneau
@ 2023-08-28  8:03 ` Nicolas Ferre
  2023-08-28 10:29   ` Yann Sionneau
  2023-08-29 12:57 ` Andi Shyti
  2023-08-30 19:20 ` Wolfram Sang
  2 siblings, 1 reply; 6+ messages in thread
From: Nicolas Ferre @ 2023-08-28  8:03 UTC (permalink / raw
  To: Yann Sionneau, Codrin Ciubotariu, Andi Shyti, Alexandre Belloni,
	Claudiu Beznea
  Cc: linux-i2c, linux-arm-kernel, linux-kernel

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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()
  2023-08-28  8:03 ` Nicolas Ferre
@ 2023-08-28 10:29   ` Yann Sionneau
  0 siblings, 0 replies; 6+ messages in thread
From: Yann Sionneau @ 2023-08-28 10:29 UTC (permalink / raw
  To: Nicolas Ferre, Codrin Ciubotariu, Andi Shyti, Alexandre Belloni,
	Claudiu Beznea
  Cc: linux-i2c, linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()
  2023-08-25 14:32 [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err() Yann Sionneau
  2023-08-28  8:03 ` Nicolas Ferre
@ 2023-08-29 12:57 ` Andi Shyti
  2023-08-29 13:00   ` Andi Shyti
  2023-08-30 19:20 ` Wolfram Sang
  2 siblings, 1 reply; 6+ messages in thread
From: Andi Shyti @ 2023-08-29 12:57 UTC (permalink / raw
  To: Yann Sionneau
  Cc: Codrin Ciubotariu, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, linux-i2c, linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()
  2023-08-29 12:57 ` Andi Shyti
@ 2023-08-29 13:00   ` Andi Shyti
  0 siblings, 0 replies; 6+ messages in thread
From: Andi Shyti @ 2023-08-29 13:00 UTC (permalink / raw
  To: Yann Sionneau
  Cc: Codrin Ciubotariu, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, linux-i2c, linux-arm-kernel, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err()
  2023-08-25 14:32 [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err() Yann Sionneau
  2023-08-28  8:03 ` Nicolas Ferre
  2023-08-29 12:57 ` Andi Shyti
@ 2023-08-30 19:20 ` Wolfram Sang
  2 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2023-08-30 19:20 UTC (permalink / raw
  To: Yann Sionneau
  Cc: Codrin Ciubotariu, Andi Shyti, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, linux-i2c, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

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!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-08-30 23:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-25 14:32 [PATCH RESEND] i2c: at91: Use dev_err_probe() instead of dev_err() Yann Sionneau
2023-08-28  8:03 ` Nicolas Ferre
2023-08-28 10:29   ` Yann Sionneau
2023-08-29 12:57 ` Andi Shyti
2023-08-29 13:00   ` Andi Shyti
2023-08-30 19:20 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).