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

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

[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

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

[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Nicolas Ferre]

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
> 

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

[Nicolas Ferre]

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
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Yann Sionneau]

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

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

[Yann Sionneau]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andi Shyti]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andi Shyti]

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

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

[Andi Shyti]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andi Shyti]

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

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

[Wolfram Sang]

[-- Attachment #1.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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Wolfram Sang]

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